Skip to content

Remove ProofReplayCache and keep the holder-proof path stateless #104

Description

@imran-siddique

Follow-up to #97.

ProofReplayCache was added in #97 to make a holder proof single-use, because the challenge underneath is stateless and therefore cannot be consumed. The decision is to keep the path stateless instead: remove the cache and document the challenge window as the bound.

Two reasons.

It contradicts the design challenge.py already chose. That module states a challenge store "is the option this one was chosen over". Reintroducing per-instance state one layer up gives the same operational cost the stateless choice was made to avoid: a multi-instance deployment needs sticky routing or a shared store for the guarantee to mean anything, and without either it silently degrades to the window anyway.

The current implementation is a denial of service well before its own bound. _expire rebuilds a list over every entry on every record() call, so the cost is linear in cache size and every inbound request pays it. Measured:

entries per record()
500 34us
2,000 146us
8,000 1.23ms
100,000 (DEFAULT_MAX_REMEMBERED) 6.35ms

Filling toward the default bound is quadratic; a 100,000-entry fill did not complete in 120 seconds. The docstring claims a flood "degrades to the challenge window rather than causing an outage", which is inverted: the flood is what causes the outage.

An expiry-ordered fix exists and is one line, since a constant TTL makes insertion order equal expiry order:

def _expire(self, now: float) -> None:
    while self._seen:
        sig, expires_at = next(iter(self._seen.items()))
        if expires_at >= now:
            break
        del self._seen[sig]

That measures 0.99us at 8k entries and fills 100k in 0.14s, with single-use, TTL expiry and the bound all preserved. It is recorded here because it is the right fix if the cache stays. It is not staying, so the work is removal.

Scope

  • Remove ProofReplayCache and the seen_proofs parameter from holder.verify_holder_proof, peer.verify_caller_holds_leaf, peer.handle_peer_request and PeerNode (including the _Unset sentinel in node.py).
  • State the guarantee honestly in holder.py and docs/spec/: at-most-once per challenge window, not exactly-once. A captured request replays until its challenge expires.
  • Drop or rewrite the tests that pin cache behaviour (test_a_proof_is_honoured_once, test_the_cache_only_remembers_proofs_that_verified, test_cache_entries_expire_with_their_challenge, test_cache_is_bounded_and_says_what_that_costs, test_a_node_remembers_proofs_by_default, test_a_node_can_opt_out_of_remembering, test_replay_over_http_is_refused). Keep test_without_a_cache_the_guarantee_is_only_the_window as the statement of the remaining guarantee.
  • Note the window in the threat model row added by fix(delegation): require holder binding before acting on a chain #97, so the residual replay surface is written down rather than implied.

@zohebk8s offered to do this in the original PR and is welcome to take it.

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