Prompt user after upgrading to RDTS-enabled version#256
Prompt user after upgrading to RDTS-enabled version#256luke-jr wants to merge 4 commits intobitcoinknots:29.x-knotsfrom
Conversation
9cb51e9 to
c6ac662
Compare
|
Concept ACK; it's very important to get explicit agreement from users regarding consensus changes. |
| } else { | ||
| setting = CONSENSUSRULES_REQUIRED; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Return value of WriteSettingsFile() is silently discarded. If writing fails (disk full, permissions), the user's consent is lost and they'll be re-prompted on every startup despite having already consented. Should at least log a warning on failure.
There was a problem hiding this comment.
WriteSettingsFile itself logs already?
c6ac662 to
dcaa85a
Compare
dcaa85a to
197b098
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a user consent mechanism for the "Reduced Data Temporary Softfork" (RDTS) protocol change in Bitcoin Knots. On mainnet, the software requires the user to explicitly acknowledge the consensus rule change via the -consensusrules=rdts config option or an interactive GUI prompt before the node will start. This is designed to work alongside PR #238 which implements the actual RDTS deployment.
Changes:
- Added
UserProtocolRulesConsent()ininit.cppthat checks forconsensusrules=rdtsconfig, prompts GUI users interactively, and persists consent tosettings.json - Added rich-text URL linkification to all GUI
ThreadSafeMessageBox/ThreadSafeQuestionmessages using a new regex-basedMakeHtmlLinkoverload, plus a<qt>prefix convention for rich text rendering - Updated
resetSettings()to preserve the consensus rules consent setting across GUI settings resets, and added the-consensusrulesargument to available CLI options
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/init.cpp |
Core consent logic: UserProtocolRulesConsent() function and its invocation during AppInitMain (skipped for test chains) |
src/chainparamsbase.h |
Defines CONSENSUSRULES_CONFIG_NAME and CONSENSUSRULES_REQUIRED constants |
src/chainparamsbase.cpp |
Registers -consensusrules CLI argument |
src/qt/bitcoingui.cpp |
Adds <qt> prefix-based rich text rendering and auto-linking of HTTPS URLs in all message boxes |
src/qt/guiutil.h |
Declares new MakeHtmlLink(const QString&) overload |
src/qt/guiutil.cpp |
Implements regex-based URL-to-HTML-link conversion for HTTPS URLs |
src/node/interfaces.cpp |
Preserves consensusrules setting during resetSettings() and adds chainparamsbase.h include |
test/functional/mining_mainnet.py |
Adds -consensusrules=rdts to mainnet test args |
test/functional/rpc_validateaddress.py |
Adds -consensusrules=rdts to mainnet test args |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
62244f6 to
de3ef29
Compare
de3ef29 to
bc826d7
Compare
Require
consensusrules=rdtsin config to start; if missing, give a brief explanation and let the GUI user consent on the spot.Intended to only be merged together with #238