backport: Merge bitcoin-core/gui#749, 28318, 29241, 29375, 29117, 28605, 28784, 29035, 24097 by vijaydasmp · Pull Request #7049 · dashpay/dash

vijaydasmp · 2025-12-06T03:25:23Z

Bitcoin backports

github-actions · 2025-12-06T03:25:59Z

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

vijaydasmp · 2025-12-08T13:01:35Z

Hello @knst @UdjinM6 @PastaPastaPasta , requesting review

coderabbitai · 2025-12-08T13:09:59Z

Walkthrough

This PR makes coordinated changes across logging, banning, wallet, and Qt UI subsystems plus tests and docs. Logging: new levels (Trace/Debug/Info/Warning/Error), removal of Level::None, new GetLogPrefix, new macros (LogInfo/LogWarning/LogError/LogDebug/LogTrace), and related callsite/test updates. BanMan: replace RecursiveMutex with Mutex, add m_banned_mutex guards, consolidate dirty-state handling, add dump_mutex, and remove BannedSetIsDirty/SetBannedSetDirty. Wallet: replace CWallet with WalletDatabase in DumpWallet, remove accept_no_keys from CheckDecryptionKey and Unlock overloads. Qt: propagate show_loading_minimized through GUI → WalletController → LoadWalletsActivity → progress dialog. Misc: many logging call and comment/text fixes, lint/test updates.

Sequence Diagram(s)

mermaid
sequenceDiagram
actor Startup as "Startup"
participant GUI as "BitcoinGUI"
participant WC as "WalletController"
participant LA as "LoadWalletsActivity"
participant PDA as "ProgressDialog"

Startup->>GUI: create and configure GUI
Startup->>GUI: setWalletController(walletController, show_loading_minimized)
GUI->>WC: setWalletController(..., show_loading_minimized)
WC->>LA: load(show_loading_minimized)
LA->>PDA: showProgressDialog(title, label, show_minimized)
alt show_minimized == true
    PDA-->>LA: shown minimized
else
    PDA-->>LA: shown normally
end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

src/logging.h / src/logging.cpp — core API and macro semantics, LogLevel parsing, GetLogPrefix, and effects on formatting/tests.
src/banman.h / src/banman.cpp — mutex semantics, guard annotations, dump_mutex behavior, and removal of dirty accessors.
Wallet surface changes:
- src/wallet/dump.h / src/wallet/dump.cpp / src/wallet/wallet.h / src/wallet/wallet.cpp
- src/wallet/scriptpubkeyman.{h,cpp}
  — ensure callsites updated and decryption/unlock semantics preserved.
Qt UI startup flow: src/qt/bitcoin.cpp / src/qt/bitcoingui.{h,cpp} / src/qt/walletcontroller.{h,cpp} — parameter propagation and UI behavior.
Tests and lint: src/test/logging_tests.cpp, test/lint/lint-format-strings.py — updated expectations and lint rules.

Suggested reviewers

UdjinM6
PastaPastaPasta

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.05% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Description check	❓ Inconclusive	The description 'Bitcoin backports' is vague and generic, using non-descriptive terminology that doesn't convey meaningful information about the specific changes.	Expand the description to specify which features or improvements are being backported and their impact on the project.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies this as a backport of multiple Bitcoin Core pull requests and GUI changes, accurately representing the main purpose of the changeset.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

UdjinM6

utACK c5b6af07fd66e9c26788ba0f2bce18f2b3bf41bf

github-actions · 2025-12-16T14:26:57Z

This pull request has conflicts, please rebase.

32db154 gui: make '-min' minimize wallet loading dialog (furszy) Pull request description: Simple fix for dashpay#748. When '-min' is enabled, no loading dialog should be presented on screen during startup. ACKs for top commit: hebasto: ACK 32db154, tested on Debian 11 + XFCE. Tree-SHA512: d08060b044938c67e8309db77b49ca645850fc21fdd7d78d5368d336fb9f602dcc66ea398a7505b00bf7d43afa07108347c7260480319fad3ec84cb41332f780

e60fc7d logging: Replace uses of LogPrintfCategory (Anthony Towns) f7ce5ac logging: add LogError, LogWarning, LogInfo, LogDebug, LogTrace (Anthony Towns) fbd7642 logging: add -loglevelalways=1 option (Anthony Towns) 782bb6a logging: treat BCLog::ALL like BCLog::NONE (Anthony Towns) 667ce3e logging: Drop BCLog::Level::None (Anthony Towns) ab34dc6 logging: Log Info messages unconditionally (Anthony Towns) dfe98b6 logging: make [cat:debug] and [info] implicit (Anthony Towns) c5c76dc logging: refactor: pull prefix code out (Anthony Towns) Pull request description: Replace `LogPrint*` functions with severity based logging functions: * `LogInfo(...)`, `LogWarning(...)`, `LogError(...)` for unconditional (uncategorised) logging (replaces `LogPrintf`) * `LogDebug(CATEGORY, ...)` and `LogTrace(CATEGORY, ...)` for conditional logging (replaces `LogPrint`) * `LogPrintLevel(CATEGORY, LEVEL, ...)` for when the level isn't known in advance, or a category needs to be added for an info/warning/error log message (mostly unchanged, but rarely needed) Logs look roughly as they do now with `LogInfo` not having an `[info]` prefix, and `LogDebug` having a `[cat]` prefix, rather than a `[cat:debug]` prefix. This removes `BCLog::Level::None` entirely -- for `LogFlags::NONE` just use `Level::Info`, for any actual category, use `Level::Debug`. Adds docs to developer-notes about when to use which level. Adds `-loglevelalways=1` option so that you get `[net:debug]`, `[all:info]`, `[all:warning]` etc, which might be helpful for automated parsing, or just if you like everything to be consistent. Defaults to off to reduce noise in the default config, and to avoid unnecessary changes on upgrades. Changes the behaviour of `LogPrintLevel(CATEGORY, BCLog::Level::Info, ...)` to be logged unconditionally, rather than only being an additional optional logging level in addition to trace and debug. Does not change the behaviour of `LogPrintLevel(NONE, Debug, ...)` and `LogPrintLevel(NONE, Trace, ...)` being no-ops. ACKs for top commit: maflcko: re-ACK e60fc7d 🌚 achow101: ACK e60fc7d stickies-v: ACK e60fc7d jamesob: ACK e60fc7d ([`jamesob/ackr/28318.1.ajtowns.logging_simplify_api_for`](https://github.com/jamesob/bitcoin/tree/ackr/28318.1.ajtowns.logging_simplify_api_for)) Tree-SHA512: e7a4588779b148242495b7b6f64198a00c314cd57100affab11c43e9d39c9bbf85118ee2002792087fdcffdea08c84576e20844b3079f27083e26ddd7ca15d7f

…ging section c003562 doc: Add missing backtick in developer notes logging section (Fabian Jahr) Pull request description: Newly added logging section from bitcoin#28318 is missing a single backtick. Also fixes some minor punctuation errors in that section. ACKs for top commit: jonatack: ACK c003562 alfonsoromanz: ACK c003562 Tree-SHA512: 2f75f9472d212ce7c7ebf3e7404f86b3bd8c695f63e2a7447d7a55bb54dcdb5e3bfd15e8eac5b92efbdcf1216c4e8d699cae0250d021f72b9d1c32a7db91989d

…decryption process 2bb25ce wallet: remove unused 'accept_no_keys' arg from decryption process (furszy) Pull request description: Found it while reviewing other PR. Couldn't contain myself from cleaning it up. The wallet decryption process (`CheckDecryptionKey()` and `Unlock()`) contains an arg 'accept_no_keys,' introduced in bitcoin#13926, that has never been used. Additionally, this also removes the unimplemented `SplitWalletPath` function. ACKs for top commit: delta1: ACK 2bb25ce epiccurious: utACK 2bb25ce. achow101: ACK 2bb25ce theStack: Code-review ACK 2bb25ce Tree-SHA512: e0537c994be19ca0032551d8a64cf1755c8997e04d21ee0522b31de26ad90b9eb02a8b415448257b60bced484b9d2a23b37586e12dc5ff6e35bdd8ff2165c6bf

…tabase d83bea4 wallettool: Don't create CWallet when dumping DB (Andrew Chow) 40c80e3 wallettool: Don't unilaterally reset wallet_instance if loading error (Ava Chow) Pull request description: bitcoin#29109 (comment) reports that a wallet with noncritical errors cannot be dumped with `bitcoin-wallet dump`. This was caused by an erroneous reset of the wallet pointer when the loading the wallet returns something other than `LOAD_OK`. Not all errors are errors that require aborting, so unilaterally resetting the pointer at that time is incorrect. The first commit resolves this issue. Furthermore, if a wallet has loading errors, that should not prevent the wallet tool from dumping the wallet. The wallet application logic should not get in the way of performing such a low level database operation, especially when it's primary usage is for debugging potentially corrupted wallets. The 2nd commit is taken from bitcoin#28710 and changes the `dump` to stop at making a `WalletDatabase` rather than making a `CWallet` only to retrieve the underlying `WalletDatabase`. ACKs for top commit: furszy: Code review ACK d83bea4 BrandonOdiwuor: Code Review ACK d83bea4 Tree-SHA512: 425d712dfff1002bd81272aca0bae1016f9126a3c89506f8cb7cf0a0ec9f33d0c03b8d03896394f3a45c2998e59047e19218dfd08dc8a5f40e8625134e886b0f

43de4d3 doc: fix typos (Sjors Provoost) Pull request description: This PR fixes typos found by lint-spelling.py using codespell 2.2.6. Our CI linter job uses codespell 2.2.5 and found fewer typos that I did locally. In any case it's happy now. ACKs for top commit: pablomartin4btc: re ACK 43de4d3 Tree-SHA512: c032fe86cb49c924a468385653b31f309a9db68c478d70335bba3e65a1ff3826abe80284fe00a090ab5a509e1edbf17e476f6922fb15d055e50f1103dad2ccb0

7cb9367 rpc: keep .cookie if it was not generated (Roman Zeyde) Pull request description: Otherwise, starting bitcoind twice may cause the `.cookie` file generated by the first instance to be deleted by the second instance shutdown (after failing to obtain a lock). ACKs for top commit: willcl-ark: re-ACK 7cb9367 achow101: ACK 7cb9367 kristapsk: re-ACK 7cb9367 stickies-v: ACK 7cb9367 Tree-SHA512: 0960dbc457975b0e0535f3d814824a879d7f85c9f1191537415b3fc253429a316a8e4badde56c8bc139778f132392983cec5fbe03891fb15ff61d3bc3f6e681b

…penBSD fd0bde2 test: fix `addnode` functional test failure on OpenBSD (Sebastian Falbesoner) Pull request description: This is the functional test counterpart of PR bitcoin#28891 / commit 007d6f0 (unfortunately, I missed it back then and only ran the unit tests -- sorry for the noise). master branch on OpenBSD 7.4: ``` $ ./test/functional/rpc_net.py 2023-12-08T17:29:05.057000Z TestFramework (INFO): PRNG seed is: 6024296850131317403 2023-12-08T17:29:05.058000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_au3zchif 2023-12-08T17:29:05.618000Z TestFramework (INFO): Test getconnectioncount 2023-12-08T17:29:05.618000Z TestFramework (INFO): Test getpeerinfo 2023-12-08T17:29:06.643000Z TestFramework (INFO): Check getpeerinfo output before a version message was sent 2023-12-08T17:29:06.709000Z TestFramework (INFO): Test getnettotals 2023-12-08T17:29:06.773000Z TestFramework (INFO): Test getnetworkinfo 2023-12-08T17:29:06.978000Z TestFramework (INFO): Test addnode and getaddednodeinfo 2023-12-08T17:29:06.980000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main self.run_test() File "/home/thestack/bitcoin/./test/functional/rpc_net.py", line 65, in run_test self.test_addnode_getaddednodeinfo() File "/home/thestack/bitcoin/./test/functional/rpc_net.py", line 224, in test_addnode_getaddednodeinfo assert_raises_rpc_error(-23, "Node already added", self.nodes[0].addnode, node=ip_port2, command='add') File "/home/thestack/bitcoin/test/functional/test_framework/util.py", line 131, in assert_raises_rpc_error assert try_rpc(code, message, fun, *args, **kwds), "No exception raised" AssertionError: No exception raised ``` On the PR branch, the same call succeeds. ACKs for top commit: kevkevinpal: ACK [fd0bde2](bitcoin@fd0bde2) Tree-SHA512: ae20816fa4025c212e115ebd267b5e5784bfcdf0051219eb686faaade47ec4f91a3947af6d24258b159290000d2dcc3f6e65e788b83b8a9297282945dbdafbfb

…nd rename it 37d150d refactor: Add more negative `!m_banned_mutex` thread safety annotations (Hennadii Stepanov) 0fb2908 refactor: replace RecursiveMutex m_banned_mutex with Mutex (w0xlt) 784c316 scripted-diff: rename m_cs_banned -> m_banned_mutex (w0xlt) 46709c5 refactor: Get rid of `BanMan::SetBannedSetDirty()` (Hennadii Stepanov) d88c0d8 refactor: Get rid of `BanMan::BannedSetIsDirty()` (Hennadii Stepanov) Pull request description: This PR is an alternative to bitcoin#24092. Last two commit have been cherry-picked from the latter. ACKs for top commit: maflcko: ACK 37d150d 🎾 achow101: ACK 37d150d theStack: Code-review ACK 37d150d vasild: ACK 37d150d Tree-SHA512: 5e9d40101a09af6e0645a6ede67432ea68631a1b960f9e6af0ad07415ca7718a30fcc1aad5182d1d5265dc54c26aba2008fc9973840255c09adbab8fedf10075

vijaydasmp · 2025-12-16T15:09:34Z

requesting review @UdjinM6 @knst @PastaPastaPasta

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/rpc/request.cpp (1)

135-138: LGTM: Conditional deletion prevents removing externally-created cookies.

The guard correctly ensures only cookies generated by the current process are deleted, improving safety when multiple processes might use the same cookie path. The existing error handling is preserved.

Note: g_generated_cookie lacks atomic or synchronization protection, but is safe in practice since GenerateAuthCookie is called during single-threaded initialization and DeleteAuthCookie is called during shutdown after RPC is stopped. This backport matches the upstream Bitcoin design.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f177719396b14f84aef27c2e5e3384a5cfc4467 and 12a96f7.

📒 Files selected for processing (40)

ci/README.md (1 hunks)
depends/description.md (1 hunks)
doc/developer-notes.md (3 hunks)
src/banman.cpp (10 hunks)
src/banman.h (1 hunks)
src/httpserver.cpp (2 hunks)
src/init.cpp (1 hunks)
src/init/common.cpp (2 hunks)
src/logging.cpp (4 hunks)
src/logging.h (5 hunks)
src/net_processing.cpp (3 hunks)
src/qt/bitcoin.cpp (1 hunks)
src/qt/bitcoingui.cpp (2 hunks)
src/qt/bitcoingui.h (1 hunks)
src/qt/transactiontablemodel.cpp (1 hunks)
src/qt/walletcontroller.cpp (3 hunks)
src/qt/walletcontroller.h (2 hunks)
src/rpc/request.cpp (3 hunks)
src/rpc/server.cpp (1 hunks)
src/test/fuzz/FuzzedDataProvider.h (1 hunks)
src/test/logging_tests.cpp (2 hunks)
src/test/orphanage_tests.cpp (1 hunks)
src/test/script_tests.cpp (1 hunks)
src/test/versionbits_tests.cpp (1 hunks)
src/torcontrol.cpp (2 hunks)
src/txmempool.h (1 hunks)
src/wallet/db.h (0 hunks)
src/wallet/dump.cpp (2 hunks)
src/wallet/dump.h (1 hunks)
src/wallet/scriptpubkeyman.cpp (4 hunks)
src/wallet/scriptpubkeyman.h (3 hunks)
src/wallet/spend.cpp (2 hunks)
src/wallet/wallet.cpp (3 hunks)
src/wallet/wallet.h (3 hunks)
src/wallet/wallettool.cpp (1 hunks)
test/functional/feature_filelock.py (2 hunks)
test/functional/rpc_net.py (2 hunks)
test/functional/test_framework/blockfilter.py (1 hunks)
test/lint/lint-format-strings.py (1 hunks)
test/lint/spelling.ignore-words.txt (1 hunks)

💤 Files with no reviewable changes (1)

src/wallet/db.h

✅ Files skipped from review due to trivial changes (1)

src/test/versionbits_tests.cpp

🚧 Files skipped from review as they are similar to previous changes (20)

src/qt/transactiontablemodel.cpp
src/wallet/spend.cpp
src/rpc/server.cpp
src/qt/walletcontroller.cpp
ci/README.md
test/functional/test_framework/blockfilter.py
src/banman.cpp
src/net_processing.cpp
src/torcontrol.cpp
test/functional/feature_filelock.py
test/lint/lint-format-strings.py
src/wallet/dump.cpp
src/init.cpp
src/txmempool.h
src/test/fuzz/FuzzedDataProvider.h
depends/description.md
src/httpserver.cpp
src/wallet/dump.h
src/test/orphanage_tests.cpp
src/logging.cpp

🧰 Additional context used

📓 Path-based instructions (6)

src/**/*.{cpp,h,hpp,cc}