Skip to content

fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives#2580

Merged
shamardy merged 5 commits intodevfrom
fix/ignore-self-keep-alive
Aug 12, 2025
Merged

fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives#2580
shamardy merged 5 commits intodevfrom
fix/ignore-self-keep-alive

Conversation

@shamardy
Copy link
Copy Markdown
Collaborator

@shamardy shamardy commented Aug 7, 2025

Summary

This PR hardens order matching to prevent “ghost orders” where a cancelled maker order can reappear or stick in the order book.

Changes

  • Ignore loop-back messages at process_msg

    • If an incoming message’s pubkey matches the node’s own (persistent or per-order), it’s treated as loop-back and ignored. Prevents acting on echoed versions of our own messages.
  • Clear pair on null or hashed-null trie root

    • When a PubkeyKeepAlive advertises a zero or hashed-null root for a pair, delete all local orders for that (pubkey, pair) and persist the null root (“tombstone”) in pubkeys_state.trie_roots. Avoids unnecessary re-sync until a non-null root appears.
  • Reject stale/replayed PubkeyKeepAlive by timestamp

    • Track the maker-published timestamp per pubkey (latest_maker_timestamp).
    • Drop PubkeyKeepAlive if message.timestamp <= latest_maker_timestamp to prevent out-of-order/replayed keep-alives from triggering stale syncs.
    • Preserve GC semantics: last_keep_alive remains a local receive timestamp (now_sec()) used by inactivity cleanup.
  • Cleanup

    • Removed redundant self-origin checks from downstream handlers (e.g., process_maker_reserved, process_taker_request) since loop-back filtering happens at the entry point.
    • recently_cancelled cache remains unchanged to handle out-of-order messages from other peers.

Impact

  • Prevents self-corruption via looped-back messages.
  • Blocks stale state from being reintroduced via replayed or out-of-order keep-alives.
  • Ensures null-root deletions are remembered and propagated.

QA plan (high level)

  • Cancellation propagation: create and cancel a maker order; verify it disappears locally and across peers and does not reappear after network churn or node restarts.
  • Null-root handling: after cancellation, verify the affected (pubkey, pair) is cleared and remains empty until a new non-null root is observed (a new maker order is created remotely).
  • Replay/out-of-order robustness: under normal network conditions (including transient delays), verify cancelled orders do not reappear and the order book converges across peers.
  • Regression checks: normal order creation/updates continue to propagate; inactivity cleanup still purges inactive pubkeys as before. Also make sure that memory on seednodes are not affected and that this fix Seednodes memory leaks fix #1054 is still actual.

Related: #2575

@shamardy shamardy force-pushed the fix/ignore-self-keep-alive branch from 27d5122 to 19e769a Compare August 7, 2025 20:17
@shamardy shamardy changed the title fix(ordermatch): prevent ghost orders by ignoring looped-back messages fix(ordermatch): ignore loop-back messages; clear pair on null trie root Aug 7, 2025
@shamardy shamardy changed the title fix(ordermatch): ignore loop-back messages; clear pair on null trie root fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives Aug 8, 2025
@shamardy shamardy marked this pull request as ready for review August 8, 2025 09:10
@shamardy shamardy requested a review from cipig August 8, 2025 09:11
@shamardy
Copy link
Copy Markdown
Collaborator Author

shamardy commented Aug 8, 2025

@cipig this needs to be run for some time to make sure there are no regressions as it changes some things for order matching and syncing.

@dimxy
Copy link
Copy Markdown
Collaborator

dimxy commented Aug 8, 2025

Do we know how to reliably reproduce the ghost orders situation (before this fix)?

@cipig
Copy link
Copy Markdown

cipig commented Aug 8, 2025

Do we know how to reliably reproduce the ghost orders situation (before this fix)?

unfortunately i don't... i will bring a seed node back online which i think might have been responsible for triggering this... problem is though that i think that the DC was under DDoS attack by the time this happened (or had other networking issues) and that's a thing that i can't reproduce

Copy link
Copy Markdown
Collaborator

@dimxy dimxy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked thru the fix, like it.
LGTM

Copy link
Copy Markdown
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Thanks

@shamardy
Copy link
Copy Markdown
Collaborator Author

@cipig I will merge this and we can test it in dev

@shamardy shamardy merged commit f5f0b18 into dev Aug 12, 2025
18 of 24 checks passed
@shamardy shamardy deleted the fix/ignore-self-keep-alive branch August 12, 2025 22:29
dimxy pushed a commit that referenced this pull request Aug 13, 2025
* dev:
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
dimxy pushed a commit that referenced this pull request Aug 15, 2025
* dev: (24 commits)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  improve note for docker test start failure (#2550)
  fix(DOCS): add note for macos to fix docker containers startup failure (#2544)
  refactor(toolchain): general stabilization for stable rust (#2528)
  ...

# Conflicts:
#	mm2src/coins/eth.rs
#	mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs
#	mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs
#	mm2src/coins/eth/eth_tests.rs
#	mm2src/coins/eth/eth_withdraw.rs
#	mm2src/coins/eth/v2_activation.rs
#	mm2src/coins/nft.rs
#	mm2src/coins/qrc20.rs
#	mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs
#	mm2src/mm2_main/src/rpc/lp_commands/tokens.rs
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
shamardy added a commit that referenced this pull request Aug 18, 2025
…#2597)

This commit uses `subscribe_to_orderbook_topic` in the kickstart flow so `topics_subscribed_to` is set and keep-alive–driven `SyncPubkeyOrderbookState` can run. It also prevents our own ephemeral / private maker pubkeys from being garbage collected, this is needed now that a maker ignores its own keepalives since #2580.
dimxy pushed a commit that referenced this pull request Aug 19, 2025
* dev:
  improvement(`static mut`s): `static mut` removal (#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  chore(release): v2.3.0-beta (#2284)

# Conflicts:
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
dimxy pushed a commit that referenced this pull request Aug 21, 2025
* dev:
  fix build script failing to find .git/HEAD (#2601)
  refactor(EVM): rename fn, fix timeouts, add activation req validation (#2543)
  improvement(`static mut`s): `static mut` removal (#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
dimxy pushed a commit that referenced this pull request Aug 21, 2025
* dev:
  fix build script failing to find .git/HEAD (#2601)
  refactor(EVM): rename fn, fix timeouts, add activation req validation (#2543)
  improvement(`static mut`s): `static mut` removal (#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)

# Conflicts:
#	mm2src/mm2_main/src/lp_swap/taker_swap.rs
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Aug 21, 2025
* dev: (28 commits)
  fix build script failing to find .git/HEAD (GLEECBTC#2601)
  refactor(EVM): rename fn, fix timeouts, add activation req validation (GLEECBTC#2543)
  improvement(`static mut`s): `static mut` removal (GLEECBTC#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (GLEECBTC#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (GLEECBTC#2580)
  fix(clippy): fix clippy warnings for GLEECBTC#2565 (GLEECBTC#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (GLEECBTC#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (GLEECBTC#2564)
  feat(protocol): [0] solana support (GLEECBTC#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (GLEECBTC#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (GLEECBTC#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (GLEECBTC#2572)
  improvement(dep-stack): security bumps (GLEECBTC#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (GLEECBTC#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (GLEECBTC#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (GLEECBTC#2454)
  ci(pull-requests): review notification bot (GLEECBTC#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (GLEECBTC#2538)
  bless clippy (GLEECBTC#2560)
  refactor(toolchain): use latest available stable compiler (GLEECBTC#2557)
  ...

# Conflicts:
#	mm2src/coins/eth.rs
#	mm2src/coins/eth/eth_rpc.rs
#	mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs
#	mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs
#	mm2src/coins/eth/eth_withdraw.rs
#	mm2src/coins/eth/v2_activation.rs
#	mm2src/coins/lightning.rs
#	mm2src/coins/lp_coins.rs
#	mm2src/coins/nft.rs
#	mm2src/coins/qrc20.rs
#	mm2src/coins/siacoin.rs
#	mm2src/coins/tendermint/tendermint_token.rs
#	mm2src/coins/test_coin.rs
#	mm2src/coins/utxo/bch.rs
#	mm2src/coins/utxo/qtum.rs
#	mm2src/coins/utxo/slp.rs
#	mm2src/coins/utxo/utxo_standard.rs
#	mm2src/coins/z_coin.rs
#	mm2src/coins_activation/src/bch_with_tokens_activation.rs
#	mm2src/coins_activation/src/erc20_token_activation.rs
#	mm2src/coins_activation/src/eth_with_token_activation.rs
#	mm2src/coins_activation/src/init_erc20_token_activation.rs
#	mm2src/coins_activation/src/init_token.rs
#	mm2src/coins_activation/src/platform_coin_with_tokens.rs
#	mm2src/coins_activation/src/slp_token_activation.rs
#	mm2src/coins_activation/src/tendermint_with_assets_activation.rs
#	mm2src/coins_activation/src/token.rs
#	mm2src/mm2_main/src/lp_swap.rs
#	mm2src/mm2_main/src/lp_swap/check_balance.rs
#	mm2src/mm2_main/src/lp_swap/maker_swap.rs
#	mm2src/mm2_main/src/lp_swap/max_maker_vol_rpc.rs
#	mm2src/mm2_main/src/lp_swap/swap_v2_rpcs.rs
#	mm2src/mm2_main/src/lp_swap/taker_swap.rs
#	mm2src/mm2_main/src/lp_swap/trade_preimage.rs
#	mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
#	mm2src/mm2_main/src/rpc/lp_commands/legacy.rs
#	mm2src/mm2_main/src/rpc/lp_commands/lr_swap.rs
#	mm2src/mm2_main/src/rpc/lp_commands/lr_swap/lr_impl.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/errors.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs
#	mm2src/mm2_main/src/rpc/lp_commands/tokens.rs
#	mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs
#	mm2src/mm2_main/tests/docker_tests/docker_tests_inner.rs
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
#	mm2src/mm2_main/tests/integration_tests_common/mod.rs
#	mm2src/trading_api/src/one_inch_api/client.rs
shamardy added a commit that referenced this pull request Oct 3, 2025
shamardy added a commit that referenced this pull request Oct 6, 2025
…; reject stale keep-alives (#2580)""

This reverts commit fa3fc5d.
dimxy pushed a commit that referenced this pull request Oct 8, 2025
* 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)
  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)"
dimxy pushed a commit that referenced this pull request Oct 9, 2025
* 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)"
dimxy pushed a commit that referenced this pull request Oct 15, 2025
…eep-alives (#2580)

This commit does 3 things:
- Tries to prevent ghost orders by ignoring looped-back messages
- Purges on null keep-alive roots and persists a tombstone
- Drops stale/replayed PubkeyKeepAlive messages via monotonic maker timestamp while keeping GC based on the receive time
dimxy pushed a commit that referenced this pull request Oct 15, 2025
…#2597)

This commit uses `subscribe_to_orderbook_topic` in the kickstart flow so `topics_subscribed_to` is set and keep-alive–driven `SyncPubkeyOrderbookState` can run. It also prevents our own ephemeral / private maker pubkeys from being garbage collected, this is needed now that a maker ignores its own keepalives since #2580.
dimxy pushed a commit that referenced this pull request Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants