Fix Review #2
Fix Review #2
Conversation
| @@ -325,14 +325,18 @@ contract RedemptionVault is ManageableVault, IRedemptionVault { | |||
|
|
|||
| mToken.burn(address(this), request.amountMToken); | |||
|
|
|||
| if (request.tokenOut != MANUAL_FULLFILMENT_TOKEN) { | |||
| uint256 tokenDecimals = _tokenDecimals(request.tokenOut); | |||
| bool isFiat = request.tokenOut == MANUAL_FULLFILMENT_TOKEN; | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
| (request.amountMToken * newMTokenRate) / request.tokenOutRate, | ||
| tokenDecimals | ||
| ); | ||
| uint256 tokenDecimals = isFiat ? 18 : _tokenDecimals(request.tokenOut); |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
| uint256 amountTokenOutWithoutFee = _truncate( | ||
| (request.amountMToken * newMTokenRate) / request.tokenOutRate, | ||
| tokenDecimals | ||
| ); | ||
|
|
||
| _requireAndUpdateAllowance(request.tokenOut, amountTokenOutWithoutFee); | ||
|
|
||
| if (!isFiat) { |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
| // assigning the default value which is gonna be used | ||
| // only for fiat redemptions | ||
| uint256 tokenOutRate = 1e18; | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
| /** | ||
| * @notice minimum amount of BUIDL to redeem. Will redeem at least this amount of BUIDL. | ||
| */ | ||
| uint256 public minBuidlToRedeem; |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| /** | ||
| * @param minBuidlToRedeem new min amount of BUIDL to redeem | ||
| * @param sender address who set new min amount of BUIDL to redeem | ||
| */ | ||
| event SetMinBuidlToRedeem(uint256 minBuidlToRedeem, address sender); | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| @@ -53,7 +64,9 @@ contract RedemptionVaultWIthBUIDL is RedemptionVault { | |||
| uint256 _minAmount, | |||
| FiatRedeptionInitParams calldata _fiatRedemptionInitParams, | |||
| address _requestRedeemer, | |||
| address _buidlRedemption | |||
| address _buidlRedemption, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| buidlSettlement = ISettlement(buidlRedemption.settlement()); | ||
| buidlLiquiditySource = ILiquiditySource(buidlRedemption.liquidity()); | ||
| buidl = IERC20(buidlRedemption.asset()); | ||
| minBuidlToRedeem = _minBuidlToRedeem; |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| } | ||
|
|
||
| /** | ||
| * @notice set min amount of BUIDL to redeem. | ||
| * @param _minBuidlToRedeem min amount of BUIDL to redeem | ||
| */ | ||
| function setMinBuidlToRedeem(uint256 _minBuidlToRedeem) | ||
| external | ||
| onlyVaultAdmin | ||
| { | ||
| minBuidlToRedeem = _minBuidlToRedeem; | ||
|
|
||
| emit SetMinBuidlToRedeem(_minBuidlToRedeem, msg.sender); |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| @@ -97,7 +135,7 @@ contract RedemptionVaultWIthBUIDL is RedemptionVault { | |||
| { | |||
| address user = msg.sender; | |||
|
|
|||
| tokenOut = buidlLiquiditySource.token(); | |||
| require(buidlRedemption.liquidity() == tokenOut, "RVB: invalid token"); | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| uint256 buidlToRedeem = amountTokenOut - contractBalanceTokenOut; | ||
|
|
||
| if (buidlToRedeem < minBuidlToRedeem) { | ||
| buidlToRedeem = minBuidlToRedeem; | ||
| } |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| IERC20 buidl = IERC20(buidlRedemption.asset()); | ||
| uint256 buidlBalance = buidl.balanceOf(address(this)); | ||
| require(buidlBalance >= buidlToRedeem, "RVB: buidlToRedeem > balance"); |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| buidlToRedeem = buidlBalance; | ||
| } |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| buidl.safeIncreaseAllowance(address(buidlRedemption), buidlToRedeem); | ||
| buidlRedemption.redeem(buidlToRedeem); |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| if (token != MANUAL_FULLFILMENT_TOKEN) { | ||
| _requireTokenExists(token); | ||
| } | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
| /** | ||
| * @inheritdoc IManageableVault | ||
| * @dev reverts address zero or equal address(this) | ||
| */ | ||
| function setTokensReceiver(address receiver) external onlyVaultAdmin { | ||
| _validateAddress(receiver, true); | ||
|
|
||
| tokensReceiver = receiver; | ||
|
|
||
| emit SetTokensReceiver(msg.sender, receiver); | ||
| } | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#66.
Fixes issues: #111
| @@ -17,6 +17,11 @@ abstract contract WithSanctionsList is WithMidasAccessControl { | |||
| */ | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#64.
Fixes issues: #103
| @@ -17,6 +17,11 @@ abstract contract WithSanctionsList is WithMidasAccessControl { | |||
| */ | |||
| address public sanctionsList; | |||
|
|
|||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#64.
Fixes issues: #103
| /** | ||
| * @dev leaving a storage gap for futures updates | ||
| */ | ||
| uint256[50] private __gap; | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#64.
Fixes issues: #103
| /** | ||
| * @dev leaving a storage gap for futures updates | ||
| */ | ||
| uint256[50] private __gap; | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#64.
Fixes issues: #103
| /** | ||
| * @dev leaving a storage gap for futures updates | ||
| */ | ||
| uint256[50] private __gap; | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#64.
Fixes issues: #103
| /** | ||
| * @param caller function caller (msg.sender) | ||
| * @param reciever new reciever address | ||
| */ | ||
| event SetTokensReceiver(address indexed caller, address indexed reciever); | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#66.
Fixes issues: #111
| /** | ||
| * @notice set new reciever for tokens. | ||
| * can be called only from permissioned actor. | ||
| * @param reciever new token reciever address | ||
| */ | ||
| function setTokensReceiver(address reciever) external; | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#66.
Fixes issues: #111
| _requireAndUpdateLimit(amountMTokenInCopy); | ||
| _requireAndUpdateAllowance(tokenOutCopy, amountTokenOut); | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#65.
Fixes issues: #110
| address _liquidity, | ||
| address _settlement | ||
| ) { | ||
| constructor(address _asset, address _liquidity) { |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| function settlement() external view returns (address) {} | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| IERC20(asset).transferFrom(msg.sender, address(this), amount); | ||
| IERC20(liquidity).transfer(msg.sender, amount); |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| @@ -25,6 +25,8 @@ import { | |||
| setVariabilityToleranceTest, | |||
| withdrawTest, | |||
| changeTokenFeeTest, | |||
| setTokensReceiverTest, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#66.
Fixes issues: #111
| describe('setTokensReceiver()', () => { | ||
| it('should fail: call from address without REDEMPTION_VAULT_ADMIN_ROLE role', async () => { | ||
| const { owner, redemptionVault, regularAccounts } = await loadFixture( | ||
| defaultDeploy, | ||
| ); | ||
|
|
||
| await setTokensReceiverTest( | ||
| { vault: redemptionVault, owner }, | ||
| regularAccounts[0].address, | ||
| { | ||
| from: regularAccounts[0], | ||
| revertMessage: acErrors.WMAC_HASNT_ROLE, | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it('should fail: call with zero address receiver', async () => { | ||
| const { owner, redemptionVault } = await loadFixture(defaultDeploy); | ||
|
|
||
| await setTokensReceiverTest( | ||
| { vault: redemptionVault, owner }, | ||
| constants.AddressZero, | ||
| { | ||
| revertMessage: 'zero address', | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it('should fail: call with address(this) receiver', async () => { | ||
| const { owner, redemptionVault } = await loadFixture(defaultDeploy); | ||
|
|
||
| await setTokensReceiverTest( | ||
| { vault: redemptionVault, owner }, | ||
| redemptionVault.address, | ||
| { | ||
| revertMessage: 'invalid address', | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it('call from address without REDEMPTION_VAULT_ADMIN_ROLE role', async () => { | ||
| const { owner, redemptionVault, regularAccounts } = await loadFixture( | ||
| defaultDeploy, | ||
| ); | ||
|
|
||
| await setTokensReceiverTest( | ||
| { vault: redemptionVault, owner }, | ||
| regularAccounts[0].address, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#66.
Fixes issues: #111
| await changeTokenAllowanceTest( | ||
| { vault: redemptionVault, owner }, | ||
| constants.AddressZero, | ||
| parseUnits('100'), | ||
| ); | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
| @@ -25,6 +25,7 @@ import { | |||
| setVariabilityToleranceTest, | |||
| withdrawTest, | |||
| changeTokenFeeTest, | |||
| setMinBuidlToRedeem, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| expect(await redemptionVaultWithBUIDL.minBuidlToRedeem()).eq( | ||
| parseUnits('250000', 6), | ||
| ); |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| describe('setMinBuidlToRedeem()', () => { | ||
| it('should fail: call from address without REDEMPTION_VAULT_ADMIN_ROLE role', async () => { | ||
| const { owner, redemptionVaultWithBUIDL, regularAccounts } = | ||
| await loadFixture(defaultDeploy); | ||
|
|
||
| await setMinBuidlToRedeem( | ||
| { vault: redemptionVaultWithBUIDL, owner }, | ||
| parseUnits('100000', 6), | ||
| { | ||
| from: regularAccounts[0], | ||
| revertMessage: acErrors.WMAC_HASNT_ROLE, | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it('call from address with REDEMPTION_VAULT_ADMIN_ROLE role', async () => { | ||
| const { owner, redemptionVaultWithBUIDL } = await loadFixture( | ||
| defaultDeploy, | ||
| ); | ||
| await setMinBuidlToRedeem( | ||
| { vault: redemptionVaultWithBUIDL, owner }, | ||
| parseUnits('100000', 6), | ||
| ); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| @@ -1169,7 +1197,7 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| mTBILL, | |||
| mTokenToUsdDataFeed, | |||
| }, | |||
| stableCoins.dai, | |||
| stableCoins.usdc, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| @@ -1200,7 +1228,7 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| mTBILL, | |||
| mTokenToUsdDataFeed, | |||
| }, | |||
| stableCoins.dai, | |||
| stableCoins.usdc, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| @@ -1277,7 +1305,7 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| mTBILL, | |||
| mTokenToUsdDataFeed, | |||
| }, | |||
| stableCoins.dai, | |||
| stableCoins.usdc, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| @@ -1352,7 +1380,7 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| mTBILL, | |||
| mTokenToUsdDataFeed, | |||
| }, | |||
| stableCoins.dai, | |||
| stableCoins.usdc, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| @@ -1367,14 +1395,53 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| mTBILL, | |||
| mTokenToUsdDataFeed, | |||
| }, | |||
| stableCoins.dai, | |||
| stableCoins.usdc, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| @@ -1387,7 +1454,7 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| } = await loadFixture(defaultDeploy); | |||
| await addPaymentTokenTest( | |||
| { vault: redemptionVaultWithBUIDL, owner }, | |||
| stableCoins.dai, | |||
| stableCoins.usdc, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| @@ -1409,14 +1476,46 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| mTBILL, | |||
| mTokenToUsdDataFeed, | |||
| }, | |||
| stableCoins.dai, | |||
| stableCoins.usdc, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| it('should fail: call when token is invalid', async () => { | ||
| const { | ||
| redemptionVaultWithBUIDL, | ||
| mockedAggregator, | ||
| owner, | ||
| mTBILL, | ||
| stableCoins, | ||
| dataFeed, | ||
| mTokenToUsdDataFeed, | ||
| } = await loadFixture(defaultDeploy); | ||
| await addPaymentTokenTest( | ||
| { vault: redemptionVaultWithBUIDL, owner }, | ||
| stableCoins.usdc, | ||
| dataFeed.address, | ||
| 0, | ||
| true, | ||
| ); | ||
| await redeemInstantTest( | ||
| { | ||
| redemptionVault: redemptionVaultWithBUIDL, | ||
| owner, | ||
| mTBILL, | ||
| mTokenToUsdDataFeed, | ||
| }, | ||
| stableCoins.dai, | ||
| 99_999, | ||
| { | ||
| revertMessage: 'RVB: invalid token', | ||
| }, | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| @@ -1685,10 +1784,10 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| mTBILL, | |||
| mTokenToUsdDataFeed, | |||
| }, | |||
| constants.AddressZero, | |||
| stableCoins.usdc, | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#71.
Fixes issues: #41
| 100000, | ||
| { | ||
| revertMessage: 'ERC20: transfer amount exceeds balance', | ||
| revertMessage: 'RVB: buidlToRedeem > balance', |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| @@ -1772,7 +1871,7 @@ describe('RedemptionVaultWithBUIDL', function () { | |||
| redemptionVaultWithBUIDL.address, | |||
| ); | |||
| expect(buidlBalanceAfter).eq( | |||
| buidlBalanceBefore.sub(parseUnits('1000', 8)), | |||
| buidlBalanceBefore.sub(parseUnits('250000', 6)), | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| await changeTokenAllowanceTest( | ||
| { vault: redemptionVault, owner }, | ||
| constants.AddressZero, | ||
| parseUnits('100'), | ||
| ); |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
| const buidlRedemption = await new RedemptionTest__factory(owner).deploy( | ||
| buidl.address, | ||
| liquiditySource.address, | ||
| settlement.address, | ||
| stableCoins.usdc.address, |
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| ); | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| @@ -775,6 +765,8 @@ export const defaultDeploy = async () => { | |||
| }, | |||
| requestRedeemer.address, | |||
| buidlRedemption.address, | |||
| parseUnits('250000', 6), | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| @@ -1,6 +1,6 @@ | |||
| import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; | |||
| import { expect } from 'chai'; | |||
| import { BigNumberish } from 'ethers'; | |||
| import { BigNumber, BigNumberish } from 'ethers'; | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#68.
Fixes issues: #100
| export const setTokensReceiverTest = async ( | ||
| { vault, owner }: CommonParamsChangePaymentToken, | ||
| newReceiver: string, | ||
| opt?: OptionalCommonParams, | ||
| ) => { | ||
| if (opt?.revertMessage) { | ||
| await expect( | ||
| vault.connect(opt?.from ?? owner).setTokensReceiver(newReceiver), | ||
| ).revertedWith(opt?.revertMessage); | ||
| return; | ||
| } | ||
|
|
||
| await expect(vault.connect(opt?.from ?? owner).setTokensReceiver(newReceiver)) | ||
| .to.emit( | ||
| vault, | ||
| vault.interface.events['SetTokensReceiver(address,address)'].name, | ||
| ) | ||
| .withArgs((opt?.from ?? owner).address, newReceiver).to.not.reverted; | ||
|
|
||
| const feeReceiver = await vault.tokensReceiver(); | ||
| expect(feeReceiver).eq(newReceiver); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#66.
Fixes issues: #111
| @@ -276,7 +276,7 @@ export const redeemFiatRequestTest = async ( | |||
| expect(request.tokenOut).eq(manualToken); | |||
| expect(request.amountMToken).eq(amountInWithoutFee); | |||
| expect(request.mTokenRate).eq(mTokenRate); | |||
| expect(request.tokenOutRate).eq(0); | |||
| expect(request.tokenOutRate).eq(parseUnits('1')); | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
| @@ -462,6 +462,8 @@ export const safeApproveRedeemRequestTest = async ( | |||
|
|
|||
| const tokenDecimals = await tokenContract.decimals(); | |||
|
|
|||
| console.log(requestDataBefore.amountMToken, requestDataBefore.tokenOutRate); | |||
There was a problem hiding this comment.
Change related to RedDuck-Software/midas-contracts#69.
Fixes issues: #102
Fix Review of
Repo:
RedDuck-Software/midas-contractsCommit Hash:
21ab5ffa59ed9265c771bc9df37f6794b9b4f73a