Addressing PR comments

This commit is contained in:
Tianjie Wei 2022-03-31 08:27:31 -07:00
parent 7534ddd4ad
commit 74a7effdb4
6 changed files with 953 additions and 953 deletions

View File

@ -1,3 +1,4 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6; pragma solidity ^0.7.6;
contract Events { contract Events {

View File

@ -1,3 +1,4 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6; pragma solidity ^0.7.6;
pragma abicoder v2; pragma abicoder v2;
@ -11,55 +12,36 @@ abstract contract Helpers is DSMath, Basic {
uint8 internal constant BORROW_TRADE = 1; uint8 internal constant BORROW_TRADE = 1;
uint256 internal constant INTERNAL_TOKEN_PRECISION = 1e8; uint256 internal constant INTERNAL_TOKEN_PRECISION = 1e8;
uint256 internal constant ETH_CURRENCY_ID = 1; uint256 internal constant ETH_CURRENCY_ID = 1;
uint256 internal constant MAX_DEPOSIT = uint256(-1); uint256 internal constant MAX_DEPOSIT = type(uint256).max;
/// @dev Contract address is different on Kovan: 0x0EAE7BAdEF8f95De91fDDb74a89A786cF891Eb0e /// @dev Contract address is different on Kovan: 0x0EAE7BAdEF8f95De91fDDb74a89A786cF891Eb0e
NotionalInterface internal constant notional = NotionalInterface internal constant notional =
NotionalInterface(0x1344A36A1B56144C3Bc62E7757377D288fDE0369); NotionalInterface(0x1344A36A1B56144C3Bc62E7757377D288fDE0369);
/// @notice Returns the address of the underlying token for a given currency id, /// @notice Returns the address of the underlying token for a given currency id,
function getUnderlyingToken(uint16 currencyId) function getAssetOrUnderlyingToken(uint16 currencyId, bool underlying)
internal internal
view view
returns (address) returns (address)
{ {
// prettier-ignore // prettier-ignore
(/* assetToken */, Token memory underlyingToken) = notional.getCurrency(currencyId); (Token memory assetToken, Token memory underlyingToken) = notional.getCurrency(currencyId);
return underlyingToken.tokenAddress; return
underlying ? underlyingToken.tokenAddress : assetToken.tokenAddress;
} }
/// @notice Returns the address of the asset token for a given currency id function getCashOrNTokenBalance(uint16 currencyId, bool nToken)
function getAssetToken(uint16 currencyId) internal view returns (address) {
// prettier-ignore
(Token memory assetToken, /* underlyingToken */) = notional.getCurrency(currencyId);
return assetToken.tokenAddress;
}
function getCashBalance(uint16 currencyId)
internal
view
returns (int256 cashBalance)
{
// prettier-ignore
(
cashBalance,
/* int256 nTokenBalance */,
/* int256 lastClaimTime */
) = notional.getAccountBalance(currencyId, address(this));
}
function getNTokenBalance(uint16 currencyId)
internal internal
view view
returns (uint256) returns (uint256)
{ {
// prettier-ignore // prettier-ignore
( (
/* int256 cashBalance */, int256 cashBalance,
int256 nTokenBalance, int256 nTokenBalance,
/* int256 lastClaimTime */ /* int256 lastClaimTime */
) = notional.getAccountBalance(currencyId, address(this)); ) = notional.getAccountBalance(currencyId, address(this));
return toUint(nTokenBalance); return toUint(nToken ? nTokenBalance : cashBalance);
} }
function getNTokenRedeemAmount( function getNTokenRedeemAmount(
@ -67,12 +49,22 @@ abstract contract Helpers is DSMath, Basic {
uint96 _tokensToRedeem, uint96 _tokensToRedeem,
uint256 getId uint256 getId
) internal returns (uint96 tokensToRedeem) { ) internal returns (uint96 tokensToRedeem) {
tokensToRedeem = uint96(getUint(getId, _tokensToRedeem)); tokensToRedeem = toUint96(getUint(getId, _tokensToRedeem));
if (tokensToRedeem == uint96(-1)) { if (tokensToRedeem == type(uint96).max) {
tokensToRedeem = uint96(getNTokenBalance(currencyId)); tokensToRedeem = toUint96(getCashOrNTokenBalance(currencyId, true));
} }
} }
function toUint96(uint256 value) internal pure returns (uint96) {
require(value <= type(uint96).max, "uint96 value overflow");
return uint96(value);
}
function toUint88(uint256 value) internal pure returns (uint88) {
require(value <= type(uint88).max, "uint88 value overflow");
return uint88(value);
}
function getMsgValue( function getMsgValue(
uint16 currencyId, uint16 currencyId,
bool useUnderlying, bool useUnderlying,
@ -140,9 +132,10 @@ abstract contract Helpers is DSMath, Basic {
: depositAmount; : depositAmount;
} }
address tokenAddress = useUnderlying address tokenAddress = getAssetOrUnderlyingToken(
? getUnderlyingToken(currencyId) currencyId,
: getAssetToken(currencyId); useUnderlying
);
if (depositAmount == MAX_DEPOSIT) { if (depositAmount == MAX_DEPOSIT) {
depositAmount = TokenInterface(tokenAddress).balanceOf( depositAmount = TokenInterface(tokenAddress).balanceOf(
@ -170,10 +163,7 @@ abstract contract Helpers is DSMath, Basic {
return ethAddr; return ethAddr;
} }
return return getAssetOrUnderlyingToken(currencyId, useUnderlying);
useUnderlying
? getUnderlyingToken(currencyId)
: getAssetToken(currencyId);
} }
/// @dev Executes a trade action and sets the balance change to setId /// @dev Executes a trade action and sets the balance change to setId

View File

@ -1,3 +1,4 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6; pragma solidity ^0.7.6;
pragma abicoder v2; pragma abicoder v2;
@ -118,7 +119,8 @@ interface NotionalInterface {
address redeemer, address redeemer,
uint16 currencyId, uint16 currencyId,
uint96 tokensToRedeem_, uint96 tokensToRedeem_,
bool sellTokenAssets bool sellTokenAssets,
bool acceptResidualAssets
) external returns (int256); ) external returns (int256);
function batchBalanceAction( function batchBalanceAction(

View File

@ -1,3 +1,4 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6; pragma solidity ^0.7.6;
pragma abicoder v2; pragma abicoder v2;
@ -96,11 +97,9 @@ abstract contract NotionalResolver is Events, Helpers {
returns (string memory _eventName, bytes memory _eventParam) returns (string memory _eventName, bytes memory _eventParam)
{ {
withdrawAmount = getUint(getId, withdrawAmount); withdrawAmount = getUint(getId, withdrawAmount);
uint88 amountInternalPrecision = withdrawAmount == uint256(-1) uint88 amountInternalPrecision = withdrawAmount == type(uint256).max
? uint88(getCashBalance(currencyId)) ? toUint88(getCashOrNTokenBalance(currencyId, false))
: uint88( : toUint88(convertToInternal(currencyId, withdrawAmount));
convertToInternal(currencyId, withdrawAmount)
);
uint256 amountWithdrawn = notional.withdraw( uint256 amountWithdrawn = notional.withdraw(
currencyId, currencyId,
@ -152,6 +151,7 @@ abstract contract NotionalResolver is Events, Helpers {
uint16 currencyId, uint16 currencyId,
bool sellTokenAssets, bool sellTokenAssets,
uint96 tokensToRedeem, uint96 tokensToRedeem,
bool acceptResidualAssets,
uint256 getId, uint256 getId,
uint256 setId uint256 setId
) )
@ -169,7 +169,8 @@ abstract contract NotionalResolver is Events, Helpers {
address(this), address(this),
currencyId, currencyId,
tokensToRedeem, tokensToRedeem,
sellTokenAssets sellTokenAssets,
acceptResidualAssets
); );
// Floor asset cash change at zero in order to properly set the uint. If the asset cash change is negative // Floor asset cash change at zero in order to properly set the uint. If the asset cash change is negative
@ -225,7 +226,7 @@ abstract contract NotionalResolver is Events, Helpers {
action[0].currencyId = currencyId; action[0].currencyId = currencyId;
action[0].depositActionAmount = tokensToRedeem; action[0].depositActionAmount = tokensToRedeem;
action[0].redeemToUnderlying = redeemToUnderlying; action[0].redeemToUnderlying = redeemToUnderlying;
if (amountToWithdraw == uint256(-1)) { if (amountToWithdraw == type(uint256).max) {
// This setting will override the withdrawAmountInternalPrecision // This setting will override the withdrawAmountInternalPrecision
action[0].withdrawEntireCashBalance = true; action[0].withdrawEntireCashBalance = true;
} else { } else {
@ -342,7 +343,7 @@ abstract contract NotionalResolver is Events, Helpers {
action[0].depositActionAmount = depositAmount; action[0].depositActionAmount = depositAmount;
// withdraw amount, withdraw cash and redeem to underlying are all 0 and false // withdraw amount, withdraw cash and redeem to underlying are all 0 and false
uint256 nTokenBefore = getNTokenBalance(currencyId); uint256 nTokenBefore = getCashOrNTokenBalance(currencyId, true);
uint256 msgValue = getMsgValue( uint256 msgValue = getMsgValue(
currencyId, currencyId,
useUnderlying, useUnderlying,
@ -351,7 +352,10 @@ abstract contract NotionalResolver is Events, Helpers {
notional.batchBalanceAction{ value: msgValue }(address(this), action); notional.batchBalanceAction{ value: msgValue }(address(this), action);
uint256 nTokenBalanceChange = sub(getNTokenBalance(currencyId), nTokenBefore); uint256 nTokenBalanceChange = sub(
getCashOrNTokenBalance(currencyId, true),
nTokenBefore
);
if (setId != 0) { if (setId != 0) {
// Set the amount of nTokens minted // Set the amount of nTokens minted
@ -388,8 +392,8 @@ abstract contract NotionalResolver is Events, Helpers {
returns (string memory _eventName, bytes memory _eventParam) returns (string memory _eventName, bytes memory _eventParam)
{ {
cashBalanceToMint = getUint(getId, cashBalanceToMint); cashBalanceToMint = getUint(getId, cashBalanceToMint);
if (cashBalanceToMint == uint256(-1)) if (cashBalanceToMint == type(uint256).max)
cashBalanceToMint = uint256(getCashBalance(currencyId)); cashBalanceToMint = getCashOrNTokenBalance(currencyId, false);
BalanceAction[] memory action = new BalanceAction[](1); BalanceAction[] memory action = new BalanceAction[](1);
action[0].actionType = DepositActionType.ConvertCashToNToken; action[0].actionType = DepositActionType.ConvertCashToNToken;
@ -397,11 +401,14 @@ abstract contract NotionalResolver is Events, Helpers {
action[0].depositActionAmount = cashBalanceToMint; action[0].depositActionAmount = cashBalanceToMint;
// NOTE: withdraw amount, withdraw cash and redeem to underlying are all 0 and false // NOTE: withdraw amount, withdraw cash and redeem to underlying are all 0 and false
uint256 nTokenBefore = getNTokenBalance(currencyId); uint256 nTokenBefore = getCashOrNTokenBalance(currencyId, true);
notional.batchBalanceAction(address(this), action); notional.batchBalanceAction(address(this), action);
uint256 nTokenBalanceChange = sub(getNTokenBalance(currencyId), nTokenBefore); uint256 nTokenBalanceChange = sub(
getCashOrNTokenBalance(currencyId, true),
nTokenBefore
);
if (setId != 0) { if (setId != 0) {
// Set the amount of nTokens minted // Set the amount of nTokens minted

View File

@ -95,7 +95,7 @@ const redeemNTokenRaw = async (
{ {
connector: "NOTIONAL-TEST-A", connector: "NOTIONAL-TEST-A",
method: "redeemNTokenRaw", method: "redeemNTokenRaw",
args: [currencyId, sellTokenAssets, tokensToRedeem, 0, 0] args: [currencyId, sellTokenAssets, tokensToRedeem, false, 0, 0]
} }
]; ];

View File

@ -52,7 +52,7 @@ describe("Notional", function () {
forking: { forking: {
//@ts-ignore //@ts-ignore
jsonRpcUrl: hre.config.networks.hardhat.forking.url, jsonRpcUrl: hre.config.networks.hardhat.forking.url,
blockNumber: 13798624, blockNumber: 14483893,
}, },
}, },
], ],
@ -240,7 +240,7 @@ describe("Notional", function () {
expect( expect(
await daiToken.balanceOf(dsaWallet0.address), await daiToken.balanceOf(dsaWallet0.address),
"expect DSA wallet to contain borrowed balance minus fees" "expect DSA wallet to contain borrowed balance minus fees"
).to.be.gte(ethers.utils.parseEther("990")); ).to.be.gte(ethers.utils.parseEther("985"));
}); });
it("test_deposit_ETH_and_borrow_DAI_asset", async function () { it("test_deposit_ETH_and_borrow_DAI_asset", async function () {
@ -256,7 +256,7 @@ describe("Notional", function () {
expect( expect(
await cdaiToken.balanceOf(dsaWallet0.address), await cdaiToken.balanceOf(dsaWallet0.address),
"expect DSA wallet to contain borrowed balance minus fees" "expect DSA wallet to contain borrowed balance minus fees"
).to.be.gte(ethers.utils.parseUnits("4500000000000", 0)); ).to.be.gte(ethers.utils.parseUnits("4490000000000", 0));
}); });
it("test_deposit_DAI_underlying_and_borrow_ETH", async function () { it("test_deposit_DAI_underlying_and_borrow_ETH", async function () {