refactor: move ChainLocks impl. to src/chainlock/ and split masternode logic out of manager source
#6761
Conversation
|
This pull request has conflicts, please rebase. |
|
WalkthroughThis change set refactors the ChainLocks subsystem by moving chainlock-related source and header files from the Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Complexity label: Complex Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (45)
💤 Files with no reviewable changes (4)
✅ Files skipped from review due to trivial changes (8)
🚧 Files skipped from review as they are similar to previous changes (30)
🧰 Additional context used📓 Path-based instructions (1)src/**/*.{cpp,h,cc,cxx,hpp}📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
🧠 Learnings (21)📓 Common learnings📚 Learning: in pr #6761, kwvg acknowledged a null pointer check issue in chainlocksigner::cleanup() method but d...Applied to files:
📚 Learning: code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded...Applied to files:
📚 Learning: in refactoring prs like #6761, kwvg prefers to defer code formatting fixes to separate follow-up prs...Applied to files:
📚 Learning: keep suggestions focused on the scope of the current commit/pr. avoid suggesting unrelated improveme...Applied to files:
📚 Learning: in refactoring prs like #6761, kwvg acknowledges code safety improvements (like null pointer checks ...Applied to files:
📚 Learning: when reviewing prs, ensure that suggestions are directly related to the pr's primary objectives rath...Applied to files:
📚 Learning: in backport prs like #6786, udjinm6 prefers to defer non-critical fixes (such as shell command expan...Applied to files:
📚 Learning: in the test framework consolidation pr (#6718), user kwvg prefers to limit functional changes to tho...Applied to files:
📚 Learning: pull request #6543 is focused on move-only changes and refactoring, specifically backporting from bi...Applied to files:
📚 Learning: when clang-format suggestions significantly harm readability (like splitting logical parameter group...Applied to files:
📚 Learning: in dash core, kwvg prefers using assert() statements to document expected behavior in code paths, es...Applied to files:
📚 Learning: the getancestor() function in cblockindex safely handles negative heights by returning nullptr rathe...Applied to files:
📚 Learning: the `getwallet()` function calls in `src/wallet/rpcwallet.cpp` are properly validated with null chec...Applied to files:
📚 Learning: llmqcontext manages concurrency for the `cinstantsendmanager`. previously, this was handled globally...Applied to files:
📚 Learning: the `cquorummanager` is fully initialized by `llmqcontext`, addressing any concerns about the manage...Applied to files:
📚 Learning: in the dash core llmq implementation, signingactivequorumcount is never 0 in the actual parameters d...Applied to files:
📚 Learning: the updatetransactionsfromblock() method in txmempool.cpp takes parameters in the order: vhashupdate...Applied to files:
📚 Learning: applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : masternode lists must use immutable data s...Applied to files:
📚 Learning: dash core uses bitcoin_ prefix for header guards as the standard convention, inherited from bitcoin ...Applied to files:
📚 Learning: applies to src/**/*.{cpp,h,cc,cxx,hpp} : dash core c++ codebase must be written in c++20 and require...Applied to files:
🪛 GitHub Actions: Clang Diff Format Checksrc/chainlock/signing.cpp[error] 15-15: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix formatting issues. src/chainlock/chainlock.cpp[error] 38-38: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix formatting issues. src/chainlock/chainlock.h[error] 58-58: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix formatting issues. ⏰ 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). (7)
🔇 Additional comments (14)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (1)
src/rpc/blockchain.cpp (1)
254-261: Doc/implementation mismatch: returns “blockhash”, spec says “hash”The RPC result declaration advertises a
hashfield, but the implementation serialises underblockhash(lines 278-281). This breaks API-contract guarantees and existing clients that rely on the help-text.@@ - {RPCResult::Type::STR_HEX, "hash", "The block hash hex-encoded"}, + {RPCResult::Type::STR_HEX, "blockhash", "The block hash hex-encoded"},(or rename the
pushKVkey to"hash").Please align the two before merging.
Also applies to: 278-281
🧹 Nitpick comments (4)
src/test/miner_tests.cpp (1)
21-27: Possible superfluous include<instantsend/instantsend.h>
The test file does not appear to reference any InstantSend symbols. Consider dropping the include to keep compile times lean and avoid unnecessary dependencies.-#include <instantsend/instantsend.h>src/rpc/blockchain.cpp (2)
51-52: Consider a lighter-weight include
chainlocks/chainlocks.hpulls in the full handler implementation even though this TU only needs theChainLockSigdeclaration. A forward declaration (or includingchainlocks/clsig.h) would be enough and keeps rebuilds faster.-#include <chainlocks/chainlocks.h> +// Only need the signature type here – use the smaller header +#include <chainlocks/clsig.h>
270-272: Namespace churn – hide withautoThe local variable’s type just flipped from
llmq::CChainLockSigtochainlocks::ChainLockSig. Usingautohere would insulate call-sites from further namespace relocations and reduce diff noise next time:- const chainlocks::ChainLockSig clsig = llmq_ctx.clhandler->GetBestChainLock(); + const auto clsig = llmq_ctx.clhandler->GetBestChainLock();src/chainlocks/signing.h (1)
24-46: Consider consolidating the multiple private sections.The class has three separate
private:sections (lines 24, 32, 39). This could be simplified by grouping all private members together for better readability.private: CChainState& m_chainstate; llmq::CChainLocksHandler& m_clhandler; llmq::CSigningManager& m_sigman; llmq::CSigSharesManager& m_shareman; CSporkManager& m_sporkman; const CMasternodeSync& m_mn_sync; -private: // We keep track of txids from recently received blocks so that we can check if all TXs got islocked struct BlockHasher { size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); } }; using BlockTxs = std::unordered_map<uint256, std::shared_ptr<std::unordered_set<uint256, StaticSaltedHasher>>, BlockHasher>; -private: mutable Mutex cs_signer; BlockTxs blockTxs GUARDED_BY(cs_signer); int32_t lastSignedHeight GUARDED_BY(cs_signer){-1}; uint256 lastSignedRequestId GUARDED_BY(cs_signer); uint256 lastSignedMsgHash GUARDED_BY(cs_signer);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
src/Makefile.am(2 hunks)src/bench/rpc_blockchain.cpp(1 hunks)src/chainlocks/chainlocks.cpp(11 hunks)src/chainlocks/chainlocks.h(4 hunks)src/chainlocks/clsig.cpp(1 hunks)src/chainlocks/clsig.h(1 hunks)src/chainlocks/signing.cpp(1 hunks)src/chainlocks/signing.h(1 hunks)src/coinjoin/coinjoin.cpp(1 hunks)src/dsnotificationinterface.cpp(2 hunks)src/dsnotificationinterface.h(1 hunks)src/evo/chainhelper.cpp(1 hunks)src/evo/specialtxman.cpp(2 hunks)src/instantsend/instantsend.cpp(1 hunks)src/instantsend/signing.cpp(1 hunks)src/interfaces/chain.h(2 hunks)src/llmq/clsig.cpp(0 hunks)src/llmq/clsig.h(0 hunks)src/llmq/context.cpp(1 hunks)src/net_processing.cpp(3 hunks)src/node/interfaces.cpp(2 hunks)src/node/miner.cpp(2 hunks)src/rest.cpp(1 hunks)src/rpc/blockchain.cpp(2 hunks)src/rpc/mining.cpp(1 hunks)src/rpc/quorums.cpp(4 hunks)src/rpc/rawtransaction.cpp(1 hunks)src/test/fuzz/process_message.cpp(0 hunks)src/test/miner_tests.cpp(1 hunks)src/validation.cpp(1 hunks)src/validationinterface.cpp(1 hunks)src/validationinterface.h(3 hunks)src/wallet/rpc/wallet.cpp(0 hunks)src/wallet/wallet.cpp(1 hunks)src/wallet/wallet.h(1 hunks)src/zmq/zmqabstractnotifier.cpp(1 hunks)src/zmq/zmqabstractnotifier.h(2 hunks)src/zmq/zmqnotificationinterface.cpp(1 hunks)src/zmq/zmqnotificationinterface.h(1 hunks)src/zmq/zmqpublishnotifier.cpp(4 hunks)src/zmq/zmqpublishnotifier.h(2 hunks)test/lint/lint-circular-dependencies.py(1 hunks)test/util/data/non-backported.txt(1 hunks)
💤 Files with no reviewable changes (4)
- src/test/fuzz/process_message.cpp
- src/wallet/rpc/wallet.cpp
- src/llmq/clsig.h
- src/llmq/clsig.cpp
🧰 Additional context used
📓 Path-based instructions (4)
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/context.cppsrc/rest.cppsrc/instantsend/signing.cppsrc/bench/rpc_blockchain.cppsrc/instantsend/instantsend.cppsrc/rpc/mining.cppsrc/test/miner_tests.cppsrc/evo/chainhelper.cppsrc/rpc/rawtransaction.cppsrc/coinjoin/coinjoin.cppsrc/validationinterface.cppsrc/node/miner.cppsrc/evo/specialtxman.cppsrc/zmq/zmqabstractnotifier.cppsrc/wallet/wallet.hsrc/zmq/zmqnotificationinterface.cppsrc/zmq/zmqnotificationinterface.hsrc/rpc/quorums.cppsrc/chainlocks/clsig.cppsrc/dsnotificationinterface.cppsrc/zmq/zmqabstractnotifier.hsrc/dsnotificationinterface.hsrc/interfaces/chain.hsrc/rpc/blockchain.cppsrc/node/interfaces.cppsrc/validationinterface.hsrc/zmq/zmqpublishnotifier.hsrc/chainlocks/clsig.hsrc/wallet/wallet.cppsrc/net_processing.cppsrc/validation.cppsrc/chainlocks/signing.cppsrc/zmq/zmqpublishnotifier.cppsrc/chainlocks/signing.hsrc/chainlocks/chainlocks.hsrc/chainlocks/chainlocks.cpp
src/bench/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Performance benchmarks should be placed in src/bench/ and use nanobench
Files:
src/bench/rpc_blockchain.cpp
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Files:
src/test/miner_tests.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/chainhelper.cppsrc/evo/specialtxman.cpp
🧠 Learnings (30)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: dashpay/dash#6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.
src/llmq/context.cpp (1)
Learnt from: kwvg
PR: #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.
src/rest.cpp (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/instantsend/signing.cpp (3)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
src/bench/rpc_blockchain.cpp (4)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/bench/**/*.{cpp,h,cc,cxx,hpp} : Performance benchmarks should be placed in src/bench/ and use nanobench
src/instantsend/instantsend.cpp (3)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
src/rpc/mining.cpp (2)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
src/test/miner_tests.cpp (6)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
test/util/data/non-backported.txt (8)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/crypto/{ctaes,x11}/** : Do not make changes under any circumstances to vendored dependencies in src/crypto/ctaes and src/crypto/x11
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to 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
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/**/*.{cpp,h,cc,cxx,hpp} : Dash uses unordered_lru_cache for efficient caching with LRU eviction
src/evo/chainhelper.cpp (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/rpc/rawtransaction.cpp (4)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: #6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The GetWallet() function calls in src/wallet/rpcwallet.cpp are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/coinjoin/coinjoin.cpp (4)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/node/miner.cpp (6)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: kwvg
PR: #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.
Learnt from: kwvg
PR: #6665
File: src/evo/providertx.h:82-82
Timestamp: 2025-06-06T11:53:09.094Z
Learning: In ProTx serialization code (SERIALIZE_METHODS), version checks should use hardcoded maximum flags (/is_basic_scheme_active=/true, /is_extended_addr=/true) rather than deployment-based flags. This is because serialization code should be able to deserialize any structurally valid ProTx up to the maximum version the code knows how to handle, regardless of current consensus validity. Validation code, not serialization code, is responsible for checking whether a ProTx version is consensus-valid based on deployment status.
src/evo/specialtxman.cpp (5)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #6665
File: src/evo/providertx.h:82-82
Timestamp: 2025-06-06T11:53:09.094Z
Learning: In ProTx serialization code (SERIALIZE_METHODS), version checks should use hardcoded maximum flags (/is_basic_scheme_active=/true, /is_extended_addr=/true) rather than deployment-based flags. This is because serialization code should be able to deserialize any structurally valid ProTx up to the maximum version the code knows how to handle, regardless of current consensus validity. Validation code, not serialization code, is responsible for checking whether a ProTx version is consensus-valid based on deployment status.
Learnt from: kwvg
PR: #6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
test/lint/lint-circular-dependencies.py (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
src/rpc/quorums.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
src/dsnotificationinterface.cpp (2)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/zmq/zmqabstractnotifier.h (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/Makefile.am (5)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #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.
src/interfaces/chain.h (2)
Learnt from: kwvg
PR: #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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/rpc/blockchain.cpp (5)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: #6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The GetWallet() function calls in src/wallet/rpcwallet.cpp are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: kwvg
PR: #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.
src/node/interfaces.cpp (2)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #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.
src/validationinterface.h (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/wallet/wallet.cpp (1)
Learnt from: kwvg
PR: #6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The GetWallet() function calls in src/wallet/rpcwallet.cpp are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
src/net_processing.cpp (2)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/validation.cpp (4)
Learnt from: kwvg
PR: #6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The GetWallet() function calls in src/wallet/rpcwallet.cpp are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.777Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/zmq/zmqpublishnotifier.cpp (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.778Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/chainlocks/signing.h (1)
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
src/chainlocks/chainlocks.h (3)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: #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.
Learnt from: kwvg
PR: #6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The CQuorumManager is fully initialized by LLMQContext, addressing any concerns about the manager’s initialization sequence.
src/chainlocks/chainlocks.cpp (6)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: #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.
Learnt from: kwvg
PR: #6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The CQuorumManager is fully initialized by LLMQContext, addressing any concerns about the manager’s initialization sequence.
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Learnt from: knst
PR: #6658
File: src/evo/creditpool.cpp:177-185
Timestamp: 2025-05-05T12:45:44.781Z
Learning: The GetAncestor() function in CBlockIndex safely handles negative heights by returning nullptr rather than asserting, making it safe to call with potentially negative values.
Learnt from: kwvg
PR: #6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
🧬 Code Graph Analysis (11)
src/validationinterface.cpp (5)
src/dsnotificationinterface.cpp (2)
NotifyChainLock(148-155)NotifyChainLock(148-149)src/zmq/zmqabstractnotifier.cpp (2)
NotifyChainLock(21-24)NotifyChainLock(21-21)src/zmq/zmqnotificationinterface.cpp (2)
NotifyChainLock(160-165)NotifyChainLock(160-160)src/zmq/zmqpublishnotifier.cpp (6)
NotifyChainLock(235-243)NotifyChainLock(235-235)NotifyChainLock(325-344)NotifyChainLock(325-325)NotifyChainLock(346-366)NotifyChainLock(346-346)src/instantsend/instantsend.cpp (2)
NotifyChainLock(627-630)NotifyChainLock(627-627)
src/evo/specialtxman.cpp (1)
src/chainlocks/clsig.cpp (3)
ChainLockSig(12-12)ChainLockSig(13-13)ChainLockSig(15-20)
src/zmq/zmqabstractnotifier.cpp (3)
src/dsnotificationinterface.cpp (2)
NotifyChainLock(148-155)NotifyChainLock(148-149)src/zmq/zmqnotificationinterface.cpp (2)
NotifyChainLock(160-165)NotifyChainLock(160-160)src/zmq/zmqpublishnotifier.cpp (6)
NotifyChainLock(235-243)NotifyChainLock(235-235)NotifyChainLock(325-344)NotifyChainLock(325-325)NotifyChainLock(346-366)NotifyChainLock(346-346)
src/wallet/wallet.h (5)
src/wallet/wallet.cpp (2)
notifyChainLock(3458-3461)notifyChainLock(3458-3458)src/chainlocks/clsig.h (1)
chainlocks(15-40)src/interfaces/chain.h (1)
chainlocks(34-36)src/validationinterface.h (1)
chainlocks(28-30)src/zmq/zmqabstractnotifier.h (1)
chainlocks(17-19)
src/zmq/zmqnotificationinterface.cpp (4)
src/dsnotificationinterface.cpp (2)
NotifyChainLock(148-155)NotifyChainLock(148-149)src/validationinterface.cpp (2)
NotifyChainLock(280-287)NotifyChainLock(280-280)src/zmq/zmqabstractnotifier.cpp (2)
NotifyChainLock(21-24)NotifyChainLock(21-21)src/zmq/zmqpublishnotifier.cpp (6)
NotifyChainLock(235-243)NotifyChainLock(235-235)NotifyChainLock(325-344)NotifyChainLock(325-325)NotifyChainLock(346-366)NotifyChainLock(346-346)
src/rpc/quorums.cpp (1)
src/chainlocks/clsig.cpp (3)
ChainLockSig(12-12)ChainLockSig(13-13)ChainLockSig(15-20)
src/chainlocks/clsig.cpp (1)
src/evo/mnhftx.h (1)
sig(36-36)
src/validationinterface.h (7)
src/chainlocks/clsig.h (1)
chainlocks(15-40)src/zmq/zmqabstractnotifier.h (1)
chainlocks(17-19)src/dsnotificationinterface.cpp (2)
NotifyChainLock(148-155)NotifyChainLock(148-149)src/validationinterface.cpp (2)
NotifyChainLock(280-287)NotifyChainLock(280-280)src/zmq/zmqabstractnotifier.cpp (2)
NotifyChainLock(21-24)NotifyChainLock(21-21)src/zmq/zmqnotificationinterface.cpp (2)
NotifyChainLock(160-165)NotifyChainLock(160-160)src/zmq/zmqpublishnotifier.cpp (6)
NotifyChainLock(235-243)NotifyChainLock(235-235)NotifyChainLock(325-344)NotifyChainLock(325-325)NotifyChainLock(346-366)NotifyChainLock(346-346)
src/wallet/wallet.cpp (1)
src/node/interfaces.cpp (2)
pindexChainLock(656-659)pindexChainLock(656-656)
src/zmq/zmqpublishnotifier.cpp (4)
src/dsnotificationinterface.cpp (2)
NotifyChainLock(148-155)NotifyChainLock(148-149)src/validationinterface.cpp (2)
NotifyChainLock(280-287)NotifyChainLock(280-280)src/zmq/zmqabstractnotifier.cpp (2)
NotifyChainLock(21-24)NotifyChainLock(21-21)src/zmq/zmqnotificationinterface.cpp (2)
NotifyChainLock(160-165)NotifyChainLock(160-160)
src/chainlocks/chainlocks.h (5)
src/chainlocks/clsig.h (2)
chainlocks(15-40)nodiscard(31-33)src/interfaces/chain.h (1)
chainlocks(34-36)src/validationinterface.h (2)
chainlocks(28-30)llmq(37-39)src/chainlocks/signing.h (2)
ChainLockSigner(22-72)llmq(13-19)src/chainlocks/chainlocks.cpp (20)
CChainLocksHandler(48-63)CChainLocksHandler(65-69)GetChainLockByHash(101-112)GetChainLockByHash(101-101)GetBestChainLock(114-118)GetBestChainLock(114-114)UpdateTxFirstSeenMap(120-127)UpdateTxFirstSeenMap(120-120)ProcessNewChainLock(129-194)ProcessNewChainLock(129-130)VerifyChainLock(371-377)VerifyChainLock(371-371)GetBestChainLockHeight(293-298)GetBestChainLockHeight(293-293)IsTxSafeForMining(300-312)IsTxSafeForMining(300-300)Cleanup(430-481)Cleanup(430-430)AreChainLocksEnabled(43-46)AreChainLocksEnabled(43-43)
🪛 GitHub Actions: Clang Diff Format Check
src/chainlocks/chainlocks.cpp
[error] 40-40: Clang format differences found. Code style does not match clang-format rules. Please run clang-format to fix formatting issues.
⏰ 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). (1)
- GitHub Check: linux64_ubsan-test / Test source
🔇 Additional comments (72)
src/llmq/context.cpp (1)
10-10: Include path correctly updated to newchainlockslocationThe switch to
<chainlocks/chainlocks.h>aligns with the new directory structure introduced in this PR and maintains consistency with the namespace move. No further action needed.src/bench/rpc_blockchain.cpp (1)
8-8: Include path update aligns with refactor effort – no issues spottedThe header reference has been correctly migrated to the new
chainlocksdirectory. Build should continue to succeed as no other code changes are required here. Good job keeping the benchmark in sync with the namespace move.src/validation.cpp (1)
53-53: No namespace inconsistency—no changes requiredThe new header at src/chainlocks/chainlocks.h still declares CChainLocksHandler inside
namespace llmq(lines 34–110), so all existing uses ofllmq::CChainLocksHandlerremain valid. You can safely ignore the suggestion to rename or add aliases.Likely an incorrect or invalid review comment.
src/rest.cpp (1)
9-9: LGTM! Include path updated correctly.The include path change from
<llmq/chainlocks.h>to<chainlocks/chainlocks.h>correctly reflects the ChainLocks refactoring that moves the implementation to a dedicatedchainlocksdirectory.src/evo/chainhelper.cpp (1)
9-9: LGTM! Include path updated correctly.The include path change from
<llmq/chainlocks.h>to<chainlocks/chainlocks.h>correctly reflects the ChainLocks refactoring and maintains existing functionality.src/instantsend/signing.cpp (1)
14-14: LGTM! Include path updated correctly.The include path change from
<llmq/chainlocks.h>to<chainlocks/chainlocks.h>correctly aligns with the ChainLocks refactoring that moves the implementation to thechainlocksdirectory.test/util/data/non-backported.txt (1)
7-8: LGTM! Non-backported files correctly updated.The addition of
src/chainlocks/*.cppandsrc/chainlocks/*.hto the non-backported list is appropriate, as ChainLocks is Dash-specific functionality that wouldn't be backported to Bitcoin Core. The entries are correctly placed alphabetically in the list.src/rpc/rawtransaction.cpp (1)
51-51: Include path update verified – no stale<llmq/chainlocks.h>includes foundRan
rg -n '<llmq/chainlocks\.h>'against the codebase and confirmed zero matches. No further action required.src/instantsend/instantsend.cpp (2)
16-17: Include path correctly updated to new ChainLocks location
Header moved fromllmq/tochainlocks/, matches the namespace refactor. No further action required.
21-23: Re-ordering of stats/client include is benign
Pure include-order change, does not affect semantics.src/rpc/mining.cpp (1)
8-9: Header path switched tochainlocks/chainlocks.h— looks good
Aligns RPC mining code with the new module layout.src/coinjoin/coinjoin.cpp (1)
18-24: LGTM: Clean include reorganization for ChainLocks refactoring.The reorganization of includes aligns well with the PR's objective of moving ChainLocks from the
llmqnamespace to a dedicatedchainlocksnamespace and directory. The grouping is logical and maintains consistent organization.test/lint/lint-circular-dependencies.py (1)
28-33: LGTM: Proper maintenance of circular dependency expectations.The addition of these circular dependencies for the new
chainlocksmodule is expected and necessary given the refactoring fromllmq/chainlocksto the dedicatedchainlocksnamespace and directory. The dependencies accurately reflect the interactions between the new chainlocks modules and existing components.src/node/miner.cpp (2)
27-27: LGTM: Include path updated for ChainLocks refactoring.The include path change from
llmq/chainlocks.htochainlocks/chainlocks.hproperly reflects the module migration to the dedicated chainlocks directory.
130-130: LGTM: Namespace updated for ChainLockSig type.The type change from
llmq::CChainLockSig{}tochainlocks::ChainLockSig{}correctly implements the namespace migration while preserving identical functionality for creating an empty chainlock signature.src/evo/specialtxman.cpp (2)
11-17: LGTM: Include reorganization for ChainLocks refactoring.The include updates properly reflect the migration of ChainLocks code from the
llmqnamespace to the dedicatedchainlocksnamespace and directory. The addition ofchainlocks/chainlocks.handchainlocks/clsig.hreplaces the previousllmqincludes.
82-82: LGTM: Namespace updated for ChainLockSig constructor.The type change from
llmq::CChainLockSigtochainlocks::ChainLockSigcorrectly implements the namespace migration. The constructor parameters (height, block hash, signature) match the expected signature for the newchainlocks::ChainLockSigclass.src/zmq/zmqabstractnotifier.cpp (1)
21-21: LGTM: Parameter type updated for ChainLocks refactoring.The parameter type change from
std::shared_ptr<const llmq::CChainLockSig>tostd::shared_ptr<const chainlocks::ChainLockSig>correctly implements the namespace migration and maintains consistency with all concrete ZMQ notifier implementations that use the newchainlocks::ChainLockSigtype.src/dsnotificationinterface.cpp (2)
17-17: LGTM - Include path updated correctly for refactoring.The include path change from
llmq/chainlocks.htochainlocks/chainlocks.hcorrectly reflects the relocation of ChainLocks code to the dedicatedchainlocksdirectory.
148-149: LGTM - Parameter type correctly updated for namespace change.The parameter type change from
std::shared_ptr<const llmq::CChainLockSig>tostd::shared_ptr<const chainlocks::ChainLockSig>correctly reflects the movement of the ChainLockSig class to the newchainlocksnamespace while preserving the method's functionality.src/rpc/quorums.cpp (2)
17-17: LGTM - Include updates support the refactoring correctly.The include changes appropriately update dependencies:
- Adding
chainlocks/chainlocks.hprovides access to the newchainlocks::ChainLockSigtype- The
masternode/node.hinclude reordering maintains necessary dependencies while accommodating the new structureAlso applies to: 30-30
1024-1025: LGTM - Type usage correctly updated for new namespace.The type changes from
llmq::CChainLockSigtochainlocks::ChainLockSigcorrectly use the new namespace while maintaining identical functionality in the RPC commands. The constructor calls match the signature defined in the newchainlocks/clsig.cpp.Also applies to: 1136-1136
src/dsnotificationinterface.h (1)
49-49: LGTM - Method declaration correctly updated for namespace change.The parameter type change from
std::shared_ptr<const llmq::CChainLockSig>tostd::shared_ptr<const chainlocks::ChainLockSig>correctly updates the interface declaration to match the implementation and use the newchainlocksnamespace.src/zmq/zmqnotificationinterface.h (1)
34-34: LGTM! Correct namespace migration.The method signature correctly updates to use the new
chainlocks::ChainLockSigtype, consistent with the broader refactoring effort to move ChainLocks functionality fromllmqto its dedicated namespace.src/validationinterface.cpp (1)
280-280: LGTM! Consistent type update for the refactoring.The method signature correctly updates the parameter type from
llmq::CChainLockSigtochainlocks::ChainLockSig, maintaining identical functionality while aligning with the namespace refactoring.src/wallet/wallet.h (1)
926-926: LGTM! Method signature correctly updated for the refactoring.The
notifyChainLockmethod signature properly updates to usechainlocks::ChainLockSig, consistent with the broader ChainLocks namespace migration and the implementation insrc/wallet/wallet.cpp.src/Makefile.am (2)
153-155: LGTM! Build configuration correctly updated for the new directory structure.The header files are properly added for the new
chainlocks/directory, including the newsigning.hfile for the modularized ChainLock signing functionality.
458-460: LGTM! Source files correctly added for the ChainLocks refactoring.The build system properly includes the relocated and new ChainLocks source files, including the new
signing.cppthat contains the extractedChainLockSignerclass functionality.src/zmq/zmqnotificationinterface.cpp (1)
160-160: LGTM: Consistent namespace migrationThe method signature update from
llmq::CChainLockSigtochainlocks::ChainLockSigis consistent with the refactoring objectives and aligns with similar changes across the ZMQ notification system.src/node/interfaces.cpp (2)
10-10: LGTM: Include path updated for refactored directory structureThe include path change from
<llmq/chainlocks.h>to<chainlocks/chainlocks.h>correctly reflects the migration of ChainLocks code to the new directory structure.
656-656: LGTM: Method signature updated for namespace consistencyThe parameter type change from
llmq::CChainLockSigtochainlocks::ChainLockSigmaintains the same interface while using the new namespace, consistent with the refactoring objectives.src/zmq/zmqabstractnotifier.h (2)
17-19: LGTM: Forward declaration updated for new namespaceThe forward declaration of
chainlocks::ChainLockSigcorrectly establishes the new type within the appropriate namespace, supporting the refactored code organization.
73-73: LGTM: Virtual method signature consistently updatedThe parameter type change from
llmq::CChainLockSigtochainlocks::ChainLockSigin the virtual method correctly updates the base class interface to support the namespace refactoring.src/net_processing.cpp (3)
62-62: LGTM! Include path correctly updated for refactoring.The include directive has been properly updated from
llmq/chainlocks.htochainlocks/chainlocks.h, which aligns with the ChainLocks refactoring objective to move the implementation to a dedicated namespace and directory.
5285-5285: Type correctly updated for ChainLocks refactoring.The variable type has been properly changed from
llmq::CChainLockSigtochainlocks::ChainLockSigin the CLSIG message handling code, maintaining consistency with the namespace refactoring while preserving the existing deserialization and processing logic.
2861-2861: GetChainLockByHash Signature Uses New Type
Confirmed thatCChainLocksHandler::GetChainLockByHash(const uint256&, chainlocks::ChainLockSig&)is declared in src/chainlocks/chainlocks.h (line 81) and defined in src/chainlocks/chainlocks.cpp (line 101), matching its invocation in src/net_processing.cpp (lines 2861–2862). No references tollmq::CChainLockSigremain.src/interfaces/chain.h (2)
34-36: Forward declaration correctly moved tochainlocksMoving the
ChainLockSigforward declaration into its own namespace keeps the header lightweight and avoids pulling in heavy includes. Looks good to me.
270-271: No remainingllmq::CChainLockSigoverrides – all updated tochainlocks::ChainLockSigI’ve verified every override of
notifyChainLock:
- src/interfaces/chain.h (virtual signature)
- src/wallet/wallet.h / wallet.cpp (CWallet implementation)
- src/node/interfaces.cpp (invocation via m_notifications)
There are no references to the old
llmq::CChainLockSigin the codebase. All implementations correctly usestd::shared_ptr<const chainlocks::ChainLockSig>.src/chainlocks/clsig.cpp (5)
1-8: LGTM!The copyright headers and includes are correct and follow standard conventions.
9-11: LGTM!The namespace declaration and constant definition follow proper conventions and align with the refactoring objectives.
12-21: LGTM!The constructors and destructor follow modern C++ best practices with explicit defaulting and efficient member initialization using uniform initialization syntax.
22-26: LGTM!The ToString() method implementation is clean and follows established patterns in the codebase. Including only the height and block hash (not the signature) is appropriate for readability.
27-27: LGTM!The namespace closing follows best practices with proper commenting.
src/validationinterface.h (2)
28-30: LGTM!The forward declaration for
chainlocks::ChainLockSigis correctly added and follows the same pattern as other namespace forward declarations in the file.
169-169: LGTM!The method signature changes consistently update both the virtual interface method and the concrete method declaration to use
chainlocks::ChainLockSig, maintaining proper const-correctness with shared_ptr.Also applies to: 237-237
src/zmq/zmqpublishnotifier.h (3)
48-48: LGTM!The method signature correctly updates to use
chainlocks::ChainLockSigwhile maintaining the proper shared_ptr pattern.
96-96: LGTM!The method signature change is consistent with other notifier classes and correctly uses the new chainlocks namespace.
102-102: LGTM!The method signature change completes the consistent update across all chainlock notifier classes to use the new namespace.
src/zmq/zmqpublishnotifier.cpp (4)
13-13: LGTM!The include directive correctly updates to use the new
chainlocks/clsig.hheader, aligning with the file reorganization.
235-235: LGTM!The method signature correctly updates to use the new namespace while maintaining the existing implementation logic.
325-325: LGTM!The method signature update is correct and the implementation appropriately publishes raw block data as intended.
346-346: LGTM!The method signature update is correct and this implementation appropriately uses the clsig parameter to serialize both block and signature data.
src/chainlocks/clsig.h (6)
1-14: LGTM!The header guards follow proper conventions and includes are minimal and necessary for the class definition.
15-17: LGTM!The namespace declaration and extern constant declaration follow proper conventions.
18-23: LGTM!The struct definition with private members is well-designed, using appropriate types and default initialization for the height sentinel value.
24-28: LGTM!The constructor and destructor declarations are properly designed with efficient parameter passing using const references for complex types.
30-35: LGTM!The getter methods and utility functions are well-designed with proper const-qualification and [[nodiscard]] attributes. The IsNull() method provides convenient state checking.
36-44: LGTM!The serialization implementation correctly includes all member variables and the file is properly closed with appropriate comments.
src/chainlocks/signing.h (1)
22-72: Well-structured class with proper thread safety annotations.The
ChainLockSignerclass is well-designed with:
- Clear separation of concerns from the main handler
- Proper thread safety annotations on all methods
- Consistent use of EXCLUSIVE_LOCKS_REQUIRED for synchronization
- Appropriate encapsulation of signing logic
src/chainlocks/signing.cpp (6)
19-32: Constructor and destructor implementation look correct.The constructor properly initializes all member references using the member initializer list, and the destructor is appropriately defaulted.
43-143: TrySignChainTip implementation correctly handles signing attempts.The method properly:
- Validates all preconditions before attempting to sign
- Maintains thread safety with appropriate locking
- Implements the transaction safety checks for InstantSend
- Correctly tracks the last signed height to avoid duplicates
152-168: Block transaction tracking is properly implemented.The method correctly:
- Ensures thread safety with proper locking
- Creates entries even for blocks without lockable transactions (as documented)
- Filters out coinbase and input-less transactions
170-216: GetBlockTxs correctly implements lazy loading with proper synchronization.The method properly:
- Prevents potential deadlocks with lock assertions
- Implements a cache-first approach with disk fallback
- Updates the transaction first seen map for newly loaded blocks
- Handles disk read failures gracefully
218-240: HandleNewRecoveredSig correctly processes signing results.The method properly validates that the recovered signature matches the last signing attempt and creates a ChainLockSig to be processed by the handler.
242-259: Cleanup method efficiently manages memory by removing obsolete entries.The implementation correctly:
- Removes transaction sets for blocks that have chainlocks
- Removes entries for blocks with conflicting chainlocks
- Returns the removed transaction sets for caller cleanup
- Maintains proper lock ordering (cs_main before cs_signer)
src/chainlocks/chainlocks.h (2)
5-8: Header guard and include path updates are correct.The changes properly reflect the new file location and namespace organization.
41-107: CChainLocksHandler refactoring properly delegates signing logic.The changes correctly:
- Add a unique_ptr to ChainLockSigner for masternode-specific functionality
- Update all method signatures to use chainlocks::ChainLockSig
- Maintain thread safety annotations throughout
- Preserve the public interface for compatibility
src/chainlocks/chainlocks.cpp (5)
48-63: Constructor properly implements conditional ChainLockSigner creation.The implementation correctly creates the ChainLockSigner only for masternodes, which aligns with the PR objective of splitting masternode logic.
71-93: Lifecycle methods correctly handle optional signer.The Start and Stop methods properly check for the existence of m_signer before delegating calls, ensuring non-masternode instances work correctly.
120-127: UpdateTxFirstSeenMap provides thread-safe transaction timestamp updates.The method correctly implements the interface for ChainLockSigner to update transaction timestamps with proper locking.
263-291: Block event handlers correctly delegate to signer.The implementation properly:
- Maintains transaction timestamp tracking in the handler
- Delegates block transaction mapping to the signer when present
- Maintains thread safety with appropriate locking
452-460: Cleanup method properly coordinates with signer.The implementation correctly delegates block transaction cleanup to the signer and uses the returned transaction sets to clean up the handler's transaction timestamp map.
src/coinjoin/coinjoin.cpp
Outdated
| #include <bls/bls.h> | ||
| #include <chainlocks/chainlocks.h> | ||
| #include <governance/common.h> | ||
| #include <instantsend/instantsend.h> | ||
| #include <masternode/node.h> | ||
| #include <masternode/sync.h> |
There was a problem hiding this comment.
nit: should not split it to own group IMO, it's not something special here about them
There was a problem hiding this comment.
IMO Dash-specific headers should be grouped separately, gives a quick glance of what Dash-specific dependencies are present. Would like this even in Dash-specific code as it better expresses interplay with various components, prefer to do grouping where it is trivial to do.
There was a problem hiding this comment.
Should keep them on top then maybe?
There was a problem hiding this comment.
Would prefer to keep them below Bitcoin headers as we'd have to follow that convention in Bitcoin-derived code to avoid merge conflicts anyways (another reason why I want to keep Dash-specific headers grouped separately), so have a consistent convention seems ideal.
There was a problem hiding this comment.
I agree with your point about backports, but src/coinjoin/coinjoin.cpp is our file, there's no backports to this file.
kwvg
changed the title
src/chainlocks/ and split masternode logic out of manager sourcesrc/chainlock/ and split masternode logic out of manager source
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
src/chainlock/signing.h (1)
67-90: Fix code formatting to match project styleThe code formatting doesn't match the expected clang-format style as indicated by the pipeline failure.
Run clang-format on this file to fix the formatting issues.
src/chainlock/chainlock.h (1)
58-90: Fix formatting issues in method declarationsThe pipeline indicates formatting issues in this section. Please run clang-format to fix.
🧹 Nitpick comments (2)
src/chainlock/signing.cpp (1)
63-66: TODO: Implement DIP8 signing attempts processThe comment indicates that the full DIP8 "Signing attempts" process is not yet implemented. This simplified approach may fail when multiple blocks compete.
Would you like me to create an issue to track the implementation of the complete DIP8 signing attempts process?
src/chainlock/chainlock.h (1)
116-116: Update header guard closing commentThe closing comment should match the updated header guard.
-#endif // BITCOIN_CHAINLOCK_CHAINLOCK_H +#endif // DASH_CHAINLOCK_CHAINLOCK_H
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
src/Makefile.am(2 hunks)src/bench/rpc_blockchain.cpp(1 hunks)src/chainlock/chainlock.cpp(12 hunks)src/chainlock/chainlock.h(3 hunks)src/chainlock/clsig.cpp(1 hunks)src/chainlock/clsig.h(1 hunks)src/chainlock/signing.cpp(1 hunks)src/chainlock/signing.h(1 hunks)src/coinjoin/coinjoin.cpp(1 hunks)src/dsnotificationinterface.cpp(2 hunks)src/dsnotificationinterface.h(1 hunks)src/evo/chainhelper.cpp(1 hunks)src/evo/specialtxman.cpp(2 hunks)src/instantsend/instantsend.cpp(1 hunks)src/instantsend/signing.cpp(1 hunks)src/interfaces/chain.h(2 hunks)src/llmq/clsig.cpp(0 hunks)src/llmq/clsig.h(0 hunks)src/llmq/context.cpp(1 hunks)src/net_processing.cpp(3 hunks)src/node/interfaces.cpp(2 hunks)src/node/miner.cpp(2 hunks)src/rest.cpp(1 hunks)src/rpc/blockchain.cpp(2 hunks)src/rpc/mining.cpp(1 hunks)src/rpc/quorums.cpp(4 hunks)src/rpc/rawtransaction.cpp(1 hunks)src/test/fuzz/process_message.cpp(0 hunks)src/test/miner_tests.cpp(1 hunks)src/validation.cpp(1 hunks)src/validationinterface.cpp(1 hunks)src/validationinterface.h(3 hunks)src/wallet/rpc/wallet.cpp(0 hunks)src/wallet/wallet.cpp(1 hunks)src/wallet/wallet.h(1 hunks)src/zmq/zmqabstractnotifier.cpp(1 hunks)src/zmq/zmqabstractnotifier.h(2 hunks)src/zmq/zmqnotificationinterface.cpp(1 hunks)src/zmq/zmqnotificationinterface.h(1 hunks)src/zmq/zmqpublishnotifier.cpp(4 hunks)src/zmq/zmqpublishnotifier.h(2 hunks)test/lint/lint-circular-dependencies.py(1 hunks)test/util/data/non-backported.txt(1 hunks)
📓 Path-based instructions (1)
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/chainlock/signing.cppsrc/chainlock/signing.hsrc/chainlock/chainlock.hsrc/chainlock/chainlock.cpp
🧠 Learnings (3)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.
src/chainlock/chainlock.h (3)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #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.
Learnt from: kwvg
PR: #6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The CQuorumManager is fully initialized by LLMQContext, addressing any concerns about the manager’s initialization sequence.
src/chainlock/chainlock.cpp (8)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: kwvg
PR: #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.
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Learnt from: kwvg
PR: #6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The CQuorumManager is fully initialized by LLMQContext, addressing any concerns about the manager’s initialization sequence.
Learnt from: knst
PR: #6658
File: src/evo/creditpool.cpp:177-185
Timestamp: 2025-05-05T12:45:44.781Z
Learning: The GetAncestor() function in CBlockIndex safely handles negative heights by returning nullptr rather than asserting, making it safe to call with potentially negative values.
Learnt from: kwvg
PR: #6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
🧬 Code Graph Analysis (1)
src/chainlock/chainlock.h (6)
src/chainlock/signing.h (1)
llmq(14-20)src/validationinterface.h (2)
llmq(37-39)chainlock(28-30)src/chainlock/chainlock.cpp (16)
CChainLocksHandler(47-62)CChainLocksHandler(64-68)UpdateTxFirstSeenMap(119-126)UpdateTxFirstSeenMap(119-119)ProcessNewChainLock(128-193)ProcessNewChainLock(128-129)HasChainLock(378-401)HasChainLock(378-378)HasConflictingChainLock(403-427)HasConflictingChainLock(403-403)GetBestChainLockHeight(292-297)GetBestChainLockHeight(292-292)IsTxSafeForMining(299-311)IsTxSafeForMining(299-299)Cleanup(429-480)Cleanup(429-429)src/chainlock/clsig.h (2)
chainlock(15-40)nodiscard(31-33)src/interfaces/chain.h (1)
chainlock(34-36)src/chainlock/signing.cpp (4)
ChainLockSigner(18-28)ChainLockSigner(30-30)Cleanup(241-258)Cleanup(241-241)
🪛 GitHub Actions: Clang Diff Format Check
src/chainlock/signing.cpp
[error] 15-250: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix.
src/chainlock/signing.h
[error] 67-90: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix.
src/chainlock/chainlock.h
[error] 58-90: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix.
src/chainlock/chainlock.cpp
[error] 39-50: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix.
💤 Files with no reviewable changes (4)
- src/test/fuzz/process_message.cpp
- src/wallet/rpc/wallet.cpp
- src/llmq/clsig.h
- src/llmq/clsig.cpp
✅ Files skipped from review due to trivial changes (12)
- src/bench/rpc_blockchain.cpp
- src/evo/specialtxman.cpp
- src/instantsend/instantsend.cpp
- src/test/miner_tests.cpp
- src/node/miner.cpp
- src/validation.cpp
- src/Makefile.am
- src/coinjoin/coinjoin.cpp
- src/chainlock/clsig.cpp
- src/zmq/zmqpublishnotifier.cpp
- src/chainlock/clsig.h
- src/validationinterface.h
🚧 Files skipped from review as they are similar to previous changes (23)
- src/evo/chainhelper.cpp
- test/util/data/non-backported.txt
- src/rest.cpp
- src/rpc/mining.cpp
- src/llmq/context.cpp
- src/instantsend/signing.cpp
- src/rpc/rawtransaction.cpp
- src/dsnotificationinterface.cpp
- test/lint/lint-circular-dependencies.py
- src/rpc/blockchain.cpp
- src/rpc/quorums.cpp
- src/wallet/wallet.h
- src/zmq/zmqabstractnotifier.h
- src/node/interfaces.cpp
- src/zmq/zmqabstractnotifier.cpp
- src/wallet/wallet.cpp
- src/validationinterface.cpp
- src/zmq/zmqnotificationinterface.h
- src/dsnotificationinterface.h
- src/zmq/zmqnotificationinterface.cpp
- src/zmq/zmqpublishnotifier.h
- src/interfaces/chain.h
- src/net_processing.cpp
🧰 Additional context used
📓 Path-based instructions (1)
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/chainlock/signing.cppsrc/chainlock/signing.hsrc/chainlock/chainlock.hsrc/chainlock/chainlock.cpp
🧠 Learnings (3)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.
src/chainlock/chainlock.h (3)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #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.
Learnt from: kwvg
PR: #6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The CQuorumManager is fully initialized by LLMQContext, addressing any concerns about the manager’s initialization sequence.
src/chainlock/chainlock.cpp (8)
Learnt from: kwvg
PR: #6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: kwvg
PR: #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.
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Learnt from: kwvg
PR: #6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The CQuorumManager is fully initialized by LLMQContext, addressing any concerns about the manager’s initialization sequence.
Learnt from: knst
PR: #6658
File: src/evo/creditpool.cpp:177-185
Timestamp: 2025-05-05T12:45:44.781Z
Learning: The GetAncestor() function in CBlockIndex safely handles negative heights by returning nullptr rather than asserting, making it safe to call with potentially negative values.
Learnt from: kwvg
PR: #6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
🧬 Code Graph Analysis (1)
src/chainlock/chainlock.h (6)
src/chainlock/signing.h (1)
llmq(14-20)src/validationinterface.h (2)
llmq(37-39)chainlock(28-30)src/chainlock/chainlock.cpp (16)
CChainLocksHandler(47-62)CChainLocksHandler(64-68)UpdateTxFirstSeenMap(119-126)UpdateTxFirstSeenMap(119-119)ProcessNewChainLock(128-193)ProcessNewChainLock(128-129)HasChainLock(378-401)HasChainLock(378-378)HasConflictingChainLock(403-427)HasConflictingChainLock(403-403)GetBestChainLockHeight(292-297)GetBestChainLockHeight(292-292)IsTxSafeForMining(299-311)IsTxSafeForMining(299-299)Cleanup(429-480)Cleanup(429-429)src/chainlock/clsig.h (2)
chainlock(15-40)nodiscard(31-33)src/interfaces/chain.h (1)
chainlock(34-36)src/chainlock/signing.cpp (4)
ChainLockSigner(18-28)ChainLockSigner(30-30)Cleanup(241-258)Cleanup(241-241)
🪛 GitHub Actions: Clang Diff Format Check
src/chainlock/signing.cpp
[error] 15-250: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix.
src/chainlock/signing.h
[error] 67-90: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix.
src/chainlock/chainlock.h
[error] 58-90: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix.
src/chainlock/chainlock.cpp
[error] 39-50: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix.
⏰ 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). (7)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: Lint / Run linters
- GitHub Check: x86_64-apple-darwin / Build depends
- GitHub Check: x86_64-pc-linux-gnu / Build depends
- GitHub Check: arm-linux-gnueabihf / Build depends
🔇 Additional comments (14)
src/chainlock/signing.h (2)
49-52: Efficient custom hasher implementationGood use of a custom hasher that reads only the first 8 bytes of the hash for the unordered_map key, which is more efficient than hashing the entire 32-byte uint256.
55-61: Proper thread safety annotationsExcellent use of thread safety annotations with
GUARDED_BYfor member variables andEXCLUSIVE_LOCKS_REQUIREDfor methods. This helps prevent data races at compile time.src/chainlock/signing.cpp (6)
18-40: Clean lifecycle management implementationThe constructor properly initializes all members, and Start/Stop methods correctly manage the listener registration lifecycle.
94-125: Robust transaction safety verification for InstantSendGood implementation of transaction safety checks when InstantSend is enabled. The logic properly walks back through recent blocks and verifies that all transactions are either InstantSend locked or old enough to be considered safe.
144-167: Correct block transaction tracking implementationThe methods properly manage the block-to-transaction mapping with appropriate locking. Good design choice to create entries even for blocks without lockable transactions.
169-215: Well-designed block transaction retrieval with cachingExcellent implementation with proper fallback to disk reads for cache misses. The method correctly updates the first-seen time map when loading transactions from disk.
217-239: Proper recovered signature handlingThe method correctly validates that the recovered signature matches the last signed request before creating and processing the ChainLockSig.
241-258: Efficient cleanup implementationGood cleanup logic that properly removes entries for both chainlocked and conflicting blocks. The method correctly returns removed transaction sets for cascading cleanup operations.
src/chainlock/chainlock.h (2)
40-40: Clean architectural separation with ChainLockSignerExcellent refactoring that properly separates signing responsibilities. The handler now inherits from
ChainLockSignerParentand delegates signing operations to theChainLockSignerinstance.Also applies to: 51-51
82-107: Proper virtual method overrides for parent interfaceThe virtual method overrides correctly implement the
ChainLockSignerParentinterface, maintaining clean separation between the handler and signer components.src/chainlock/chainlock.cpp (4)
47-62: Efficient conditional signer initializationGood design to only create the
ChainLockSignerinstance for masternode nodes. This saves resources on non-masternode nodes that don't participate in signing.
70-92: Proper lifecycle management with signer delegationThe Start and Stop methods correctly delegate to the signer when present, maintaining clean separation of responsibilities.
451-459: Well-structured cleanup delegationThe cleanup method properly delegates to the signer and processes the returned transaction sets to maintain consistency in the handler's data structures.
42-45: Improved function namingThe rename from
ChainLocksSigningEnabledtoAreChainLocksEnabledbetter reflects the function's purpose of checking overall ChainLocks enablement status.
|
This pull request has conflicts, please rebase. |
There was a problem hiding this comment.
utACK 524f5f1
NOTE: conflicts with #6786 so should probably merge that one first and then rebase this on top of new develop
NOTE2: @PastaPastaPasta Check Potential Conflicts job is broken it seems: Error: Resource not accessible by integration, see https://github.com/dashpay/dash/actions/runs/16695519542/job/47259067541?pr=6761
|
This pull request has conflicts, please rebase. |
We can safely move the entire chunk to `CleanupSigner()` because `blockTxes` is populated only for the signer's sake, if we aren't signing, `blockTxes` won't be populated and there would be nothing to iterate upon. This will become more apparent as we isolate signer code from the manager.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
…ons, ChainLock and InstantSend refactoring 34a90e6 chore: remove unused `IsInvInFilter` from `PeerManager` interface (Kittywhiskers Van Gogh) f3224ae refactor: consolidate `INPUTLOCK_REQUESTID_PREFIX` usage to `lock.cpp` (Kittywhiskers Van Gogh) 7b4ee6b trivial: document transaction confirmation safety threshold (Kittywhiskers Van Gogh) 25f05c1 refactor: make unknown block clsig flow easier to follow (Kittywhiskers Van Gogh) 9578146 refactor: document `pindex` assumptions in chainlocks code (Kittywhiskers Van Gogh) 4a744c7 refactor: use `std::chrono` for time variables, reduce resolution (Kittywhiskers Van Gogh) b051c22 refactor: consolidate `CLSIG_REQUESTID_PREFIX` usage to `clsig.cpp` (Kittywhiskers Van Gogh) 024b466 chore: move lock annotations in `chainlock.h` to the next line (Kittywhiskers Van Gogh) c6e99fb chore: apply most `clang-format` suggestions (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Depends on #6761 * Dependency for #6821 * Assumptions surrounding `pindex` usage have been documented in response to reviewer comments in [dash#6761](#6761) ([comment](#6761 (comment)), [comment](#6761 (comment))). * The internal structures of `CChainLocksHandler` (`txFirstSeenTime`, `seenChainLocks`, `lastCleanupTime`) have their time resolution reduced from milliseconds to seconds while migrating to `std::chrono`. * `CInstantSendManager::AskNodesForLockedTx()` was moved as `PeerManagerImpl::AskPeersForTransaction()` in [dash#6425](#6425) and with that, the sole external usage of `PeerManagerImpl::IsInvInFilter()` was moved internally, we can therefore safely remove it from the `PeerManager` interface. ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 34a90e6 Tree-SHA512: 28b532cb5b8b5e6d7c1331c7c6c0ecd4d45b186922c279db6d2d3e8974d422ec8b67d75aeadce77986d409a8ed071e85359ee08609e0c2dde657e4520c546817
Additional Information
A continuation of dash#6742, this pull request extends the same refactoring to ChainLocks, separating its masternode mode functionality to a dedicated signer object and migrating some definitions to the newly created
chainlocknamespace.Changes are mostly move-only except for decoupling, which should be behavior-identical.
Breaking Changes
None expected.
Checklist