You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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.
Follow-up to #97.
ProofReplayCachewas 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.pyalready 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.
_expirerebuilds a list over every entry on everyrecord()call, so the cost is linear in cache size and every inbound request pays it. Measured:record()DEFAULT_MAX_REMEMBERED)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:
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
ProofReplayCacheand theseen_proofsparameter fromholder.verify_holder_proof,peer.verify_caller_holds_leaf,peer.handle_peer_requestandPeerNode(including the_Unsetsentinel innode.py).holder.pyanddocs/spec/: at-most-once per challenge window, not exactly-once. A captured request replays until its challenge expires.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). Keeptest_without_a_cache_the_guarantee_is_only_the_windowas the statement of the remaining guarantee.@zohebk8s offered to do this in the original PR and is welcome to take it.