refactor: introduce new helpers Uint256HashMap, Uint256HashSet, Uint256LruHashMap #6851
Conversation
…shMap for code simplification
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughAdds typed aliases for salted uint256 containers in src/saltedhasher.h: Uint256HashMap, Uint256HashSet, and Uint256LruHashMap. Replaces many direct uses of std::unordered_map/std::unordered_set keyed by uint256 (with StaticSaltedHasher) across many subsystems (chainlock, instantsend, llmq, evo, net, spork, etc.) with these aliases and corresponding LRU variants. Several function signatures, return types, class members, and public APIs were updated to use the new aliases (e.g., quorum and masternode APIs, various caches, and sig-share managers). One behavioral change: SetMasternodeQuorumRelayMembers now propagates QSENDRECSIGS to relay members. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)src/**/*.{cpp,h,cc,cxx,hpp}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (4)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/evo/mnhftx.h (1)
15-16: Include order for alias: include unordered_lru_cache before saltedhasherIf saltedhasher.h defines Uint256LruHashMap in terms of unordered_lru_cache, it must see that template first. Swap these two includes to avoid transitive‑include fragility.
Apply:
-#include <saltedhasher.h> -#include <unordered_lru_cache.h> +#include <unordered_lru_cache.h> +#include <saltedhasher.h>src/llmq/dkgsessionmgr.cpp (1)
99-186: Fix merge conflict reported by CIResolve the conflict in this file before merge.
src/instantsend/instantsend.cpp (1)
692-713: Bug: wrong key type used for mempool lookup.
mapNextTxis keyed byCOutPoint; passingCTxInwill not match and can fail to compile. Usein.prevout.Apply:
- auto it = mempool.mapNextTx.find(in); + auto it = mempool.mapNextTx.find(in.prevout);
🧹 Nitpick comments (8)
src/evo/deterministicmns.h (1)
630-632: LGTM: private caches migrated to aliasesNo behavior change; aliases keep hashing semantics via StaticSaltedHasher. Resolve the CI‑reported conflict in this header.
src/evo/cbtx.cpp (1)
62-62: Cache alias swap OK; add explicit include if not already indirect.If saltedhasher.h doesn’t get pulled in via another header here, include it to define Uint256LruHashMap.
Suggested include:
#include <llmq/utils.h> #include <node/blockstorage.h> +#include <saltedhasher.h> + #include <chain.h>Also, CI flags a merge conflict for this file—please rebase/resolve.
src/chainlock/chainlock.cpp (1)
116-116: Prefer std::chrono::seconds parameter and pass-by-value.Current signature uses const int64_t&; switch to std::chrono::seconds to avoid unit mismatches and drop the unnecessary reference.
Apply with the header change below:
-void CChainLocksHandler::UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time) +void CChainLocksHandler::UpdateTxFirstSeenMap(const Uint256HashSet& tx, std::chrono::seconds time) { AssertLockNotHeld(cs); LOCK(cs); for (const auto& txid : tx) { txFirstSeenTime.emplace(txid, time); } }Note: CI indicates a merge conflict in chainlock files—please rebase.
src/chainlock/chainlock.h (1)
90-90: Tighten time parameter type for UpdateTxFirstSeenMap.Recommend std::chrono::seconds by value instead of const int64_t& for clarity and type safety. Keep override signature in sync with parent.
- void UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time) override EXCLUSIVE_LOCKS_REQUIRED(!cs); + void UpdateTxFirstSeenMap(const Uint256HashSet& tx, std::chrono::seconds time) override EXCLUSIVE_LOCKS_REQUIRED(!cs);src/net.cpp (3)
3398-3400: Fix typo in comment and consider clearer name.
- s/fInboud/inbound/.
- Optional: rename
connectedProRegTxHashestoprotx_is_inboundfor clarity.Apply:
- // it maps protx hashes to fInboud flags - Uint256HashMap<bool> connectedProRegTxHashes; + // Maps proTx hashes to IsInboundConn() flags + Uint256HashMap<bool> connectedProRegTxHashes;
3467-3471: Avoid accidental insertion on lookup.Use iterator instead of
operator[](even though guarded by count) to prevent accidental insertion and double lookup.Apply:
- bool connectedAndOutbound = connectedProRegTxHashes.count(dmn->proTxHash) && !connectedProRegTxHashes[dmn->proTxHash]; + bool connectedAndOutbound = false; + if (auto it = connectedProRegTxHashes.find(dmn->proTxHash); + it != connectedProRegTxHashes.end()) { + connectedAndOutbound = !it->second; // present and outbound + }
3470-3471: Fix typo in log message.s/theed/need/.
Apply:
- // we already have an outbound connection to this MN so there is no theed to probe it again + // we already have an outbound connection to this MN so there is no need to probe it againsrc/llmq/utils.cpp (1)
973-989: Explicit instantiations: simplify overly verbose one.The
std::map<... , Uint256LruHashMap<std::shared_ptr<llmq::CQuorum>>, std::less<...>, std::allocator<...>>instantiation is needlessly specific. Prefer the shorter default-form to reduce template noise.-template void InitQuorumsCache< - std::map<Consensus::LLMQType, Uint256LruHashMap<std::shared_ptr<llmq::CQuorum>>, std::less<Consensus::LLMQType>, - std::allocator<std::pair<Consensus::LLMQType const, Uint256LruHashMap<std::shared_ptr<llmq::CQuorum>>>>>>( - std::map<Consensus::LLMQType, Uint256LruHashMap<std::shared_ptr<llmq::CQuorum>>, std::less<Consensus::LLMQType>, - std::allocator<std::pair<Consensus::LLMQType const, Uint256LruHashMap<std::shared_ptr<llmq::CQuorum>>>>>& cache, - bool limit_by_connections); +template void InitQuorumsCache<std::map<Consensus::LLMQType, Uint256LruHashMap<std::shared_ptr<llmq::CQuorum>>>>( + std::map<Consensus::LLMQType, Uint256LruHashMap<std::shared_ptr<llmq::CQuorum>>>& cache, + bool limit_by_connections);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
src/chainlock/chainlock.cpp(1 hunks)src/chainlock/chainlock.h(2 hunks)src/chainlock/signing.cpp(3 hunks)src/chainlock/signing.h(3 hunks)src/evo/cbtx.cpp(1 hunks)src/evo/creditpool.cpp(1 hunks)src/evo/creditpool.h(1 hunks)src/evo/deterministicmns.h(1 hunks)src/evo/mnhftx.h(1 hunks)src/instantsend/db.cpp(3 hunks)src/instantsend/db.h(2 hunks)src/instantsend/instantsend.cpp(5 hunks)src/instantsend/instantsend.h(2 hunks)src/instantsend/signing.cpp(1 hunks)src/instantsend/signing.h(2 hunks)src/llmq/blockprocessor.h(1 hunks)src/llmq/dkgsession.cpp(1 hunks)src/llmq/dkgsession.h(2 hunks)src/llmq/dkgsessionmgr.cpp(1 hunks)src/llmq/quorums.cpp(1 hunks)src/llmq/quorums.h(1 hunks)src/llmq/signing.cpp(1 hunks)src/llmq/signing.h(2 hunks)src/llmq/signing_shares.cpp(8 hunks)src/llmq/signing_shares.h(4 hunks)src/llmq/snapshot.h(1 hunks)src/llmq/utils.cpp(5 hunks)src/llmq/utils.h(1 hunks)src/net.cpp(4 hunks)src/net.h(2 hunks)src/saltedhasher.h(2 hunks)src/spork.h(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/llmq/dkgsession.cppsrc/instantsend/signing.cppsrc/llmq/signing.cppsrc/spork.hsrc/llmq/quorums.cppsrc/chainlock/chainlock.hsrc/chainlock/chainlock.cppsrc/llmq/blockprocessor.hsrc/evo/deterministicmns.hsrc/llmq/dkgsessionmgr.cppsrc/instantsend/db.hsrc/evo/creditpool.hsrc/evo/creditpool.cppsrc/evo/mnhftx.hsrc/saltedhasher.hsrc/net.hsrc/llmq/snapshot.hsrc/instantsend/instantsend.hsrc/llmq/dkgsession.hsrc/evo/cbtx.cppsrc/net.cppsrc/chainlock/signing.cppsrc/llmq/signing.hsrc/instantsend/signing.hsrc/llmq/signing_shares.cppsrc/chainlock/signing.hsrc/llmq/utils.cppsrc/llmq/quorums.hsrc/instantsend/db.cppsrc/llmq/signing_shares.hsrc/llmq/utils.hsrc/instantsend/instantsend.cpp
src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/evo/deterministicmns.hsrc/evo/creditpool.hsrc/evo/creditpool.cppsrc/evo/mnhftx.hsrc/evo/cbtx.cpp
🧠 Learnings (2)
📚 Learning: 2025-01-07T18:50:44.838Z
Learnt from: knst
PR: dashpay/dash#6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions `MigrateDBIfNeeded` and `MigrateDBIfNeeded2` in `src/evo/deterministicmns.cpp` are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Applied to files:
src/evo/deterministicmns.h
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
PR: dashpay/dash#6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Applied to files:
src/instantsend/instantsend.h
🧬 Code graph analysis (13)
src/chainlock/chainlock.h (1)
src/chainlock/chainlock.cpp (2)
UpdateTxFirstSeenMap(116-123)UpdateTxFirstSeenMap(116-116)
src/llmq/blockprocessor.h (1)
src/llmq/params.h (1)
LLMQType(14-507)
src/evo/deterministicmns.h (1)
src/chainlock/chainlock.h (2)
cs(59-128)cs(60-125)
src/instantsend/db.h (2)
src/instantsend/db.cpp (2)
RemoveConfirmedInstantSendLocks(119-165)RemoveConfirmedInstantSendLocks(119-119)src/llmq/quorums.h (1)
cs_db(238-314)
src/net.h (1)
src/net.cpp (8)
SetMasternodeQuorumNodes(4299-4306)SetMasternodeQuorumNodes(4299-4299)SetMasternodeQuorumRelayMembers(4308-4331)SetMasternodeQuorumRelayMembers(4308-4308)HasMasternodeQuorumNodes(4333-4337)HasMasternodeQuorumNodes(4333-4333)GetMasternodeQuorums(4339-4350)GetMasternodeQuorums(4339-4339)
src/instantsend/instantsend.h (3)
src/instantsend/lock.h (1)
instantsend(18-41)src/saltedhasher.h (1)
uint256(38-44)src/instantsend/instantsend.cpp (4)
ProcessPendingInstantSendLocks(168-231)ProcessPendingInstantSendLocks(168-168)ProcessPendingInstantSendLocks(233-343)ProcessPendingInstantSendLocks(233-236)
src/evo/cbtx.cpp (8)
src/evo/creditpool.cpp (1)
GUARDED_BY(71-71)src/llmq/utils.cpp (2)
GUARDED_BY(207-208)GUARDED_BY(210-210)src/net.cpp (1)
GUARDED_BY(132-132)src/llmq/dkgsessionmgr.cpp (1)
GUARDED_BY(99-99)src/llmq/quorums.cpp (1)
GUARDED_BY(37-37)src/evo/specialtxman.cpp (2)
GUARDED_BY(37-37)GUARDED_BY(38-38)src/validation.cpp (1)
GUARDED_BY(2147-2147)src/evo/simplifiedmns.cpp (3)
GUARDED_BY(107-107)GUARDED_BY(109-109)GUARDED_BY(110-110)
src/chainlock/signing.cpp (3)
src/llmq/signing_shares.cpp (2)
Cleanup(1285-1421)Cleanup(1285-1285)src/chainlock/chainlock.cpp (2)
Cleanup(432-485)Cleanup(432-432)src/chainlock/signing.h (1)
cs_signer(61-88)
src/instantsend/signing.h (1)
src/instantsend/signing.cpp (2)
ClearInputsFromQueue(58-64)ClearInputsFromQueue(58-58)
src/chainlock/signing.h (3)
src/chainlock/chainlock.cpp (4)
UpdateTxFirstSeenMap(116-123)UpdateTxFirstSeenMap(116-116)Cleanup(432-485)Cleanup(432-432)src/saltedhasher.h (1)
uint256(38-44)src/chainlock/signing.cpp (2)
Cleanup(246-265)Cleanup(246-246)
src/llmq/quorums.h (1)
src/llmq/params.h (1)
LLMQType(14-507)
src/llmq/signing_shares.h (2)
src/chainlock/chainlock.h (2)
cs(59-128)cs(60-125)src/llmq/signing_shares.cpp (8)
CollectSigSharesToRequest(860-955)CollectSigSharesToRequest(860-860)CollectSigSharesToSend(957-1005)CollectSigSharesToSend(957-957)CollectSigSharesToSendConcentrated(1007-1050)CollectSigSharesToSendConcentrated(1007-1007)CollectSigSharesToAnnounce(1052-1108)CollectSigSharesToAnnounce(1052-1053)
src/llmq/utils.h (1)
src/llmq/utils.cpp (4)
GetQuorumConnections(736-760)GetQuorumConnections(736-739)GetQuorumRelayMembers(762-817)GetQuorumRelayMembers(762-765)
🪛 GitHub Actions: Check Potential Conflicts
src/llmq/dkgsession.cpp
[error] Merge conflict in 'src/llmq/dkgsession.cpp' file.
src/llmq/signing.cpp
[error] Merge conflict in 'src/llmq/signing.cpp' file.
src/spork.h
[error] Merge conflict in 'src/spork.h' file.
src/llmq/quorums.cpp
[error] Merge conflict in 'src/llmq/quorums.cpp' and 'src/llmq/quorums.h' files.
src/chainlock/chainlock.h
[error] Merge conflict in 'src/chainlock/chainlock.cpp' and 'src/chainlock/chainlock.h' files.
src/chainlock/chainlock.cpp
[error] Merge conflict in 'src/chainlock/chainlock.cpp' and 'src/chainlock/chainlock.h' files.
src/llmq/blockprocessor.h
[error] Merge conflict in 'src/llmq/blockprocessor.h' file.
src/evo/deterministicmns.h
[error] Merge conflict in 'src/evo/deterministicmns.h' file.
src/llmq/dkgsessionmgr.cpp
[error] Merge conflict in 'src/llmq/dkgsessionmgr.cpp'
src/instantsend/db.h
[error] Merge conflict in 'src/instantsend/db.cpp' and 'src/instantsend/db.h' files.
src/evo/creditpool.h
[error] Merge conflict in 'src/evo/creditpool.cpp' and 'src/evo/creditpool.h' files.
src/evo/creditpool.cpp
[error] Merge conflict in 'src/evo/creditpool.cpp' and 'src/evo/creditpool.h' files.
src/evo/mnhftx.h
[error] Merge conflict in 'src/evo/mnhftx.h' file.
src/net.h
[error] Merge conflict in 'src/net.cpp' and 'src/net.h' files.
src/llmq/snapshot.h
[error] Merge conflict in 'src/llmq/snapshot.h'
src/instantsend/instantsend.h
[error] Merge conflict in 'src/instantsend/instantsend.cpp' and 'src/instantsend/instantsend.h' files.
src/llmq/dkgsession.h
[error] Merge conflict in 'src/llmq/dkgsession.cpp' and 'src/llmq/dkgsession.h'
src/evo/cbtx.cpp
[error] Merge conflict in 'src/evo/cbtx.cpp' file.
src/net.cpp
[error] Merge conflict in 'src/net.cpp' and 'src/net.h' files.
src/llmq/signing.h
[error] Merge conflict in 'src/llmq/signing.cpp' and 'src/llmq/signing.h'
src/instantsend/signing.h
[error] Merge conflict in 'src/instantsend/signing.h' file.
src/llmq/signing_shares.cpp
[error] Merge conflict in 'src/llmq/signing_shares.cpp' and 'src/llmq/signing_shares.h' files.
src/llmq/utils.cpp
[error] Merge conflict in 'src/llmq/utils.cpp' and 'src/llmq/utils.h' files.
src/llmq/quorums.h
[error] Merge conflict in 'src/llmq/quorums.cpp' and 'src/llmq/quorums.h' files.
src/instantsend/db.cpp
[error] Merge conflict in 'src/instantsend/db.cpp' and 'src/instantsend/db.h' files.
src/llmq/signing_shares.h
[error] Merge conflict in 'src/llmq/signing_shares.cpp' and 'src/llmq/signing_shares.h' files.
src/llmq/utils.h
[error] Merge conflict in 'src/llmq/utils.cpp' and 'src/llmq/utils.h' files.
src/instantsend/instantsend.cpp
[error] Merge conflict in 'src/instantsend/instantsend.cpp' and 'src/instantsend/instantsend.h' files.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: x86_64-w64-mingw32 / Build depends
- GitHub Check: x86_64-apple-darwin / Build depends
- GitHub Check: arm-linux-gnueabihf / Build depends
- GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
- GitHub Check: x86_64-pc-linux-gnu / Build depends
- GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
🔇 Additional comments (74)
src/llmq/dkgsession.cpp (1)
472-479: LGTM: alias swap is correctLocal map keyed by uint256 → Uint256HashMap is a no‑op behaviorally. Header transitively provides the alias.
src/llmq/dkgsession.h (2)
301-301: Container type for relayMembers updated: OKInternal storage moved to Uint256HashSet. No issues.
393-393: No changes required — callers accept const referenceOnly usage found: src/llmq/dkgsessionhandler.cpp:442 —
const auto& relayMembers = session.RelayMembers();(compatible; no updates needed).src/llmq/blockprocessor.h (1)
54-54: Fix merge conflict; Uint256LruHashMap alias confirmed
- Found: src/saltedhasher.h:89 —
using Uint256LruHashMap = unordered_lru_cache<...>;and usage at src/llmq/blockprocessor.h:54. Resolve the CI-reported merge conflict in src/llmq/blockprocessor.h before merging.src/llmq/quorums.cpp (1)
374-384: No changes required — saltedhasher is already included and net API matchessrc/llmq/quorums.h includes <saltedhasher.h>, and CConnman::SetMasternodeQuorumNodes / SetMasternodeQuorumRelayMembers use const Uint256HashSet& (src/net.h:1483–1484, src/net.cpp:4299–4308).
src/instantsend/signing.cpp (1)
58-64: Signature change to Uint256HashSet — OK
Declaration and definition match; call sites updated.
Decl: src/instantsend/signing.h:81
Def: src/instantsend/signing.cpp:58
Call sites: src/instantsend/instantsend.cpp:601,609src/evo/creditpool.cpp (1)
71-72: Alias migration to Uint256LruHashMap looks good.Capacity and locking remain intact; no logic change. Note: CI reports a merge conflict for this file—please rebase/resolve before merge.
src/spork.h (1)
168-168: Spork hash index switched to Uint256HashMap — OK.Serialization stays the same (unordered container), threading annotations unchanged. Also resolve the reported merge conflict in this header before merging.
src/evo/creditpool.h (1)
115-115: creditPoolCache → Uint256LruHashMap is consistent.Header already includes saltedhasher.h; locking and size init preserved. CI shows a merge conflict touching this header—please resolve.
src/llmq/snapshot.h (1)
219-219: quorumSnapshotCache alias migration LGTM.Constructor size param retained; thread-safety annotations unchanged. Address the CI-reported merge conflict before merge.
src/chainlock/chainlock.h (1)
62-62: txFirstSeenTime switched to Uint256HashMap — OK.Type aligns with alias strategy elsewhere; no behavior change.
src/llmq/signing.cpp (1)
567-567: Alias swap LGTM — ensure saltedhasher.h is includedUint256HashSet is correct for dedup; ripgrep returned "No files were searched" so verification is inconclusive. Confirm saltedhasher.h is included transitively or add it to this TU (src/llmq/signing.cpp — ~line 567).
Suggested include:
#include <validation.h> +#include <saltedhasher.h> + #include <algorithm> #include <unordered_set>Run this local check (use -uu to avoid gitignore hiding files):
#!/bin/bash rg -nP -S -uu --glob '!**/third_party/**' '(unordered_(map|set)\s*<\s*uint256\b|unordered_lru_cache\s*<\s*uint256\b)' rg -nP -S -uu --glob '!**/third_party/**' 'Uint256(Hash(Map|Set)|LruHashMap)' -C2src/saltedhasher.h (3)
12-13: LGTM! Proper includes added for type aliases.The standard library includes for
<unordered_map>and<unordered_set>are correctly added to support the new template aliases.
80-83: LGTM! Clean and well-structured template aliases.The
Uint256HashMap<T>andUint256HashSetaliases provide a clean abstraction for 256-bit keyed containers usingStaticSaltedHasher. This follows C++ best practices for template aliases and reduces boilerplate throughout the codebase.
86-89: LGTM! Proper forward declaration and LRU cache alias.The forward declaration of
unordered_lru_cacheis necessary and correctly placed. TheUint256LruHashMapalias maintains consistency with the naming convention while providing sensible defaults forMaxSizeandTruncateThreshold.src/instantsend/signing.h (3)
60-60: LGTM! Container type updated consistently.The
inputRequestIdsfield is correctly updated to useUint256HashSet, maintaining thread safety with the existingGUARDED_BY(cs_input_requests)annotation.
67-69: LGTM! Internal container types updated appropriately.Both
creatingInstantSendLocksandtxToCreatingInstantSendLocksare correctly updated to use the new type aliases while preserving their thread safety annotations and logical relationship (where one maps to values and the other to pointers).
81-81: LGTM! Public API signature updated correctly.The
ClearInputsFromQueuemethod signature is correctly updated to acceptconst Uint256HashSet&, which matches the implementation shown in the relevant code snippets fromsrc/instantsend/signing.cpp.src/instantsend/db.cpp (3)
119-119: LGTM! Return type updated consistently.The
RemoveConfirmedInstantSendLocksfunction return type is correctly updated toUint256HashMap<InstantSendLockPtr>, matching the header declaration.
136-136: LGTM! Local variable type updated appropriately.The local
retvariable type is correctly updated toUint256HashMap<InstantSendLockPtr>to match the function's return type and usage pattern.
362-362: LGTM! Helper set type updated consistently.The local
addedset inRemoveChainedInstantSendLocksis appropriately updated to useUint256HashSetfor tracking processed transaction IDs.src/chainlock/signing.cpp (3)
161-161: LGTM! Container creation updated consistently.The shared pointer creation is correctly updated to use
std::make_shared<Uint256HashSet>(), maintaining consistency with the new type aliases.
203-203: LGTM! Consistent container type usage.The block transaction set creation is properly updated to use
std::make_shared<Uint256HashSet>(), maintaining consistency throughout the function.
246-249: LGTM! Function signature and implementation updated correctly.Both the function signature and local variable declaration in
Cleanup()are correctly updated to usestd::vector<std::shared_ptr<Uint256HashSet>>, maintaining consistency with the type alias changes.src/instantsend/instantsend.h (4)
69-71: LGTM! Pending lock containers updated appropriately.Both
pendingInstantSendLocksandpendingNoTxInstantSendLocksare correctly updated to useUint256HashMap<std::pair<NodeId, instantsend::InstantSendLockPtr>>, maintaining the existing thread safety annotations.
78-78: LGTM! Nested container type updated consistently.The
childrenfield inNonLockedTxInfois appropriately updated to useUint256HashSetfor tracking child transaction IDs.
82-82: LGTM! Container types updated across the board.All three containers (
nonLockedTxs,pendingRetryTxs, andtimingsTxSeen) are correctly updated to use the appropriateUint256HashMapandUint256HashSetaliases while preserving their thread safety annotations.Also applies to: 86-86, 89-89
113-115: LGTM! Method signature updated consistently.The
ProcessPendingInstantSendLocksmethod signature is correctly updated to returnUint256HashSetand acceptconst Uint256HashMap<std::pair<NodeId, instantsend::InstantSendLockPtr>>&, which matches the implementation shown in the relevant code snippets.src/chainlock/signing.h (3)
37-37: LGTM! Interface method signature updated correctly.The
UpdateTxFirstSeenMapvirtual method signature is properly updated to acceptconst Uint256HashSet&, which aligns with the implementation changes across the chainlock module.
55-55: LGTM! Type alias updated appropriately.The
BlockTxstype alias is correctly updated to map tostd::shared_ptr<Uint256HashSet>, maintaining consistency with the container type changes throughout the file.
83-83: LGTM! Return type updated consistently.The
Cleanupmethod return type is appropriately updated tostd::vector<std::shared_ptr<Uint256HashSet>>, matching the implementation and maintaining consistency with the type alias changes.src/instantsend/db.h (2)
39-40: LGTM! Cache container types updated appropriately.Both
islockCacheandtxidCacheare correctly updated to useUint256LruHashMapwith appropriate template parameters, maintaining their cache sizes and thread safety annotations.
104-104: LGTM! Method return type updated consistently.The
RemoveConfirmedInstantSendLocksmethod signature is properly updated to returnUint256HashMap<InstantSendLockPtr>, which matches the implementation in the corresponding source file.src/llmq/signing.h (2)
118-119: LGTM! Cache container types updated consistently.Both
hasSigForSessionCacheandhasSigForHashCacheare correctly updated fromunordered_lru_cachewithStaticSaltedHashertoUint256LruHashMap<bool, 30000>, maintaining their cache functionality and thread safety.
168-168: LGTM! Pending container type updated appropriately.The
pendingReconstructedRecoveredSigscontainer is correctly updated to useUint256HashMap<std::shared_ptr<const CRecoveredSig>>, maintaining its thread safety annotation and logical purpose.src/net.cpp (3)
4308-4331: Behavioral change: proactively enabling REC_SIG relays — verify scope.Sending
QSENDRECSIGS(true)on all verified quorum-relay members is reasonable. Please confirm:
- This only runs after MNAUTH (verified proRegTxHash is non-null) — it does.
- No duplicates: guarded by
!m_masternode_iqr_connection— OK.- Net processing side accepts this at
GetCommonVersion()used here — confirm min protocol where QSENDRECSIGS is valid.
4339-4349: LGTM: alias adoption and return type.
Uint256HashSetusage and construction look correct.
1-10: Resolve CI merge conflicts in src/net.cppRebase and remove any conflict markers (<<<<<<<, =======, >>>>>>>) from src/net.cpp (reported around lines 1–10). Sandbox ripgrep produced no matches; if CI still reports conflicts, verify locally with: rg -n '^(<{7}|={7}|>{7})' src/net.cpp src/net.h.
src/net.h (3)
1746-1748: LGTM: internal maps moved to Uint256HashSet.Matches the public API and reduces boilerplate.
1-10: Rebase and confirm no merge-conflict markers remain in src/net.h.rg scan of src/net.h returned no matches. If CI still reports conflicts, run from the repo root and paste the output or confirm the rebase completed:
rg -n --hidden -S '^(<{7}|={7}|>{7})' .
1482-1487: Public API switch to Uint256HashSet — callers updated. Confirmed SetMasternodeQuorumNodes/SetMasternodeQuorumRelayMembers are called with Uint256HashSet in src/llmq/utils.cpp and src/llmq/quorums.cpp; no raw std::unordered_set<uint256, StaticSaltedHasher> usages remain and the alias is defined in src/saltedhasher.h.src/instantsend/instantsend.cpp (4)
40-43: LGTM: alias return type and reserve.
Uint256HashSet+ reserve is correct and efficient.
723-724: LGTM: nested container alias.Outer map by block index with inner
Uint256HashMapis fine.
233-236: LGTM — aliases and signatures verified. Uint256HashMap/Uint256HashSet aliases are defined in src/saltedhasher.h and both ProcessPendingInstantSendLocks declarations in src/instantsend/instantsend.h match the implementations in src/instantsend/instantsend.cpp; types for pend/recSigs/badISLocks are consistent.
1-10: Resolve CI merge conflicts (instantsend headers) — verification required.CI reports conflicts between src/instantsend/instantsend.cpp and src/instantsend/instantsend.h; searching for merge conflict markers in the workspace returned no matches. Provide the CI failure log or the conflicting hunk(s), or run locally and paste the output of:
rg -n '^(<{7}|={7}|>{7})' src/instantsend/instantsend.* || truesrc/llmq/quorums.h (4)
256-256: Cleanup cache alias LGTM.No behavior change expected.
251-251: Good swap to alias for quorum caches; capacity init confirmed.
utils::InitQuorumsCache(mapQuorumsCache, false) is called in the constructor (src/llmq/quorums.cpp:228); cleanup path also initializes cleanupQuorumsCache via InitQuorumsCache(cleanupQuorumsCache, false) (src/llmq/quorums.cpp:1112).
253-254: Consistent alias use for scan cache — runtime capacity initialization verified.scanQuorumsCache is initialized in src/llmq/quorums.cpp via try_emplace(llmq.type, llmq.max_cycles(llmq.keepOldConnections) * (llmq.dkgMiningWindowEnd - llmq.dkgMiningWindowStart)); — no change required.
260-262: Explicit 128-size LRU retained — no capacity change.
Parent commit used unordered_lru_cache<..., 128>, so max_size is unchanged.src/llmq/signing_shares.cpp (9)
1011-1011: proTxToNode switched to Uint256HashMap — OK.Lookup/emplace usage remains correct.
1052-1054: Announce-map now Uint256HashMap — OK.Consistent with other per-node maps.
1112-1116: Outbound message buffers updated to aliases — OK.Allocations/iteration semantics unchanged.
1321-1321: inactiveQuorumSessions: alias set — OK.No change in semantics.
1337-1337: doneSessions: alias set — OK.No change in semantics.
1351-1351: timeoutSessions: alias set — OK.No change in semantics.
957-957: Send-map now Uint256HashMap — OK.
Declaration in src/llmq/signing_shares.h:482 matches definition in src/llmq/signing_shares.cpp:957 — no functional change.
860-860: Signature now uses Uint256HashMap — verified.Declaration (src/llmq/signing_shares.h:480), definition (src/llmq/signing_shares.cpp:860) and call site (src/llmq/signing_shares.cpp:1142) match; no further changes required.
1-1: Resolved — header and source signatures now match; no conflict markers found in signing_shares.{h,cpp}.CollectSigSharesToRequest, CollectSigSharesToSend and CollectSigSharesToAnnounce declarations in the header match their implementations in the .cpp.
src/llmq/signing_shares.h (5)
160-160: SigShareMap internalMap switched to Uint256HashMap — OK.Public surface unchanged; storage clearer.
334-334: sessions now Uint256HashMap — OK.Find/insert semantics preserved.
385-385: signedSessions now Uint256HashMap — OK.No behavior change.
388-388: timeSeenForSessions now Uint256HashMap — OK.No behavior change.
479-487: Collect methods’ params updated to Uint256HashMap — header and implementation match.*
Signatures match; no changes required.src/llmq/utils.cpp (8)
208-209: Quorum members cache alias — OK.Runtime size still configured via InitQuorumsCache; good.
736-740: GetQuorumConnections: alias return + spork param — OK.Logic untouched; consistent with header.
743-743: Use Uint256HashSet for result — OK.
762-766: GetQuorumRelayMembers: alias return — OK.
768-768: Result set alias — OK.
774-774: Local outbound set alias — OK.
864-866: connections/relayMembers now alias sets — OK.Callers (connman setters) unchanged.
1-1: Incorrect — no merge-conflict markers in src/llmq/utils.h or src/llmq/utils.cpp.
Searched both files for '<<<<<<<', '=======', '>>>>>>>' and found none.Likely an incorrect or invalid review comment.
src/llmq/utils.h (3)
41-45: Return type migrated to Uint256HashSet; sporkman param added — callers updated.Verified call sites in src/rpc/quorums.cpp and src/llmq/utils.cpp updated to pass sporkman and use the new return type.
51-56: EnsureQuorumConnections signature expanded — callers updatedDeclaration/definition updated; verified call sites in src/llmq/quorums.cpp and src/llmq/dkgsessionhandler.cpp pass the new parameters — no further changes required.
45-48: Verify callers handle Uint256HashSet return typeGetQuorumRelayMembers returns Uint256HashSet — OK. Call sites: src/llmq/utils.cpp:759, 762, 869 and src/llmq/dkgsession.cpp:137 — confirm the assigned variable (relayMembers) is declared as Uint256HashSet or explicitly converted where used.
| Uint256LruHashMap<Signals> mnhfCache GUARDED_BY(cs_cache){MNHFCacheSize}; | ||
|
|
There was a problem hiding this comment.
Fix merge conflict and validate constructor call
- CI flags a conflict in this header; resolve it.
- Brace‑init with MNHFCacheSize is correct for unordered_lru_cache; no further changes needed.
🧰 Tools
🪛 GitHub Actions: Check Potential Conflicts
[error] Merge conflict in 'src/evo/mnhftx.h' file.
🤖 Prompt for AI Agents
In src/evo/mnhftx.h around lines 100 to 101, resolve the leftover merge conflict
markers so the declaration reads as a single clean line, ensuring the
Uint256LruHashMap<Signals> mnhfCache GUARDED_BY(cs_cache){MNHFCacheSize};
remains intact; confirm the brace-init with MNHFCacheSize is preserved (it is
the correct constructor for unordered_lru_cache) and remove any conflict
artifacts or duplicated declarations so the header compiles cleanly.
| static std::map<Consensus::LLMQType, Uint256LruHashMap<int>> indexedQuorumsCache GUARDED_BY(cs_indexedQuorumsCache); | ||
|
|
There was a problem hiding this comment.
Missing include for alias in this TU
This file uses Uint256LruHashMap but doesn’t include saltedhasher.h. Add it to avoid relying on transitive includes.
Apply:
#include <spork.h>
#include <unordered_lru_cache.h>
+#include <saltedhasher.h>
#include <util/irange.h>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static std::map<Consensus::LLMQType, Uint256LruHashMap<int>> indexedQuorumsCache GUARDED_BY(cs_indexedQuorumsCache); | |
| #include <spork.h> | |
| #include <unordered_lru_cache.h> | |
| #include <saltedhasher.h> | |
| #include <util/irange.h> | |
| static std::map<Consensus::LLMQType, Uint256LruHashMap<int>> indexedQuorumsCache GUARDED_BY(cs_indexedQuorumsCache); |
🧰 Tools
🪛 GitHub Actions: Check Potential Conflicts
[error] Merge conflict in 'src/llmq/dkgsessionmgr.cpp'
🤖 Prompt for AI Agents
In src/llmq/dkgsessionmgr.cpp around lines 99-100, the code uses the
Uint256LruHashMap alias but does not directly include saltedhasher.h; add
#include "saltedhasher.h" (or the correct relative path to that header in the
project) near the other includes at the top of the TU so the alias/type is
defined here rather than relying on transitive includes.
Issue being fixed or feature implemented
Instead typing
std::unordered_map<uint256, CDeterministicMNList, StaticSaltedHasher> mnListsCachebetter to just writeUint256HashMap<CDeterministicMNList> mnListsCacheto minimize a boiler code. This PR aims to replace #6839, but this PR reduces scope significantly, no extra header is introduced.What was done?
Main idea is authored by kwvg's PR:
These helpers are used to prepare a PR:
Leftover changes are done manually.
Please, note, that compilation time is not affected by including
<unordered_set>and<unordered_map>because they are already included indirectly by chain<saltedhasher.h> -> <hash.h> -> <serialize.h>How Has This Been Tested?
Grep all over codebase by
StaticSaltedHasherBreaking Changes
N/A
Checklist: