Skip to content

dash-spv: SegmentedStorage::get_items() panics on segments with non-zero first_valid_offset (checkpoint-based / sparse segments) #792

Open

Description

@QuantumExplorer

Summary

SegmentedStorage::get_items() (in dash-spv/src/storage/segments.rs) panics in debug builds when a requested height range spans into a segment whose data does not begin at offset 0 (i.e. first_valid_offset() > seg_start). This crashes the SPV sync worker task and aborts header sync.

Observed on a real iOS (debug) build syncing mainnet headers; the SPV runtime starts, enters the sync loop, then panics ~3s later:

INFO platform_wallet::spv::runtime: SpvRuntime::run() starting client...
INFO platform_wallet::spv::runtime: SpvRuntime::run() client started, entering sync loop

thread 'tokio-rt-worker' panicked at dash-spv/src/storage/segments.rs:236:26:
Trying to access invalid offset (0) in segment with first_valid_offset = Some(13824)

WARN platform_wallet::spv::runtime: SpvRuntime background run exited with error:
  SPV error: Sync error: Invalid sync state: Manager task panicked:
  task 72 panicked with message
  "Trying to access invalid offset (0) in segment with first_valid_offset = Some(13824)"

rev: eb889af13f667ed39c35e8e8a0830eeedf523476

Where

dash-spv/src/storage/segments.rs, in get_items():

for segment_id in start_segment..=end_segment {
    let segment = self.get_segment_mut(&segment_id).await?;

    let seg_start = if segment_id == start_segment {
        Self::height_to_offset(start)
    } else {
        0                                   // <-- assumes every non-start segment begins at offset 0
    };

    let seg_end = if segment_id == end_segment {
        Self::height_to_offset(end)
    } else {
        Segment::<I>::ITEMS_PER_SEGMENT
    };

    #[cfg(debug_assertions)]
    {
        match segment.first_valid_offset() {
            Some(offset) if offset <= seg_start => {}
            _ => panic!("Trying to access invalid offset ({seg_start}) in segment with first_valid_offset = {:?}", segment.first_valid_offset()),
        }
    }
    ...
}

(ITEMS_PER_SEGMENT = 50_000; first_valid_offset() returns the index of the first non-sentinel item in the segment.)

Root cause

When a multi-segment range is read, every segment after the first has seg_start hardcoded to 0. The debug assertion then requires that segment to have valid data starting at offset 0 (first_valid_offset() <= 0, i.e. == Some(0)).

That invariant does not hold for a segment that was first populated partway through — e.g. when header storage begins from a checkpoint rather than from the genesis-aligned segment boundary. Here the segment's data starts at offset 13824 (first_valid_offset() = Some(13824)), but the read path requests it from offset 0, so the assertion fires.

In other words: the code assumes contiguous, segment-boundary-aligned density across all interior segments of a range, but checkpoint-based / partially-populated segments violate that assumption. The lower-bound assertion is too strict (or, symmetrically, the read range is being computed without clamping to the segment's valid region).

Impact

  • Debug builds: hard panic → the tokio SPV worker task dies → header sync aborts with Manager task panicked. On mobile (iOS debug builds ship with debug_assertions on) this manifests as "sync does nothing": the user taps Start, the runtime spins up and immediately dies, and the UI sits at 0/— with no surfaced error.
  • Release builds: the #[cfg(debug_assertions)] guard compiles out, so instead of panicking the code proceeds to segment.get(seg_start..seg_end) and reads sentinel (empty) items at [0..first_valid_offset) — i.e. it returns placeholder/sentinel data as if it were real, which is a silent correctness bug rather than a crash.

Reproduction

  1. Build dash-spv with debug_assertions (default cargo build / any debug profile).
  2. Sync mainnet headers such that a segment is populated starting from a non-zero offset (checkpoint-based start), then trigger a get_items() range that spans from an earlier segment through that segment (so it hits the else { 0 } branch for the partially-populated segment).
  3. The assertion at segments.rs:236 panics.

Suggested fix directions

A couple of options (maintainers will know which matches intent):

  1. Clamp seg_start to the segment's valid region instead of asserting — e.g. for interior segments use max(0, segment.first_valid_offset().unwrap_or(0)) as the start, and skip the sentinel prefix. This makes reads tolerant of checkpoint-aligned / sparse segments.
  2. Relax / remove the lower-bound assertion and have get_items() (or its callers) never request a range that starts below a segment's first_valid_offset — i.e. fix the range computation at the call site so it doesn't ask for the sentinel prefix in the first place.
  3. At minimum, the release-build behavior should not silently return sentinel items as data — whichever fix is chosen should close the silent-correctness gap too, since the assertion is the only thing currently "guarding" this in debug.

Happy to test a patch against the same mainnet store that reproduced this.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions