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

[shamardy]

This is a follow‑up to #2618.

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.

Previous PR (#2618)

• Validated the maker’s payment offline from signed bytes on the taker side to remove the hard dependency on mempool presence. This was done for EVM transactions only.
• Deferred on‑chain visibility for EVM maker payment to the existing confirmation step, eliminating false negatives caused by private/MEV relays for that stage.

What this PR changes

• Extends offline validation 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.
• Taker (v2)
  • Validates the maker payment offline, from signed bytes.
  • Requires maker payment to become visible within a short grace window, if still invisible after a one‑shot rebroadcast, refunds the funding transaction.
  • If configured, waits for on‑chain confirmations before spending the funding tx.
• Maker (v2)
  • Validates taker funding offline (no RPC lookups).
  • Before sending the maker payment, either:
    • requires funding tx visibility within a grace window (one‑shot rebroadcast on first miss), or
    • waits up to one on‑chain confirmation if configured.
• Adds a generic visibility helper
  • ensure_tx_is_broadcasted(…): polls by tx hash within a grace window, on the first miss performs a one‑shot rebroadcast, continues polling until timeout.
• EVM specifics
  • validate_taker_funding_impl: now parses SignedEthTx and validates from/to, ABI inputs and value offline (no RPC fetch for the tx).
• UTXO specifics
  • Removed strict byte‑for‑byte RPC match for taker funding in the v2 path to avoid false negatives.
  • Legacy v1 validation remains unchanged, but validate_maker_payment_v2 uses it so a change in validate_maker_payment_v2 might follow, it's not a big issue for UTXO but will be done for completeness.

How this fixes the problem

• Decouples correctness from mempool presence:
  • Offline semantic checks verify that each signed transaction matches the expected sender/recipient, method/inputs, and value without any RPC dependency.
• Guarantees a clear and fast outcome:
  • Visibility gate with a one‑shot rebroadcast ensures that transactions sent via private/MEV relays either appear on public mempools quickly or trigger a deterministic fallback.
  • Outcomes by role:
    • Taker:
      • checks maker payment:
      • visible within the grace window → proceed;
      • still invisible after a single rebroadcast and the window elapses → refund taker funding immediately.
    • Maker
      • checks taker funding:
        • visible (or 1 confirmation, if configured) → send maker payment;
        • not visible after the window → do not send maker payment.
• Net effect: no more false negatives when private relays are used, while keeping the original confirmation‑based safety for operators who want it.

Not done from prior “Future Work” in #2618 and why

• “Add taker rebroadcast fallback after a short pre‑wait”

  • Chosen approach: rebroadcast immediately on first miss.
  • Why better: reduces latency and avoids an extra tunable, the grace window already accounts for propagation variance.

• “Treat rebroadcast as success only based on transport responses”

  • Chosen approach: never infer success from transport strings, success is defined solely by tx visibility by hash.
  • Why better: avoids brittle, client‑specific error matching and yields consistent behavior across RPC providers and coins.

• “Do not treat ‘nonce too low’ or ‘replacement underpriced’ as success, query tx by hash”

  • Effectively addressed: transport errors are not considered success, we only proceed when the expected tx hash becomes visible (pending or included). Until then, we keep polling within the grace window.

Defaults and constants

• Maker policy default: require_taker_funding_confirm_before_maker_payment = true (wait up to 1 confirmation).
• Visibility timing defaults:
  • SWAP_TX_VISIBILITY_GRACE_SECS = 30s
  • SWAP_TX_VISIBILITY_POLL_SECS = 1s

Todo in this PR:

• Make sure that validate_maker_payment_v2 validates tx offline only (UTXO parity). Not required for resolving the private‑relay false‑negative, but desirable for completeness.

[dimxy]

A question (maybe because I have done full review yet):
In this PR we are fixing a problem when a party sends a transaction via a private relay, so other party may need to rebroadcast that transaction.
In legacy swaps we have a broadcast_p2p_tx_msg feature. We do not like using it in TPU to ensure a transaction to be in the chain?

[mariocynicys]

only 2 comments.
looks good otherwise :D

[mariocynicys]

i fear this extra check might create problems.
i don't think this check is necessary here though (for a non confirm-required case). being visible in the mempool isn't any guarantee that it won't be rbf or replaced by nonce later. we already confirm this tx at later stage also (i guess, but must be true otherwise the protocol is broken xD)

[shamardy]

being visible in the mempool isn't any guarantee that it won't be rbf or replaced by nonce later

I am aware, this is about quick refund if maker sent taker a p2p message that they broadcasted their payment but it's not to be seen. Otherwise, the taker will wait maker_payment_conf_timeout for refund which is a lot.

[dimxy]

Why rush to refund though, if we did not see the tx in mempool?
Indeed it's not guaranteed tx was in the node mempool (even if we did rebroadcast we still may not see it in the connected nodes) but tx may eventually appear in the block.

[shamardy]

Why rush to refund though, if we did not see the tx in mempool?

I don't think it's rushed, 30 seconds is 2 eth blocktimes and any other EVM is less than that. As for UTXOs, it should be in the mempool by then. It's on the other side (the maker) to have a good fee to make it appear quickly, we don't want taker funds or the swap to hang for a long time as this is bad for usability. In the future, makers reputation or other means (slashing) should be tied to that to since taker will lose network / mining fees. Basically when maker sends the taker a p2p message that they broadcasted the tx they should guarantee it will appear in less than 30 seconds.

[shamardy]

P.S. after testing and seeing how this works in real environment the 30 seconds wait time can be changed if needed, this is just my own thinking of what a good value should be to strike balance between returning funds early and giving the tx time to appear. I guess there are other factors like web3 nodes / electrums being down etc... but I don't think a swap should continue in such conditions.

[cipig]

Should we even do / allow swaps in such times?

on DOGE, the problem persisted for a pretty long time, like couple months.... after i figured out those mitigations swaps worked fine
so i think, if we can keep it working, then we should also allow the users to do the swaps... they will complain anyway if we don't :-)

[shamardy]

ok, seems like a config option that is documented in a good way and lists the cons of increasing it's value should be good enough. I might improve on this code as well, in this PR or in the future, I didn't want to rely on errors returned from nodes, but maybe a mixture of both will make things more robust.

[dimxy]

IMHO, as a general case when we did not see a tx in mempool we could not know for sure about it: the tx may be not valid or may be valid but just did not get into a specific node mempool (e.g. due to the node policy).
Maybe it's better to wait for the next block to be sure (well I think at least this is okay for refund: better to wait longer than disrupt the swap) .

[shamardy]

I agree not seeing a tx does not prove it is invalid, but in the non cooperating model we are following, the maker must produce quick public evidence once they claim that they broadcasted the transaction by sending the p2p message. The taker cannot wait open ended for something that might be withheld or stuck. Waiting a block does not guarantee visibility within this block either in cases like the Doge case mentioned by @cipig.
As mentioned in this comment:
https://github.com/KomodoPlatform/komodo-defi-framework/blob/30d0638eb5651c776e8586ac3b34a0ddb8e393f5/mm2src/mm2_main/src/lp_swap/swap_v2_common.rs#L89-L93
For non trusted makers we can require maker payment first, this is the cost to gain trust. We can add makers explicitly as trusted once they have enough volume, until a reputation index or other means are in place. Trusted makers keep the current flow while allowing longer waits or confirmations, so this PR is only a step in this direction.
I suggest to keep this 30 seconds for now and test it in real scenarios and maybe add an RPC later for takers to choose to refund immediately whenever they want as they incur the network / mining fees, or make the wait time configurable per coin or whatever we find necessary later.

[dimxy]

I guess this fix indeed matters for TPU as its whole idea was to make it grief-free.
That is, in case of any signs of manipulation from the Maker side the Taker should be able to release their funds ASAP.

[mariocynicys]

this might return 0 if taker_coin_confs is set to 0 making this require_taker_funding_confirm_before_maker_payment parameter essentially useless.

[shamardy]

I found problems with this approach ref. #2646 (comment)

[shamardy]

this might return 0 if taker_coin_confs is set to 0 making this require_taker_funding_confirm_before_maker_payment parameter essentially useless.

require_taker_funding_confirm_before_maker_payment is a global requirement that can be overridden by specific coins configs, so coin config takes precedence here as an opt-out mechanism for makers. Eventually, the plan is to make this flag (and similar ones) user-togglable, but for now this is how it’s wired, this was my thinking at least. In any case, please wait for 2 more commits from my side before continuing reviews.

[shamardy]

Will be disabled in next commit with lots of code comments/notes about the decision and all the different tradeoffs.

30d0638

[mariocynicys]

LGTM

[onur-ozkan]

Perhaps, add a offline prefix to the function (e.g., offline_maker_payment_validation_v2)?

[shamardy]

Well, validation in the crypto sense is an offline operation after fetching or getting all the needed info from the chain / p2p / light nodes / etc..., I think it would be excessive to add offline to this but it's fine by me if you insist.

[dimxy]

TBH the idea of refund if we did not find the payment tx in mempool looks debatable for me.
Apart from that, LGTM

[shamardy]

TBH the idea of refund if we did not find the payment tx in mempool looks debatable for me.

Thanks @dimxy, I answered here #2646 (comment) with my rationale. Like I said, this can be changed / enhanced later once tested.

[shamardy]

@onur-ozkan all your comments should have been addressed in the latest commit