refactor: move InstantSend impl. to src/instantsend/ and split sources for islock, database and masternode logic out of manager source
#6742
Conversation
src/instantsend/instantsend.cpp
Outdated
| #include <chainparams.h> | ||
| #include <consensus/validation.h> | ||
| #include <dbwrapper.h> | ||
| #include <index/txindex.h> | ||
| #include <masternode/sync.h> | ||
| #include <net_processing.h> | ||
| #include <node/blockstorage.h> | ||
| #include <spork.h> | ||
| #include <stats/client.h> | ||
| #include <txmempool.h> | ||
| #include <util/irange.h> | ||
| #include <util/ranges.h> | ||
| #include <util/thread.h> | ||
| #include <validation.h> | ||
|
|
||
| #include <bls/bls_batchverifier.h> | ||
| #include <instantsend/lock.h> | ||
| #include <llmq/chainlocks.h> | ||
| #include <llmq/commitment.h> | ||
| #include <llmq/quorums.h> | ||
| #include <llmq/signing_shares.h> | ||
| #include <masternode/sync.h> | ||
| #include <spork.h> |
There was a problem hiding this comment.
nit: let's group all of them together. Before it was like "llmq" and all others; but now instantsend.cpp doesn't belong to this group anymore; they can be all together now
|
This pull request has conflicts, please rebase. |
src/instantsend, split islock, database and (master)node logic, spin off signing to instantsend::CSigningManagersrc/instantsend/ and split sources for islock, database and masternode logic out of manager source
WalkthroughThe changes comprehensively refactor the InstantSend module in the codebase. All InstantSend-related source and header files are moved from the ✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
src/rpc/mempool.cpp (1)
54-56: RPC field name mismatch: “instantsend” vs “instantlock”
MempoolEntryDescription()documents a boolean field named instantsend, butentryToJSON()actually fills instantlock.
Clients relying on the help text will look for the wrong key.@@ - RPCResult{RPCResult::Type::BOOL, "instantsend", "True if this transaction was locked via InstantSend"}, + RPCResult{RPCResult::Type::BOOL, "instantlock", "True if this transaction was locked via InstantSend"}, @@ - info.pushKV("instantlock", isman ? (isman->IsLocked(tx.GetHash()) ? "true" : "false") : "unknown"); + info.pushKV("instantlock", isman ? (isman->IsLocked(tx.GetHash()) ? "true" : "false") : "unknown");Keeping the internal key
instantlock(aligned with other RPCs) and updating the help string is the minimal, non-breaking fix.
Please adjust to avoid API confusion before the next release.Also applies to: 112-114
♻️ Duplicate comments (3)
test/lint/lint-circular-dependencies.py (1)
47-51: The circular dependencies reflect the InstantSend refactoring but could be improved.The added circular dependencies are consistent with moving InstantSend from the
llmqdirectory to theinstantsenddirectory. However, as noted in the past review comment, introducing circular dependencies is not ideal design.Consider the suggestion from the past review comment about moving
InstantSendStoragetoinstantsend.handCInstantSendManagerto a separate header likeinstantsend/is_impl.hto reduce circular dependencies.#!/bin/bash # Description: Verify the actual circular dependencies exist in the new structure # Check for circular dependencies in the instantsend directory ast-grep --pattern 'instantsend/instantsend' ast-grep --pattern 'instantsend/signing'src/instantsend/signing.cpp (1)
63-63: Fix the typo in mutex name (continuation from header file).The mutex name
cs_inputReqests(with typo) is used here. This will be fixed when you correct the typo in the header file.Also applies to: 87-87, 333-333
src/instantsend/instantsend.cpp (1)
7-23: Group all includes together.As suggested in the previous review, since instantsend.cpp no longer belongs to the "llmq" group, all includes can be grouped together now.
#include <chainparams.h> #include <consensus/validation.h> #include <net_processing.h> #include <node/blockstorage.h> #include <stats/client.h> #include <txmempool.h> #include <util/thread.h> #include <validation.h> - #include <bls/bls_batchverifier.h> #include <instantsend/signing.h> #include <llmq/chainlocks.h> #include <llmq/commitment.h> #include <llmq/quorums.h> #include <masternode/sync.h> #include <spork.h>
🧹 Nitpick comments (13)
src/test/ratecheck_tests.cpp (1)
2-3: License header year range is staleThe header was just modified, yet the copyright range stops at 2022.
Consider updating to the current year for consistency with other freshly-touched files.-// Copyright (c) 2014-2022 The Dash Core developers +// Copyright (c) 2014-2025 The Dash Core developerssrc/test/governance_validators_tests.cpp (1)
2-3: Align copyright yearsSame nit as in the other test files: adjust the end year to 2025 so newly-edited sources share a uniform header.
-// Copyright (c) 2014-2023 The Dash Core developers +// Copyright (c) 2014-2025 The Dash Core developerssrc/test/cachemap_tests.cpp (1)
2-3: Refresh year in license bannerUpdate the upper bound of the year range to reflect the 2025 edit.
-// Copyright (c) 2014-2023 The Dash Core developers +// Copyright (c) 2014-2025 The Dash Core developerssrc/evo/chainhelper.cpp (1)
9-10: Remove unused header to avoid unnecessary coupling
lock.his not referenced anywhere in this translation unit. Dropping the include trims build times and prevents accidental reliance on unrelated InstantSend internals.-#include <instantsend/instantsend.h> -#include <instantsend/lock.h> +#include <instantsend/instantsend.h>src/txmempool.cpp (1)
28-28: Consider forward-declaringCInstantSendManagerinstead of including the full header
txmempool.cpponly needs the pointer tollmq::CInstantSendManager(seeConnectManagers).
A forward declaration intxmempool.hplus removing the heavy header here would reduce compile time and dependency churn.-#include <instantsend/instantsend.h> +// forward declaration placed in txmempool.h: +// namespace llmq { class CInstantSendManager; }src/test/cachemultimap_tests.cpp (1)
1-3: Minor nit — extend copyright year to 2025 for consistency.Most updated source files now show
2014-2025; consider aligning this header to avoid year drift.-// Copyright (c) 2014-2023 The Dash Core developers +// Copyright (c) 2014-2025 The Dash Core developerssrc/init.cpp (1)
88-92: Include ordering ‑ consider deferring heavy headers
instantsend/instantsend.hpulls in the full InstantSend manager implementation, althoughinit.cpponly needs the opaque pointer thatllmq/context.halready forward-declares. You could avoid an extra compilation dependency (and speed up rebuilds) by dropping this include and relying on the one transitively brought in byllmq/context.h.If a direct include is preferred for clarity, keep it; otherwise, removing it is safe and trims compile time ever so slightly.
src/wallet/wallet.cpp (1)
3384-3385: Parameterislockis now unused – add[[maybe_unused]]or remove it to silence compiler warningsThe parameter is kept only for interface symmetry but isn’t referenced in the body. With
-Wunused-parameterenabled this will trigger a warning. Easiest fix is to mark it[[maybe_unused]]:-void CWallet::notifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const instantsend::InstantSendLock>& islock) +void CWallet::notifyTransactionLock(const CTransactionRef &tx, [[maybe_unused]] const std::shared_ptr<const instantsend::InstantSendLock>& islock)(or remove the parameter entirely if no caller needs it).
src/instantsend/instantsend.h (1)
59-59: Consider renamingm_activemanfor clarity.The member variable
m_activemanis of typeInstantSendSigner, but the name suggests it's an "active manager". Consider renaming it tom_signerorm_is_signerto better reflect its actual type and purpose.- std::unique_ptr<instantsend::InstantSendSigner> m_activeman{nullptr}; + std::unique_ptr<instantsend::InstantSendSigner> m_signer{nullptr};src/instantsend/db.cpp (2)
12-19: Fix code formatting issues.The pipeline indicates clang-format differences. Please run clang-format to ensure consistent code style.
124-131: Good defensive programming with the safety check.The check against going backwards in confirmation height prevents potential issues. However, please fix the formatting issues indicated by the pipeline.
src/instantsend/instantsend.cpp (2)
27-956: Fix extensive code formatting issues.The pipeline indicates that almost the entire file has formatting differences. Please run clang-format on the entire file to ensure consistent code style.
927-951: Simplify the control flow by removing unnecessary do-while(0) pattern.The do-while(0) pattern here adds unnecessary complexity. The breaks could be replaced with early returns or proper if-else chains.
- bool fMoreWork{false}; - do { - if (!IsInstantSendEnabled()) break; - fMoreWork = ProcessPendingInstantSendLocks(peerman); - if (!m_activeman) break; - // Construct set of non-locked transactions that are pending to retry - std::vector<CTransactionRef> txns{}; - { - LOCK2(cs_nonLocked, cs_pendingRetry); - if (pendingRetryTxs.empty()) break; - txns.reserve(pendingRetryTxs.size()); - for (const auto& txid : pendingRetryTxs) { - if (auto it = nonLockedTxs.find(txid); it != nonLockedTxs.end()) { - const auto& [_, tx_info] = *it; - if (tx_info.tx) { - txns.push_back(tx_info.tx); - } - } - } - txns.shrink_to_fit(); - } - // Retry processing them - m_activeman->ProcessPendingRetryLockTxs(txns); - } while (0); + bool fMoreWork{false}; + if (IsInstantSendEnabled()) { + fMoreWork = ProcessPendingInstantSendLocks(peerman); + if (m_activeman) { + // Construct set of non-locked transactions that are pending to retry + std::vector<CTransactionRef> txns{}; + { + LOCK2(cs_nonLocked, cs_pendingRetry); + if (!pendingRetryTxs.empty()) { + txns.reserve(pendingRetryTxs.size()); + for (const auto& txid : pendingRetryTxs) { + if (auto it = nonLockedTxs.find(txid); it != nonLockedTxs.end()) { + const auto& [_, tx_info] = *it; + if (tx_info.tx) { + txns.push_back(tx_info.tx); + } + } + } + txns.shrink_to_fit(); + } + } + // Retry processing them + if (!txns.empty()) { + m_activeman->ProcessPendingRetryLockTxs(txns); + } + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
src/Makefile.am(2 hunks)src/bench/rpc_blockchain.cpp(1 hunks)src/coinjoin/coinjoin.cpp(1 hunks)src/dsnotificationinterface.cpp(1 hunks)src/evo/chainhelper.cpp(1 hunks)src/init.cpp(1 hunks)src/instantsend/db.cpp(1 hunks)src/instantsend/db.h(1 hunks)src/instantsend/instantsend.cpp(20 hunks)src/instantsend/instantsend.h(1 hunks)src/instantsend/lock.cpp(1 hunks)src/instantsend/lock.h(1 hunks)src/instantsend/signing.cpp(1 hunks)src/instantsend/signing.h(1 hunks)src/interfaces/chain.h(2 hunks)src/llmq/chainlocks.cpp(1 hunks)src/llmq/context.cpp(1 hunks)src/llmq/context.h(1 hunks)src/llmq/instantsend.h(0 hunks)src/net_processing.cpp(2 hunks)src/node/interfaces.cpp(2 hunks)src/node/miner.cpp(1 hunks)src/rest.cpp(1 hunks)src/rpc/blockchain.cpp(1 hunks)src/rpc/mempool.cpp(1 hunks)src/rpc/mining.cpp(0 hunks)src/rpc/rawtransaction.cpp(2 hunks)src/test/cachemap_tests.cpp(1 hunks)src/test/cachemultimap_tests.cpp(1 hunks)src/test/evo_deterministicmns_tests.cpp(1 hunks)src/test/evo_islock_tests.cpp(3 hunks)src/test/fuzz/process_message.cpp(0 hunks)src/test/governance_validators_tests.cpp(1 hunks)src/test/miner_tests.cpp(0 hunks)src/test/ratecheck_tests.cpp(1 hunks)src/txmempool.cpp(1 hunks)src/validationinterface.cpp(1 hunks)src/validationinterface.h(3 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/miner_tests.cpp
- src/rpc/mining.cpp
- src/test/fuzz/process_message.cpp
- src/llmq/instantsend.h
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
src/bench/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (42)
📓 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#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-10T16:26:00.371Z
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-10T16:26:00.371Z
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/governance_validators_tests.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, `IsVersionChangeValid()` is only called by transaction types that update existing masternode entries (like `ProUpServTx`, `ProUpRegTx`, `ProUpRevTx`), not by `ProRegTx` which creates new entries. This means validation logic in `IsVersionChangeValid()` only applies to the subset of transaction types that actually call it, not all masternode transaction types.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: dashpay/dash#6530
File: src/validation.cpp:4514-4516
Timestamp: 2025-01-14T08:40:00.994Z
Learning: In Dash Core, error() is used (instead of state.Invalid()) to report critical failures that require immediate user intervention, particularly for database/disk errors and data corruption scenarios. This is an established pattern seen in validation.cpp.
src/dsnotificationinterface.cpp (5)
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#6532
File: src/net.cpp:4329-4329
Timestamp: 2025-01-14T09:07:12.446Z
Learning: Keep suggestions focused on the scope of the current commit/PR. Avoid suggesting unrelated improvements that should be handled in separate PRs, even if technically valid.
Learnt from: knst
PR: dashpay/dash#6533
File: test/functional/feature_llmq_singlenode.py:98-106
Timestamp: 2025-01-22T08:33:31.405Z
Learning: When reviewing PRs, ensure that suggestions are directly related to the PR's primary objectives rather than general code improvements that could be addressed separately.
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#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/test/cachemultimap_tests.cpp (2)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/**/*.{cpp,h,cc,cxx,hpp} : Dash uses unordered_lru_cache for efficient caching with LRU eviction
src/test/ratecheck_tests.cpp (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
src/evo/chainhelper.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: 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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
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.
src/test/evo_deterministicmns_tests.cpp (5)
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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
src/bench/rpc_blockchain.cpp (2)
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/bench/**/*.{cpp,h,cc,cxx,hpp} : Performance benchmarks should be implemented in src/bench/ using nanobench
src/rpc/mempool.cpp (3)
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.
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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 (2)
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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 (4)
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
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.
src/rpc/blockchain.cpp (4)
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
</retrieved_learning>
<retrieved_learning>
Learnt from: knst
PR: #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.
</retrieved_learning>
src/llmq/chainlocks.cpp (7)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: knst
PR: dashpay/dash#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: 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.
Learnt from: kwvg
PR: dashpay/dash#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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, `IsVersionChangeValid()` is only called by transaction types that update existing masternode entries (like `ProUpServTx`, `ProUpRegTx`, `ProUpRevTx`), not by `ProRegTx` which creates new entries. This means validation logic in `IsVersionChangeValid()` only applies to the subset of transaction types that actually call it, not all masternode transaction types.
src/llmq/context.h (3)
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.
Learnt from: kwvg
PR: dashpay/dash#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: dashpay/dash#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/test/evo_islock_tests.cpp (5)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
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#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-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
src/llmq/context.cpp (1)
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.
src/init.cpp (3)
undefined
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
</retrieved_learning>
src/rpc/rawtransaction.cpp (5)
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#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-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: 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.
src/test/cachemap_tests.cpp (2)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/**/*.{cpp,h,cc,cxx,hpp} : Dash uses unordered_lru_cache for efficient caching with LRU eviction
src/instantsend/lock.cpp (1)
Learnt from: kwvg
PR: dashpay/dash#6629
File: src/test/evo_netinfo_tests.cpp:39-45
Timestamp: 2025-05-10T00:55:16.184Z
Learning: In Dash's validation testing for network entries, checking `IsTriviallyValid()` is specifically used to ensure all three validation methods (`AddEntry()`, `Validate()`, and `IsTriviallyValid()`) consistently work together rather than replacing one check with another. `AddEntry()` and `Validate()` handle the consensus rule checks, while `IsTriviallyValid()` confirms the basic validation steps.
test/util/data/non-backported.txt (7)
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: 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-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/**/*.{cpp,h,cc,cxx,hpp} : Dash Core codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
src/interfaces/chain.h (2)
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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/validationinterface.cpp (3)
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.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, `IsVersionChangeValid()` is only called by transaction types that update existing masternode entries (like `ProUpServTx`, `ProUpRegTx`, `ProUpRevTx`), not by `ProRegTx` which creates new entries. This means validation logic in `IsVersionChangeValid()` only applies to the subset of transaction types that actually call it, not all masternode transaction types.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
src/wallet/wallet.cpp (2)
undefined
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
src/zmq/zmqabstractnotifier.cpp (1)
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.
src/zmq/zmqnotificationinterface.h (1)
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.
src/Makefile.am (5)
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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-10T16:26:00.371Z
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-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
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.
src/txmempool.cpp (6)
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
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: 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.
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.
test/lint/lint-circular-dependencies.py (2)
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-10T16:26:00.371Z
Learning: Applies to depends/** : Unless specifically prompted, avoid making changes to the depends directory (dependency build system)
src/zmq/zmqnotificationinterface.cpp (1)
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.
src/instantsend/lock.h (1)
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.
src/validationinterface.h (3)
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.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, `IsVersionChangeValid()` is only called by transaction types that update existing masternode entries (like `ProUpServTx`, `ProUpRegTx`, `ProUpRevTx`), not by `ProRegTx` which creates new entries. This means validation logic in `IsVersionChangeValid()` only applies to the subset of transaction types that actually call it, not all masternode transaction types.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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/interfaces.cpp (2)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: 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.
src/net_processing.cpp (5)
undefined
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
</retrieved_learning>
<retrieved_learning>
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: knst
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
</retrieved_learning>
src/wallet/wallet.h (2)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
Learnt from: kwvg
PR: dashpay/dash#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/zmq/zmqpublishnotifier.cpp (3)
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#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-10T16:26:00.371Z
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: 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.
src/instantsend/signing.cpp (2)
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.
Learnt from: knst
PR: dashpay/dash#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/instantsend/db.h (1)
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.
src/instantsend/instantsend.h (3)
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.
Learnt from: knst
PR: dashpay/dash#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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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.h (2)
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.
Learnt from: knst
PR: dashpay/dash#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/instantsend/instantsend.cpp (8)
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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: 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#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: 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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: dashpay/dash#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: kwvg
PR: dashpay/dash#6629
File: src/test/evo_netinfo_tests.cpp:39-45
Timestamp: 2025-05-10T00:55:16.184Z
Learning: In Dash's validation testing for network entries, checking `IsTriviallyValid()` is specifically used to ensure all three validation methods (`AddEntry()`, `Validate()`, and `IsTriviallyValid()`) consistently work together rather than replacing one check with another. `AddEntry()` and `Validate()` handle the consensus rule checks, while `IsTriviallyValid()` confirms the basic validation steps.
🧬 Code Graph Analysis (6)
src/interfaces/chain.h (2)
src/node/interfaces.cpp (12)
tx(497-500)tx(497-497)tx(620-623)tx(620-620)tx(624-627)tx(624-624)tx(645-648)tx(645-645)tx(890-897)tx(890-890)tx(909-924)tx(909-909)src/instantsend/instantsend.h (1)
islock(117-117)
src/validationinterface.cpp (4)
src/zmq/zmqabstractnotifier.cpp (2)
NotifyTransactionLock(51-54)NotifyTransactionLock(51-51)src/zmq/zmqnotificationinterface.cpp (2)
NotifyTransactionLock(216-221)NotifyTransactionLock(216-216)src/zmq/zmqpublishnotifier.cpp (6)
NotifyTransactionLock(256-264)NotifyTransactionLock(256-256)NotifyTransactionLock(418-425)NotifyTransactionLock(418-418)NotifyTransactionLock(427-435)NotifyTransactionLock(427-427)src/validationinterface.h (5)
tx(152-152)tx(152-152)tx(166-166)tx(166-166)tx(234-234)
src/zmq/zmqabstractnotifier.cpp (3)
src/validationinterface.cpp (2)
NotifyTransactionLock(269-275)NotifyTransactionLock(269-269)src/zmq/zmqnotificationinterface.cpp (2)
NotifyTransactionLock(216-221)NotifyTransactionLock(216-216)src/zmq/zmqpublishnotifier.cpp (6)
NotifyTransactionLock(256-264)NotifyTransactionLock(256-256)NotifyTransactionLock(418-425)NotifyTransactionLock(418-418)NotifyTransactionLock(427-435)NotifyTransactionLock(427-427)
src/zmq/zmqnotificationinterface.h (2)
src/instantsend/instantsend.h (5)
tx(111-111)tx(115-115)tx(133-133)tx(139-139)islock(117-117)src/validationinterface.h (5)
tx(152-152)tx(152-152)tx(166-166)tx(166-166)tx(234-234)
src/zmq/zmqpublishnotifier.h (3)
src/zmq/zmqabstractnotifier.h (4)
transaction(66-66)transaction(68-68)transaction(70-70)transaction(72-72)src/instantsend/instantsend.h (1)
islock(117-117)src/instantsend/signing.h (1)
islock(73-73)
src/instantsend/instantsend.h (9)
src/instantsend/db.h (12)
unitTests(80-80)hash(72-72)hash(91-91)hash(97-97)hash(123-123)txid(77-77)txid(133-133)txid(143-143)islockHash(111-111)islockHash(157-157)pblock(109-109)pblock(110-110)src/instantsend/instantsend.cpp (2)
CInstantSendManager(56-74)CInstantSendManager(76-76)src/net_processing.cpp (24)
pfrom(619-619)pfrom(642-643)pfrom(717-719)pfrom(725-725)pfrom(733-733)pfrom(736-736)pfrom(738-738)pfrom(740-740)pfrom(833-833)pfrom(1015-1015)from(1018-1018)hash(986-986)hash(992-992)tx(1043-1043)txid(637-637)txid(652-652)txid(654-654)pindex(614-614)pindex(831-831)pblock(606-606)ret(657-657)ret(658-658)id(676-676)id(680-680)src/instantsend/signing.h (5)
islock(73-73)tx(81-81)tx(85-85)tx(96-97)tx(99-99)src/interfaces/chain.h (9)
hash(57-57)hash(57-57)hash(128-128)hash(143-143)txid(193-193)txid(196-196)txid(207-207)pindexChainLock(270-270)pindexChainLock(270-270)src/node/interfaces.cpp (25)
hash(741-758)hash(741-741)hash(780-784)hash(780-780)hash(806-811)hash(806-806)tx(497-500)tx(497-497)tx(620-623)tx(620-620)tx(624-627)tx(624-624)tx(645-648)tx(645-645)tx(890-897)txid(877-882)txid(877-877)txid(883-889)txid(883-883)txid(898-903)txid(898-898)pindexChainLock(641-644)pindexChainLock(641-641)id(407-413)id(407-407)src/validationinterface.h (6)
pindex(167-167)pindex(167-167)pindex(200-200)pindex(232-232)pindex(233-233)pindex(235-235)src/zmq/zmqabstractnotifier.h (4)
pindex(60-60)pindex(62-62)pindex(64-64)pindex(71-71)src/zmq/zmqpublishnotifier.h (3)
pindex(42-42)pindex(48-48)pindex(90-90)
🪛 GitHub Actions: Clang Diff Format Check
src/instantsend/signing.cpp
[error] 48-373: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
src/instantsend/db.h
[error] 33-70: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
[error] 100-161: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
src/instantsend/instantsend.h
[error] 63-147: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
src/instantsend/signing.h
[error] 50-98: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
src/instantsend/db.cpp
[error] 19-19: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
[error] 124-127: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
[error] 221-224: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
[error] 241-245: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
[error] 354-359: Clang format differences found. Code formatting does not match clang-format style. Please run clang-format to fix.
src/instantsend/instantsend.cpp
[error] 27-932: Clang format differences found. Code formatting does not match clang-format 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). (3)
- GitHub Check: linux64_tsan-test / Test source
- GitHub Check: linux64-test / Test source
- GitHub Check: linux64_multiprocess-build / Build source
🔇 Additional comments (55)
src/rpc/mempool.cpp (1)
19-19: Include path migration looks correctThe switch to the new
instantsend/instantsend.hlocation compiles cleanly and keeps the dependency minimal.
No further action required.src/rest.cpp (1)
14-14: Header path update acknowledgedThe new InstantSend include path is correct and keeps REST build-able after the refactor.
src/rpc/blockchain.cpp (1)
57-57: Header path updated; confirm build picks up new locationThe include has been switched to
instantsend/instantsend.h, which aligns with the module relocation.
Please double-check that:
- The build system (
Makefile.am/ CMake, etc.) already addssrc/instantsend/to the compiler’s header-search paths.instantsend/instantsend.hstill definesllmq::CInstantSendManager, as this TU continues to rely on that namespace (e.g.isman.IsLocked()below).A quick sanity compile should surface any mismatch early.
src/test/evo_deterministicmns_tests.cpp (1)
26-26: Header path update looks goodThe test now pulls in the correct InstantSend header; no further action needed.
src/dsnotificationinterface.cpp (1)
19-19: Include path update looks correct.
dsnotificationinterface.cppnow includes the refactored InstantSend header (instantsend/instantsend.h), matching the new module layout. No further action required.src/bench/rpc_blockchain.cpp (1)
9-9: Bench file brought in sync with refactor.The include was updated to
<instantsend/instantsend.h>, ensuring the benchmark continues to compile after the module move. All good.test/util/data/non-backported.txt (1)
17-18: List update acknowledged.Adding the new
src/instantsend/*patterns keeps the non-backported registry up-to-date with the module split. ✔️src/coinjoin/coinjoin.cpp (1)
12-12: Include path correctly migrated to new InstantSend moduleThe update from the old
llmqlocation to<instantsend/instantsend.h>is consistent with the refactor and compiles cleanly.
No further issues spotted.src/node/miner.cpp (1)
36-36: Header moved to new namespace – looks good
<instantsend/instantsend.h>is now the canonical header; the include order remains logically grouped.
All good.src/llmq/context.cpp (1)
10-10: Include update aligns with refactorSwitching to
<instantsend/instantsend.h>removes the obsolete dependency; no circular-include concerns observed.src/llmq/chainlocks.cpp (1)
23-26: Updated include block is accurateThe new InstantSend header and reordered LLMQ headers compile without warnings; dependency graph unaffected.
No further action needed.src/llmq/context.h (1)
72-72: Comment removal only – safeOnly a stale TODO was deleted; no functional impact.
src/rpc/rawtransaction.cpp (2)
53-54: LGTM: Include updates align with InstantSend namespace refactoring.The includes are correctly updated from
llmq/instantsend.hto the new dedicatedinstantsend/lock.handinstantsend/instantsend.hheaders, consistent with the PR's goal of moving InstantSend functionality out of thellmqnamespace.
459-459: LGTM: Type update correctly reflects namespace changes.The type change from
llmq::CInstantSendLockPtrtoinstantsend::InstantSendLockPtris consistent with the InstantSend refactoring and maintains the same functionality while using the new namespace.src/interfaces/chain.h (2)
34-36: LGTM: Forward declaration correctly added for new namespace.The forward declaration for
instantsend::InstantSendLockis properly added to support the namespace refactoring while maintaining clean header dependencies.
271-271: LGTM: Interface method signature correctly updated.The parameter type change from
llmq::CInstantSendLockPtrtostd::shared_ptr<const instantsend::InstantSendLock>is consistent with the InstantSend refactoring and maintains the same const-correctness and shared pointer semantics.src/test/evo_islock_tests.cpp (6)
1-1: Copyright year update is appropriate for refactoring.The copyright year update to 2025 is consistent with the refactoring effort.
7-7: Include path correctly updated for the new module structure.The include path change from
llmq/instantsend.htoinstantsend/lock.hcorrectly reflects the refactoring of InstantSend into a dedicated module.
14-16: Include additions are appropriate for the test file.The added includes for
<string_view>and<boost/test/unit_test.hpp>are correctly formatted and appropriate for the test file's needs.
31-31: Type reference correctly updated to new namespace.The change from
llmq::CInstantSendLocktoinstantsend::InstantSendLockis consistent with the refactoring objectives and maintains the same functionality.
34-34: Comment updated to reflect new type name.The comment correctly refers to
InstantSendLockinstead ofCInstantSendLock, maintaining consistency with the new naming convention.
79-79: Consistent type usage throughout the test.The second instance of type change from
llmq::CInstantSendLocktoinstantsend::InstantSendLockmaintains consistency throughout the test file.src/zmq/zmqabstractnotifier.cpp (1)
51-51: Parameter type correctly updated for namespace refactoring.The change from
std::shared_ptr<const llmq::CInstantSendLock>tostd::shared_ptr<const instantsend::InstantSendLock>is consistent with the refactoring objectives and maintains the same method signature semantics. This aligns with the corresponding updates insrc/zmq/zmqnotificationinterface.cppandsrc/zmq/zmqpublishnotifier.cpp.src/net_processing.cpp (2)
65-66: LGTM! Include directives updated correctly.The new include directives for
<instantsend/lock.h>and<instantsend/instantsend.h>properly reflect the InstantSend module extraction from thellmqnamespace to the dedicatedinstantsendnamespace.
2869-2869: LGTM! Type rename is consistent with the refactor.The change from
llmq::CInstantSendLocktoinstantsend::InstantSendLockis consistent with the comprehensive refactoring that moves InstantSend implementation to theinstantsendnamespace. The functionality remains identical, just using the new type definition.src/wallet/wallet.h (1)
916-916: LGTM! Type signature correctly updated for the InstantSend refactoring.The method signature change from
llmq::CInstantSendLocktoinstantsend::InstantSendLockis consistent with the refactoring that moves InstantSend functionality from thellmqnamespace to the dedicatedinstantsendnamespace.src/zmq/zmqnotificationinterface.h (1)
35-35: LGTM: Namespace change correctly implemented.The parameter type update from
llmq::CInstantSendLocktoinstantsend::InstantSendLockis consistent with the move-only refactor to reorganize InstantSend functionality into a dedicated module.src/zmq/zmqnotificationinterface.cpp (1)
216-216: LGTM: Implementation correctly updated to match header.The method implementation properly updates the parameter type to
instantsend::InstantSendLock, maintaining consistency with the header file change and preserving the same functionality.src/validationinterface.cpp (1)
269-269: LGTM: Consistent namespace update in validation interface.The parameter type change to
instantsend::InstantSendLockis consistent with the move-only refactor and properly maintains the same notification functionality.src/node/interfaces.cpp (2)
21-21: LGTM: Include directive properly updated.The include directive correctly reflects the new file location of the InstantSend module under the
instantsenddirectory structure.
645-645: LGTM: Method signature consistently updated.The parameter type change to
instantsend::InstantSendLockis consistent with the namespace reorganization and maintains the same notification functionality.src/Makefile.am (2)
233-236: LGTM: Header files properly added to build system.The new
instantsendmodule header files are correctly added to theBITCOIN_CORE_Hsection, ensuring they will be included in the build.
493-496: LGTM: Source files properly added to build system.The new
instantsendmodule source files are correctly added to thelibbitcoin_node_a_SOURCESsection, completing the integration of the refactored module into the build system.src/instantsend/lock.cpp (3)
13-13: LGTM! Good use of string_view for constant prefix.Using
std::string_viewfor the constant prefix is efficient as it avoids unnecessary string copies during hashing operations.
16-22: LGTM! Request ID generation is correctly implemented.The implementation properly hashes the prefix and inputs to generate a unique request ID, which is consistent with the expected behavior for InstantSend locks.
28-43: LGTM! Validation logic is comprehensive and correct.The trivial validation correctly checks for:
- Null transaction ID
- Empty inputs vector
- Duplicate inputs using efficient set-based detection
The duplicate detection logic using
std::set::emplace().secondis correct and efficient.src/instantsend/lock.h (2)
19-41: LGTM! Well-designed struct with proper serialization support.The struct definition is well-structured with:
- Appropriate version field for future compatibility
- Proper member types (vector, uint256, CBLSLazySignature)
- Comprehensive serialization support
- Forward declaration usage to minimize dependencies
The default constructor and serialization macros are correctly implemented.
43-43: LGTM! Convenient type alias for shared pointer usage.The
InstantSendLockPtralias provides a clean way to use shared pointers to InstantSendLock throughout the codebase.src/validationinterface.h (3)
31-33: LGTM! Proper namespace organization.The forward declaration of
instantsend::InstantSendLockis correctly placed and follows the established pattern for other namespace declarations in the file.
166-166: LGTM! Interface updated to use the new InstantSend type.The
NotifyTransactionLockmethod signature correctly usesstd::shared_ptr<const instantsend::InstantSendLock>instead of the previousllmq::CInstantSendLock, which aligns with the refactoring objectives.
234-234: LGTM! Consistent method signature update.The
CMainSignals::NotifyTransactionLockmethod signature is correctly updated to match the interface change, maintaining consistency across the validation interface.src/zmq/zmqpublishnotifier.h (3)
60-60: LGTM! Consistent type update for hash transaction lock notifier.The method signature correctly updates to use
std::shared_ptr<const instantsend::InstantSendLock>instead of the previousllmq::CInstantSendLock, maintaining consistency with the refactoring.
123-123: LGTM! Consistent type update for raw transaction lock notifier.The method signature correctly updates to use
std::shared_ptr<const instantsend::InstantSendLock>instead of the previousllmq::CInstantSendLock, maintaining consistency with the refactoring.
129-129: LGTM! Consistent type update for raw transaction lock signature notifier.The method signature correctly updates to use
std::shared_ptr<const instantsend::InstantSendLock>instead of the previousllmq::CInstantSendLock, maintaining consistency with the refactoring.src/zmq/zmqabstractnotifier.h (3)
17-26: LGTM! Well-organized namespace declarations.The namespace declarations are properly organized with:
- Consistent formatting and style
- Correct forward declaration of
instantsend::InstantSendLock- Removal of the old
llmq::CInstantSendLockdeclaration- Retention of other necessary
llmqforward declarationsThis aligns with the refactoring objectives.
28-28: LGTM! Modern C++ style improvement.The change from
typedeftousingdeclaration is a good modernization and follows C++11+ best practices.
72-72: LGTM! Method signature correctly updated.The
NotifyTransactionLockmethod signature is correctly updated to usestd::shared_ptr<const instantsend::InstantSendLock>instead of the previousllmq::CInstantSendLock, maintaining consistency with the refactoring across all interface files.src/zmq/zmqpublishnotifier.cpp (2)
15-20: LGTM!The include directive changes correctly reflect the refactoring of InstantSend into its own module.
256-256: LGTM!The parameter type updates correctly use the new
instantsend::InstantSendLocktype, maintaining consistency with the namespace refactoring.Also applies to: 418-418, 427-427
src/instantsend/signing.h (1)
28-102: Well-designed class structure!The
InstantSendSignerclass has a clear separation of concerns, extracting the signing logic from the main manager class. The thread-safety annotations are comprehensive and will help prevent concurrency issues.src/instantsend/db.h (1)
29-158: Excellent database interface design!The
CInstantSendDbclass provides a comprehensive and well-documented interface for managing InstantSend locks. The use of GSL'snot_nullpointers and thorough thread-safety annotations demonstrates attention to safety and correctness.src/instantsend/instantsend.h (1)
46-167: Excellent refactoring of the InstantSend manager!The refactored
CInstantSendManagersuccessfully delegates signing responsibilities to theInstantSendSignerclass, resulting in a cleaner separation of concerns. The comprehensive thread-safety annotations will help maintain correctness in concurrent scenarios.src/instantsend/signing.cpp (1)
30-396: High-quality implementation of the InstantSend signer!The implementation is comprehensive with proper error handling, detailed logging for debugging, and appropriate thread safety measures. The extraction of signing logic into this dedicated class improves modularity and maintainability.
src/instantsend/db.cpp (1)
357-394: Well-designed implementation for removing chained locks.The stack-based approach avoids recursion issues, and the use of an unordered_set to track processed transactions prevents infinite loops from circular dependencies. The batch operations ensure atomicity.
src/instantsend/instantsend.cpp (1)
620-627: Good helper function implementation.The
TryEmplacePendingLockfunction properly checks if the lock is already known before adding it to pending locks, with appropriate locking.
| const CMasternodeSync& m_mn_sync; | ||
|
|
||
| private: | ||
| mutable Mutex cs_inputReqests; |
There was a problem hiding this comment.
Fix the typo in the mutex name.
The mutex name has a typo: cs_inputReqests should be cs_inputRequests.
Apply this diff to fix the typo:
- mutable Mutex cs_inputReqests;
+ mutable Mutex cs_inputRequests;Also update the corresponding GUARDED_BY annotation:
- std::unordered_set<uint256, StaticSaltedHasher> inputRequestIds GUARDED_BY(cs_inputReqests);
+ std::unordered_set<uint256, StaticSaltedHasher> inputRequestIds GUARDED_BY(cs_inputRequests);Also applies to: 49-49
🤖 Prompt for AI Agents
In src/instantsend/signing.h at lines 42 and 49, there is a typo in the mutex
name `cs_inputReqests` which should be corrected to `cs_inputRequests`. Rename
the mutex variable to fix the typo and update any corresponding `GUARDED_BY`
annotations to use the corrected name to maintain consistency and proper
thread-safety annotations.
src/instantsend/ and split sources for islock, database and masternode logic out of manager sourcesrc/instantsend/ and split sources for islock, database and masternode logic out of manager source
| // Check that each input is unique | ||
| std::set<COutPoint> dups; | ||
| for (const auto& o : inputs) { | ||
| if (!dups.emplace(o).second) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
@coderabbitai can we use std algorithms here to improve it?
(nit not for this PR)
There was a problem hiding this comment.
@PastaPastaPasta Yes, there are several ways to use standard algorithms to improve the duplicate detection in the TriviallyValid() method. Here are a few alternatives:
Option 1: Using std::unordered_set (potentially faster)
bool InstantSendLock::TriviallyValid() const
{
if (txid.IsNull() || inputs.empty()) {
return false;
}
std::unordered_set<COutPoint> seen;
for (const auto& o : inputs) {
if (!seen.insert(o).second) {
return false;
}
}
return true;
}Option 2: Using std::ranges::unique (C++20, more expressive)
bool InstantSendLock::TriviallyValid() const
{
if (txid.IsNull() || inputs.empty()) {
return false;
}
auto sorted_inputs = inputs;
std::ranges::sort(sorted_inputs);
auto unique_end = std::ranges::unique(sorted_inputs);
return unique_end.begin() == sorted_inputs.end();
}Option 3: Using std::adjacent_find after sorting
bool InstantSendLock::TriviallyValid() const
{
if (txid.IsNull() || inputs.empty()) {
return false;
}
auto sorted_inputs = inputs;
std::sort(sorted_inputs.begin(), sorted_inputs.end());
return std::adjacent_find(sorted_inputs.begin(), sorted_inputs.end()) == sorted_inputs.end();
}Trade-offs:
- Current approach: O(n log n), early termination on duplicates, clear intent
- unordered_set: O(n) average case, but requires good hash function for
COutPoint - sort + unique/adjacent_find: O(n log n), more "algorithmic" style, but copies the vector
The current implementation is actually quite good - it's efficient and clear. The std::unordered_set approach might be the most beneficial performance-wise if COutPoint has a good hash function.
🧠 Learnings used
Learnt from: kwvg
PR: dashpay/dash#6629
File: src/test/evo_netinfo_tests.cpp:39-45
Timestamp: 2025-05-10T00:55:16.184Z
Learning: In Dash's validation testing for network entries, checking `IsTriviallyValid()` is specifically used to ensure all three validation methods (`AddEntry()`, `Validate()`, and `IsTriviallyValid()`) consistently work together rather than replacing one check with another. `AddEntry()` and `Validate()` handle the consensus rule checks, while `IsTriviallyValid()` confirms the basic validation steps.
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.
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.
| } | ||
| } | ||
|
|
||
| void CInstantSendDb::RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, InstantSendLockPtr islock, bool keep_cache) |
There was a problem hiding this comment.
nit not for this pr
@coderabbitai, can this islock be taken in as a raw pointer to avoid having to copy in the shared_ptr?
There was a problem hiding this comment.
🧩 Analysis chain
@PastaPastaPasta Looking at the RemoveInstantSendLock method, you're right that it could potentially take a raw pointer instead of InstantSendLockPtr to avoid copying the shared_ptr.
The method currently:
- Uses the shared_ptr only for temporary access (
islock->txid,islock->inputs) - Has null-checking logic that fetches from database if needed
- Doesn't store or pass the shared_ptr elsewhere
A raw pointer would work, but would require:
- Caller to handle shared_ptr→raw pointer conversion
- Careful lifetime management (caller must ensure object stays alive)
- Reconsidering the null-checking logic (maybe require non-null raw pointer)
Since the method only needs temporary access and doesn't maintain ownership, a raw pointer would indeed avoid the shared_ptr copy overhead. Good catch for a future optimization!
🏁 Script executed:
#!/bin/bash
# Let's examine the call sites and usage patterns
echo "=== Call sites for RemoveInstantSendLock ==="
rg "RemoveInstantSendLock" -A 2 -B 2
echo -e "\n=== How InstantSendLockPtr is defined ==="
rg "InstantSendLockPtr" -A 1 -B 1 src/instantsend/lock.h
Length of output: 2964
Looking at the call sites and usage patterns, you're absolutely right that this could be optimized!
Current usage analysis:
- The method takes
InstantSendLockPtr(shared_ptr) but only uses it for temporary access (islock->txid,islock->inputs) - It doesn't store or pass the shared_ptr elsewhere - just reads from it
- Call sites are:
RemoveInstantSendLock(batch, islockHash, islock)- passes shared_ptr fromGetInstantSendLockByHashInternalRemoveInstantSendLock(batch, childIslockHash, childIsLock, false)- similarRemoveInstantSendLock(batch, islockHash, nullptr, false)- passes nullptr explicitly
Optimization potential:
Yes, a raw pointer would work and avoid the shared_ptr copy overhead. The method only needs temporary read access to the lock data.
Design considerations:
The current method serves dual purposes:
- Remove a lock the caller already has
- Remove a lock the caller doesn't have (fetches internally when passed nullptr)
For a raw pointer approach, you could:
- Require non-null raw pointer (caller must fetch first)
- Have two overloads:
RemoveInstantSendLock(batch, hash, const InstantSendLock*)andRemoveInstantSendLock(batch, hash) - Keep current design but change signature to
const InstantSendLock* islock = nullptr
The shared_ptr→raw pointer conversion at call sites would be trivial (islock.get()), and lifetime is guaranteed since callers already hold the shared_ptr.
Good catch for a future optimization - it would reduce reference counting overhead for what's essentially a read-only parameter! 🐰
🧬 Code Graph Analysis Results
Relevant snippets from src/instantsend/db.cpp (entire file, lines 1-280 approx.)
The entire file src/instantsend/db.cpp is relevant as it contains the implementation of the CInstantSendDb class, which manages InstantSend locks (ISLocks) in the database. Key points include:
- The class uses a
std::shared_ptr<InstantSendLock>type aliasInstantSendLockPtrfor caching ISLocks. - The method
WriteNewInstantSendLock(const uint256& hash, const InstantSendLock& islock)inserts a new ISLock into the database and caches it by creating astd::shared_ptr<InstantSendLock>from the passedInstantSendLockreference. - The method
RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, InstantSendLockPtr islock, bool keep_cache)accepts anInstantSendLockPtrshared pointer for the ISLock to remove. If the pointer is null, it fetches the ISLock internally. - The caching mechanism uses
std::shared_ptr<InstantSendLock>extensively. - The code uses locking (
LOCK(cs_db)) to protect database and cache access. - The user comment suggests a nit about passing the ISLock as a raw pointer instead of a shared pointer to avoid copying the shared pointer.
Relevant snippets from src/instantsend/db.h
- Declaration of
RemoveInstantSendLockmethod:
void RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, InstantSendLockPtr islock, bool keep_cache = true);- Declaration of
WriteNewInstantSendLockmethod:
void WriteNewInstantSendLock(const uint256& hash, const InstantSendLock& islock);- Declaration of
InstantSendLockPtras a shared pointer type alias forInstantSendLock. - The header defines the interface for the database operations on ISLocks, including caching and batch operations.
Summary of relevant context from src/instantsend/db.cpp
WriteNewInstantSendLocktakes the ISLock by const reference and then creates astd::shared_ptr<InstantSendLock>to insert into the cache.RemoveInstantSendLocktakes anInstantSendLockPtrshared pointer, which may be null, and if so, fetches the ISLock internally.- The user comment suggests that the
islockparameter inRemoveInstantSendLockcould be passed as a raw pointer instead of a shared pointer to avoid copying the shared pointer. - The code uses shared pointers for caching and lifetime management of ISLocks.
- The locking and batch write patterns are consistent throughout the file.
Possibly relevant snippets from src/instantsend/instantsend.cpp
At lines 863-870:
instantsend::InstantSendLockPtr CInstantSendManager::GetInstantSendLockByTxid(const uint256& txid) const
{
if (!IsInstantSendEnabled()) {
return nullptr;
}
return db.GetInstantSendLockByTxid(txid);
}- Shows usage of
InstantSendLockPtrreturned from the DB interface.
At lines 919-922:
size_t CInstantSendManager::GetInstantSendLockCount() const
{
return db.GetInstantSendLockCount();
}- Shows delegation to DB for lock count.
Possibly relevant snippets from src/instantsend/instantsend.h
- Declaration of methods related to ISLocks, including:
RemoveConflictingLock(const uint256& islockHash, const instantsend::InstantSendLock& islock);
TryEmplacePendingLock(const uint256& hash, const NodeId id, const instantsend::InstantSendLockPtr& islock);
GetInstantSendLockByTxid(const uint256& txid) const;- Shows that
InstantSendLockPtris used in the interface.
Possibly relevant snippets from src/zmq/zmqnotificationinterface.h
At lines 31-32:
BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override
BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexDisconnected) override- Shows usage of
std::shared_ptr<const CBlock>in block connected/disconnected notifications, similar to howstd::shared_ptris used inWriteBlockInstantSendLocksandRemoveBlockInstantSendLocksindb.cpp.
Possibly relevant snippets from src/dbwrapper.h
At lines 687-689:
std::unique_ptr<CDBTransactionIterator<CDBTransaction>> NewIteratorUniquePtr() {
return std::make_unique<CDBTransactionIterator<CDBTransaction>>(*this);
}- Shows usage of
std::unique_ptrfor DB iterators, relevant to how iterators are used inRemoveConfirmedInstantSendLocksandRemoveArchivedInstantSendLocks.
Summary
The main relevant context is the use of std::shared_ptr<InstantSendLock> in the CInstantSendDb class, especially in the RemoveInstantSendLock method where the user suggests passing a raw pointer instead of a shared pointer to avoid copying the shared pointer. The file shows consistent use of shared pointers for caching and lifetime management of ISLocks. Other snippets show similar usage patterns of shared pointers in related code areas.
🧠 Learnings used
Learnt from: kwvg
PR: dashpay/dash#6629
File: src/evo/netinfo.h:248-256
Timestamp: 2025-05-10T00:54:30.887Z
Learning: The Dash codebase includes a specialization for serializing std::shared_ptr objects in src/serialize.h, which allows direct serialization of shared_ptr instances with stream operators.
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.
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.
| static const std::string_view DB_ARCHIVED_BY_HASH = "is_a2"; | ||
| static const std::string_view DB_ARCHIVED_BY_HEIGHT_AND_HASH = "is_a1"; | ||
| static const std::string_view DB_HASH_BY_OUTPOINT = "is_in"; | ||
| static const std::string_view DB_HASH_BY_TXID = "is_tx"; | ||
| static const std::string_view DB_ISLOCK_BY_HASH = "is_i"; | ||
| static const std::string_view DB_MINED_BY_HEIGHT_AND_HASH = "is_m"; | ||
| static const std::string_view DB_VERSION = "is_v"; |
There was a problem hiding this comment.
nit @coderabbitai can these be constexpr
There was a problem hiding this comment.
@PastaPastaPasta Yes, these can definitely be constexpr!
Since C++17, std::string_view is a literal type and its constructor for string literals is constexpr. All of these variables are initialized with string literals, which are constant expressions, so they can be evaluated at compile time.
static constexpr std::string_view DB_ARCHIVED_BY_HASH = "is_a2";
static constexpr std::string_view DB_ARCHIVED_BY_HEIGHT_AND_HASH = "is_a1";
static constexpr std::string_view DB_HASH_BY_OUTPOINT = "is_in";
static constexpr std::string_view DB_HASH_BY_TXID = "is_tx";
static constexpr std::string_view DB_ISLOCK_BY_HASH = "is_i";
static constexpr std::string_view DB_MINED_BY_HEIGHT_AND_HASH = "is_m";
static constexpr std::string_view DB_VERSION = "is_v";This is a nice optimization that makes these compile-time constants rather than runtime constants, and better expresses the intent that these values are never meant to change.
🐰💭
🧠 Learnings used
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.
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.
| using node::GetTransaction; | ||
|
|
||
| namespace llmq { | ||
| static const std::string_view INPUTLOCK_REQUESTID_PREFIX = "inlock"; |
src/instantsend/signing.cpp
Outdated
| uint256 hashBlock{}; | ||
| const auto tx = GetTransaction(nullptr, &mempool, txid, Params().GetConsensus(), hashBlock); | ||
| if (!tx) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
nit mark hashBlock sas unused via _
src/instantsend/signing.cpp
Outdated
| CTransactionRef tx; | ||
| { | ||
| { | ||
| LOCK(cs_nonLocked); | ||
| auto it = nonLockedTxs.find(txid); | ||
| if (it == nonLockedTxs.end()) { | ||
| continue; | ||
| } | ||
| tx = it->second.tx; | ||
| } |
There was a problem hiding this comment.
nit: rework this to use a lambda that returns the tx
|
@coderabbitai can you generate the Sequence Diagram for the new architecture? |
|
Tip For best results, initiate chat on the files or code changes. @PastaPastaPasta I'll generate a sequence diagram showing the new InstantSend architecture after the refactoring. Let me create a diagram that illustrates the key interactions between the components. sequenceDiagram
participant TxPool as Transaction Pool
participant ISMgr as CInstantSendManager
participant ISSigner as InstantSendSigner
participant ISDb as CInstantSendDb
participant QuorumMgr as CQuorumManager
participant SigningMgr as CSigningManager
participant SigSharesMgr as CSigSharesManager
participant PeerMgr as PeerManager
Note over TxPool, PeerMgr: New Transaction Processing Flow
TxPool->>ISMgr: TransactionAddedToMempool()
ISMgr->>ISSigner: ProcessTx()
ISSigner->>ISSigner: CheckCanLock()
alt Transaction can be locked
ISSigner->>ISSigner: TrySignInputLocks()
ISSigner->>QuorumMgr: GetQuorumForSigning()
QuorumMgr-->>ISSigner: Return quorum info
ISSigner->>SigningMgr: AsyncSignIfMember()
SigningMgr->>SigSharesMgr: ProcessMessage()
Note over ISSigner, SigningMgr: Input Lock Recovery
SigningMgr-->>ISSigner: HandleNewInputLockRecoveredSig()
ISSigner->>ISSigner: All inputs locked?
alt All inputs have recovered signatures
ISSigner->>ISSigner: TrySignInstantSendLock()
ISSigner->>QuorumMgr: GetQuorumForSigning()
QuorumMgr-->>ISSigner: Return quorum info
ISSigner->>SigningMgr: AsyncSignIfMember()
Note over ISSigner, SigningMgr: InstantSend Lock Recovery
SigningMgr-->>ISSigner: HandleNewInstantSendLockRecoveredSig()
ISSigner->>ISDb: WriteNewInstantSendLock()
ISDb->>ISDb: Update caches
ISSigner-->>ISMgr: Lock created
ISMgr->>PeerMgr: Broadcast lock
end
end
Note over TxPool, PeerMgr: Incoming InstantSend Lock Processing
PeerMgr->>ISMgr: ProcessMessageInstantSendLock()
ISMgr->>ISDb: KnownInstantSendLock()
ISDb-->>ISMgr: Lock status
alt Lock not known
ISMgr->>ISMgr: ProcessInstantSendLock()
ISMgr->>ISDb: WriteNewInstantSendLock()
ISDb->>ISDb: Update caches
ISMgr->>ISMgr: RemoveMempoolConflictsForLock()
ISMgr->>TxPool: Remove conflicting transactions
end
Note over TxPool, PeerMgr: Block Connection Events
ISMgr->>ISMgr: BlockConnected()
ISMgr->>ISDb: WriteBlockInstantSendLocks()
ISDb->>ISDb: Mark locks as mined
ISMgr->>ISDb: RemoveConfirmedInstantSendLocks()
ISDb->>ISDb: Cleanup old locks
Note over TxPool, PeerMgr: Query Operations
ISMgr->>ISDb: GetInstantSendLockByTxid()
ISDb-->>ISMgr: Return lock
ISMgr->>ISDb: GetInstantSendLockByInput()
ISDb-->>ISMgr: Return lock
ISMgr->>ISDb: GetInstantSendLockCount()
ISDb-->>ISMgr: Return count
This sequence diagram illustrates the key aspects of the new InstantSend architecture: Key Changes in the New Architecture:
Main Flow:
This architecture better separates networking-dependent code (in 🧠 Learnings used |
|
generally LGTM; a few minor changes to do. |
|
This pull request has conflicts, please rebase. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/instantsend/db.h (1)
33-70: Fix clang-format issues and member initialization.The code has clang-format violations that need to be addressed, and the member initialization should use proper brace initialization.
Apply this fix for the member initialization:
- int best_confirmed_height GUARDED_BY(cs_db) {0}; + int best_confirmed_height GUARDED_BY(cs_db){0};Please run clang-format on the entire file to fix the formatting issues.
src/instantsend/instantsend.h (1)
63-63: Fix clang-format violation.The line has a clang-format violation that needs to be addressed.
Please run clang-format on this file to fix the formatting issue.
src/instantsend/db.cpp (2)
12-18: Make database constants constexpr for better performance.These constants should be
constexprsince they're initialized with string literals and never change.Apply this diff:
-static const std::string_view DB_ARCHIVED_BY_HASH = "is_a2"; -static const std::string_view DB_ARCHIVED_BY_HEIGHT_AND_HASH = "is_a1"; -static const std::string_view DB_HASH_BY_OUTPOINT = "is_in"; -static const std::string_view DB_HASH_BY_TXID = "is_tx"; -static const std::string_view DB_ISLOCK_BY_HASH = "is_i"; -static const std::string_view DB_MINED_BY_HEIGHT_AND_HASH = "is_m"; -static const std::string_view DB_VERSION = "is_v"; +static constexpr std::string_view DB_ARCHIVED_BY_HASH = "is_a2"; +static constexpr std::string_view DB_ARCHIVED_BY_HEIGHT_AND_HASH = "is_a1"; +static constexpr std::string_view DB_HASH_BY_OUTPOINT = "is_in"; +static constexpr std::string_view DB_HASH_BY_TXID = "is_tx"; +static constexpr std::string_view DB_ISLOCK_BY_HASH = "is_i"; +static constexpr std::string_view DB_MINED_BY_HEIGHT_AND_HASH = "is_m"; +static constexpr std::string_view DB_VERSION = "is_v";
283-290: Remove redundant database read attempts.The code redundantly attempts to read and deserialize the same data twice with identical logic. The second attempt will produce the same result as the first.
Apply this diff to remove the redundant read:
- ret = std::make_shared<InstantSendLock>(); - bool exists = db->Read(std::make_tuple(DB_ISLOCK_BY_HASH, hash), *ret); - if (!exists || (::SerializeHash(*ret) != hash)) { - ret = std::make_shared<InstantSendLock>(); - exists = db->Read(std::make_tuple(DB_ISLOCK_BY_HASH, hash), *ret); - if (!exists || (::SerializeHash(*ret) != hash)) { - ret = nullptr; - } - } + ret = std::make_shared<InstantSendLock>(); + bool exists = db->Read(std::make_tuple(DB_ISLOCK_BY_HASH, hash), *ret); + if (!exists || (::SerializeHash(*ret) != hash)) { + ret = nullptr; + }
🧹 Nitpick comments (1)
src/instantsend/db.cpp (1)
71-71: Consider using raw pointer to avoid shared_ptr copy overhead.The
RemoveInstantSendLockmethod only needs temporary read access to the lock data but currently takes ashared_ptrwhich involves copy overhead.As suggested in past reviews, consider changing the signature to use a raw pointer since the method only needs temporary access and doesn't maintain ownership:
void RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, const InstantSendLock* islock = nullptr, bool keep_cache = true)This would require callers to use
.get()on their shared_ptr but would eliminate the reference counting overhead for read-only access.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
src/instantsend/db.cpp(1 hunks)src/instantsend/db.h(1 hunks)src/instantsend/instantsend.cpp(20 hunks)src/instantsend/instantsend.h(1 hunks)src/instantsend/lock.cpp(1 hunks)src/instantsend/lock.h(1 hunks)src/instantsend/signing.cpp(1 hunks)src/instantsend/signing.h(1 hunks)src/interfaces/chain.h(2 hunks)src/net_processing.cpp(2 hunks)src/node/interfaces.cpp(2 hunks)src/rpc/mining.cpp(0 hunks)src/rpc/rawtransaction.cpp(2 hunks)src/test/cachemap_tests.cpp(1 hunks)src/test/cachemultimap_tests.cpp(1 hunks)src/test/evo_islock_tests.cpp(3 hunks)src/test/fuzz/process_message.cpp(0 hunks)src/test/governance_validators_tests.cpp(1 hunks)src/test/miner_tests.cpp(0 hunks)src/test/ratecheck_tests.cpp(1 hunks)src/validationinterface.cpp(1 hunks)src/validationinterface.h(3 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)
💤 Files with no reviewable changes (3)
- src/test/miner_tests.cpp
- src/rpc/mining.cpp
- src/test/fuzz/process_message.cpp
✅ Files skipped from review due to trivial changes (3)
- src/test/cachemap_tests.cpp
- src/test/cachemultimap_tests.cpp
- src/zmq/zmqpublishnotifier.cpp
🚧 Files skipped from review as they are similar to previous changes (20)
- src/test/governance_validators_tests.cpp
- src/test/ratecheck_tests.cpp
- src/zmq/zmqabstractnotifier.cpp
- src/test/evo_islock_tests.cpp
- src/rpc/rawtransaction.cpp
- src/zmq/zmqnotificationinterface.cpp
- src/validationinterface.cpp
- src/wallet/wallet.h
- src/zmq/zmqnotificationinterface.h
- src/instantsend/lock.cpp
- src/interfaces/chain.h
- src/instantsend/lock.h
- src/zmq/zmqpublishnotifier.h
- src/validationinterface.h
- src/zmq/zmqabstractnotifier.h
- src/node/interfaces.cpp
- src/wallet/wallet.cpp
- src/instantsend/signing.cpp
- src/instantsend/signing.h
- src/net_processing.cpp
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (5)
📓 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#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-10T16:26:00.371Z
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-10T16:26:00.371Z
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/instantsend/db.h (4)
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.
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.
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.
src/instantsend/instantsend.h (5)
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.
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: knst
PR: dashpay/dash#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/instantsend/db.cpp (4)
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.
Learnt from: kwvg
PR: dashpay/dash#6629
File: src/evo/netinfo.h:248-256
Timestamp: 2025-05-10T00:54:30.887Z
Learning: The Dash codebase includes a specialization for serializing std::shared_ptr objects in src/serialize.h, which allows direct serialization of shared_ptr instances with stream operators.
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#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/instantsend/instantsend.cpp (8)
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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: 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.
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#6532
File: src/net.cpp:4329-4329
Timestamp: 2025-01-14T09:07:12.446Z
Learning: Keep suggestions focused on the scope of the current commit/PR. Avoid suggesting unrelated improvements that should be handled in separate PRs, even if technically valid.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: dashpay/dash#6629
File: src/test/evo_netinfo_tests.cpp:39-45
Timestamp: 2025-05-10T00:55:16.184Z
Learning: In Dash's validation testing for network entries, checking `IsTriviallyValid()` is specifically used to ensure all three validation methods (`AddEntry()`, `Validate()`, and `IsTriviallyValid()`) consistently work together rather than replacing one check with another. `AddEntry()` and `Validate()` handle the consensus rule checks, while `IsTriviallyValid()` confirms the basic validation steps.
🪛 GitHub Actions: Clang Diff Format Check
src/instantsend/db.h
[error] 33-33: Clang format differences found. Code style does not match clang-format rules. Please run clang-format to fix formatting issues.
src/instantsend/instantsend.h
[error] 63-63: Clang format differences found. Code style does not match clang-format rules. Please run clang-format to fix formatting issues.
src/instantsend/db.cpp
[error] 19-19: Clang format differences found. Code style does not match clang-format rules. Please run clang-format to fix formatting issues.
src/instantsend/instantsend.cpp
[error] 27-27: 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). (6)
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: x86_64-pc-linux-gnu / Build depends
🔇 Additional comments (18)
src/instantsend/db.cpp (1)
19-19: Fix clang-format violation.The line has a clang-format violation that needs to be addressed.
Please run clang-format on this file to fix the formatting issue.
src/instantsend/instantsend.cpp (17)
5-5: LGTM: Include path reflects new structureThe include path change from
llmq/instantsend.htoinstantsend/instantsend.hcorrectly reflects the new directory structure after the refactoring.
16-23: LGTM: Includes properly organized for new architectureThe includes are properly organized to support the new architecture, with
instantsend/signing.hfor the new signing component and the necessary LLMQ headers maintained for continued integration.
41-53: LGTM: Modern C++20 concepts implementationThe template function correctly uses C++20 concepts with
requiresclause, which provides better error messages and aligns with modern C++ best practices. The implementation is clean and efficient.
55-72: LGTM: Constructor properly integrates new signer architectureThe constructor correctly integrates the new
InstantSendSignercomponent, conditionally creating it only for masternodes. The parameter forwarding and initialization are appropriate for the new architecture.
85-88: LGTM: Signer lifecycle managementThe Start method properly manages the signer lifecycle by calling
m_signer->Start()when the signer is available.
92-94: LGTM: Consistent signer lifecycle managementThe Stop method properly manages the signer lifecycle by calling
m_signer->Stop()before stopping the work thread.
110-110: LGTM: Type update to new InstantSendLockThe type change from
llmq::CInstantSendLocktoinstantsend::InstantSendLockis consistent with the namespace refactoring.
355-357: LGTM: Signer integration for queue managementThe code correctly delegates to the signer for clearing locks from the queue when the signer is available, maintaining proper separation of concerns.
427-427: LGTM: Masternode status checkThe masternode status check using
m_signer != nullptris appropriate and consistent with the new architecture.
456-458: LGTM: Transaction processing delegationThe code correctly delegates transaction processing to the signer when available, maintaining the proper separation between manager and signing responsibilities.
497-499: LGTM: Retroactive transaction processingThe code correctly delegates retroactive transaction processing to the signer for block-connected transactions, maintaining consistency with the new architecture.
602-605: LGTM: Conflict resolution with signer integrationThe method correctly delegates input clearing to the signer when available, ensuring proper queue management during conflict resolution.
607-616: LGTM: Signature truncation with signer integrationThe method correctly integrates signer queue clearing with signature truncation, maintaining consistency in the cleanup process.
618-625: LGTM: Pending lock managementThe method correctly manages pending locks with appropriate duplicate checking and locking mechanisms.
721-721: LGTM: Masternode status check consistencyThe masternode status check using
m_signer != nullptris consistent with the pattern used throughout the refactored code.
925-947: LGTM: Modern lambda-based work loopThe refactored work loop using an immediately invoked lambda expression (IIFE) is more modern and readable than the previous
do...while(0)pattern. The performance characteristics remain identical while improving code clarity.
1-1: Please run clang-format locally to fix formatting in src/instantsend/instantsend.cppI wasn’t able to verify formatting changes in the CI environment (no clang-format installed). Please confirm and apply clang-format, for example:
clang-format -i src/instantsend/instantsend.cppThen re-run the pipeline to ensure no formatting diffs remain.
|
This pull request has conflicts, please rebase. |
…p,h}` Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Also, since CInstantSendDb is used only by CInstantSendManager, we can move it to the instantsend namespace easier. Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
src/instantsend/db.h (1)
33-33: Fix clang-format issue.The pipeline indicates a formatting issue on this line. Please run clang-format to fix the formatting.
src/instantsend/instantsend.h (1)
9-9: Fix clang-format issue.The pipeline indicates a formatting issue on this line. Please run clang-format to fix the formatting.
src/instantsend/signing.cpp (2)
31-31: Use constexpr for compile-time constant.This constant should be declared as
constexprinstead ofconstsince it's a compile-time constant.-static const std::string_view INPUTLOCK_REQUESTID_PREFIX = "inlock"; +static constexpr std::string_view INPUTLOCK_REQUESTID_PREFIX = "inlock";
48-48: Fix clang-format issue.The pipeline indicates a formatting issue on this line. Please run clang-format to fix the formatting.
src/instantsend/signing.h (4)
50-50: Fix clang-format issue.The pipeline indicates a formatting issue on this line. Please run clang-format to fix the formatting.
42-42: Fix the typo in the mutex name.The mutex name has a typo:
cs_inputReqestsshould becs_inputRequests.- mutable Mutex cs_inputReqests; + mutable Mutex cs_inputRequests;
49-49: Update GUARDED_BY annotation to use correct mutex name.This should use the corrected mutex name
cs_inputRequests.- std::unordered_set<uint256, StaticSaltedHasher> inputRequestIds GUARDED_BY(cs_inputReqests); + std::unordered_set<uint256, StaticSaltedHasher> inputRequestIds GUARDED_BY(cs_inputRequests);
77-77: Update EXCLUSIVE_LOCKS_REQUIRED annotation to use correct mutex name.This should use the corrected mutex name
cs_inputRequests.- EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests); + EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputRequests);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
src/Makefile.am(2 hunks)src/bench/rpc_blockchain.cpp(1 hunks)src/coinjoin/coinjoin.cpp(1 hunks)src/dsnotificationinterface.cpp(1 hunks)src/evo/chainhelper.cpp(1 hunks)src/init.cpp(1 hunks)src/instantsend/db.cpp(1 hunks)src/instantsend/db.h(1 hunks)src/instantsend/instantsend.cpp(20 hunks)src/instantsend/instantsend.h(1 hunks)src/instantsend/lock.cpp(1 hunks)src/instantsend/lock.h(1 hunks)src/instantsend/signing.cpp(1 hunks)src/instantsend/signing.h(1 hunks)src/interfaces/chain.h(2 hunks)src/llmq/chainlocks.cpp(1 hunks)src/llmq/context.cpp(1 hunks)src/llmq/context.h(1 hunks)src/llmq/instantsend.h(0 hunks)src/net_processing.cpp(2 hunks)src/node/interfaces.cpp(2 hunks)src/node/miner.cpp(1 hunks)src/rest.cpp(1 hunks)src/rpc/blockchain.cpp(1 hunks)src/rpc/mempool.cpp(1 hunks)src/rpc/mining.cpp(0 hunks)src/rpc/rawtransaction.cpp(2 hunks)src/test/cachemap_tests.cpp(1 hunks)src/test/cachemultimap_tests.cpp(1 hunks)src/test/evo_deterministicmns_tests.cpp(1 hunks)src/test/evo_islock_tests.cpp(3 hunks)src/test/fuzz/process_message.cpp(0 hunks)src/test/governance_validators_tests.cpp(1 hunks)src/test/miner_tests.cpp(0 hunks)src/test/ratecheck_tests.cpp(1 hunks)src/txmempool.cpp(1 hunks)src/validationinterface.cpp(1 hunks)src/validationinterface.h(3 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/rpc/mining.cpp
- src/test/miner_tests.cpp
- src/llmq/instantsend.h
✅ Files skipped from review due to trivial changes (10)
- src/test/cachemultimap_tests.cpp
- src/bench/rpc_blockchain.cpp
- src/dsnotificationinterface.cpp
- src/rpc/blockchain.cpp
- src/test/cachemap_tests.cpp
- src/test/ratecheck_tests.cpp
- src/llmq/context.h
- src/test/evo_deterministicmns_tests.cpp
- src/rpc/mempool.cpp
- src/node/miner.cpp
🚧 Files skipped from review as they are similar to previous changes (29)
- src/test/governance_validators_tests.cpp
- src/evo/chainhelper.cpp
- src/rest.cpp
- src/llmq/context.cpp
- test/util/data/non-backported.txt
- src/coinjoin/coinjoin.cpp
- src/llmq/chainlocks.cpp
- src/txmempool.cpp
- src/zmq/zmqnotificationinterface.h
- src/init.cpp
- src/rpc/rawtransaction.cpp
- src/zmq/zmqabstractnotifier.cpp
- src/wallet/wallet.h
- src/test/evo_islock_tests.cpp
- src/interfaces/chain.h
- src/instantsend/lock.cpp
- src/node/interfaces.cpp
- src/net_processing.cpp
- src/zmq/zmqnotificationinterface.cpp
- src/validationinterface.cpp
- src/wallet/wallet.cpp
- src/instantsend/lock.h
- src/Makefile.am
- src/zmq/zmqpublishnotifier.h
- test/lint/lint-circular-dependencies.py
- src/zmq/zmqabstractnotifier.h
- src/zmq/zmqpublishnotifier.cpp
- src/validationinterface.h
- src/instantsend/db.cpp
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (6)
📓 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#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-10T16:26:00.371Z
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-10T16:26:00.371Z
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/instantsend/signing.cpp (4)
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.
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.
Learnt from: knst
PR: dashpay/dash#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/instantsend/db.h (4)
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.
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.
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.
src/instantsend/signing.h (4)
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.
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.
Learnt from: knst
PR: dashpay/dash#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/instantsend/instantsend.h (4)
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.
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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/instantsend.cpp (8)
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.
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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: 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#6532
File: src/net.cpp:4329-4329
Timestamp: 2025-01-14T09:07:12.446Z
Learning: Keep suggestions focused on the scope of the current commit/PR. Avoid suggesting unrelated improvements that should be handled in separate PRs, even if technically valid.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
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: dashpay/dash#6629
File: src/test/evo_netinfo_tests.cpp:39-45
Timestamp: 2025-05-10T00:55:16.184Z
Learning: In Dash's validation testing for network entries, checking `IsTriviallyValid()` is specifically used to ensure all three validation methods (`AddEntry()`, `Validate()`, and `IsTriviallyValid()`) consistently work together rather than replacing one check with another. `AddEntry()` and `Validate()` handle the consensus rule checks, while `IsTriviallyValid()` confirms the basic validation steps.
🧬 Code Graph Analysis (3)
src/instantsend/db.h (1)
src/instantsend/db.cpp (6)
CInstantSendDb(28-32)CInstantSendDb(34-34)GetInstantSendLockByHashInternal(270-293)GetInstantSendLockByHashInternal(270-270)GetInstantSendLockHashByTxidInternal(295-306)GetInstantSendLockHashByTxidInternal(295-295)
src/instantsend/signing.h (6)
src/validation.h (1)
chainstate(371-377)src/instantsend/signing.cpp (2)
InstantSendSigner(33-47)InstantSendSigner(49-49)src/instantsend/instantsend.h (9)
islock(117-117)tx(111-111)tx(115-115)tx(133-133)tx(139-139)txHash(131-131)txHash(132-132)txid(113-113)txid(147-147)src/validationinterface.h (4)
tx(152-152)tx(152-152)tx(166-166)tx(166-166)src/instantsend/db.h (4)
outpoint(149-149)txid(77-77)txid(133-133)txid(143-143)src/node/interfaces.cpp (6)
txid(892-897)txid(892-892)txid(898-904)txid(898-898)txid(913-918)txid(913-913)
src/instantsend/instantsend.cpp (5)
src/net_processing.cpp (11)
ret(657-657)ret(658-658)hash(986-986)hash(992-992)from(1018-1018)tx(1043-1043)id(676-676)id(680-680)txid(637-637)txid(652-652)txid(654-654)src/instantsend/instantsend.h (19)
CInstantSendManager(89-92)CInstantSendManager(93-93)islock(117-117)hash(145-145)hash(155-155)from(108-108)peerman(95-95)peerman(101-101)peerman(119-119)peerman(124-124)peerman(137-137)tx(111-111)tx(115-115)tx(133-133)tx(139-139)islockHash(121-121)islockHash(154-154)txid(113-113)txid(147-147)src/instantsend/signing.cpp (2)
Stop(56-59)Stop(56-56)src/llmq/chainlocks.cpp (2)
Stop(78-82)Stop(78-78)src/instantsend/db.cpp (2)
GetInstantSendLockByTxid(308-312)GetInstantSendLockByTxid(308-308)
🪛 GitHub Actions: Clang Diff Format Check
src/instantsend/signing.cpp
[error] 48-48: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix formatting issues.
src/instantsend/db.h
[error] 33-33: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix formatting issues.
src/instantsend/signing.h
[error] 50-50: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix formatting issues.
src/instantsend/instantsend.h
[error] 9-9: Clang format differences found. Code formatting does not match the expected style. Please run clang-format to fix formatting issues.
src/instantsend/instantsend.cpp
[error] 27-27: 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). (9)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: Lint / Run linters
- GitHub Check: x86_64-w64-mingw32 / Build depends
🔇 Additional comments (26)
src/instantsend/instantsend.cpp (26)
5-5: LGTM: Include path correctly updated for refactoring.The include path change from
llmq/instantsend.htoinstantsend/instantsend.hcorrectly reflects the new directory structure.
16-23: LGTM: New includes support the refactored architecture.The additional includes for
instantsend/signing.hand other dependencies are necessary to support the split of functionality into separate components.
40-53: LGTM: Excellent use of C++20 concepts.The refactoring to use
requires std::same_as<T, CTxIn> || std::same_as<T, COutPoint>instead ofstatic_assertis a modern C++20 improvement that provides better error messages and clearer type constraints.
55-72: LGTM: Constructor properly delegates to new components.The conditional creation of
InstantSendSigneronly for masternodes is correct, and the use ofstd::make_uniquewith proper parameter forwarding demonstrates good modern C++ practices.
85-88: LGTM: Start method properly delegates to signer.The conditional starting of the signer component when it exists is correct and follows the delegation pattern consistently.
92-95: LGTM: Stop method properly delegates to signer.The conditional stopping of the signer component mirrors the Start method and ensures proper cleanup of the signing subsystem.
110-110: LGTM: Correctly uses new InstantSendLock type.The change from
llmq::CInstantSendLocktoinstantsend::InstantSendLockis consistent with the namespace refactoring.
122-122: LGTM: Parameter type correctly updated.The parameter type change to
instantsend::InstantSendLockPtris consistent with the refactoring and maintains type safety.
242-242: LGTM: Method signature correctly updated.The parameter type change to use
instantsend::InstantSendLockPtrin the method signature is consistent with the refactoring.
351-357: LGTM: Method properly delegates to signer component.The signature update to use
instantsend::InstantSendLockPtrand the conditional delegation tom_signer->ClearLockFromQueue()correctly implements the separation of concerns.
427-427: LGTM: Correctly uses signer existence check.The
/*is_masternode=*/m_signer != nullptrparameter correctly determines masternode status based on signer existence.
437-437: LGTM: Type correctly updated.The type change to
instantsend::InstantSendLockPtris consistent with the refactoring.
456-459: LGTM: Method properly delegates to signer.The conditional delegation to
m_signer->ProcessTx()correctly implements the separation of transaction processing logic.
472-472: LGTM: Type correctly updated.The type change to
instantsend::InstantSendLockPtris consistent with the refactoring.
497-499: LGTM: Method properly delegates to signer.The conditional delegation to
m_signer->ProcessTx()correctly implements the separation of transaction processing logic for block-connected transactions.
602-616: LGTM: Methods properly delegate to signer.The conditional delegation to
m_signer->ClearInputsFromQueue()in both methods correctly implements the separation of signing-related logic. The signature update to useinstantsend::InstantSendLockis consistent with the refactoring.
925-947: LGTM: Excellent refactoring to use lambda pattern.The immediately invoked lambda expression (IIFE) is a modern and cleaner approach compared to the previous
do...while(0)pattern. The conditional delegation tom_signer->ProcessPendingRetryLockTxs()correctly implements the separation of retry logic.
618-625: LGTM: Method signature correctly updated.The signature update to use
instantsend::InstantSendLockPtris consistent with the refactoring.
692-692: LGTM: Parameter type correctly updated.The parameter type change to
instantsend::InstantSendLockis consistent with the refactoring.
721-721: LGTM: Correctly uses signer existence check.The
/*is_masternode=*/m_signer != nullptrparameter correctly determines masternode status based on signer existence.
725-725: LGTM: Parameter type correctly updated.The parameter type change to
instantsend::InstantSendLockis consistent with the refactoring.
813-813: LGTM: Parameter type correctly updated.The parameter type change to
instantsend::InstantSendLockis consistent with the refactoring.
836-836: LGTM: Return type correctly updated.The return type change to
instantsend::InstantSendLockis consistent with the refactoring.
861-861: LGTM: Return type correctly updated.The return type change to
instantsend::InstantSendLockPtris consistent with the refactoring.
898-898: LGTM: Return type correctly updated.The return type change to
instantsend::InstantSendLockPtris consistent with the refactoring.
27-27: Fix clang-format issue.The pipeline failure indicates formatting differences on this line. Please run
clang-formatto fix the formatting issue.
| void ProcessPendingRetryLockTxs(const std::vector<CTransactionRef>& retryTxs) | ||
| EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests); | ||
| void ProcessTx(const CTransaction& tx, bool fRetroactive, const Consensus::Params& params) | ||
| EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests); |
There was a problem hiding this comment.
Update EXCLUSIVE_LOCKS_REQUIRED annotation to use correct mutex name.
This should use the corrected mutex name cs_inputRequests.
- EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests);
+ EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputRequests);📝 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.
| EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests); | |
| EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputRequests); |
🤖 Prompt for AI Agents
In src/instantsend/signing.h at line 82, the EXCLUSIVE_LOCKS_REQUIRED annotation
uses the incorrect mutex name 'cs_inputReqests'. Update this annotation to use
the correct mutex name 'cs_inputRequests' to fix the typo.
| void Stop(); | ||
|
|
||
| void ClearInputsFromQueue(const std::unordered_set<uint256, StaticSaltedHasher>& ids) | ||
| EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests); |
There was a problem hiding this comment.
Update EXCLUSIVE_LOCKS_REQUIRED annotation to use correct mutex name.
This should use the corrected mutex name cs_inputRequests.
- EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests);
+ EXCLUSIVE_LOCKS_REQUIRED(!cs_inputRequests);📝 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.
| EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests); | |
| EXCLUSIVE_LOCKS_REQUIRED(!cs_inputRequests); |
🤖 Prompt for AI Agents
In src/instantsend/signing.h at line 71, the EXCLUSIVE_LOCKS_REQUIRED annotation
uses an incorrect mutex name `cs_inputReqests`. Update this annotation to use
the correct mutex name `cs_inputRequests` to ensure proper lock requirement
specification.
|
|
||
| [[nodiscard]] bool TrySignInputLocks(const CTransaction& tx, bool allowResigning, Consensus::LLMQType llmqType, | ||
| const Consensus::Params& params) | ||
| EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests); |
There was a problem hiding this comment.
Update EXCLUSIVE_LOCKS_REQUIRED annotation to use correct mutex name.
This should use the corrected mutex name cs_inputRequests.
- EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests);
+ EXCLUSIVE_LOCKS_REQUIRED(!cs_inputRequests);📝 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.
| EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests); | |
| EXCLUSIVE_LOCKS_REQUIRED(!cs_inputRequests); |
🤖 Prompt for AI Agents
In src/instantsend/signing.h at line 98, the EXCLUSIVE_LOCKS_REQUIRED annotation
uses an incorrect mutex name 'cs_inputReqests'. Update this annotation to use
the correct mutex name 'cs_inputRequests' to ensure proper lock requirement
specification.
| EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests); | ||
|
|
||
| void ProcessPendingRetryLockTxs(const std::vector<CTransactionRef>& retryTxs) | ||
| EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests); |
There was a problem hiding this comment.
Update EXCLUSIVE_LOCKS_REQUIRED annotation to use correct mutex name.
This should use the corrected mutex name cs_inputRequests.
- EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests);
+ EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputRequests);📝 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.
| EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests); | |
| EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputRequests); |
🤖 Prompt for AI Agents
In src/instantsend/signing.h at line 80, the EXCLUSIVE_LOCKS_REQUIRED annotation
uses the incorrect mutex name cs_inputReqests. Update the annotation to use the
correct mutex name cs_inputRequests to fix the typo.
…ons, avoid redundant transaction queries f488c43 fix: remove unnecessary tx fetching in `RemoveMempoolConflictsForLock` (Kittywhiskers Van Gogh) dd54011 refactor: move inexpensive checks earlier in `ProcessInstantSendLock()` (Kittywhiskers Van Gogh) c86d886 refactor: abstract away InstantSend parent implementation from signer (Kittywhiskers Van Gogh) cbfd325 refactor: avoid shared_ptr copies in `{WriteNew,Remove}InstantSendLock()` (Kittywhiskers Van Gogh) 234a16a refactor: use unordered set for faster duplicates checking (Kittywhiskers Van Gogh) 0459848 refactor: `constexpr` static string definitions (Kittywhiskers Van Gogh) 710c504 refactor: s/cs_inputReqests/cs_input_requests/g (Kittywhiskers Van Gogh) 31065d1 chore: mark `hashBlock` as unused in `HandleNewInputLockRecoveredSig()` (Kittywhiskers Van Gogh) cf47f14 chore: apply most `clang-format` suggestions (Kittywhiskers Van Gogh) 63371e0 trivial: move forward declaration above `using` declarations (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #6742 * This pull request aims to address reviewer concerns raised during [dash#6742](#6742), the following changes were made with that in mind * Marking unused variable `hashBlock` by prefixing with underscore, [comment](#6742 (comment)) * Correct typo in variable name `cs_inputReqests` (now `cs_input_requests`, [comment](#6770 (comment))), [comment](#6742 (comment)) * Using `constexpr` for `std::string_view`s, [comment](#6742 (comment)) * Utilizing a faster way to validate uniqueness of entries (`std::unordered_set`), [comment](#6742 (comment)). The construction of the unordered set is inspired by similar usage in `policy/package.cpp` ([source](https://github.com/dashpay/dash/blob/f648039258d2192ec5e505721e34edaa0bda568c/src/policy/packages.cpp#L51)). * Avoiding unnecessary `std::shared_ptr` copying (`InstantSendLockPtr` is a `std::shared_ptr<InstantSendLock>`), [comment](#6742 (comment)) * Resolving circular dependency by cherry-picking 1e2e854, [comment](#6742 (comment)) but has been renamed to `InstantSendSignerParent`. For rationale, see [comment](#6761 (comment)). * Additionally, optimizations have been made to `ProcessInstantSendLock()` * The _cheaper_ database checks have been moved _earlier_ in the function to allow for faster bail-out. This is alongside consolidating network requests to the tail of the function and general cleanup for better clarity. * Dropping the peer request for a transaction we know about in `RemoveMempoolConflictsForLock()` (introduced in [dash#2898](#2898)) as `ProcessInstantSendLock()` calls it when we _know_ about the transaction and seek to remove conflicts ([source](https://github.com/dashpay/dash/blob/f648039258d2192ec5e505721e34edaa0bda568c/src/instantsend/instantsend.cpp#L419-L420)) The only other caller, `TransactionAddedToMempool()` calls it when we have knowledge of the transaction and want to purge conflicts from the mempool ([source](https://github.com/dashpay/dash/blob/f648039258d2192ec5e505721e34edaa0bda568c/src/instantsend/instantsend.cpp#L455-L463)). In both cases, we already know the _good_ transaction. Additionally, tests introduced in [dash#2898](#2898) still pass ([source](https://github.com/dashpay/dash/blob/f648039258d2192ec5e505721e34edaa0bda568c/test/functional/p2p_instantsend.py#L95-L142)), indicating a lack of regression but this change may require additional scrutiny. ## 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 **(note: N/A)** - [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: PastaPastaPasta: utACK f488c43 knst: utACK f488c43 UdjinM6: utACK f488c43 Tree-SHA512: dcd153e14747639f8dbbbd5c3bea1123e4f84f82884787992e20f494227913763bf19a73b485e345dddb63ad551c856bc56dcad72e67304f84df60cb76deaa74
…split masternode logic out of manager source fd8af9c refactor: abstract away ChainLocks parent implementation from signer (Kittywhiskers Van Gogh) 733f7d3 refactor: remove `Internal*()` function definitions, consolidate (Kittywhiskers Van Gogh) e742b7b refactor: remove need for mutual access to private members (Kittywhiskers Van Gogh) 74c1b28 refactor: separate masternode mode logic into dedicated manager class (Kittywhiskers Van Gogh) f469c46 refactor: inline single-use `ChainLocksSigningEnabled()` definition (Kittywhiskers Van Gogh) 7e0a3f8 refactor: move masternode mode logic to `chainlock/signer.{cpp,h}` (Kittywhiskers Van Gogh) 5663eac refactor: split `Cleanup()`'s masternode and non-masternode concerns (Kittywhiskers Van Gogh) ef15fee refactor: cleanup `src/chainlock/clsig.*`, move to dedicated namespace (Kittywhiskers Van Gogh) 021062c refactor: `llmq/chainlocks.{cpp,h}` > `chainlock/chainlock.{cpp,h}` (Kittywhiskers Van Gogh) f10912b refactor: `llmq/clsig.{cpp,h}` > `chainlock/clsig.{cpp,h}` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information A continuation of [dash#6742](#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 `chainlock` namespace. Changes are mostly move-only except for decoupling, which should be behavior-identical. ## 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 **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [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 fd8af9c Tree-SHA512: b9de697d2c3d762824449972ce206ccfca9d878a9a8c1cf38efdb9223ee563dd7305a00ebd80ddb09bc8805133b8b5e45fbbc5e472d23b4bd710454c1048be22
Motivation
Currently, the
llmqnamespace includes the LLMQ subsystem itself and capabilities built on them, primarily InstantSend and ChainLocks. Both capabilities play a key part in block and validation logic (seeCChainHelperpassthrough functions, source). Currently, we have to setup all LLMQ infrastructure (i.e.LLMQContext) in order to validate on-disk information.This ceases to be feasible when extracting validation code that interacts with on-disk state into
libbitcoinkernelthroughbitcoin-chainstate(see bitcoin#24304) as this thinned down library is devoid of networking capabilities.As LLMQ is inherently an interactive system that is tied to networking capabilities, to extract on-disk state validation logic, we need to separate out networking from non-networking concerns. This pull request is the first step in extracting networking logic by first splitting out the monolithic InstantSend implementation into smaller concerns and then spinning off masternode-mode logic into a dedicated manager.
Future work should aim to spit out non-masternode networking code into a separate concern and eventually thin down
llmq::CInstantSendManagerto be capable of running with a networking-disabled mode, which would be adequate forlibbitcoinkernel.Breaking Changes
None expected.
Checklist