From 9763126a0d5b82b8b0313b735d4113750b06c522 Mon Sep 17 00:00:00 2001 From: Nelson Galdeman Date: Tue, 17 Jan 2023 15:59:08 +0000 Subject: [PATCH 1/2] feat: added min residual token transfers for A & B tokens in replace of boolean Allows the dapp to decide if the extra gas on the transfer is worth over the residual tokens value by setting a minimum --- contracts/Zap.sol | 39 ++++++++----- test/Zap.test.ts | 145 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 149 insertions(+), 35 deletions(-) diff --git a/contracts/Zap.sol b/contracts/Zap.sol index 3b1c733..8b18b00 100644 --- a/contracts/Zap.sol +++ b/contracts/Zap.sol @@ -127,7 +127,8 @@ contract Zap is Ownable, ReentrancyGuard { @param swapTokenB Data for swap tx pool's token B - amounts, path & DEX @param receiver LP token receiver address @param affiliate Affiliate address - @param transferResidual Set false to save gas by donating the residual remaining after a ZapTx + @param transferResidualMinTokenA Set mimimum amount of token A transfer back any residual amount, otherwise donated to contract + @param transferResidualMinTokenB Set mimimum amount of token B transfer back any residual amount, otherwise donated to contract @return lpBought Amount of LP tokens transferred to receiver @return lpToken LP token address */ @@ -137,7 +138,8 @@ contract Zap is Ownable, ReentrancyGuard { SwapTx calldata swapTokenB, address receiver, address affiliate, - bool transferResidual + uint256 transferResidualMinTokenA, + uint256 transferResidualMinTokenB ) external payable nonReentrant stopInEmergency returns (uint256 lpBought, address lpToken) { // check if start token is the same for both paths if (swapTokenA.path[0] != swapTokenB.path[0]) revert InvalidStartPath(); @@ -149,7 +151,8 @@ contract Zap is Ownable, ReentrancyGuard { swapTokenA, swapTokenB, zap, - transferResidual + transferResidualMinTokenA, + transferResidualMinTokenB ); if (lpBought < zap.amountLPMin) revert InsufficientMinAmount(); @@ -332,7 +335,8 @@ contract Zap is Ownable, ReentrancyGuard { SwapTx calldata swapTokenA, SwapTx calldata swapTokenB, ZapInTx calldata zap, - bool transferResidual + uint256 transferResidualMinTokenA, + uint256 transferResidualMinTokenB ) internal returns (uint256 liquidity, address lpToken) { // check if dex address is valid and supported (address router, address factory) = getSupportedDEX(zap.dexIndex); @@ -357,7 +361,8 @@ contract Zap is Ownable, ReentrancyGuard { swapTokenA.amountMin, swapTokenB.amountMin, router, - transferResidual + transferResidualMinTokenA, + transferResidualMinTokenB ); } @@ -586,7 +591,8 @@ contract Zap is Ownable, ReentrancyGuard { @param amountAMin The minimum amount of token A to receive @param amountBMin The minimum amount of token A to receive @param router The address of platform's router - @param transferResidual Set false to save gas by donating the residual remaining after a ZapTx + @param transferResidualMinTokenA Set mimimum amount of token A transfer back any residual amount, otherwise donated to contract + @param transferResidualMinTokenB Set mimimum amount of token B transfer back any residual amount, otherwise donated to contract @return amountA Token A amount added to LP @return amountB Token B amount added to LP @return liquidity LP tokens minted @@ -599,7 +605,8 @@ contract Zap is Ownable, ReentrancyGuard { uint256 amountAMin, uint256 amountBMin, address router, - bool transferResidual + uint256 transferResidualMinTokenA, + uint256 transferResidualMinTokenB ) internal returns (uint256 amountA, uint256 amountB, uint256 liquidity) { _approveTokenIfNeeded(tokenA, amountADesired, router); _approveTokenIfNeeded(tokenB, amountBDesired, router); @@ -615,16 +622,16 @@ contract Zap is Ownable, ReentrancyGuard { deadline ); - if (transferResidual) { - // returning residue in tokenA, if any - if (amountADesired - amountA > 0) { - TransferHelper.safeTransfer(tokenA, msg.sender, (amountADesired - amountA)); - } + uint256 residualTokenA = amountADesired - amountA; + // returning residue in tokenA, if any + if (residualTokenA > 0 && residualTokenA >= transferResidualMinTokenA) { + TransferHelper.safeTransfer(tokenA, msg.sender, residualTokenA); + } - // returning residue in tokenB, if any - if (amountBDesired - amountB > 0) { - TransferHelper.safeTransfer(tokenB, msg.sender, (amountBDesired - amountB)); - } + uint256 residualTokenB = amountBDesired - amountB; + // returning residue in tokenB, if any + if (residualTokenB > 0 && residualTokenB >= transferResidualMinTokenB) { + TransferHelper.safeTransfer(tokenB, msg.sender, residualTokenB); } } diff --git a/test/Zap.test.ts b/test/Zap.test.ts index 0c47de2..e4a79a7 100644 --- a/test/Zap.test.ts +++ b/test/Zap.test.ts @@ -132,7 +132,8 @@ describe.only("Zap", function () { {amount: zeroBN, amountMin: zeroBN, path: [WXDAI.address, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true + zeroBN, + zeroBN ) ).to.be.revertedWith("InvalidInputAmount()") @@ -143,7 +144,8 @@ describe.only("Zap", function () { {amount: zeroBN, amountMin: zeroBN, path: [AddressZero, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) ).to.be.revertedWith("InvalidInputAmount()") @@ -155,7 +157,8 @@ describe.only("Zap", function () { {amount: amountIn, amountMin: zeroBN, path: [WXDAI.address, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true + zeroBN, + zeroBN ) ).to.be.revertedWith("InvalidStartPath()") @@ -293,7 +296,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [DXD.address, WETH.address], dexIndex: dexIndex1}, impersonated.address, randomSigner.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -319,7 +323,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [DXD.address, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -350,7 +355,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path:[DXD.address] , dexIndex: dexIndex1}, impersonated.address, randomSigner.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -379,7 +385,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [DXD.address, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -406,7 +413,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [DXD.address, WXDAI.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -433,7 +441,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [WXDAI.address, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -448,7 +457,7 @@ describe.only("Zap", function () { .withArgs(impersonated.address, impersonated.address,WXDAI.address, totalAmount, cowWeth.address, lpBought) }) - it("zap in native currency (xdai) token to cow/weth", async function () { + it("zap in native currency (xdai) token to cow/weth with residual transfer", async function () { const totalAmount = ethers.utils.parseEther("1") const nativeCurrencyBalanceInit = await impersonated.getBalance() expect(nativeCurrencyBalanceInit).to.be.above(0) @@ -463,7 +472,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [AddressZero, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: totalAmount, gasLimit: 9999999} ) @@ -474,12 +484,102 @@ describe.only("Zap", function () { expect(lpBought).to.be.above(0) expect(nativeCurrencyBalanceInit).to.be.above(nativeCurrencyBalance) - await expect(txZapIn).to.emit(zap, "ZapIn") .withArgs(impersonated.address, impersonated.address,AddressZero, totalAmount, cowWeth.address, lpBought) + + // Get transfer event data + const receipt = await ethers.provider.getTransactionReceipt(txZapIn.hash) + const interfaceEvent = new ethers.utils.Interface(["event Transfer(address indexed from, address indexed to, uint256 amount)"]) + + // Search WETH Transfer event + let wethTransferIndex = -1; + for (let i = 0; i < receipt.logs.length; i++) { + if (receipt.logs[i].address.toLowerCase() == WETH.address.toLowerCase()) { + // Test we got a transfer back from token A residuals + let data = receipt.logs[i].data + let topics = receipt.logs[i].topics + let event; + try { + event = interfaceEvent.decodeEventLog("Transfer", data, topics) + } catch { + continue + } + + if (event.from.toLowerCase() == zap.address.toLowerCase() + && event.to.toLowerCase() == impersonated.address.toLowerCase()) { + wethTransferIndex = i; + break; + } + } + } + + if (wethTransferIndex === -1) { + expect("").to.be.equal("Missing residual token transfer") + } }) + it("zap in native currency (xdai) token to cow/weth without residual transfer", async function () { + const totalAmount = ethers.utils.parseEther("1") + const nativeCurrencyBalanceInit = await impersonated.getBalance() + expect(nativeCurrencyBalanceInit).to.be.above(0) + + const lpBalanceInit = await cowWeth.balanceOf(impersonated.address) + const tokenInBalanceInit = await WXDAI.balanceOf(impersonated.address) + + await WXDAI.connect(impersonated).approve(zap.address, totalAmount) + const txZapIn = await zap.connect(impersonated).zapIn( + {amountAMin: zeroBN, amountBMin: zeroBN, amountLPMin: zeroBN, dexIndex: dexIndex1}, + {amount: totalAmount.div(2), amountMin: zeroBN, path:[AddressZero, WETH.address, COW.address] , dexIndex: dexIndex1}, + {amount: totalAmount.div(2), amountMin: zeroBN, path: [AddressZero, WETH.address], dexIndex: dexIndex1}, + impersonated.address, + impersonated.address, + zeroBN, + BigNumber.from("9999999999999999999999999"), + {value: totalAmount, gasLimit: 9999999} + ) + + const tokenInBalance = await WXDAI.balanceOf(impersonated.address) + const lpBalance = await cowWeth.balanceOf(impersonated.address) + const lpBought = lpBalance.sub(lpBalanceInit) + const nativeCurrencyBalance = await impersonated.getBalance() + + expect(lpBought).to.be.above(0) + expect(nativeCurrencyBalanceInit).to.be.above(nativeCurrencyBalance) + + await expect(txZapIn).to.emit(zap, "ZapIn") + .withArgs(impersonated.address, impersonated.address,AddressZero, totalAmount, cowWeth.address, lpBought) + + // Get transfer event data + const receipt = await ethers.provider.getTransactionReceipt(txZapIn.hash) + const interfaceEvent = new ethers.utils.Interface(["event Transfer(address indexed from, address indexed to, uint256 amount)"]) + + // Search WETH Transfer event + let wethTransferIndex = -1; + for (let i = 0; i < receipt.logs.length; i++) { + if (receipt.logs[i].address.toLowerCase() == WETH.address.toLowerCase()) { + // Test we got a transfer back from token A residuals + let data = receipt.logs[i].data + let topics = receipt.logs[i].topics + let event; + try { + event = interfaceEvent.decodeEventLog("Transfer", data, topics) + } catch { + continue + } + + if (event.from.toLowerCase() == zap.address.toLowerCase() + && event.to.toLowerCase() == impersonated.address.toLowerCase()) { + wethTransferIndex = i; + break; + } + } + } + + if (wethTransferIndex !== -1) { + expect("").to.be.equal("Residual token transfer done") + } + }) }) describe("Zap Out", function () { @@ -495,7 +595,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path:[DXD.address] , dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -545,7 +646,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [DXD.address, WXDAI.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -593,7 +695,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [WXDAI.address, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -644,7 +747,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [AddressZero, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: totalAmount, gasLimit: 9999999} ) @@ -696,7 +800,8 @@ describe.only("Zap", function () { {amount: totalAmount.div(2), amountMin: zeroBN, path: [AddressZero, WETH.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: totalAmount, gasLimit: 9999999} ) @@ -754,7 +859,8 @@ describe.only("Zap", function () { {amount: amountB, amountMin: zeroBN, path: [WXDAI.address, GNO.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) @@ -786,7 +892,8 @@ describe.only("Zap", function () { {amount: amountB, amountMin: zeroBN, path: [WXDAI.address, GNO.address], dexIndex: dexIndex1}, impersonated.address, impersonated.address, - true, + zeroBN, + zeroBN, {value: zeroBN, gasLimit: 9999999} ) From 03b4c8fe4ccb99c4d6a17d7773a3b80c68751af3 Mon Sep 17 00:00:00 2001 From: Nelson Galdeman Date: Tue, 24 Jan 2023 16:29:17 +0100 Subject: [PATCH 2/2] refactor: updates transfer residual condition to save gas --- contracts/Zap.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/Zap.sol b/contracts/Zap.sol index 8b18b00..66b58ef 100644 --- a/contracts/Zap.sol +++ b/contracts/Zap.sol @@ -624,13 +624,13 @@ contract Zap is Ownable, ReentrancyGuard { uint256 residualTokenA = amountADesired - amountA; // returning residue in tokenA, if any - if (residualTokenA > 0 && residualTokenA >= transferResidualMinTokenA) { + if (residualTokenA > transferResidualMinTokenA) { TransferHelper.safeTransfer(tokenA, msg.sender, residualTokenA); } uint256 residualTokenB = amountBDesired - amountB; // returning residue in tokenB, if any - if (residualTokenB > 0 && residualTokenB >= transferResidualMinTokenB) { + if (residualTokenB > transferResidualMinTokenB) { TransferHelper.safeTransfer(tokenB, msg.sender, residualTokenB); } }