From d6cd01ea04724209e776a69a5fca9851c6bc0e47 Mon Sep 17 00:00:00 2001 From: John Date: Mon, 15 Nov 2021 10:08:03 +0100 Subject: [PATCH 1/2] Fix exchange fee collection --- contracts/multiply/Exchange.sol | 13 ++++-- test/exchange.js | 81 +++++++++++---------------------- 2 files changed, 37 insertions(+), 57 deletions(-) diff --git a/contracts/multiply/Exchange.sol b/contracts/multiply/Exchange.sol index bb7d3c1..5dc1377 100644 --- a/contracts/multiply/Exchange.sol +++ b/contracts/multiply/Exchange.sol @@ -92,13 +92,20 @@ contract Exchange { return balance; } - function _collectFee(address asset, uint256 fromAmount) internal returns (uint256) { + function _collectFeeTokenToDai(address asset, uint256 fromAmount) internal returns (uint256) { uint256 feeToTransfer = (fromAmount.mul(fee)).div(feeBase); IERC20(asset).safeTransfer(feeBeneficiaryAddress, feeToTransfer); emit FeePaid(feeBeneficiaryAddress, feeToTransfer); return fromAmount.sub(feeToTransfer); } + function _collectFeeDaiToToken(address asset, uint256 fromAmount) internal returns (uint256) { + uint256 feeToTransfer = fromAmount.sub((fromAmount.mul(feeBase)).div(feeBase.add(fee))); + IERC20(asset).safeTransfer(feeBeneficiaryAddress, feeToTransfer); + emit FeePaid(feeBeneficiaryAddress, feeToTransfer); + return fromAmount.sub(feeToTransfer); + } + function _transferOut( address asset, address to, @@ -116,7 +123,7 @@ contract Exchange { ) public { _transferIn(msg.sender, DAI_ADDRESS, amount); - uint256 _amount = _collectFee(DAI_ADDRESS, amount); + uint256 _amount = _collectFeeDaiToToken(DAI_ADDRESS, amount); uint256 balance = _swap(DAI_ADDRESS, asset, _amount, receiveAtLeast, callee, withData); uint256 daiBalance = IERC20(DAI_ADDRESS).balanceOf(address(this)); @@ -138,7 +145,7 @@ contract Exchange { _transferIn(msg.sender, asset, amount); uint256 balance = _swap(asset, DAI_ADDRESS, amount, receiveAtLeast, callee, withData); - uint256 _balance = _collectFee(DAI_ADDRESS, balance); + uint256 _balance = _collectFeeTokenToDai(DAI_ADDRESS, balance); uint256 assetBalance = IERC20(asset).balanceOf(address(this)); diff --git a/test/exchange.js b/test/exchange.js index 450f551..c1b947c 100644 --- a/test/exchange.js +++ b/test/exchange.js @@ -21,6 +21,8 @@ const BigNumber = require('bignumber.js') const { zero } = require('./utils') const _ = require('lodash') +BigNumber.config({ EXPONENTIAL_AT: 100 }) + const AGGREGATOR_V3_ADDRESS = '0x11111112542d85b3ef69ae05771c2dccff4faa26' const ethers = hre.ethers @@ -39,7 +41,8 @@ function asPercentageValue(value, base) { } describe('Exchange', async function () { - let provider, signer, address, exchange, WETH, DAI, feeBeneficiary, slippage, fee, snapshotId + let provider, signer, address, exchange, WETH, DAI, feeBeneficiary, slippage, snapshotId + let fee = asPercentageValue(FEE, FEE_BASE) this.beforeAll(async function () { let [_provider, _signer] = await init(undefined, provider, signer) @@ -49,7 +52,6 @@ describe('Exchange', async function () { feeBeneficiary = addressRegistryFactory(undefined, undefined).feeRecepient slippage = asPercentageValue(8, 100) - fee = asPercentageValue(FEE, FEE_BASE) const Exchange = await ethers.getContractFactory('Exchange', signer) exchange = await Exchange.deploy(address, feeBeneficiary, fee.value.toString()) @@ -97,11 +99,11 @@ describe('Exchange', async function () { await expect(tx).to.revertedWith('Exchange / Unauthorized Caller') }) - it.only('should allow beneficiary to update the fee', async function () { - const toTransferAmount = "0x"+amountToWei(1,18).toString(16); - let tx0 = await signer.populateTransaction({to:feeBeneficiary,value:toTransferAmount}); - await signer.sendTransaction(tx0); - await provider.send("hardhat_impersonateAccount", [feeBeneficiary]); + it('should allow beneficiary to update the fee', async function () { + const toTransferAmount = '0x' + amountToWei(1, 18).toString(16) + let tx0 = await signer.populateTransaction({ to: feeBeneficiary, value: toTransferAmount }) + await signer.sendTransaction(tx0) + await provider.send('hardhat_impersonateAccount', [feeBeneficiary]) const benef = await ethers.provider.getSigner(feeBeneficiary) let tx = await exchange.connect(benef).setFee('3') }) @@ -118,7 +120,7 @@ describe('Exchange', async function () { amountInWei, exchange.address, slippage.value.toString(), - ALLOWED_PROTOCOLS + ALLOWED_PROTOCOLS, ) initialDaiWalletBalance = convertToBigNumber(await balanceOf(MAINNET_ADRESSES.ETH, address)) @@ -139,19 +141,6 @@ describe('Exchange', async function () { await provider.send('evm_revert', [snapshotId]) }) - it('should not happen if it is triggered from unauthorized caller', async () => { - let tx = exchange - .connect(provider.getSigner(1)) - .swapTokenForDai( - MAINNET_ADRESSES.ETH, - amountToWei(1).toFixed(0), - amountFromWei(1).toFixed(0), - AGGREGATOR_V3_ADDRESS, - 0, - ) - await expect(tx).to.revertedWith('Exchange / Unauthorized Caller') - }) - describe('when transferring an exact amount to the exchange', async function () { let localSnapshotId, initialWethWalletBalance @@ -468,18 +457,18 @@ describe('Exchange', async function () { }) describe('DAI for Asset', async function () { - let initialDaiWalletBalance, amountWithFeeInWei + let initialDaiWalletBalance, amountInWei, amountWithFeeInWei this.beforeAll(async function () { - const amountInWei = amountToWei(new BigNumber(1000)) - amountWithFeeInWei = amountInWei.div(ONE.minus(fee.asDecimal)).toFixed(0) + amountInWei = amountToWei(new BigNumber(1000)) + amountWithFeeInWei = amountInWei.times(ONE.plus(fee.asDecimal)).toFixed(0) const response = await exchangeFromDAI( MAINNET_ADRESSES.ETH, amountInWei.toFixed(0), slippage.value.toString(), exchange.address, - ALLOWED_PROTOCOLS + ALLOWED_PROTOCOLS, ) const { @@ -495,20 +484,6 @@ describe('Exchange', async function () { receiveAtLeastInWei = amountToWei(receiveAtLeast).toFixed(0) }) - it('should not happen if it is triggered from unauthorized caller', async () => { - let tx = exchange - .connect(provider.getSigner(1)) - .swapDaiForToken( - MAINNET_ADRESSES.ETH, - amountToWei(1).toFixed(0), - amountFromWei(1).toFixed(0), - AGGREGATOR_V3_ADDRESS, - 0, - ) - - await expect(tx).to.revertedWith('Exchange / Unauthorized Caller') - }) - describe('when transferring an exact amount to the exchange', async function () { let localSnapshotId @@ -579,7 +554,7 @@ describe('Exchange', async function () { await balanceOf(MAINNET_ADRESSES.MCD_DAI, feeBeneficiary), ) - const expectedCollectedFee = convertToBigNumber(amountWithFeeInWei).times(fee.asDecimal) + const expectedCollectedFee = convertToBigNumber(amountInWei).times(fee.asDecimal) expect(beneficiaryDaiBalance.toFixed(0)).to.be.eq(expectedCollectedFee.toFixed(0)) }) }) @@ -630,11 +605,11 @@ describe('Exchange', async function () { it('should exchange all needed amount and return the surplus', async function () { const wethBalance = convertToBigNumber(await balanceOf(MAINNET_ADRESSES.ETH, address)) const daiBalance = convertToBigNumber(await balanceOf(MAINNET_ADRESSES.MCD_DAI, address)) + const amount = amountFromWei(new BigNumber(moreThanTheTransferAmount)) - const surplusFee = amountToWei(surplusAmount.times(fee.asDecimal)) - - expect(daiBalance.toFixed(0)).to.equals( - initialDaiWalletBalance.minus(amountWithFeeInWei).minus(surplusFee).toFixed(0), + const swapFee = amountToWei(amount.minus(amount.div(ONE.plus(fee.asDecimal)))).toFixed(0) + expect(amountFromWei(daiBalance).toFixed(6)).to.equals( + amountFromWei(initialDaiWalletBalance.minus(amountInWei).minus(swapFee)).toFixed(6), ) expect(wethBalance.gte(convertToBigNumber(receiveAtLeastInWei))).to.be.true }) @@ -656,16 +631,14 @@ describe('Exchange', async function () { }) it('should have collected fee', async function () { - const beneficiaryDaiBalance = convertToBigNumber( - await balanceOf(MAINNET_ADRESSES.MCD_DAI, feeBeneficiary), + const beneficiaryDaiBalance = amountFromWei( + convertToBigNumber(await balanceOf(MAINNET_ADRESSES.MCD_DAI, feeBeneficiary)), ) - const surplusFee = amountToWei(surplusAmount.times(fee.asDecimal)) + const amount = amountFromWei(moreThanTheTransferAmount) + const expectedCollectedFee = amount.minus(amount.div(ONE.plus(fee.asDecimal))) - const expectedCollectedFee = convertToBigNumber(amountWithFeeInWei).times(fee.asDecimal) - expect(beneficiaryDaiBalance.toFixed(0)).to.be.eq( - expectedCollectedFee.plus(surplusFee).toFixed(0), - ) + expect(beneficiaryDaiBalance.toFixed(10)).to.be.eq(expectedCollectedFee.toFixed(10)) }) }) @@ -956,7 +929,7 @@ describe('Exchange', async function () { amountInWei, exchange.address, slippage.value.toString(), - ALLOWED_PROTOCOLS + ALLOWED_PROTOCOLS, ) const { @@ -1070,7 +1043,7 @@ describe('Exchange', async function () { amountInWei.toFixed(0), slippage.value.toString(), exchange.address, - ALLOWED_PROTOCOLS + ALLOWED_PROTOCOLS, ) const { @@ -1168,7 +1141,7 @@ describe('Exchange', async function () { this.beforeAll(async function () { localSnapshotId = await provider.send('evm_snapshot', []) const amountInWei = amountToWei(new BigNumber(1000)) - amountWithFeeInWei = amountInWei.div(ONE.minus(fee.asDecimal)).toFixed(0) + amountWithFeeInWei = amountInWei.times(ONE.plus(fee.asDecimal)).toFixed(0) await swapTokens( MAINNET_ADRESSES.ETH, From af3ae9d494508d6704554012c876bbc35e027abd Mon Sep 17 00:00:00 2001 From: John Date: Mon, 15 Nov 2021 11:04:10 +0100 Subject: [PATCH 2/2] Hardcode DAI address in the fee collection functions --- contracts/multiply/Exchange.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/multiply/Exchange.sol b/contracts/multiply/Exchange.sol index 5dc1377..8d2da8e 100644 --- a/contracts/multiply/Exchange.sol +++ b/contracts/multiply/Exchange.sol @@ -92,16 +92,16 @@ contract Exchange { return balance; } - function _collectFeeTokenToDai(address asset, uint256 fromAmount) internal returns (uint256) { + function _collectFeeTokenToDai(uint256 fromAmount) internal returns (uint256) { uint256 feeToTransfer = (fromAmount.mul(fee)).div(feeBase); - IERC20(asset).safeTransfer(feeBeneficiaryAddress, feeToTransfer); + IERC20(DAI_ADDRESS).safeTransfer(feeBeneficiaryAddress, feeToTransfer); emit FeePaid(feeBeneficiaryAddress, feeToTransfer); return fromAmount.sub(feeToTransfer); } - function _collectFeeDaiToToken(address asset, uint256 fromAmount) internal returns (uint256) { + function _collectFeeDaiToToken(uint256 fromAmount) internal returns (uint256) { uint256 feeToTransfer = fromAmount.sub((fromAmount.mul(feeBase)).div(feeBase.add(fee))); - IERC20(asset).safeTransfer(feeBeneficiaryAddress, feeToTransfer); + IERC20(DAI_ADDRESS).safeTransfer(feeBeneficiaryAddress, feeToTransfer); emit FeePaid(feeBeneficiaryAddress, feeToTransfer); return fromAmount.sub(feeToTransfer); } @@ -123,7 +123,7 @@ contract Exchange { ) public { _transferIn(msg.sender, DAI_ADDRESS, amount); - uint256 _amount = _collectFeeDaiToToken(DAI_ADDRESS, amount); + uint256 _amount = _collectFeeDaiToToken(amount); uint256 balance = _swap(DAI_ADDRESS, asset, _amount, receiveAtLeast, callee, withData); uint256 daiBalance = IERC20(DAI_ADDRESS).balanceOf(address(this)); @@ -145,7 +145,7 @@ contract Exchange { _transferIn(msg.sender, asset, amount); uint256 balance = _swap(asset, DAI_ADDRESS, amount, receiveAtLeast, callee, withData); - uint256 _balance = _collectFeeTokenToDai(DAI_ADDRESS, balance); + uint256 _balance = _collectFeeTokenToDai(balance); uint256 assetBalance = IERC20(asset).balanceOf(address(this));