From 2212b95d1b73282e259723981183f1b98c52d623 Mon Sep 17 00:00:00 2001 From: Pano Skylakis <49353965+pano-skylakis@users.noreply.github.com> Date: Fri, 8 Dec 2023 16:28:16 +1300 Subject: [PATCH] [TP-1811] Always call refundETH for native token-in swaps (#1267) Co-authored-by: Pano Skylakis --- .../sdk-sample-app/src/components/Example.tsx | 41 +++++----- ...ange.getUnsignedSwapTxFromAmountIn.test.ts | 82 +++++++++++++++++++ .../dex/sdk/src/lib/transactionUtils/swap.ts | 14 +++- 3 files changed, 112 insertions(+), 25 deletions(-) diff --git a/packages/internal/dex/sdk-sample-app/src/components/Example.tsx b/packages/internal/dex/sdk-sample-app/src/components/Example.tsx index 03b5798891..1fb9fae56e 100644 --- a/packages/internal/dex/sdk-sample-app/src/components/Example.tsx +++ b/packages/internal/dex/sdk-sample-app/src/components/Example.tsx @@ -27,6 +27,8 @@ const allTokens: Token[] = [ { symbol: 'zkSRE', address: '0x43566cAB87CC147C95e2895E7b972E19993520e4' }, { symbol: 'zkCORE', address: '0x4B96E7b7eA673A996F140d5De411a97b7eab934E' }, { symbol: 'zkWAT', address: '0xaC953a0d7B67Fae17c87abf79f09D0f818AC66A2' }, + { symbol: 'zkCATS', address: '0xb95B75B4E4c09F04d5DA6349861BF1b6F163D78c' }, + { symbol: 'zkYEET', address: '0x8AC26EfCbf5D700b37A27aA00E6934e6904e7B8e' }, ]; const buildExchange = (secondaryFeeRecipient: string, secondaryFeePercentage: number) => { @@ -109,7 +111,7 @@ export function Example() { const performSwap = async (result: TransactionResponse) => { setSwapTransaction(null); setIsFetching(true); - const provider = new ethers.providers.Web3Provider((window as any).ethereum) + const provider = new ethers.providers.Web3Provider((window as any).ethereum); const signer = provider.getSigner(); // Approve the ERC20 spend @@ -157,13 +159,13 @@ export function Example() {
@@ -176,6 +178,7 @@ export function Example() {
{ setOutputToken({ @@ -208,9 +211,8 @@ export function Example() { symbol: addressToSymbolMapping[e.target.value], }); setResult(null); - setSwapTransaction(null) - }} - > + setSwapTransaction(null); + }}> {allTokens.map((token) => (
-
+
{tradeType === 'exactInput' && inputToken && ( @@ -232,15 +234,14 @@ export function Example() { {inputToken && outputToken && ( )} -
+
{error && } {result && ( <> @@ -262,10 +263,9 @@ export function Example() { <> {isFetching &&

loading...

} @@ -275,10 +275,9 @@ export function Example() { Swap successful! Check your metamask to see updated token balances + target='_blank'> Transaction diff --git a/packages/internal/dex/sdk/src/exchange.getUnsignedSwapTxFromAmountIn.test.ts b/packages/internal/dex/sdk/src/exchange.getUnsignedSwapTxFromAmountIn.test.ts index b7b21f19bb..3fee4e5d52 100644 --- a/packages/internal/dex/sdk/src/exchange.getUnsignedSwapTxFromAmountIn.test.ts +++ b/packages/internal/dex/sdk/src/exchange.getUnsignedSwapTxFromAmountIn.test.ts @@ -42,6 +42,7 @@ import { expectToBeString, decodeMulticallExactInputWithoutFees, buildBlock, + refundETHFunctionSignature, TEST_MAX_PRIORITY_FEE_PER_GAS, TEST_BASE_FEE, } from './test/utils'; @@ -433,6 +434,44 @@ describe('getUnsignedSwapTxFromAmountIn', () => { expect(result.approval).toBeNull(); }); + + it('should include a call to refundETH as the final step of the multicall calldata', async () => { + mockRouterImplementation({ + pools: [createPool(nativeTokenService.wrappedToken, FUN_TEST_TOKEN)], + }); + + const swapRouterInterface = SwapRouter.INTERFACE; + const paymentsInterface = PaymentsExtended.INTERFACE; + const exchange = new Exchange(TEST_DEX_CONFIGURATION); + + // Sell 100 native tokens for X amount of FUN where the exchange rate is 1 token-in : 10 token-out + // Route is WIMX > FUN + const { swap } = await exchange.getUnsignedSwapTxFromAmountIn( + TEST_FROM_ADDRESS, + 'native', + FUN_TEST_TOKEN.address, + newAmountFromString('100', nativeTokenService.nativeToken).value, + 3, // 3 % slippage + ); + + expectToBeDefined(swap.transaction.data); + expectToBeDefined(swap.transaction.value); + const calldata = swap.transaction.data.toString(); + + const topLevelParams = swapRouterInterface.decodeFunctionData('multicall(uint256,bytes[])', calldata); + + expect(topLevelParams.data.length).toBe(2); // expect that there are two calls in the multicall + const swapTransactionCalldata = topLevelParams.data[0]; + const refundETHTransactionCalldata = topLevelParams.data[1]; + + expectToBeString(swapTransactionCalldata); + expectToBeString(refundETHTransactionCalldata); + + const decodedRefundEthTx = paymentsInterface.decodeFunctionData('refundETH', refundETHTransactionCalldata); + + expect(topLevelParams.data[1]).toEqual(refundETHFunctionSignature); + expect(decodedRefundEthTx.length).toEqual(0); // expect that the refundETH call has no parameters + }); }); describe('with a single pool and a native token out', () => { @@ -583,6 +622,49 @@ describe('getUnsignedSwapTxFromAmountIn', () => { }); describe('with multiple pools', () => { + describe('with a native token in', () => { + it('should include a call to refundETH as the final step of the multicall calldata', async () => { + mockRouterImplementation({ + pools: [ + createPool(nativeTokenService.wrappedToken, USDC_TEST_TOKEN), + createPool(USDC_TEST_TOKEN, FUN_TEST_TOKEN), + ], + }); + + const swapRouterInterface = SwapRouter.INTERFACE; + const paymentsInterface = PaymentsExtended.INTERFACE; + const exchange = new Exchange(TEST_DEX_CONFIGURATION); + + // Sell 100 native tokens for X amount of FUN where the exchange rate is 1 token-in : 10 token-out + // Route is WIMX > USDC > FUN + const { swap } = await exchange.getUnsignedSwapTxFromAmountIn( + TEST_FROM_ADDRESS, + 'native', + FUN_TEST_TOKEN.address, + newAmountFromString('100', nativeTokenService.nativeToken).value, + 3, // 3 % slippage + ); + + expectToBeDefined(swap.transaction.data); + expectToBeDefined(swap.transaction.value); + const calldata = swap.transaction.data.toString(); + + const topLevelParams = swapRouterInterface.decodeFunctionData('multicall(uint256,bytes[])', calldata); + + expect(topLevelParams.data.length).toBe(2); // expect that there are two calls in the multicall + const swapTransactionCalldata = topLevelParams.data[0]; + const refundETHTransactionCalldata = topLevelParams.data[1]; + + expectToBeString(swapTransactionCalldata); + expectToBeString(refundETHTransactionCalldata); + + const decodedRefundEthTx = paymentsInterface.decodeFunctionData('refundETH', refundETHTransactionCalldata); + + expect(topLevelParams.data[1]).toEqual(refundETHFunctionSignature); + expect(decodedRefundEthTx.length).toEqual(0); // expect that the refundETH call has no parameters + }); + }); + describe('with a native token out', () => { it('should specify the Router contract as the recipient of the swap function call', async () => { mockRouterImplementation({ diff --git a/packages/internal/dex/sdk/src/lib/transactionUtils/swap.ts b/packages/internal/dex/sdk/src/lib/transactionUtils/swap.ts index bcece56934..848eca9df9 100644 --- a/packages/internal/dex/sdk/src/lib/transactionUtils/swap.ts +++ b/packages/internal/dex/sdk/src/lib/transactionUtils/swap.ts @@ -67,9 +67,12 @@ function buildSinglePoolSwap( ); } - const shouldRefundNativeTokens = trade.tradeType === Uniswap.TradeType.EXACT_OUTPUT && isNative(tokenIn); + const shouldRefundNativeTokens = isNative(tokenIn); if (shouldRefundNativeTokens) { - // Refund ETH if the input token is native and the swap is exact output + // Refund ETH if the input token is native. + // In some cases, the user may have specified an input amount that is greater than what + // the liqudiity of the pool can provide. + // To account for this case, always call `refundETH` to refund any excess native tokens. calldatas.push(paymentsContract.encodeFunctionData('refundETH')); } @@ -182,9 +185,12 @@ function buildMultiPoolSwap( ); } - const shouldRefundNativeTokens = trade.tradeType === Uniswap.TradeType.EXACT_OUTPUT && isNative(tokenIn); + const shouldRefundNativeTokens = isNative(tokenIn); if (shouldRefundNativeTokens) { - // Refund ETH if the input token is native and the swap is exact output + // Refund ETH if the input token is native. + // In some cases, the user may have specified an input amount that is greater than what + // the liqudiity of the pool can provide. + // To account for this case, always call `refundETH` to refund any excess native tokens. calldatas.push(paymentsContract.encodeFunctionData('refundETH')); }