From 8ac646a1a3ee11b0970ba77b3384ce692002785a Mon Sep 17 00:00:00 2001 From: Tianjie Wei Date: Wed, 2 Feb 2022 11:14:45 -0800 Subject: [PATCH] Addressing PR comments --- contracts/mainnet/common/math.sol | 5 ++ .../connectors/notional/SafeInt256.sol | 31 -------- .../mainnet/connectors/notional/helpers.sol | 78 +++++++++++-------- .../mainnet/connectors/notional/main.sol | 17 ++-- 4 files changed, 56 insertions(+), 75 deletions(-) delete mode 100644 contracts/mainnet/connectors/notional/SafeInt256.sol diff --git a/contracts/mainnet/common/math.sol b/contracts/mainnet/common/math.sol index f6e2e6cd..f395ba84 100644 --- a/contracts/mainnet/common/math.sol +++ b/contracts/mainnet/common/math.sol @@ -43,6 +43,11 @@ contract DSMath { require(y >= 0, "int-overflow"); } + function toUint(int256 x) internal pure returns (uint256) { + require(x >= 0, "int-overflow"); + return uint256(x); + } + function toRad(uint wad) internal pure returns (uint rad) { rad = mul(wad, 10 ** 27); } diff --git a/contracts/mainnet/connectors/notional/SafeInt256.sol b/contracts/mainnet/connectors/notional/SafeInt256.sol deleted file mode 100644 index 47013c05..00000000 --- a/contracts/mainnet/connectors/notional/SafeInt256.sol +++ /dev/null @@ -1,31 +0,0 @@ -pragma solidity ^0.7.6; - -library SafeInt256 { - int256 private constant _INT256_MIN = type(int256).min; - - function mul(int256 a, int256 b) internal pure returns (int256 c) { - c = a * b; - if (a == -1) require(b == 0 || c / b == a); - else require(a == 0 || c / a == b); - } - - function div(int256 a, int256 b) internal pure returns (int256 c) { - require(!(b == -1 && a == _INT256_MIN)); // dev: int256 div overflow - // NOTE: solidity will automatically revert on divide by zero - c = a / b; - } - - function sub(int256 x, int256 y) internal pure returns (int256 z) { - // taken from uniswap v3 - require((z = x - y) <= x == (y >= 0)); - } - - function neg(int256 x) internal pure returns (int256 y) { - return mul(-1, x); - } - - function toInt(uint256 x) internal pure returns (int256 y) { - require(x <= uint256(type(int256).max)); - return int256(x); - } -} diff --git a/contracts/mainnet/connectors/notional/helpers.sol b/contracts/mainnet/connectors/notional/helpers.sol index 0aceb6e0..79161b4f 100644 --- a/contracts/mainnet/connectors/notional/helpers.sol +++ b/contracts/mainnet/connectors/notional/helpers.sol @@ -1,18 +1,15 @@ pragma solidity ^0.7.6; pragma abicoder v2; -import {SafeMath} from "@openzeppelin/contracts/math/SafeMath.sol"; import {Token, NotionalInterface, BalanceAction, BalanceActionWithTrades, DepositActionType} from "./interface.sol"; -import {SafeInt256} from "./SafeInt256.sol"; import {Basic} from "../../common/basic.sol"; +import {DSMath} from "../../common/math.sol"; import {TokenInterface} from "../../common/interfaces.sol"; -contract Helpers is Basic { - using SafeMath for uint256; - using SafeInt256 for int256; +abstract contract Helpers is DSMath, Basic { uint8 internal constant LEND_TRADE = 0; uint8 internal constant BORROW_TRADE = 1; - int256 internal constant INTERNAL_TOKEN_PRECISION = 1e8; + uint256 internal constant INTERNAL_TOKEN_PRECISION = 1e8; uint256 internal constant ETH_CURRENCY_ID = 1; uint256 internal constant MAX_DEPOSIT = uint256(-1); @@ -20,25 +17,30 @@ contract Helpers is Basic { NotionalInterface internal constant notional = NotionalInterface(0x1344A36A1B56144C3Bc62E7757377D288fDE0369); - /// @notice Returns the address of the underlying token for a given currency id, - function getUnderlyingToken(uint16 currencyId) internal view returns (address) { - ( - /* Token memory assetToken */, - Token memory underlyingToken - ) = notional.getCurrency(currencyId); + /// @notice Returns the address of the underlying token for a given currency id, + function getUnderlyingToken(uint16 currencyId) + internal + view + returns (address) + { + // prettier-ignore + (/* assetToken */, Token memory underlyingToken) = notional.getCurrency(currencyId); return underlyingToken.tokenAddress; } /// @notice Returns the address of the asset token for a given currency id function getAssetToken(uint16 currencyId) internal view returns (address) { - ( - Token memory assetToken, - /* Token memory underlyingToken */ - ) = notional.getCurrency(currencyId); + // prettier-ignore + (Token memory assetToken, /* underlyingToken */) = notional.getCurrency(currencyId); return assetToken.tokenAddress; } - function getCashBalance(uint16 currencyId) internal view returns (int256 cashBalance) { + function getCashBalance(uint16 currencyId) + internal + view + returns (int256 cashBalance) + { + // prettier-ignore ( cashBalance, /* int256 nTokenBalance */, @@ -46,17 +48,25 @@ contract Helpers is Basic { ) = notional.getAccountBalance(currencyId, address(this)); } - function getNTokenBalance(uint16 currencyId) internal view returns (int256 nTokenBalance) { + function getNTokenBalance(uint16 currencyId) + internal + view + returns (uint256) + { + // prettier-ignore ( /* int256 cashBalance */, - nTokenBalance, + int256 nTokenBalance, /* int256 lastClaimTime */ ) = notional.getAccountBalance(currencyId, address(this)); + return toUint(nTokenBalance); } - function getNTokenRedeemAmount(uint16 currencyId, uint96 _tokensToRedeem, uint256 getId) - internal - returns (uint96 tokensToRedeem) { + function getNTokenRedeemAmount( + uint16 currencyId, + uint96 _tokensToRedeem, + uint256 getId + ) internal returns (uint96 tokensToRedeem) { tokensToRedeem = uint96(getUint(getId, _tokensToRedeem)); if (tokensToRedeem == uint96(-1)) { tokensToRedeem = uint96(getNTokenBalance(currencyId)); @@ -73,17 +83,20 @@ contract Helpers is Basic { : 0; } - function convertToInternal(uint16 currencyId, int256 amount) - internal view - returns (int256) + function convertToInternal(uint16 currencyId, uint256 amount) + internal + view + returns (uint256) { // If token decimals is greater than INTERNAL_TOKEN_PRECISION then this will truncate // down to the internal precision. Resulting dust will accumulate to the protocol. // If token decimals is less than INTERNAL_TOKEN_PRECISION then this will add zeros to the // end of amount and will not result in dust. + // prettier-ignore (Token memory assetToken, /* underlyingToken */) = notional.getCurrency(currencyId); - if (assetToken.decimals == INTERNAL_TOKEN_PRECISION) return amount; - return amount.mul(INTERNAL_TOKEN_PRECISION).div(assetToken.decimals); + uint256 decimals = toUint(assetToken.decimals); + if (decimals == INTERNAL_TOKEN_PRECISION) return amount; + return div(mul(amount, INTERNAL_TOKEN_PRECISION), decimals); } function encodeLendTrade( @@ -121,9 +134,10 @@ contract Helpers is Basic { depositAmount = getUint(getId, depositAmount); if (currencyId == ETH_CURRENCY_ID && useUnderlying) { // No approval required for ETH so we can return the deposit amount - return depositAmount == MAX_DEPOSIT - ? address(this).balance - : depositAmount; + return + depositAmount == MAX_DEPOSIT + ? address(this).balance + : depositAmount; } address tokenAddress = useUnderlying @@ -184,7 +198,7 @@ contract Helpers is Basic { if (setId != 0) { uint256 balanceAfter = getBalance(tokenAddress); - setUint(setId, balanceAfter.sub(balanceBefore)); + setUint(setId, sub(balanceAfter, balanceBefore)); } } @@ -207,7 +221,7 @@ contract Helpers is Basic { if (setId != 0) { uint256 balanceAfter = getBalance(tokenAddress); - setUint(setId, balanceAfter.sub(balanceBefore)); + setUint(setId, sub(balanceAfter, balanceBefore)); } } diff --git a/contracts/mainnet/connectors/notional/main.sol b/contracts/mainnet/connectors/notional/main.sol index 5af12ec3..df4ac1ee 100644 --- a/contracts/mainnet/connectors/notional/main.sol +++ b/contracts/mainnet/connectors/notional/main.sol @@ -7,14 +7,11 @@ pragma abicoder v2; */ import {Helpers} from "./helpers.sol"; -import {SafeInt256} from "./SafeInt256.sol"; import {Events} from "./events.sol"; import {DepositActionType, BalanceActionWithTrades, BalanceAction} from "./interface.sol"; import {TokenInterface} from "../../common/interfaces.sol"; abstract contract NotionalResolver is Events, Helpers { - using SafeInt256 for int256; - /** * @notice Deposit collateral into Notional, this should only be used for reducing risk of * liquidation. @@ -102,7 +99,7 @@ abstract contract NotionalResolver is Events, Helpers { uint88 amountInternalPrecision = withdrawAmount == uint256(-1) ? uint88(getCashBalance(currencyId)) : uint88( - convertToInternal(currencyId, SafeInt256.toInt(withdrawAmount)) + convertToInternal(currencyId, withdrawAmount) ); uint256 amountWithdrawn = notional.withdraw( @@ -345,7 +342,7 @@ abstract contract NotionalResolver is Events, Helpers { action[0].depositActionAmount = depositAmount; // withdraw amount, withdraw cash and redeem to underlying are all 0 and false - int256 nTokenBefore = getNTokenBalance(currencyId); + uint256 nTokenBefore = getNTokenBalance(currencyId); uint256 msgValue = getMsgValue( currencyId, useUnderlying, @@ -354,9 +351,7 @@ abstract contract NotionalResolver is Events, Helpers { notional.batchBalanceAction{value: msgValue}(address(this), action); - int256 nTokenBalanceChange = getNTokenBalance(currencyId).sub( - nTokenBefore - ); + uint256 nTokenBalanceChange = sub(getNTokenBalance(currencyId), nTokenBefore); if (setId != 0) { // Set the amount of nTokens minted @@ -402,13 +397,11 @@ abstract contract NotionalResolver is Events, Helpers { action[0].depositActionAmount = cashBalanceToMint; // NOTE: withdraw amount, withdraw cash and redeem to underlying are all 0 and false - int256 nTokenBefore = getNTokenBalance(currencyId); + uint256 nTokenBefore = getNTokenBalance(currencyId); notional.batchBalanceAction(address(this), action); - int256 nTokenBalanceChange = getNTokenBalance(currencyId).sub( - nTokenBefore - ); + uint256 nTokenBalanceChange = sub(getNTokenBalance(currencyId), nTokenBefore); if (setId != 0) { // Set the amount of nTokens minted