Skip to content

namespace op-log retry scans rely on decode-error-as-prefix-boundary instead of an explicit range bound #3102

Description

@chefsale

Summary

The three namespace op-log walk helpers in crates/governance-store/src/namespace/op_log.rs terminate their scan on the first key-decode error, treating it as the end of the NamespaceGovOp prefix:

  • collect_signed_group_ops_for_group
  • collect_buffered_group_op_keys
  • collect_opaque_skeleton_delta_ids_for_group
let key = match entries_iter.next() {
    None => break,
    Some(Ok(k)) => k,
    Some(Err(_)) => {
        record_namespace_decode_invalid("signed_iter_end");
        break;               // <- first KEY decode error ends the whole scan
    }
};

Why the current behavior is correct (and intentional)

iter::<NamespaceGovOp>() does not filter by prefix — it walks the entire shared Group column family, sorted by a 1-byte prefix:

0x20 GroupMeta          (33 bytes)
0x32 GroupMemberContext (65 bytes)
0x38 NamespaceGovOp     (65 bytes)  <- what we iterate
0x39 NamespaceGovHead   (33 bytes)
0x3A GroupKey           (varies)

Every 0x38 key is a fixed 65 bytes, written atomically with its value. When the walk runs past the last 0x38 entry into 0x39, borsh tries to decode a 33-byte key as a 65-byte NamespaceGovOp and fails with "unexpected length of input." That first key-decode error is the prefix boundary — every later key is higher-prefixed and would fail identically. So break is correct.

The code also distinguishes failure modes deliberately: a key-decode error → break (boundary), whereas a value-decode error → skip + warn + continue (genuine corruption of one of our own entries). A metric is recorded at the break so real corruption would still surface on dashboards.

The originally-reported concern — "a corrupt key mid-prefix drops all later retry candidates, so an op is never re-applied and a member gets stuck unable to decrypt/join" — is not reachable in practice: 0x38 keys are fixed-length and store-written, not independently corruptible mid-prefix.

Why this is still worth an issue

The termination is a negative signal ("we hit a decode error, therefore we're past the prefix") rather than a positive bound ("stop at key >= upper bound"). It is correct today only because it leans on the invariant that no in-prefix key ever fails to decode. That invariant holds by construction now, but it's implicit and fragile against future key-layout changes in the shared column family.

Possible solutions

  1. Explicit range bound (preferred, but needs store support).
    Seek from (namespace_id, [0x00; 32]) and stop when the raw key reaches (namespace_id, [0xFF; 32]) — or when the leading prefix byte changes — instead of stopping on a decode error. This requires a bounded / raw-key range iterator on calimero-store; the current StructuredIterator (seek + keys()) only yields typed keys and exposes no raw bytes or range-end, and decoding is exactly what fails at the boundary. So the real work is a small store-layer API addition (a prefix/range scan), after which all three walks become explicit and layout-change-proof.

  2. Assert the boundary rather than infer it (cheap hardening).
    Keep break-on-decode-error, but when it fires, confirm we're genuinely at/after the 0x38 upper bound (e.g. peek the raw next-key prefix if the iterator can expose it) before treating remaining entries as out-of-prefix; only then is the metric an "end" marker vs. a real-corruption alarm. Lower value than option 1 and still limited by the same iterator API gap.

  3. Do nothing / documentation-only.
    The behavior is already documented at length in op_log.rs. If the store iterator API is not going to grow a range primitive soon, tracking this as a known, bounded limitation may be sufficient.

Recommendation

Option 1 is the clean fix but is gated on a calimero-store bounded-iterator primitive; it buys robustness against future shared-column key additions rather than fixing a live bug. Reasonable to keep low-priority until the store iterator is touched for another reason.

Context: surfaced during the determinism review that produced #3096 (deliberately left out of that PR — current behavior is correct given fixed-length keys).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions