fix(orderbook): reduce invalid states and improve sync reliability#2626
Draft
fix(orderbook): reduce invalid states and improve sync reliability#2626
Conversation
shamardy
changed the title
Collaborator
|
regarding 9bd5e53 i think @dimxy also did some clippy fixes in one of his PRs. but clippy fixes will usually create conflicts anyway even if done by a single person in an isolated PR :/, albeit less messy. |
shamardy
force-pushed
the
fix/orderbook-temp-bans
branch
from
f69b690 to
9bd5e53
Compare
This reverts commit 9a76d11.
…SyncFailure to prevent bans during initial sync
- Sync: request diffs from our local “from” roots (SyncPlan), reducing FullTrie fallbacks and false mismatches; apply diffs tentatively and revert on mismatch. - Bans: add cause/role-aware temp-ban with per-peer grace (120s, TTL 600s, temp-ban 20m). Relays do not ban Unavailable light nodes; light nodes ban Unavailable relays. Remote role detected via relay mesh query. - Treat Ok(None) on SyncPubkeyOrderbookState as InvalidOrIncomplete (not Unavailable). - Keep-alive: avoid creating pubkey state on stale messages (read-only pre-scan). - Logging: single structured decision log for SyncFailure ban action. Risk/rollout: minor policy tuning; no protocol changes. Verify behavior under lossy links and observe reduced false bans.
- Route message forwarding through libp2p’s validation path instead of manual propagation - This achieves faster gossip cache cleanup while avoiding forwarding malformed/invalid messages as before
…mong other things
… keepalive messages
- Observed repeated PubkeyKeepAlive messages; root cause not confirmed yet. Hypotheses include seq no. based MessageId dedup gaps and equal-timestamp behavior. - Added investigation notes and TODOs: considered content‑derived MessageId. - Deferred code changes until we gather more evidence to avoid risky churn. No functional changes; comments and guidance only.
…erbookState` - Prepares for exact from→to trie diffs, enabling strict landing on expected roots (security/consistency hardening) while maintaining backward compatibility. Behavioral impact: - No functional change in this commit, purely protocol scaffolding.
…ate` - Plumb `expected_roots` into the sync handler rejects unsound requests
shamardy
force-pushed
the
fix/orderbook-temp-bans
branch
from
9bd5e53 to
f7baf4f
Compare
3 tasks
Collaborator
Author
Related:
|
shamardy
added a commit
that referenced
this pull request
Dec 16, 2025
Temporarily ignore tests that depend on remote Electrum servers (tracked in #2708) and orderbook sync timing (tracked in PR #2626). Unit tests ignored (Electrum): - test_unavailable_electrum_proto_version - test_one_unavailable_electrum_proto_version - test_qtum_add_delegation - test_qtum_get_delegation_infos - test_wait_for_confirmations_excepted Integration tests ignored (Electrum): - test_electrum_tx_history - test_add_delegation_qtum - test_query_delegations_info_qtum - test_qrc20_withdraw Integration test ignored (orderbook sync): - test_order_cancellation_received_before_creation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
4 tasks
shamardy
added a commit
that referenced
this pull request
Jan 7, 2026
…2707) Splits the monolithic docker test suite into 12 feature-gated modules that run in parallel CI jobs (~4x faster total execution time). **New Feature Flags:** - `docker-tests-eth` — ETH/ERC20/NFT tests - `docker-tests-slp` — BCH/SLP token tests - `docker-tests-sia` — Sia tests + DSIA swaps - `docker-tests-ordermatch` — Orderbook/matching tests - `docker-tests-swaps` — UTXO swap protocol tests - `docker-tests-watchers` — UTXO watcher tests - `docker-tests-watchers-eth` — ETH watcher tests (unstable) - `docker-tests-qrc20` — Qtum/QRC20 tests - `docker-tests-tendermint` — Cosmos/IBC tests - `docker-tests-zcoin` — ZCoin/Zombie tests - `docker-tests-integration` — Cross-chain swaps - `docker-tests-all` — All suites (local dev) **Key Changes:** - Matrix-based docker test jobs in CI workflow - Docker-compose infrastructure (`.docker/test-nodes.yml`) - Feature-gated test modules in `tests/docker_tests/` - Per-chain helper modules (`helpers/eth.rs`, `helpers/utxo.rs`, etc.) - Docker images migrated to gleec organization **Flaky Tests Ignored:** - Remote Electrum tests (tracked in #2708) - External NFT API tests (Moralis rate-limiting, nft-antispam unavailable) - Timing-dependent orderbook sync test (#2626) Closes #2417, #764 Partially addresses #2381, #835, #2388, #2231 Enables #2708
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Better to be reviewed commit by commit, no need to review the first 2 commits a73a714 and e3ad3ca as they were the ones reverted in #2631 and reapplied here. PR description to follow...