feat: add optional argument to ignore ChainLocks for reconsiderblocks RPC #6501
Conversation
WalkthroughThe pull request introduces modifications across multiple source files to enhance the blockchain RPC functionality and block validation process. The primary focus is on adding a new boolean parameter, The changes span four key files:
These modifications provide more granular control over block validation and reconsideration processes, maintaining the existing code structure while introducing new capabilities for handling block states and RPC interactions. Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🔇 Additional comments (3)src/rpc/blockchain.cpp (3)
The new
The parameter parsing follows best practices:
The Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/validation.cpp (1)
3413-3414: LGTM! Consider adding documentation for the new parameter.The new
ignore_chainlocksparameter is a good addition. Consider adding documentation to explain when this parameter should be set to true/false.Add documentation like:
/** * Reset the validation state of a block and its descendants. * @param pindex The block index to start from * @param ignore_chainlocks If true, also clear BLOCK_CONFLICT_CHAINLOCK flags */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/rpc/blockchain.cpp(3 hunks)src/rpc/client.cpp(1 hunks)src/validation.cpp(3 hunks)src/validation.h(1 hunks)
🔇 Additional comments (4)
src/rpc/blockchain.cpp (2)
2128-2128: Looks good: Parameter definition aligns with the updated functionality.
The new parameter "nochainlocks" has a clear default (false) and an explanatory description.
2150-2150: Integration with ResetBlockFailureFlags is consistent.
Once the parameter index mismatch is corrected, passing “ignore_chainlocks” here directly aligns with the updated method signature.
src/rpc/client.cpp (1)
78-78: Consistent addition of "nochainlocks" parameter.
This correctly registers the second parameter (index = 1) for JSON conversion. Ensure the caller in blockchain.cpp references this same index to avoid mismatched parameters.
src/validation.h (1)
716-716: Straightforward extension to ResetBlockFailureFlags.
Providing a default of false for "ignore_chainlocks" is a safe choice, preserving existing behavior unless explicitly overridden. The concurrency annotation (EXCLUSIVE_LOCKS_REQUIRED) helps ensure proper synchronization.
knst
force-pushed
the
reconsider-reset-cl
branch
from
80cd381 to
1cb44ab
Compare
knst
force-pushed
the
reconsider-reset-cl
branch
3 times, most recently
from
d5f0049 to
b7f1d6b
Compare
… RPC Co-Authored-By: UdjinM6 <[email protected]>
knst
force-pushed
the
reconsider-reset-cl
branch
from
b7f1d6b to
64817da
Compare
Issue being fixed or feature implemented
More general way to implement #6500 which can be merged to develop.
What was done?
It allows removing BLOCK_CONFLICT_CHAINLOCK flag via ResetBlockFailureFlags
How Has This Been Tested?
N/A
Breaking Changes
N/A
Checklist: