Skip to content

Conversation

@shamardy
Copy link
Collaborator

@shamardy shamardy commented Sep 25, 2025

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...

@shamardy shamardy changed the title fix(orderbook-sync): exact diffs to target root, validated apply, graceful temp bans fix(orderbook): reduce invalid states and improve sync reliability Sep 25, 2025
@mariocynicys
Copy link
Collaborator

regarding 9bd5e53
i kinda want us to revert back to some set version in the CI & toolchain file and upgrade to the latest version every month or so (and maybe create a reminder bot for that). instead of using stable release.

i think @dimxy also did some clippy fixes in one of his PRs.
and i avoid doing these fixes (even though they are annoying AF in the terminal) mid other PRs to avoid conflicts if somebody else happens to fix them too.

but clippy fixes will usually create conflicts anyway even if done by a single person in an isolated PR :/, albeit less messy.

…; reject stale keep-alives (#2580)""

This reverts commit fa3fc5d.
…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
- 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
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.

3 participants