fix(download): persist bad-descriptor reputation through CapturingStateStore - #134
Merged
Merged
Conversation
…teStore CapturingStateStore delegated only load/save/clear and inherited the StateStore trait's no-op reputation defaults, so a holder demoted for a lying descriptor (#1611) was never written to the inner FileStateStore — the node persisted no reputation and re-asked the same liars every call and after every restart. Forward record_bad_descriptor + bad_descriptor_peers to the inner store so a demotion survives a restart. The ticket was filed against dig-download 0.9.0's holder-reputation names; the pinned 0.13.0 API is record_bad_descriptor/bad_descriptor_peers (same feature, evolved names) — the delegation gap is identical. Regression test records a verdict through one wrapper and reads it back through a fresh wrapper over the same on-disk store (simulated restart). Closes #1629 Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
CapturingStateStore(theStateStorewrapper the node handsdig-download) delegated onlyload/save/clearand silently inherited the trait's no-op defaults for the bad-descriptor reputation methods. So a holder demoted for serving a lying descriptor (#1611) was forgotten the moment the wrapper's innerFileStateStorenever saw the record — the node persisted no reputation, and every fresh call/process re-asked the same liars from scratch.This forwards both reputation methods to the inner
FileStateStore, which persists them to disk:record_bad_descriptor(target_key, peer_id)→self.inner.record_bad_descriptor(...)bad_descriptor_peers(target_key)→self.inner.bad_descriptor_peers(...)The wrapper's only job is commitment capture (chunk_lens/root/proof); reputation is now delegated verbatim.
Note on the ticket's method names
Issue #1629 was filed against dig-download 0.9.0, which named the API
record_holder_reputation/load_holder_reputation. By the pinned dig-download 0.13.0 the API isrecord_bad_descriptor/bad_descriptor_peers(same feature, evolved names). The fix targets the current 0.13 contract; the defect — the wrapper dropping the delegation — is identical.Verification
capturing_state_store_persists_bad_descriptor_reputation_across_restart: records a verdict through one wrapper, reads it back through a fresh wrapper over the same on-disk store (simulated restart). RED before the fix (left: []), GREEN after.cargo fmt --all -- --checkclean ·cargo clippy -p dig-node-core --all-targets --all-features -D warningsclean · fulldig-node-core --libsuite green.Version
fix:→ patch bump of the released binary version[workspace.package].version0.72.4 → 0.72.5.Closes #1629.
Generated by Claude Code