fix(net): amortize ChainLock seen-cache pruning#7482
Draft
thepastaclaw wants to merge 1 commit into
Draft
Conversation
dashpay#7424 bounded ChainlockHandler::seenChainLocks by switching it to an unordered_limitedmap constructed as `seenChainLocks{MAX_SEEN_CHAINLOCKS}`. unordered_limitedmap defaults nPruneAfterSize to nMaxSize, so prune() runs on every unique insertion past 1024 entries: it allocates a vector of all 1025 iterators, std::sort()s it in full, and erases a single element, all while ChainlockHandler::cs is held. ProcessNewChainLock records the CLSIG hash before both the stale-height early return and VerifyChainLock, and stale-height CLSIGs carry no misbehavior penalty, so a peer could turn a stream of unique, unverified CLSIG hashes into a stream of O(n log n) sorts under cs -- CPU amplification on the ChainLocks relay path. Construct the cache with an explicit prune-after size of twice the retained size instead. The map now grows to 2048 entries and is pruned back to 1024 in one batch, amortising each sort over 1024 evictions rather than one, at the cost of a larger transient cache. The 2x ratio matches the default already used by unordered_lru_cache. Note that pruning less often also reduces the chance that an entry is evicted by the same prune that inserted it, since entries inserted within one second share a timestamp and tie under prune()'s comparator. Stale CLSIG duplicate suppression is unchanged: hashes are still recorded before the stale-height return, and the 24h time-based Cleanup() is unaffected. Naming now distinguishes the retained size from the temporary prune threshold (MAX_SEEN_CHAINLOCKS -> SEEN_CHAINLOCKS_RETAINED_SIZE plus a new SEEN_CHAINLOCKS_PRUNE_AFTER_SIZE), and the constructor's defaulted nPruneAfterSize is documented as the footgun it is. Tests: two new limitedmap cases prove the generic container semantics -- no prune at retained max+1, growth bounded by the trigger, and a batch prune back to the retained size on crossing it -- plus the default-threshold behavior. The ChainLock handler test no longer asserts a strict instantaneous 1024 cap; it now verifies how the handler wires the cache up and that growth past the retained size is retained until the trigger is crossed. Verified the handler test fails when the fix is reverted.
Author
|
✅ Final review complete — no blockers (commit 64cf27d) |
thepastaclaw
commented
Jul 25, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Author
There was a problem hiding this comment.
Final validation — Codex + Sonnet
The change correctly configures the ChainLock seen cache to retain 1,024 entries while allowing growth to 2,048 before batch pruning, amortizing the peer-triggerable sort cost without changing stale CLSIG duplicate suppression or cleanup behavior. The generic container semantics, handler integration, focused tests, commit history, and exact-head diff are internally consistent, with no correctness or Dash-specific integration issues identified.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
gpt-5.6-sol— final-verifier (fallback) - Sonnet reviewers:
claude-sonnet-5— general (failed),claude-sonnet-5— dash-core-commit-history (failed),claude-sonnet-5— general (failed),claude-sonnet-5— dash-core-commit-history (completed),claude-sonnet-5— general (completed)
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.
Issue being fixed or feature implemented
Dash #7424 bounded
ChainlockHandler::seenChainLockswithunordered_limitedmap, but the single-argument constructor makes the prune trigger equal to the 1,024-entry retained size. After the cache fills, every unique insertion creates and sorts a 1,025-element iterator vector and removes one entry while holdingChainlockHandler::cs.ProcessNewChainLock()records the hash before its stale-height return and before signature verification. A peer can therefore submit unlimited unique stale-height CLSIGs, receive no misbehavior penalty, and trigger the full sort on every message.What was done?
seenChainLockswith a 1,024-entry retained size and a 2,048-entry prune trigger, so one sort removes a batch of 1,025 entries instead of one.unordered_limitedmap's default prune behavior and exposed its configured threshold for focused tests.Stale CLSIG hashes are still remembered before the early return, and the existing 24-hour cleanup behavior is unchanged.
How Has This Been Tested?
Tested locally on macOS against current
develop:./autogen.shand./configurewith the existingaarch64-apple-darwindepends prefixmake -C src test/test_dash./src/test/test_dash --run_test=limitedmap_tests,llmq_chainlock_tests./src/test/test_dash --run_test=limitedmap_tests,llmq_chainlock_tests,llmq_signing_tests,llmq_dkg_tests,evo_islock_testsgit diff --check upstream/develop..HEADCOMMIT_RANGE=upstream/develop..HEAD test/lint/lint-whitespace.pytest/lint/lint-circular-dependencies.pytest/lint/lint-include-guards.pyship, zero findingsBreaking Changes
None.
Checklist: