From 8e8977006aa638b5c2705327904d1465186a0461 Mon Sep 17 00:00:00 2001 From: Alfetopito Date: Tue, 31 Dec 2024 10:30:12 +0000 Subject: [PATCH 1/2] fix: use network instead of orderParams chainId --- .../modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/cowswap-frontend/src/modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts b/apps/cowswap-frontend/src/modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts index 1c688a320d..f061616206 100644 --- a/apps/cowswap-frontend/src/modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts +++ b/apps/cowswap-frontend/src/modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts @@ -79,7 +79,7 @@ export async function signEthFlowOrderStep( ...ethTxOptions, gasLimit: calculateGasMargin(estimatedGas), }) - const txReceipt = await ethFlowContract.signer.sendTransaction({ ...tx, chainId: orderParams.chainId }) + const txReceipt = await ethFlowContract.signer.sendTransaction({ ...tx, chainId: network.chainId }) addInFlightOrderId(orderId) From 722e00cff846af16c3684efa7c4263a05a7ac402 Mon Sep 17 00:00:00 2001 From: Alfetopito Date: Tue, 31 Dec 2024 10:35:44 +0000 Subject: [PATCH 2/2] refactor: update comment --- .../services/ethFlow/steps/signEthFlowOrderStep.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts b/apps/cowswap-frontend/src/modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts index f061616206..3fda6d658c 100644 --- a/apps/cowswap-frontend/src/modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts +++ b/apps/cowswap-frontend/src/modules/swap/services/ethFlow/steps/signEthFlowOrderStep.ts @@ -67,18 +67,24 @@ export async function signEthFlowOrderStep( return GAS_LIMIT_DEFAULT }) - // This used to be done with a higher level of abstraction like this: + // Ensure the Eth flow contract network matches the network where you place the transaction. + // There are multiple wallet implementations, and potential race conditions that can cause the chain of the wallet to be different, + // and therefore leaving the chainId implicit might lead the user to place an order in an unwanted chain. + // This is especially dangerous for Eth Flow orders, because the contract address is different for the distinct networks, + // and this can lead to loss of funds. + // + // Thus, we are not using a higher level of abstraction as it doesn't allow to explicitly set the chainId: // const txReceipt = await ethFlowContract.createOrder(ethOrderParams, { // ...ethTxOptions, // gasLimit: calculateGasMargin(estimatedGas), // }) - // However, to **try** to prevent wallet issues, we want to explicitly send along the chainId - // But that wrapper doesn't accept it. - // So we must build the tx first, then send it using the contract's signer + // + // So we must build the tx first: const tx = await ethFlowContract.populateTransaction.createOrder(ethOrderParams, { ...ethTxOptions, gasLimit: calculateGasMargin(estimatedGas), }) + // Then send the is using the contract's signer where the chainId is an acceptable parameter const txReceipt = await ethFlowContract.signer.sendTransaction({ ...tx, chainId: network.chainId }) addInFlightOrderId(orderId)