backport: trivial 2025 04 22 #6648
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request has conflicts, please rebase. |
WalkthroughThis set of changes introduces new functionality and refactors several components across the codebase. The primary user-facing addition is a "Restore Wallet" action in the GUI, allowing users to restore a wallet from a backup file through a new menu item. Supporting this, a 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (15)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (14)
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:
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 (
|
… in GetSelectionWaste d2eccac doc: Clarify that change_cost cannot be negative in GetSelectionWaste (benthecarman) Pull request description: We assert that the `change_cost` must be positive so we should document it in the function's docs https://github.com/bitcoin/bitcoin/blob/4f5ad43b1e05cd7b403f87aae4c4d42e5aea810b/src/wallet/coinselection.cpp#L361 ACKs for top commit: jonatack: ACK d2eccac jarolrod: ACK d2eccac shaavan: ACK d2eccac Tree-SHA512: 089bc41d7e212b811455527ce2ac83301eae0edc03933dcbd5229b22078827395c8e3bcfda0ba029b7c60705db3119b4f80e372842a6bd8dae195dbaa0b52833
…indow a56a104 qt: Handle Android back key in the Node window (Hennadii Stepanov) f045f98 qt, android: Add GUIUtil::IsEscapeOrBack helper (Hennadii Stepanov) Pull request description: On master (4633199) there are no means to return from the Node window to the main one on Android. This PR assigns this functionality to the Android back key:  ACKs for top commit: icota: utACK bitcoin-core/gui@a56a104 Tree-SHA512: 379c1ad8c6bffa037e861b88c66eb33872d7f7d54aa2f76289a51c55d79a37a0c16262b20f22d00fda11522c7df1f3561c1ceae34cd7a85da94aee4c6cdcfaaf
…)` with `QMetaObject::invokeMethod()` 6958a26 Revert "qt: Add ObjectInvoke template function" (Hennadii Stepanov) 249984f qt: Replace `GUIUtil::ObjectInvoke()` with `QMetaObject::invokeMethod()` (Hennadii Stepanov) Pull request description: A comment in 5659e73 states that `GUIUtil::ObjectInvoke` > can be replaced by a call to the QMetaObject::invokeMethod functor overload after Qt 5.10 ACKs for top commit: w0xlt: tACK bitcoin-core/gui@6958a26 on Ubuntu 21.10, Qt 5.15.2. promag: Code review ACK 6958a26. Tree-SHA512: 6a840289568113cf38df6c1092821d626c2d206768a21d4dc6846b9dcccb4130477adb45ba718bb6bc15a3041871a7df3238983ac03db80406732be597693266
fa2102e test: Split MempoolAncestryTests into two (MacroFake) Pull request description: The two tests don't share any state, so it seems clearer to put them in separate scopes. ACKs for top commit: jnewbery: Code review ACK fa2102e Tree-SHA512: 6669f50f8d5944fed55ecc88aa1bd139bddf6a40e3c2e8f88c3cc7e70cf6d4650c0dd652c7f304813893827c3930d626268655cd9b3f17ff9c9a1a02f0359714
…p tools a0e2a31 build: Drop redundant checks for ranlib and strip tools (Hennadii Stepanov) Pull request description: These checks are handled by the `LT_INIT` macro. Inspired by bitcoin-core/secp256k1#1088. On master (f3e0ace): ``` $ ./configure --with-incompatible-bdb 2>&1 | grep -n -E 'ranlib\.\.\.|strip\.\.\.' 56:checking for strip... strip 57:checking for ranlib... ranlib 102:checking for ranlib... /usr/bin/ranlib 103:checking for strip... /usr/bin/strip 380:checking for strip... strip 381:checking for ranlib... ranlib ``` With this PR: ``` $ ./configure --with-incompatible-bdb 2>&1 | grep -n -E 'ranlib\.\.\.|strip\.\.\.' 56:checking for strip... strip 57:checking for ranlib... ranlib 377:checking for strip... strip 378:checking for ranlib... ranlib $ CONFIG_SITE=$PWD/depends/x86_64-apple-darwin/share/config.site ./configure 2>&1 | grep -n -E 'ranlib\.\.\.|strip\.\.\.' 8:checking for x86_64-apple-darwin-strip... /home/hebasto/GitHub/bitcoin/depends/x86_64-apple-darwin/native/bin/x86_64-apple-darwin-strip 61:checking for x86_64-apple-darwin-strip... (cached) /home/hebasto/GitHub/bitcoin/depends/x86_64-apple-darwin/native/bin/x86_64-apple-darwin-strip 62:checking for x86_64-apple-darwin-ranlib... /home/hebasto/GitHub/bitcoin/depends/x86_64-apple-darwin/native/bin/x86_64-apple-darwin-ranlib 188:checking whether the linker accepts -Wl,-dead_strip... yes 367:checking for x86_64-apple-darwin-strip... /home/hebasto/GitHub/bitcoin/depends/x86_64-apple-darwin/native/bin/x86_64-apple-darwin-strip 411:checking for x86_64-apple-darwin-strip... (cached) /home/hebasto/GitHub/bitcoin/depends/x86_64-apple-darwin/native/bin/x86_64-apple-darwin-strip 412:checking for x86_64-apple-darwin-ranlib... /home/hebasto/GitHub/bitcoin/depends/x86_64-apple-darwin/native/bin/x86_64-apple-darwin-ranlib ``` #### Guix builds on `x86_64`: ``` ... ``` ACKs for top commit: real-or-random: ACK a0e2a31 fanquake: ACK a0e2a31 Tree-SHA512: 17e2f54a3fc0447d7a27592d4c803538b6e0dfe02eab9a234084d71f3d9244c2488d56301f6c57050592e0d760c2d48b2b7d365454754af2ce098e77c05d33cc
…reApplication::quit()` with `QCoreApplication::exit(0)` 252f363 qt: Replace `QCoreApplication::quit()` with `QCoreApplication::exit(0)` (Hennadii Stepanov) Pull request description: ### Qt 5: - no behavior change. See https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qcoreapplication.cpp?h=5.15#n2012: ```cpp void QCoreApplication::quit() { exit(0); } ``` ### Qt 6: - this change avoids sending a duplicated `QEvent::Quit` We use `QEvent::Quit` to [handle](bitcoin-core/gui#547) macOS dock menu events. Qt 6 uses `QEvent::Quit` more [widely](qt/qtbase@89f7a27). We do not want a duplicated `QEvent::Quit` which fires `Assert(node.args);` in the [`Shutdown()`](https://github.com/bitcoin-core/gui/blob/d1b3dfb275fd98e37cfe8a0f7cea7d03595af2e8/src/init.cpp#L200) function. ACKs for top commit: promag: Code review ACK 252f363. Tree-SHA512: 6a04cbcf523c0375158a59b29afadf18da99738c8db8b8728f99319a8cdc10806d2f06dc5a7d3b8b0e1a5f1711be778a75d4ecdefef7cf66e26ae2848f7f57db
bc13ec8 doc: Add a release note about the "restore wallet" menu item (w0xlt) e7a3f69 gui: Add Wallet Restore in the GUI (w0xlt) Pull request description: This PR adds a menu item to restore a wallet from a backup file in the GUI. Currently this option exists only in RPC interface. Motivation: It makes easier for non-technical users to restore backups. Master | PR | --- | --- <img width="307" alt="master" src="https://user-images.githubusercontent.com/94266259/141673349-0bf8a237-ecec-42e4-a0d7-1d5863940036.png"> | <img width="307" alt="pr" src="https://user-images.githubusercontent.com/94266259/141673350-972dea23-ae56-4283-a365-819da62b7067.png"> | ACKs for top commit: w0xlt: Added a release note in a new commit (bitcoin-core/gui@bc13ec8) to not invalidate the ACKs for the previous one. furszy: utACK bc13ec8 shaavan: ACK bc13ec8 hebasto: ACK bc13ec8 Tree-SHA512: edc3675484238857b77e74382a4041dd5d2cbcda1e2d5bfe52c83d9d7bb7be8a243ecd97e25e994d8c30ab6d7c59ead5a1c953a46dce173666b137eeffc3c94f
…election 98ea43d test: add tests for negative waste during coin selection (ishaanam) Pull request description: bitcoin#25495 mentions that waste can be negative when the current feerate is less than the long term feerate. There are currently no waste tests for negative waste, so this PR adds two of them. ACKs for top commit: achow101: ACK 98ea43d glozow: light code review ACK 98ea43d, good to have tests for negative waste Tree-SHA512: d194d370f1257975959d3c601fea9f82c30c1aabc3e8bedc997c62659283fe681cc527e59df1a0187b3c91e8067c60374dd5ce0237561bd882edafe6a575a9b9
… `QInputDialog` 9d9a098 gui: Fix translator comment for Restore Wallet QInputDialog (w0xlt) Pull request description: Fix translator comment for Restore Wallet `QInputDialog`, as suggested in bitcoin-core/gui#471 (comment). This also changes the window title name from `Restore Name` to `Restore Wallet` as it seems clearer. ACKs for top commit: shaavan: reACK 9d9a098 Tree-SHA512: 02aec661839215ab1183e4e92fa131671daa986339373a87c0a0e2c5e79a46f362a8846f4a5f6d630a99884a7949031982d13352336bd3f0573625826406dde8
…osity 0, False, and True f663b43 QA: rpc_blockchain: Test output of getblock verbosity 0, False, and True (Luke Dashjr) Pull request description: Currently getblock's "verbosity" is documented as a NUM, though it has a fallback to Boolean for the (deprecated?) "verbose" alias. Since we've been doing more generic type-checking on RPC stuff, I think it would be a good idea to actually test the Boolean values work. I didn't see an existing test for verbosity=0, so this adds that too. ACKs for top commit: aureleoules: ACK f663b43. Tree-SHA512: 321a7795a2f32e469d28879dd323c85cb6b221828030e2a33ad9afd35a648191151a79b04e359b2f58314e43360f81c25f05be07deb42f61efdf556850a7266c
PastaPastaPasta
force-pushed
the
develop-trivial-2025-04-22-p1
branch
from
2d2630d to
df78d6f
Compare
knst
reviewed
Apr 24, 2025
| @@ -293,7 +293,9 @@ void BitcoinApplication::startThread() | |||
|
|
|||
| /* communication to and from thread */ | |||
| connect(&m_executor.value(), &InitExecutor::initializeResult, this, &BitcoinApplication::initializeResult); | |||
| connect(&m_executor.value(), &InitExecutor::shutdownResult, this, &QCoreApplication::quit); | |||
| connect(&m_executor.value(), &InitExecutor::shutdownResult, this, [] { | |||
There was a problem hiding this comment.
we have one more usage of QCoreApplication::quit in InitExecutor::restart
see src/qt/initexecutor.cpp
knst
reviewed
Apr 24, 2025
| GUI changes | ||
| -------- | ||
|
|
||
| - A new menu item to restore a wallet from a backup file has been added (#471). |
| //: Title of the Restore Wallet input dialog (where the wallet name is entered) | ||
| QString wallet_name = QInputDialog::getText(this, tr("Restore Name"), tr("Wallet Name:"), QLineEdit::Normal, "", &wallet_name_ok); | ||
| /*: Title of pop-up window shown when the user is attempting to | ||
| + restore a wallet. */ |
There was a problem hiding this comment.
consider include bitcoin-core/gui#663 which fix formatting for gui#629
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
batch of trivial backports
How Has This Been Tested?
Built locally
Breaking Changes
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.