fix(evm-swapv2): no mempool inclusion required for maker payment validation by shamardy · Pull Request #2618 · GLEECBTC/komodo-defi-framework

shamardy · 2025-09-11T01:09:40Z

Problem

Taker-side validation failed when the maker broadcasted via private/MEV-protected relays (e.g., Polygon protect RPC), because the tx wasn’t visible on public RPCs. This caused false negatives and unnecessary refunds.

What this PR changes

Validate maker payment from the signed tx bytes (offline), without requiring mempool presence, while deferring on-chain visibility to the existing confirmation step.

Why this isn’t a complete solution

Offline checks don’t prove the maker actually broadcasted the tx. If the maker never submits the tx (and taker doesn’t rebroadcast it), the taker still waits until the maker-payment confirmation timeout before switching to the “immediate refund” path (refund via taker’s secret). This fixes the false‑negatives, but the drawback is that fast‑fail isn’t kept. The taker now may wait until the timeout if the maker never broadcasts.

Future Work

If this approach is accepted, apply it to other swap steps as needed:
- validate_taker_funding_impl: Only a brief visibility delay is needed to find the transaction, since there’s no confirmation step yet (we may need to add one). Note: there is no need for a fast-fail for this step as the maker’s payment hasn’t been broadcast.
Add taker rebroadcast fallback after a short pre-wait if the maker’s tx isn’t visible
Treat rebroadcast as success only if:
- send_raw_transaction returns the expected tx hash, or
- Error explicitly indicates “already known” / “already imported” / “known transaction”
Do not treat “nonce too low” or “replacement underpriced” as success; instead:
- Immediately query eth_getTransactionByHash and proceed only if the hash is visible (pending or included)

mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs

dimxy · 2025-09-11T07:20:17Z

mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs

-        validate_from_to_addresses(tx_from_rpc, maker_address, maker_swap_v2_contract).map_mm_err()?;
+        let taker_address = self.my_addr().await;
+
+        match tx.unsigned().action() {


Why not make a function similar to the validate_from_to_addresses fn to validate to and from?

Or, we could even reuse validate_from_to_addresses fn if we change it to accept SignedEthTx:
https://github.com/KomodoPlatform/komodo-defi-framework/compare/fix/eth-swapv2-offline-maker-payment-validation...dimxy:komodo-defi-framework:validate-from-reuse-suggestion?expand=1

Thanks for the suggestion and the detailed code changes. Fixed here c13654b

…tion in `validate_maker_payment_v2_impl`

…nvert Web3 tx to `SignedEthTx` in taker/NFT paths

onur-ozkan

LGTM

mm2src/coins/eth/eth_swap_v2/mod.rs

shamardy · 2025-09-11T09:02:32Z

Was planning to work on the todo in #2618 (comment) but since I already got an approval, I will do it in another PR.

onur-ozkan · 2025-09-11T09:03:28Z

Was planning to work on the todo in #2618 (comment) but since I already got an approval, I will do it in another PR.

I thought the PR was ready for merge as it's not in a draft mode.

shamardy · 2025-09-11T09:05:16Z

I thought the PR was ready for merge as it's not in a draft mode.

It's totally fine, I think it's better to keep it short :)

mariocynicys · 2025-09-11T10:55:38Z

but in validate_taker_funding_impl and validate_nft_maker_payment_v2_impl we still require querying the tx from rpc? wouldn't these tx broadcasts suffer from the same issue? (and we already wait for their confs? not sure)

shamardy · 2025-09-11T15:21:09Z

but in validate_taker_funding_impl and validate_nft_maker_payment_v2_impl we still require querying the tx from rpc? wouldn't these tx broadcasts suffer from the same issue? (and we already wait for their confs? not sure)

There is a todo in the opening comment about validate_taker_funding_impl and this #2618 (comment)

As for validate_nft_maker_payment_v2_impl, I was still thinking about it since there is no state machine implementation for NFTs yet, it's not needed but good to do it for future coverage (I think I will just leave a todo inside the function)

dimxy

LGTM.

Validate maker payment from the signed tx bytes (offline), without requiring mempool presence, while deferring on-chain visibility to the existing confirmation step.

Agree that we should not rely on tx presence in mempool, we need at least one confirmation

* dev: fix(TPU): correct dexfee in check balance to prevent swap failures (#2600) fix(tests): fix/remove kmd rewards failing test (#2633) chore(ci): bump CI container image to debian bullseye-slim to match dev (#2641) chore(release): add changelog entries for v2.5.2-beta (#2639) chore(release): bump mm2 version to 2.5.2-beta (#2638) feat(ci): add macos universal2 build (#2628) fix(metrics): remove memory_db size metric (#2632) fix(zcoin): exact-anchor witnesses in wasm get_spendable_notes (#2629) fix(evm-swapv2): no mempool inclusion required for maker payment validation (#2618) chore(rust 1.90): make CI clippy/fmt pass Revert "fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)" Revert "fix(orderbook): validate roots before commit (#2605)"

…dation (#2618) This commit validates maker payment from the signed tx bytes (offline), without requiring mempool presence, while deferring on-chain visibility to the existing confirmation step. It fixes a problem where taker-side validation failed when the maker broadcasted via private/MEV-protected relays.

…roadcast (#2646) This commit extends offline validation done in #2618 while adding best‑effort visibility gates with a one‑shot rebroadcast fallback to all coins, this is now part of the state machine instead of being coin specific.

eth/swapv2: validate maker payment offline (no RPC required)

edfdcb4

shamardy changed the title ~~fix(eth-swapv2): don't require mempool to validate maker payment~~ fix(eth-swapv2): no mempool inclusion required for maker payment validation Sep 11, 2025

shamardy changed the title ~~fix(eth-swapv2): no mempool inclusion required for maker payment validation~~ fix(evm-swapv2): no mempool inclusion required for maker payment validation Sep 11, 2025

shamardy marked this pull request as ready for review September 11, 2025 01:47

shamardy added bug: swap priority: high Important tasks that need attention soon. labels Sep 11, 2025

onur-ozkan reviewed Sep 11, 2025

View reviewed changes

mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs Outdated Show resolved Hide resolved

dimxy reviewed Sep 11, 2025

View reviewed changes

shamardy marked this pull request as draft September 11, 2025 07:59

review fix: perform sender (tx.sender) check before action/ABI valida…

eede9be

…tion in `validate_maker_payment_v2_impl`

shamardy force-pushed the fix/eth-swapv2-offline-maker-payment-validation branch from e5196fc to 0d7d442 Compare September 11, 2025 08:30

review fix: reuse validate_from_to_addresses with SignedEthTx; co…

c13654b

…nvert Web3 tx to `SignedEthTx` in taker/NFT paths

shamardy force-pushed the fix/eth-swapv2-offline-maker-payment-validation branch from 0d7d442 to c13654b Compare September 11, 2025 08:31

onur-ozkan previously approved these changes Sep 11, 2025

View reviewed changes

mm2src/coins/eth/eth_swap_v2/mod.rs Outdated Show resolved Hide resolved

review fix: display address instead of using debug

1f3d813

shamardy dismissed onur-ozkan’s stale review via 1f3d813 September 11, 2025 09:01

shamardy marked this pull request as ready for review September 11, 2025 09:01

shamardy requested a review from dimxy September 11, 2025 09:05

dimxy approved these changes Sep 15, 2025

View reviewed changes

shamardy added the 2.6.0-beta label Oct 6, 2025

shamardy merged commit 85938ef into dev Oct 6, 2025
22 of 28 checks passed

shamardy deleted the fix/eth-swapv2-offline-maker-payment-validation branch October 6, 2025 06:34

shamardy mentioned this pull request Oct 9, 2025

fix(swapv2): offline validation, tx visibility check and one-shot rebroadcast #2646

Merged

1 task

shamardy mentioned this pull request Nov 25, 2025

chore(release): v2.6.0-beta #2696

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(evm-swapv2): no mempool inclusion required for maker payment validation#2618

fix(evm-swapv2): no mempool inclusion required for maker payment validation#2618
shamardy merged 4 commits intodevfrom
fix/eth-swapv2-offline-maker-payment-validation

shamardy commented Sep 11, 2025 •

edited

Loading

Uh oh!

Uh oh!

dimxy Sep 11, 2025 •

edited

Loading

Uh oh!

shamardy Sep 11, 2025 •

edited

Loading

Uh oh!

onur-ozkan left a comment

Uh oh!

Uh oh!

shamardy commented Sep 11, 2025

Uh oh!

onur-ozkan commented Sep 11, 2025

Uh oh!

shamardy commented Sep 11, 2025

Uh oh!

mariocynicys commented Sep 11, 2025

Uh oh!

shamardy commented Sep 11, 2025 •

edited

Loading

Uh oh!

dimxy left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

shamardy commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

What this PR changes

Why this isn’t a complete solution

Future Work

Uh oh!

Uh oh!

dimxy Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shamardy Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

onur-ozkan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shamardy commented Sep 11, 2025

Uh oh!

onur-ozkan commented Sep 11, 2025

Uh oh!

shamardy commented Sep 11, 2025

Uh oh!

mariocynicys commented Sep 11, 2025

Uh oh!

shamardy commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dimxy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shamardy commented Sep 11, 2025 •

edited

Loading

dimxy Sep 11, 2025 •

edited

Loading

shamardy Sep 11, 2025 •

edited

Loading

shamardy commented Sep 11, 2025 •

edited

Loading