feat: implement asynchronous processing for coinbase chainlocks#6947
feat: implement asynchronous processing for coinbase chainlocks#6947PastaPastaPasta wants to merge 3 commits into
Conversation
|
| } | ||
| } | ||
|
|
||
| BOOST_AUTO_TEST_CASE(coinbase_chainlock_queueing_test) |
There was a problem hiding this comment.
kinda useless test; but I guess fine? Open to thoughts
WalkthroughAdds asynchronous queuing for coinbase chainlock signatures and periodic processing of that queue. Chainlocks gains Sequence DiagramsequenceDiagram
participant Caller
participant Chainlocks
participant Queue as pendingCoinbaseChainLocks
participant Handler as ProcessPendingCoinbaseChainLocks
Caller->>Chainlocks: QueueCoinbaseChainLock(clsig)
rect rgba(200,220,240,0.5)
note over Chainlocks: chainlocks enabled && clsig.height > best
Chainlocks->>Queue: enqueue(clsig)
end
Note over Chainlocks,Handler: periodic Start(qman) scheduler tick
Chainlocks->>Handler: ProcessPendingCoinbaseChainLocks(qman)
rect rgba(220,240,200,0.5)
note over Handler: Drain queue (swap under lock)
Handler->>Handler: deduplicate & reverse-order iterate
loop for each clsig (newest first)
alt eligible & not seen
Handler->>Chainlocks: ProcessNewChainLock(-1, clsig, hash)
else skip
Handler-->>Handler: ignore outdated/duplicate
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/chainlock/chainlock.cpp(3 hunks)src/chainlock/chainlock.h(3 hunks)src/evo/chainhelper.cpp(1 hunks)src/evo/chainhelper.h(2 hunks)src/evo/specialtxman.cpp(1 hunks)src/evo/specialtxman.h(1 hunks)src/test/llmq_chainlock_tests.cpp(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/evo/specialtxman.hsrc/chainlock/chainlock.cppsrc/evo/chainhelper.hsrc/evo/chainhelper.cppsrc/evo/specialtxman.cppsrc/test/llmq_chainlock_tests.cppsrc/chainlock/chainlock.h
src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/evo/specialtxman.hsrc/evo/chainhelper.hsrc/evo/chainhelper.cppsrc/evo/specialtxman.cpp
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Files:
src/test/llmq_chainlock_tests.cpp
🧠 Learnings (8)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.
📚 Learning: 2025-07-20T18:42:49.794Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Applied to files:
src/evo/specialtxman.hsrc/evo/specialtxman.cpp
📚 Learning: 2025-07-29T14:32:48.369Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Applied to files:
src/chainlock/chainlock.cppsrc/evo/specialtxman.cppsrc/chainlock/chainlock.h
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
Repo: dashpay/dash 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.
Applied to files:
src/evo/chainhelper.h
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: Across net_processing.cpp, once LLMQContext (m_llmq_ctx) is asserted non-null, its subcomponents (e.g., isman, qdkgsman, quorum_block_processor) are treated as initialized and used without extra null checks.
Applied to files:
src/evo/chainhelper.cpp
📚 Learning: 2024-12-29T17:43:41.755Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.
Applied to files:
src/evo/chainhelper.cpp
📚 Learning: 2025-07-20T18:42:49.794Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Applied to files:
src/test/llmq_chainlock_tests.cpp
📚 Learning: 2025-10-21T11:09:34.688Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6849
File: src/governance/governance.cpp:1339-1343
Timestamp: 2025-10-21T11:09:34.688Z
Learning: In the Dash Core codebase, `CacheMap` (defined in src/cachemap.h) is internally thread-safe and uses its own `mutable CCriticalSection cs` to protect access to its members. Methods like `GetSize()`, `Insert()`, `Get()`, `HasKey()`, etc., can be called without holding external locks.
Applied to files:
src/chainlock/chainlock.h
🧬 Code graph analysis (5)
src/evo/specialtxman.h (1)
src/chainlock/chainlock.cpp (2)
CChainLocksHandler(48-59)CChainLocksHandler(61-65)
src/chainlock/chainlock.cpp (1)
src/chainlock/chainlock.h (2)
cs(64-146)cs(65-143)
src/evo/chainhelper.h (1)
src/chainlock/chainlock.cpp (2)
CChainLocksHandler(48-59)CChainLocksHandler(61-65)
src/test/llmq_chainlock_tests.cpp (2)
src/node/interfaces.cpp (8)
height(563-573)height(563-563)height(878-885)height(878-878)height(886-892)height(886-886)height(937-941)height(937-937)src/test/util/llmq_tests.h (1)
GetTestBlockHash(109-109)
src/chainlock/chainlock.h (2)
src/chainlock/clsig.h (1)
chainlock(15-38)src/chainlock/chainlock.cpp (6)
QueueCoinbaseChainLock(497-518)QueueCoinbaseChainLock(497-497)Cleanup(442-495)Cleanup(442-442)ProcessPendingCoinbaseChainLocks(520-563)ProcessPendingCoinbaseChainLocks(520-520)
🪛 GitHub Actions: Clang Diff Format Check
src/chainlock/chainlock.cpp
[error] 1-1: Clang format differences found in src/chainlock/chainlock.cpp. Run the clang-format-diff.py script to apply formatting changes.
src/evo/specialtxman.cpp
[error] 1-1: Clang format differences found in src/evo/specialtxman.cpp. Run the clang-format-diff.py script to apply formatting changes.
src/test/llmq_chainlock_tests.cpp
[error] 1-1: Clang format differences found in src/test/llmq_chainlock_tests.cpp. Run the clang-format-diff.py script to apply formatting changes.
src/chainlock/chainlock.h
[error] 1-1: Clang format differences found in src/chainlock/chainlock.h. Run the clang-format-diff.py script to apply formatting changes.
⏰ 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). (8)
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (9)
src/evo/specialtxman.h (1)
48-48: LGTM: const-to-non-const change enables queueing.The change from
const llmq::CChainLocksHandler&tollmq::CChainLocksHandler&is necessary to support the newQueueCoinbaseChainLockmutating operation introduced in this PR. The member declaration, constructor parameter, and initialization are all consistent.Also applies to: 55-55, 63-63
src/chainlock/chainlock.h (3)
23-23: LGTM: Include added for new deque member.The
<deque>include is necessary for thependingCoinbaseChainLocksmember.
73-74: LGTM: Properly guarded queue member.The
pendingCoinbaseChainLocksdeque is correctly protected by the existingcsmutex, maintaining thread safety.
133-142: Fix clang-format differences and approve API design.The new public
QueueCoinbaseChainLockand privateProcessPendingCoinbaseChainLocksmethods have correct lock annotations (EXCLUSIVE_LOCKS_REQUIRED(!cs)). However, there's a clang-format pipeline failure that must be resolved.Run the following command to fix formatting:
#!/bin/bash # Fix clang-format issues git-clang-format-diff HEAD^ -- src/chainlock/chainlock.h⛔ Skipped due to learnings
Learnt from: kwvg Repo: dashpay/dash PR: 6761 File: src/chainlock/signing.cpp:247-250 Timestamp: 2025-07-29T14:32:48.369Z Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.Learnt from: kwvg Repo: dashpay/dash PR: 6543 File: src/wallet/receive.cpp:240-251 Timestamp: 2025-02-06T14:34:30.466Z Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.Learnt from: knst Repo: dashpay/dash PR: 6916 File: src/univalue/include/univalue.h:81-88 Timestamp: 2025-10-25T07:08:51.918Z Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.src/evo/chainhelper.cpp (1)
20-20: LGTM: Parameter change consistent with header.The constructor parameter change from
const llmq::CChainLocksHandler&tollmq::CChainLocksHandler&matches the header declaration and enables the new queueing functionality.src/evo/chainhelper.h (1)
36-36: LGTM: Consistent const-to-non-const changes.The member and constructor parameter changes from
const llmq::CChainLocksHandler&tollmq::CChainLocksHandler&are consistent and necessary to support the new queueing functionality.Also applies to: 47-47
src/chainlock/chainlock.cpp (3)
19-19: LGTM: Include added for processing vector.The
<vector>include is necessary for the localtoProcessvector inProcessPendingCoinbaseChainLocks.
76-76: LGTM: Periodic processing integrated into main loop.The call to
ProcessPendingCoinbaseChainLocks()is correctly placed in theStart()scheduler loop, ensuring queued chainlocks are processed asynchronously every 5 seconds.
497-518: LGTM: Thread-safe queueing with proper deduplication.The
QueueCoinbaseChainLockimplementation correctly:
- Asserts no lock is held, then acquires the lock
- Returns early if chainlocks are disabled
- Filters chainlocks that aren't newer than the current best
- Deduplicates via
seenChainLocksbefore queueing
| void CChainLocksHandler::ProcessPendingCoinbaseChainLocks() | ||
| { | ||
| AssertLockNotHeld(cs); | ||
| AssertLockNotHeld(cs_main); | ||
|
|
||
| if (!IsEnabled()) { | ||
| return; | ||
| } | ||
|
|
||
| std::vector<chainlock::ChainLockSig> toProcess; | ||
| { | ||
| LOCK(cs); | ||
| if (pendingCoinbaseChainLocks.empty()) { | ||
| return; | ||
| } | ||
|
|
||
| // Move all pending chainlocks to a local vector for processing | ||
| toProcess.reserve(pendingCoinbaseChainLocks.size()); | ||
| while (!pendingCoinbaseChainLocks.empty()) { | ||
| toProcess.push_back(pendingCoinbaseChainLocks.front()); | ||
| pendingCoinbaseChainLocks.pop_front(); | ||
| } | ||
| } | ||
|
|
||
| // Process each chainlock outside the lock | ||
| for (const auto& clsig : toProcess) { | ||
| const uint256 hash = ::SerializeHash(clsig); | ||
|
|
||
| // Check again if we still want to process this (might have been processed via network) | ||
| { | ||
| LOCK(cs); | ||
| if (seenChainLocks.count(hash) != 0) { | ||
| continue; | ||
| } | ||
| if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight()) { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Process as if it came from a coinbase (from = -1 means internal) | ||
| // Ignore return value as we're processing internally from coinbase | ||
| (void)ProcessNewChainLock(-1, clsig, hash); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix clang-format differences; excellent lock minimization pattern.
The ProcessPendingCoinbaseChainLocks implementation demonstrates good concurrency practices:
- Drains the queue into a local vector while briefly holding the lock (lines 531-542)
- Processes each chainlock outside the lock to minimize contention (lines 544-562)
- Double-checks each chainlock's status before processing (lines 550-557) to handle race conditions where a chainlock may have been received via the network while queued
- Uses
from=-1to indicate internal/coinbase source (line 561)
However, there's a clang-format pipeline failure that must be resolved.
Run the following command to fix formatting:
#!/bin/bash
# Fix clang-format issues
git-clang-format-diff HEAD^ -- src/chainlock/chainlock.cpp🤖 Prompt for AI Agents
src/chainlock/chainlock.cpp around lines 520-563 has clang-format differences
causing CI to fail; run the provided formatting command (git-clang-format-diff
HEAD^ -- src/chainlock/chainlock.cpp) to auto-apply clang-format fixes, verify
the diff only contains whitespace/formatting changes (no logic edits), then
stage and commit the resulting changes and push the commit to resolve the
pipeline failure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/llmq_chainlock_tests.cpp (1)
173-204: Consider adding assertions to make the test more meaningful.The test currently only verifies that queueing doesn't throw exceptions. While this is valid as a smoke test, consider adding assertions to verify observable behavior.
For example, you could:
- Verify that calling
QueueCoinbaseChainLockwith the same chainlock twice is idempotent (no crash/exception)- Test queueing behavior when chainlocks are disabled via spork
- Verify queueing older chainlocks is rejected (by checking a newer one doesn't queue an older one)
However, given that comprehensive processing logic is tested in functional tests (as noted in the comment), this minimal smoke test is acceptable.
src/chainlock/chainlock.cpp (1)
497-517: LGTM! Queueing logic is efficient and thread-safe.The implementation correctly:
- Holds
csthroughout to ensure thread safety (line 499)- Returns early if chainlocks are disabled (lines 501-503)
- Skips queueing if the chainlock is not newer than the current best (lines 505-508)
- Avoids duplicate work by checking
seenChainLocks(lines 510-514)- Enqueues for asynchronous processing (line 516)
The filtering logic minimizes unnecessary queueing and processing.
Optional: Consider adding a queue size limit.
While unlikely in practice (blocks arrive infrequently and processing is fast), if there's a scenario where processing is delayed and many blocks arrive quickly, the queue could grow unbounded. Consider adding a maximum queue size check:
if (pendingCoinbaseChainLocks.size() >= MAX_PENDING_CHAINLOCKS) { return; // or pop oldest }However, given that:
- Blocks arrive at most every ~2.5 minutes
- Processing runs every 5 seconds
- Each chainlock is small (~200 bytes)
This is a nice-to-have rather than essential.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/chainlock/chainlock.cpp(3 hunks)src/chainlock/chainlock.h(3 hunks)src/evo/specialtxman.cpp(1 hunks)src/test/llmq_chainlock_tests.cpp(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/chainlock/chainlock.cppsrc/test/llmq_chainlock_tests.cppsrc/evo/specialtxman.cppsrc/chainlock/chainlock.h
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Files:
src/test/llmq_chainlock_tests.cpp
src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/evo/specialtxman.cpp
🧠 Learnings (6)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.
📚 Learning: 2025-07-29T14:32:48.369Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Applied to files:
src/chainlock/chainlock.cppsrc/evo/specialtxman.cpp
📚 Learning: 2025-08-19T14:57:31.801Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.
Applied to files:
src/chainlock/chainlock.cpp
📚 Learning: 2025-02-06T14:34:30.466Z
Learnt from: kwvg
Repo: dashpay/dash 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.
Applied to files:
src/chainlock/chainlock.cpp
📚 Learning: 2025-07-20T18:42:49.794Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Applied to files:
src/evo/specialtxman.cpp
📚 Learning: 2025-10-21T11:09:34.688Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6849
File: src/governance/governance.cpp:1339-1343
Timestamp: 2025-10-21T11:09:34.688Z
Learning: In the Dash Core codebase, `CacheMap` (defined in src/cachemap.h) is internally thread-safe and uses its own `mutable CCriticalSection cs` to protect access to its members. Methods like `GetSize()`, `Insert()`, `Get()`, `HasKey()`, etc., can be called without holding external locks.
Applied to files:
src/chainlock/chainlock.h
🧬 Code graph analysis (3)
src/chainlock/chainlock.cpp (1)
src/chainlock/chainlock.h (2)
cs(64-144)cs(65-141)
src/test/llmq_chainlock_tests.cpp (2)
src/node/interfaces.cpp (8)
height(563-573)height(563-563)height(878-885)height(878-878)height(886-892)height(886-886)height(937-941)height(937-937)src/test/util/llmq_tests.h (1)
GetTestBlockHash(109-109)
src/chainlock/chainlock.h (2)
src/chainlock/clsig.h (1)
chainlock(15-38)src/chainlock/chainlock.cpp (6)
QueueCoinbaseChainLock(497-517)QueueCoinbaseChainLock(497-497)Cleanup(442-495)Cleanup(442-442)ProcessPendingCoinbaseChainLocks(519-562)ProcessPendingCoinbaseChainLocks(519-519)
⏰ 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). (10)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (8)
src/test/llmq_chainlock_tests.cpp (1)
11-14: LGTM! Includes support the new queueing test.The added includes are appropriate for the new test case that exercises
CChainLocksHandler::QueueCoinbaseChainLock.src/chainlock/chainlock.h (3)
23-23: LGTM! Include required for the new queue.The
<deque>include is necessary for thependingCoinbaseChainLocksmember added at line 74.
73-74: LGTM! Queue member is properly guarded and well-documented.The
std::dequeis an appropriate choice for a FIFO queue, and theGUARDED_BY(cs)annotation ensures thread safety. The comment clearly describes the purpose.
133-140: LGTM! Method declarations are well-designed.The API design is clean:
QueueCoinbaseChainLockprovides a public interface for enqueueingProcessPendingCoinbaseChainLocksis private for internal processing- Both have correct
EXCLUSIVE_LOCKS_REQUIRED(!cs)annotations indicating they acquire the lock internally- Comments clearly explain the asynchronous processing model
src/evo/specialtxman.cpp (1)
648-658: LGTM! Queueing logic is correct and properly guarded.The implementation correctly:
- Validates the signature before queueing (line 649)
- Skips queueing in dry-run mode via
!fJustCheck(line 649)- Computes the chainlock height consistently with the validation logic at line 72 (line 650)
- Handles the case where
GetAncestorreturnsnullptr(lines 651-652)- Queues the chainlock for asynchronous processing (line 656)
The height calculation and null-check mirror the validation logic in
CheckCbTxBestChainlock, ensuring consistency between validation and processing.src/chainlock/chainlock.cpp (3)
19-19: LGTM! Include required for processing logic.The
<vector>include is necessary for the localtoProcessvector inProcessPendingCoinbaseChainLocks(line 528).
76-76: LGTM! Proper integration into the processing loop.The call to
ProcessPendingCoinbaseChainLocks()is correctly placed in the scheduler loop, ensuring pending coinbase chainlocks are processed asynchronously every 5 seconds without blocking main validation threads.
519-562: LGTM! Excellent concurrency pattern with minimal lock contention.The implementation demonstrates best practices for asynchronous processing:
- Asserts locks are not held to prevent deadlocks (lines 521-522)
- Returns early if disabled (lines 524-526)
- Drains the queue into a local vector while briefly holding the lock (lines 530-541)
- Processes each chainlock outside the lock to minimize contention (lines 543-561)
- Double-checks each chainlock's status before processing (lines 548-556) to handle race conditions where a chainlock may have been received via the network while queued
- Uses
from=-1to indicate internal/coinbase source (line 560)- Ignores the return value with
(void)cast since we're processing internally, not relaying (line 560)This pattern ensures the handler remains responsive while processing queued chainlocks.
knst
left a comment
knst
left a comment
There was a problem hiding this comment.
I believe this PR will use extra CPU resources without visible benefits.
This enhancement allows for improved handling of chainlocks during block validation without blocking the main processing flow.
Chainlocks inside CbTx are already validated; it's part of consensus validation during indexing.
|
|
||
| // Process as if it came from a coinbase (from = -1 means internal) | ||
| // Ignore return value as we're processing internally from coinbase | ||
| (void)ProcessNewChainLock(-1, clsig, hash); |
There was a problem hiding this comment.
2. call of ProcessnewChainLock means that for every call will be re-validated chainlock & signature -> it is waste because it's already validated with coinbase.
Nevermind, I see LIFO here
040ac27 ci: bump CTCACHE_COMMIT to e393144d5c49b060a1dbc7ae15b9c6973efb967d (UdjinM6) 7c310c7 refactor: derive ctcache Python script path from wrapper location (UdjinM6) 28577db refactor: improve ctcache configuration maintainability (UdjinM6) 7ef44b6 chore: adjust removal count message (UdjinM6) 64ff657 fix: improve file removal pipeline robustness (UdjinM6) 2944562 feat: verify clang and ctcache are installed correctly (UdjinM6) a1b98b1 chore: make paths more maintainable (UdjinM6) ead9136 feat: integrate ctcache for clang-tidy result caching in CI (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Add [ctcache](https://github.com/matus-chochlik/ctcache) to cache clang-tidy analysis results, reducing time spent in "Run linters" step from 33-34 minutes to 3-4 minutes in the best case scenarios. ## What was done? - Download ctcache in `contrib/containers/ci/ci.Dockerfile` and patch it to work correctly - Add new steps in `.github/workflows/build-src.yml` to save/restore ctcache cache (for `linux64_multiprocess`, save on pushes to default branch only) - Adjust `ci/dash/lint-tidy.sh` to configure ctcache, force `run-clang-tidy` to use it - Add manual cache cleanup in `ci/dash/lint-tidy.sh` since ctcache doesn't have a way to set internal limits like ccache does ## How Has This Been Tested? Run, check "Run linters" step in `linux64_multiprocess-build` job. develop, no ctcache: [31m 43s](https://github.com/UdjinM6/dash/actions/runs/20273923736/job/58217581581) develop*, empty cache: 0% hits, [33m 19s](https://github.com/UdjinM6/dash/actions/runs/20287332888/job/58265404189) develop*, second run, using cache: 100% hits, [3m 27s](https://github.com/UdjinM6/dash/actions/runs/20287332888/job/58281396883) New branch*, merged #6947 (chainlock, evo): 96.5% hits, [5m 13s](https://github.com/UdjinM6/dash/actions/runs/20300087485/job/58303447008) New branch*, merged #7056 (chainlock, instantsend, llmq, etc): 59.3%, [19m 50s](https://github.com/UdjinM6/dash/actions/runs/20300940029/job/58306338715) New branch*, merged #7005 (wallet, utils): 40.9% hits, [25m 49s](https://github.com/UdjinM6/dash/actions/runs/20293363566/job/58282142424) _* with this patch merged into develop_ ## Breaking Changes n/a ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: kwvg: utACK 040ac27 PastaPastaPasta: utACK 040ac27 Tree-SHA512: 3fe629b533773c6200caf0037c541180c91d7d9b9468eba410c0c51c17ae5df28d3d02380a4d4c14654eb73e20ea07d2c0748462874b11e33e1a7a1ae0dd22f4
|
This pull request has conflicts, please rebase. |
|
🕓 Ready for review — 3 ahead in queue (commit 9ca9b0c) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
The async coinbase chainlock processing adds a useful recovery path, but the deferred processing model has two correctness gaps: queued coinbase CLSIGs survive spork disable/re-enable cycles (replaying stale state), and pending items are lost on shutdown before the 5-second scheduler tick — permanently missing chainlocks that existed only in coinbase data.
Reviewed commit: 525368a
🔴 1 blocking | 🟡 1 suggestion(s)
1 additional finding
🟡 suggestion: Queued coinbase chainlocks survive SPORK_19 disable/re-enable — replays stale state
src/chainlock/chainlock.cpp (lines 246-260)
CheckActiveState() clears bestChainLock* when SPORK_19 is re-enabled to discard stale chainlock state. But the pending coinbase queue isn't cleared. If QueueCoinbaseChainLock() buffered entries before the spork was disabled, ProcessPendingCoinbaseChainLocks() will process them after re-enable, replaying pre-disable CLSIGs that the handler explicitly tried to discard.
Clear the pending queue in CheckActiveState() alongside the other state reset.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/chainlock/chainlock.cpp`:
- [BLOCKING] lines 495-517: Pending coinbase chainlocks lost on shutdown before scheduler processes them
Queued coinbase CLSIGs are only drained by the 5-second scheduler callback. `Stop()` calls `scheduler->stop()` without first draining `pendingCoinbaseChainLocks` or calling `ProcessPendingCoinbaseChainLocks()`. If the node accepts a block with a recoverable coinbase chainlock and shuts down before the scheduler runs, the CLSIG is permanently lost — on restart, already-accepted blocks aren't replayed through `ProcessSpecialTxsInBlock()`, so the node can miss that chainlock indefinitely.
Either drain the queue in `Stop()` before stopping the scheduler, or persist pending coinbase CLSIGs to disk.
- [SUGGESTION] lines 246-260: Queued coinbase chainlocks survive SPORK_19 disable/re-enable — replays stale state
`CheckActiveState()` clears `bestChainLock*` when SPORK_19 is re-enabled to discard stale chainlock state. But the pending coinbase queue isn't cleared. If `QueueCoinbaseChainLock()` buffered entries before the spork was disabled, `ProcessPendingCoinbaseChainLocks()` will process them after re-enable, replaying pre-disable CLSIGs that the handler explicitly tried to discard.
Clear the pending queue in `CheckActiveState()` alongside the other state reset.
| void CChainLocksHandler::QueueCoinbaseChainLock(const chainlock::ChainLockSig& clsig) | ||
| { | ||
| LOCK(cs); | ||
|
|
||
| if (!IsEnabled()) { | ||
| return; | ||
| } | ||
|
|
||
| // Only queue if it's potentially newer than what we have | ||
| if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight()) { | ||
| return; | ||
| } | ||
|
|
||
| // Check if we've already seen this chainlock | ||
| const uint256 hash = ::SerializeHash(clsig); | ||
| if (seenChainLocks.count(hash) != 0) { | ||
| return; | ||
| } | ||
|
|
||
| pendingCoinbaseChainLocks.push_back(clsig); | ||
| } | ||
|
|
||
| void CChainLocksHandler::ProcessPendingCoinbaseChainLocks() |
There was a problem hiding this comment.
🔴 Blocking: Pending coinbase chainlocks lost on shutdown before scheduler processes them
Queued coinbase CLSIGs are only drained by the 5-second scheduler callback. Stop() calls scheduler->stop() without first draining pendingCoinbaseChainLocks or calling ProcessPendingCoinbaseChainLocks(). If the node accepts a block with a recoverable coinbase chainlock and shuts down before the scheduler runs, the CLSIG is permanently lost — on restart, already-accepted blocks aren't replayed through ProcessSpecialTxsInBlock(), so the node can miss that chainlock indefinitely.
Either drain the queue in Stop() before stopping the scheduler, or persist pending coinbase CLSIGs to disk.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/chainlock/chainlock.cpp`:
- [BLOCKING] lines 495-517: Pending coinbase chainlocks lost on shutdown before scheduler processes them
Queued coinbase CLSIGs are only drained by the 5-second scheduler callback. `Stop()` calls `scheduler->stop()` without first draining `pendingCoinbaseChainLocks` or calling `ProcessPendingCoinbaseChainLocks()`. If the node accepts a block with a recoverable coinbase chainlock and shuts down before the scheduler runs, the CLSIG is permanently lost — on restart, already-accepted blocks aren't replayed through `ProcessSpecialTxsInBlock()`, so the node can miss that chainlock indefinitely.
Either drain the queue in `Stop()` before stopping the scheduler, or persist pending coinbase CLSIGs to disk.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
The async coinbase chainlock queue is structurally sound and lock annotations are correct, but two issues warrant attention before merge: the new functional test is non-deterministic because mockscheduler does not drive the CChainLocksHandler's private scheduler, and the BLS signature is verified twice on the recovery path this PR targets. A minor dedup gap in the queue can let identical coinbase CLSIGs pile up between scheduler ticks. No consensus or correctness defects found.
Reviewed commit: 525368a
Fresh dispatcher run for this queue item. A same-SHA review already existed, so this records the fresh verification without duplicating inline threads.
Code Review
Reviewed commit: 525368a
🟡 2 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `test/functional/feature_llmq_chainlocks.py`:
- [SUGGESTION] lines 413-419: mockscheduler does not advance the chainlock handler's scheduler — test relies on wall-clock timing
`self.nodes[4].mockscheduler(6)` only forwards `node_context->scheduler` (src/rpc/node.cpp:847-848). The new async drain runs on `CChainLocksHandler::scheduler`, a private `CScheduler` instance owned by the handler (src/chainlock/chainlock.cpp:53-55, 70-82). The RPC does not advance that scheduler, so `ProcessPendingCoinbaseChainLocks()` is not deterministically triggered by this test. The assertion at `wait_for_chainlocked_block(..., timeout=5)` happens to pass only when the handler's real 5s wall-clock tick fires within the 5s polling window — that is timing-dependent and can flake on slow CI. To deterministically exercise the new mechanism, either (a) expose the chainlock scheduler to mockscheduler (e.g., have `node_context->scheduler` mock-forward also bump `CChainLocksHandler::scheduler`), or (b) add a test-only RPC that calls `ProcessPendingCoinbaseChainLocks()` directly, or (c) increase the timeout substantially and document the dependency on real time.
In `src/evo/specialtxman.cpp`:
- [SUGGESTION] lines 648-658: Coinbase chainlock BLS signature is verified twice on the recovery path
`CheckCbTxBestChainlock` already calls `VerifyChainLock` on `(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, bestCLSignature)` at src/evo/specialtxman.cpp:81 for any block whose coinbase CLSIG isn't served from `cached_chainlock`. The same triple is then queued and re-verified inside `ProcessNewChainLock` (src/chainlock/chainlock.cpp:144) when the queue drains. The fast-path height check at chainlock.cpp:138-141 only skips the second verification once `bestChainLock` has caught up — but in the recovery scenario this PR targets, the queued CLSIG is exactly the one that advances `bestChainLock`, so the second BLS verify always runs. BLS aggregate verification dominates, so this doubles the dominant cost during reindex/IBD on every block whose coinbase advertises a non-cached CLSIG. Consider marking queued entries as pre-verified and adding a thin internal entry point on `ProcessNewChainLock` that skips `VerifyChainLock`, or threading a `bool already_verified` flag through.
- Added methods to queue and process coinbase chainlocks in CChainLocksHandler. - Introduced a deque to hold pending chainlocks for asynchronous processing. - Updated the Start method to call ProcessPendingCoinbaseChainLocks. - Added unit tests to verify the queueing mechanism for coinbase chainlocks. This enhancement allows for improved handling of chainlocks during block validation without blocking the main processing flow.
Replace std::deque with std::vector and implement several performance optimizations for processing coinbase chainlocks: 1. Zero-copy swap: Use swap() instead of copying elements one by one (~2333x faster for 400 queued chainlocks) 2. LIFO processing: Process newest chainlocks first using reverse iterators. Once a newer chainlock is accepted, older ones fail the height check immediately without expensive operations. 3. Height check before hashing: Check height first (cheap int comparison) before computing the hash. During reindex, ~99% of chainlocks fail the height check, avoiding unnecessary SHA256 hash computations. 4. Better cache locality: Vector provides contiguous memory vs deque's fragmented chunks (2-3x faster iteration). Performance impact during reindex (400 queued chainlocks): - Queue drain: 56 KB copied → 24 bytes swapped - Hash computations: 400 → ~1 (99% reduction) - Memory overhead: 14 KB → 1 KB No behavior changes during normal operation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Replace the useless unit test with a meaningful functional test that verifies nodes can learn about chainlocks from coinbase transactions when they miss the P2P broadcast. The new test: - Isolates a node before a chainlock is created - Submits blocks via RPC (not P2P) so the node gets blocks but not the chainlock message - Verifies the chainlock appears in the next block's coinbase - Uses mockscheduler to trigger async processing - Verifies the node learned the chainlock from the coinbase This tests the async chainlock queueing and processing mechanism implemented in the parent commit, ensuring nodes can recover chainlocks from block data during sync/reindex. Removed: coinbase_chainlock_queueing_test (unit test that only verified calls don't crash, provided no real validation) Added: test_coinbase_chainlock_recovery (functional test with actual validation of chainlock recovery behavior) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
PastaPastaPasta
force-pushed
the
feat/auto-clsig-in-blocks
branch
from
525368a to
9ca9b0c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/chainlock/chainlock.cpp (1)
175-189:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftPending coinbase chainlocks lost on shutdown before scheduler processes them.
Queued coinbase CLSIGs are only drained by the 5-second scheduler callback in
ChainlockHandler::Start. TheStop()method (line 99 in handler.cpp) and the destructor callscheduler->stop()without first drainingpendingCoinbaseChainLocksor invokingProcessPendingCoinbaseChainLocks(). If the node accepts a block with a recoverable coinbase chainlock and shuts down before the scheduler runs, the CLSIG is permanently lost. On restart, already-accepted blocks are not replayed throughProcessSpecialTxsInBlock(), so the node can miss that chainlock indefinitely.Either drain the queue in
ChainlockHandler::Stop()before stopping the scheduler, or persist pending coinbase CLSIGs to disk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/chainlock/chainlock.cpp` around lines 175 - 189, The queued coinbase CLSIGs can be lost on shutdown because ChainlockHandler::Stop() (and destructor) calls scheduler->stop() without draining pendingCoinbaseChainLocks; fix by draining or persisting them before stopping the scheduler: in ChainlockHandler::Stop() (and destructor) acquire the same lock used by Chainlocks, call ProcessPendingCoinbaseChainLocks() (or otherwise move/persist pendingCoinbaseChainLocks) while holding the lock to ensure no races with Chainlocks::QueueCoinbaseChainLock, then call scheduler->stop(); alternatively implement persistence of pendingCoinbaseChainLocks to disk and reload on startup so items aren’t lost across shutdown/restart.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/chainlock/chainlock.cpp`:
- Around line 184-186: The current check using if (!bestChainLock.IsNull() &&
clsig.getHeight() <= bestChainLock.getHeight()) wrongly rejects incoming
chainlocks at the same height even when they target a different block; change
the condition to only reject strictly lower heights or, if you want to continue
rejecting duplicates, reject equal heights only when the block hash matches:
update the test to if (!bestChainLock.IsNull() && (clsig.getHeight() <
bestChainLock.getHeight() || (clsig.getHeight() == bestChainLock.getHeight() &&
clsig.getBlockHash() == bestChainLock.getBlockHash()))) return; so use
clsig.getHeight(), bestChainLock.getHeight(), and
clsig.getBlockHash()/bestChainLock.getBlockHash() to locate and implement the
fix.
---
Duplicate comments:
In `@src/chainlock/chainlock.cpp`:
- Around line 175-189: The queued coinbase CLSIGs can be lost on shutdown
because ChainlockHandler::Stop() (and destructor) calls scheduler->stop()
without draining pendingCoinbaseChainLocks; fix by draining or persisting them
before stopping the scheduler: in ChainlockHandler::Stop() (and destructor)
acquire the same lock used by Chainlocks, call
ProcessPendingCoinbaseChainLocks() (or otherwise move/persist
pendingCoinbaseChainLocks) while holding the lock to ensure no races with
Chainlocks::QueueCoinbaseChainLock, then call scheduler->stop(); alternatively
implement persistence of pendingCoinbaseChainLocks to disk and reload on startup
so items aren’t lost across shutdown/restart.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7c0affca-c639-4389-9a22-205ea62c8080
📒 Files selected for processing (4)
src/chainlock/chainlock.cppsrc/chainlock/chainlock.hsrc/chainlock/handler.cppsrc/chainlock/handler.h
| if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Same-height chainlock rejection may skip valid fork chainlocks.
The condition clsig.getHeight() <= bestChainLock.getHeight() (line 184) rejects chainlocks at the same height as the current best. During a chain fork, competing blocks at the same height can both have valid chainlocks, and the coinbase chainlock for the correct fork should still be queued and processed. Using <= instead of < may cause the node to skip recovery of a valid chainlock when the incoming coinbase chainlock is at the same height but for a different block hash than bestChainLock.
Consider changing the condition to < or adding a block-hash inequality check.
Proposed fix
- if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight()) {
+ if (!bestChainLock.IsNull() && clsig.getHeight() < bestChainLock.getHeight()) {
return;
}Alternatively, if same-height entries must be filtered, check the block hash as well:
- if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight()) {
+ if (!bestChainLock.IsNull() &&
+ (clsig.getHeight() < bestChainLock.getHeight() ||
+ (clsig.getHeight() == bestChainLock.getHeight() && clsig.getBlockHash() == bestChainLock.getBlockHash()))) {
return;
}📝 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.
| if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight()) { | |
| return; | |
| } | |
| if (!bestChainLock.IsNull() && clsig.getHeight() < bestChainLock.getHeight()) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/chainlock/chainlock.cpp` around lines 184 - 186, The current check using
if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight())
wrongly rejects incoming chainlocks at the same height even when they target a
different block; change the condition to only reject strictly lower heights or,
if you want to continue rejecting duplicates, reject equal heights only when the
block hash matches: update the test to if (!bestChainLock.IsNull() &&
(clsig.getHeight() < bestChainLock.getHeight() || (clsig.getHeight() ==
bestChainLock.getHeight() && clsig.getBlockHash() ==
bestChainLock.getBlockHash()))) return; so use clsig.getHeight(),
bestChainLock.getHeight(), and clsig.getBlockHash()/bestChainLock.getBlockHash()
to locate and implement the fix.
4 participants
Issue being fixed or feature implemented
This enhancement allows for improved handling of chainlocks during block validation without blocking the main processing flow.
What was done?
How Has This Been Tested?
Tested a node on main net; launched via --no-connect and then used submit block, and saw that chain locks were processed.
Breaking Changes
None
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.