Skip to content

net: history sync accepts a peer's incomplete history as complete #2047

Description

@hmzakhalid

Problem

A node answers a historical-sync request with whatever its own event store holds and marks the answer complete, even when it lacks part of the requested range. The requesting node accepts that answer and stops fetching.

  • The responder builds the batch from its local store with no coverage check. It returns BatchCursor::Done when the scan returns fewer events than the limit (crates/net/src/network_sync/workflow.rs:117-157, handlers.rs:143). It serves requests as soon as it starts, before its own startup sync has finished.
  • The requester asks one random peer (PeerTarget::Random, crates/net/src/network_sync/effects/fetch_history.rs:23) and stops at the first Done (crates/net/src/event_buffer/model.rs:297).

Scenario

Node A restarts during an E3 and asks for the gossip since its last snapshot. The random peer B restarted or joined after that point, so B lacks part of the range. B returns its partial history with Done, and A starts without the missing messages, for example DKG coordination messages or decryption shares.

Since #2024, main re-sends DKG Ready/Roster messages and each node's decryption share until the phase ends, which covers most gaps. v0.17.0 and v0.18.0 do not re-send, so a message they published during the gap stays missing on A. Bootstrap nodes (#2046) are always connected and are therefore likely to be picked.

Constraints for a fix

  • v0.17.0, v0.18.0 and main share sync wire version 3 (SYNC_WIRE_MAJOR, crates/net/src/network.rs:16). A new cursor or response variant needs a wire version bump, which old nodes reject.
  • A responder must not refuse a request because its history is incomplete while older nodes exist. v0.17.0 and v0.18.0 treat ProtocolResponse::Error as a failed fetch with no retry (crates/net/src/direct_requester.rs:133). After three recovery rounds, fetch_history.rs:274 fails, and startup then waits until its deadline. Freshly upgraded nodes would all have short histories, so an old node restarting after a rollout could fail to start.
  • Do not ship a change here while v0.17.0 or v0.18.0 nodes are still needed for E3-2.

Suggested direction

  1. Requester only, no wire change: fetch each aggregate from two peers and merge by event ID. Older nodes see no difference.
  2. After every node runs a version that understands it: coverage-aware responses (the responder states the range it can vouch for) with a sync wire version bump, and a requester that falls back to another peer.

Found in an adversarial review of #2046 (finding 5) and verified against the code.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingciphernodeRelated to the ciphernode package

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions