Age | Commit message (Collapse) | Author |
|
Add TransactionError to node namespace and include it directly instead of
relying on indirect include through common/messages.h
This is a followup to a previous commit which moved the TransactionError enum.
These changes were done in a separate followup just to keep the previous commit
more minimal and easy to review.
|
|
Add separate PSBTError enum instead of reusing TransactionError enum for PSBT
operations, and drop unused error codes. The error codes returned by PSBT
operations and transaction broadcast functions mostly do not overlap, so using
an unified enum makes it harder to call any of these functions and know which
errors actually need to be handled.
Define PSBTError in the common library because PSBT functionality is
implemented in the common library and used by both the node (for rawtransaction
RPCs) and the wallet.
|
|
-BEGIN VERIFY SCRIPT-
perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' )
-END VERIFY SCRIPT-
|
|
|
|
The remaining places are handled easier outside a scripted-diff.
-BEGIN VERIFY SCRIPT-
sed --regexp-extended -i 's/CDataStream ([0-9a-zA-Z_]+)\(SER_[A-Z]+, [A-Z_]+_VERSION\);/DataStream \1{};/g' $( git grep -l CDataStream)
sed -i 's/, CDataStream/, DataStream/g' src/wallet/walletdb.cpp
-END VERIFY SCRIPT-
|
|
|
|
sendcoins dialog
a582b4141f0756faa3793fb1c772898a984c83e4 gui: send, left alignment for "bytes" and "change" label (furszy)
210ef1e980e0887c5675b66bcd5cbccd00aceec6 qt: remove confusing "Dust" label from coincontrol / sendcoins dialog (Sebastian Falbesoner)
Pull request description:
In contrast to to all other labels on the coin selection dialog, the displayed dust information has nothing to do with the selected coins. All that this label shows is whether at least one of the _outputs_ qualify as dust, but the outputs are set in a different dialog. (Even worse, the dust check is currently simply wrong because it only looks at an output's nValue and just assumes a P2PKH script size.)
As the label clearly doesn't help the user and is, quite the contrary, rather increasing confusion/misguidance, it seems sensible to remove it. The label from the sendcoins dialog is also removed with the same rationale. Additionally, the "bytes" and "change" labels are aligned to the left (second commit).
Closes https://github.com/bitcoin-core/gui/issues/699.
ACKs for top commit:
furszy:
ACK a582b41
hebasto:
Looks good. ACK a582b4141f0756faa3793fb1c772898a984c83e4.
Tree-SHA512: ebc00b68bdeab69f6ab643e4b89301a7e3d04a8a4027b50813314ddddb1387bc97a83313851e375dfbce97751c234686c82af7f4e55fa5ef29f4fed4e8fc11d9
|
|
In contrast to to all other labels on the coin selection dialog, the
displayed dust information has nothing to do with the selected coins.
All that this label shows is whether at least one of the _outputs_
qualify as dust, but the outputs are set in a different dialog.
(Even worse, the dust check is currently simply wrong because it only
looks at an output's nValue and just assumes a P2PKH script size.)
As the label clearly doesn't help the user and is, quite the contrary,
rather increasing confusion/misguidance, it seems sensible to remove it.
Also, remove the label from the sendcoins dialog with the same rationale.
|
|
|
|
The previous behavior for getAvailableBalance when coin control
has selected coins was to return the sum of them. Instead, we
are currently returning the wallet's available total balance minus
the selected coins total amount.
This turns into a GUI-only issue for the "use available balance"
button when the user manually select coins in the send screen.
Reason:
We missed to update the GetAvailableBalance function to include
the coin control selected coins on #25685.
Context:
Since #25685 we skip the selected coins inside `AvailableCoins`,
the reason is that there is no need to traverse the wallet's
txes map just to get coins that can directly be fetched by
their id.
|
|
|
|
|
|
|
|
|
|
9a1d73fdffa4f35e33bc187ac9b3afbba28b1950 Fix segfault when shutdown during wallet open (John Moffett)
Pull request description:
Fixes #689
## Summary
If you open a wallet and send a shutdown signal during that process, you'll get a segfault when the wallet finishes opening. That's because the `WalletController` object gets deleted manually in bitcoin.cpp during shutdown, but copies of the pointer (and pointers to child objects) are dangling in various places and are accessed in queued events after the deletion.
## Details
The issue in #689 is caused by the following sequence of events:
1. Wallet open modal dialog is shown and worker thread does the actual work.
2. Every 200ms, the main event loop checks to see if a shutdown has been requested, but only if a modal is not being shown.
3. Request a shutdown while the modal window is shown.
4. The wallet open process completes, the modal window is dismissed, and various `finish` signals are sent.
5. During handling of one of the `finish` signals, `qApp->processEvents()` is [called](https://github.com/bitcoin-core/gui/blob/e9262ea32a6e1d364fb7974844fadc36f931f8c6/src/qt/sendcoinsdialog.cpp#L603), which causes the main event loop to detect the shutdown (now that the modal window has been dismissed). The `WalletController` and all the `WalletModel`s are [deleted](https://github.com/bitcoin-core/gui/blob/65de8eeeca29e71378aa34602b287ab921b040e4/src/qt/bitcoin.cpp#L394-L401).
6. Control returns to the `finish` method, which eventually tries to send a [signal](https://github.com/bitcoin-core/gui/blob/e9262ea32a6e1d364fb7974844fadc36f931f8c6/src/qt/sendcoinsdialog.cpp#L167) from a wallet model, but it's been deleted already (and the signal is sent from a now-[dangling](https://github.com/bitcoin-core/gui/blob/d8bdee0fc889def7c5f5076da13db4fce0a3728a/src/qt/walletview.cpp#L65) pointer).
The simplest fix for that is to change the `qApp->processEvents()` into a `QueuedConnection` call. (The `qApp->processEvents() was a [workaround](https://github.com/bitcoin/bitcoin/pull/593#issuecomment-3050699) to get the GUI to scroll to the last item in a list that just got added, and this is just a safer way of doing that).
However, once that segfault is fixed, another segfault occurs due to some queued wallet events happening after the wallet controller object is deleted here:
https://github.com/bitcoin-core/gui/blob/65de8eeeca29e71378aa34602b287ab921b040e4/src/qt/bitcoin.cpp#L394-L401
Since `m_wallet_controller` is a copy of that pointer in `bitcoingui.cpp`, it's now dangling and `if(null)` checks won't work correctly. For instance, this line:
https://github.com/bitcoin-core/gui/blob/65de8eeeca29e71378aa34602b287ab921b040e4/src/qt/bitcoingui.cpp#L413
sets up a `QueuedConnection` to `setCurrentWallet`, but by the time control reaches that method (one event cycle after shutdown deleted `m_wallet_controller` in `bitcoin.cpp`), the underlying objects have been destroyed (but the pointers are still dangling).
Ideally, we'd use a `QPointer` or `std::shared_ptr / std::weak_ptr`s for these, but the changes would be more involved.
This is a minimal fix for the issues. Just set `m_wallet_controller` to `nullptr` in `bitcoingui.cpp`, check its value in a couple places, and avoid a use of `qApp->processEvents`.
ACKs for top commit:
hebasto:
ACK 9a1d73fdffa4f35e33bc187ac9b3afbba28b1950, I have reviewed the code and it looks OK.
furszy:
ACK https://github.com/bitcoin-core/gui/commit/9a1d73fdffa4f35e33bc187ac9b3afbba28b1950
Tree-SHA512: a1b94676eb2fcb7606e68fab443b1565b4122aab93c35382b561842a049f4b43fecc459535370d67a64d6ebc4bcec0ebcda981fff633ebd41bdba6f7093ea540
|
|
dependency
52f4d567d69425dfd514489079db80483024a80d refactor: remove <util/system.h> include from wallet.h (furszy)
6c9b342c306b9e17024762c4ba8f1c64e9810ee2 refactor: wallet, remove global 'ArgsManager' access (furszy)
d8f5fc446216258a68e256076c889ec23471855f wallet: set '-walletnotify' script instead of access global args manager (furszy)
3477a28dd3b4bc6c1993554c5ce589d69fa86070 wallet: set keypool_size instead of access global args manager (furszy)
Pull request description:
Structurally, the wallet class shouldn't access the global `ArgsManager` class, its internal behavior shouldn't be coupled to a global command line args parsing object.
So this PR migrates the only two places where we depend on it: (1) the keypool size, and (2) the "-walletnotify" script. And cleans up the, now unneeded, wallet `ArgsManager` ref member.
Extra note:
In the process of removing the args ref member, discovered and fixed files that were invalidly depending on the wallet header including `util/system.h`.
ACKs for top commit:
achow101:
ACK 52f4d567d69425dfd514489079db80483024a80d
TheCharlatan:
Re-ACK 52f4d567d69425dfd514489079db80483024a80d
hebasto:
re-ACK 52f4d567d69425dfd514489079db80483024a80d
Tree-SHA512: 0cffd99b4dd4864bf618aa45aeaabbef2b6441d27b6dbb03489c4e013330877682ff17b418d07aa25fbe1040bdf2c67d7559bdeb84128c5437bf0e6247719016
|
|
Since we no longer store a ref to the global `ArgsManager`
inside the wallet, we can move the util/system.h
include to the cpp.
This dependency removal opened a can of worms, as few
other places were, invalidly, depending on the wallet's
header including it.
|
|
fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e gui: Show watchonly balance only for Legacy wallets (Andrew Chow)
Pull request description:
Descriptor wallets do not have a watchonly balance as wallets are designated watchonly or not. Thus we should not be displaying the empty watchonly balance for descriptor wallets.
The result is that instead of the send page showing "Watch-only balance: 0.00000000 BTC" for watchonly descriptor wallets, we see the actual balance as "Balance: 10.00000000 BTC"
ACKs for top commit:
johnny9:
tACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e
furszy:
ACK fdb8dc8a
hebasto:
ACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e
Tree-SHA512: e5c0703a62d25c881c8dadfb9cffd482791f3d437a4ec5ae0088ce1a2069c2455ad6d3ec6c95a4404a3b55fbd727f92694529c35052236951553ca90c4ed31b5
|
|
7b7cd112444b996a8ae6a6edfed00bcee67546c8 clang-tidy, qt: Force checks for headers in `src/qt` (Hennadii Stepanov)
69eacf2c5ee1c84e92153b525fd4302aec0f5f2a clang-tidy, qt: Fix `modernize-use-default-member-init` in headers (Hennadii Stepanov)
Pull request description:
This PR split from bitcoin/bitcoin#26705 and contains only changes in `src/qt`.
Effectively, it fixes the clang-tidy's `modernize-use-default-member-init` errors, and forces clang-tidy checks for all headers in the `src/qt` directory.
ACKs for top commit:
jarolrod:
ACK 7b7cd112444b996a8ae6a6edfed00bcee67546c8
Tree-SHA512: 79525bb0f31ae7cad88c781e55091a21467c0485ddc1ed03ad62e051480fda3b3710619ea11af480437edba3c6e038f7c40edc6b373e3a37408c006d11b34686
|
|
If you open a wallet and send a shutdown signal during
that process, the GUI will segfault due to some queued
wallet events happening after the wallet controller
is deleted. This is a minimal fix for those issues.
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html
|
|
no need to waste resources calculating the wallet available coins if
they are not going to be used.
The 'm_allow_other_inputs=true` default value change is to correct
an ugly misleading behavior:
The tx creation process was having a workaround patch to automatically
fall back to select coins from the wallet if `m_allow_other_inputs=false`
(previous default value) and no manual inputs were selected.
This could be seen in master in flows like `sendtoaddress`, `sendmany`
and even the GUI, where the `m_allow_other_inputs` value isn't customized
and the wallet still selects and adds coins to the tx internally.
|
|
|
|
model cache
4584d300a40bfd84517072f7a6eee114fb7cab08 GUI: remove now unneeded 'm_balances' field from overviewpage (furszy)
050e8b139145d6991e740b0e5f2b3364663dd348 GUI: 'getAvailableBalance', use cached balance if the user did not select UTXO manually (furszy)
96e3264a82c51b456703f500bd98e8cb98115697 GUI: use cached balance in overviewpage and sendcoinsdialog (furszy)
321335bf0292034d79afa6c44f7f072942b6cc3c GUI: add getter for WalletModel::m_cached_balances field (furszy)
e62958dc81d215a1c56318d0914dfd9a33d45973 GUI: sendCoinsDialog, remove duplicate wallet().getBalances() call (furszy)
Pull request description:
As per the title says, we are recalculating the entire wallet balance on different situations calling to `wallet().getBalances()`, when should instead make use of the wallet model cached balance.
This has the benefits of (1) not spending resources calculating a balance that we already have cached, and (2) avoid blocking the main thread for a long time, in case of big wallets, walking through the entire wallet's tx map more than what it's really needed.
Changes:
1) Fix: `SendCoinsDialog` was calling `wallet().getBalances()` twice during `setModel`.
2) Use the cached balance if the user did not select any UTXO manually inside the wallet model `getAvailableBalance` call.
-----------------------
As an extra note, this work born in [#25005](https://github.com/bitcoin/bitcoin/pull/25005) but grew out of scope of it.
ACKs for top commit:
jarolrod:
ACK 4584d300a40bfd84517072f7a6eee114fb7cab08
hebasto:
re-ACK 4584d300a40bfd84517072f7a6eee114fb7cab08, only suggested changes and commit message formatting since my [recent](https://github.com/bitcoin-core/gui/pull/598#pullrequestreview-1071268192) review.
Tree-SHA512: 6633ce7f9a82a3e46e75aa7295df46c80a4cd4a9f3305427af203c9bc8670573fa8a1927f14a279260c488cc975a08d238faba2e9751588086fea1dcf8ea2b28
|
|
Descriptor wallets do not have a watchonly balance as wallets are
designated watchonly or not. Thus we should not be displaying the empty
watchonly balance for descriptor wallets.
|
|
UTXO manually
No need to walk through the entire wallet's tx map. Used for 'walletModel::prepareTransaction' and 'useAvailable' flow in sendcoinsdialog.
|
|
Plus, calculate the cached balance right when the wallet model, so the wallet widgets don't need to redo the same balance calculation multiple times when they are waiting for the model balance polling timer.
----------------------------------------------------------------------
test wise: `WalletTests` now need to trigger the walletModel balance changed manually. So the model updates its internal state and can be used by the widgets.
This is because the test does not start the balance polling timer, in the same way as does not initialize several parts of the GUI workflow. All the objects (wallet, models, views, etc) that are used on this test are manually created instead of using the `WalletController` class flow.
Rationale is that this unit test is focused on verifying the GUI widgets/views behavior only: update the presented information, etc. when they receive different signals and/or function calls from outside (in other words, focus is on the signal slots/receiver side). It's not about whether the wallet balance polling timer is functioning as expected or not (which we definitely create a new test case for it in a follow-up work).
|
|
Inside setModel, we call 'wallet().getBalances()', to set the view balance,
right before calling 'updateDisplayUnit' which calls 'wallet().getBalances()'
internally and re-sets the view balance again.
|
|
-BEGIN VERIFY SCRIPT-
sed -i "s|node/ui_interface|node/interface_ui|g" $(git grep -l "node/ui_interface" ./src)
git mv src/node/ui_interface.cpp src/node/interface_ui.cpp
git mv src/node/ui_interface.h src/node/interface_ui.h
sed -i "s|BITCOIN_NODE_UI_INTERFACE_H|BITCOIN_NODE_INTERFACE_UI_H|g" src/node/interface_ui.h
-END VERIFY SCRIPT-
|
|
`WalletModel::sendCoins()` return `void`
1f653dc2623ee2be3e1eeeaf5ce0e97966cecd6a qt, wallet, refactor: Make `WalletModel::sendCoins()` return `void` (Hennadii Stepanov)
Pull request description:
Currently, the `WalletModel::sendCoins()` function always returns the same value.
Also dead and noop (calling `processSendCoinsReturn(OK)`) code has been removed.
The other `return` statements have been removed from the `WalletModel::sendCoins()` function in bitcoin/bitcoin#17154 and bitcoin/bitcoin#17165.
ACKs for top commit:
kristapsk:
cr ACK 1f653dc2623ee2be3e1eeeaf5ce0e97966cecd6a
furszy:
Code review ACK 1f653dc2
shaavan:
Code Review ACK 1f653dc2623ee2be3e1eeeaf5ce0e97966cecd6a
w0xlt:
Code Review ACK https://github.com/bitcoin-core/gui/pull/608/commits/1f653dc2623ee2be3e1eeeaf5ce0e97966cecd6a
Tree-SHA512: 2b59495a7fc10b4de30fcc63fc3af92d50406e16031112eb72494736dce193ac1fbac0802623496cf81edcd16766e1647d9c4f3a607b3eb84cc50e273b999c04
|
|
Also dead code has been removed.
|
|
Currently, the `WalletModel::sendCoins()` function always returns the
same value.
Also dead and noop code has been removed.
|
|
0e5dedbc9eb54105ab9b0c4ce1f57afa55bcb5b6 qt/wallettests: sort includes (William Casarin)
0554251d660caa1c3f5f44ae1d9fa3c23d2aac18 qt: Skip displayUnitChanged signal if unit is not actually changed (Hennadii Stepanov)
ffbc2fe459034024cb2fce9fd94bff457b7a7d49 qt, refactor: Remove default cases for scoped enum (Hennadii Stepanov)
152d5bad50f145af922011f6ec1fd9afd9076ceb qt, refactor: Remove BitcoinUnits::valid function (Hennadii Stepanov)
aa23960fdf1deff321ecea435026c87db78498fb qt, refactor: Make BitcoinUnits::Unit a scoped enum (Hennadii Stepanov)
75832fdc37ea3fe9cf515bd1946e220fe07a440b qt: Use QVariant instead of int for BitcoinUnit in QSettings (Hennadii Stepanov)
Pull request description:
This is a rebased version of #60
Since Qt 5.5 there are [means](https://doc.qt.io/qt-5/qobject.html#Q_ENUM) to register an enum type with the meta-object system (such enum still lacks an ability to interact with [QSettings::setValue()](https://doc.qt.io/qt-5/qsettings.html#setValue) and [QSettings::value()](https://doc.qt.io/qt-5/qsettings.html#value) without defined stream operators).
In order to reduce global namespace polluting and to force strong type checking, this PR makes BitcoinUnits::Unit a scoped enum (typedef BitcoinUnits::Unit BitcoinUnit;).
No behavior change.
ACKs for top commit:
jonatack:
ACK 0e5dedbc9eb54105ab9b0c4ce1f57afa55bcb5b6, review and debug build of each commit after rebase on current master, lightly tested running the GUI, changing units a few times, and verifying persistence after restarting
promag:
Code review ACK 0e5dedbc9eb54105ab9b0c4ce1f57afa55bcb5b6
Tree-SHA512: 39ec0d7e4f0b9b25be287888121a8db6b282339674e37ec3a3554da63a9e22d6fe079e8310ca289b2a0356a19b3c7e55afa17d09dd34e0f222177f603bb053a3
|
|
Before this change the send confirmation dialog would keep the Send option disabled. The Create Unsigned choice would actually send. This is potentially confusing.
With this change the Create Unsigned button will not attempt to sign and always produce a PSBT. The Send button will attempt to sign, and only return a PSBT if more signatures are needed.
When using an external signer, the Create Unsigned option only appears when PSBT controls are enabled in the wallet settings.
This commit maintains the pre-existing behavior of filling the PSBT (without signing) even when not using an external signer.
Closes #551
Co-authored-by: Jon Atack <jon@atack.com>
|
|
Does not change behavior.
Review hint:
git show --color-moved --color-moved-ws=allow-indentation-change
|
|
This commit does not change behavior.
Review hint:
git show --color-moved --color-moved-ws=allow-indentation-change
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/ShowModalDialogAndDeleteOnClose/ShowModalDialogAsynchronously/' -- $(git grep -l -e "ShowModalDialogAndDeleteOnClose")
-END VERIFY SCRIPT-
It is important to highlight that a modal dialog is showed
asynchronously as there are cases when the synchronous QDialog::exec()
is required.
|
|
Warning: Replacing fs::system_complete calls with fs::absolute calls
in this commit may cause minor changes in behaviour because fs::absolute
no longer strips trailing slashes; however these changes are believed to
be safe.
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
Part of #20744, but this can be done now, and will simplify the diff.
|
|
QTimer methods
51250b0906e56b39488304208ad119c951b4ae7d refactor, qt: Use std::chrono for input_filter_delay constant (Hennadii Stepanov)
f3bdc143b67e8a5e763071a0774f6d994ca35c57 refactor, qt: Add SHUTDOWN_POLLING_DELAY constant (Hennadii Stepanov)
0e193deb523a4fa04e0ee69bd66f917895802ac9 refactor, qt: Use std::chrono for non-zero arguments in QTimer methods (Hennadii Stepanov)
6f0da958116ecc0e06332fad2f490e37b6884166 refactor, qt: Use std::chrono in ConfirmMessage parameter (Hennadii Stepanov)
33d520ac538fcd6285fd958578f1bd26295592e4 refactor, qt: Use std::chrono for MODEL_UPDATE_DELAY constant (Hennadii Stepanov)
Pull request description:
Since Qt 5.8 `QTimer` methods have overloads that accept `std::chrono::milliseconds` arguments:
- [`QTimer::singleShot`](https://doc.qt.io/archives/qt-5.9/qtimer.html#singleShot-8)
- [`QTimer::start`](https://doc.qt.io/archives/qt-5.9/qtimer.html#start-2)
ACKs for top commit:
promag:
Code review ACK 51250b0906e56b39488304208ad119c951b4ae7d.
shaavan:
reACK 51250b0906e56b39488304208ad119c951b4ae7d
Tree-SHA512: aa843bb2322a84c0c2bb113d3b48d7bf02d7f09a770779dcde312c32887f973ef9445cdef42f39edaa599ff0f3d0457454f6153aa130efadd989e413d39c6062
|
|
`node::` and `wallet::` namespaces
e5b6aef61221b621ad77b5f075a16897e08835bf Move CBlockFileInfo::ToString method where class is declared (Russell Yanofsky)
f7086fd8ff084ab0dd656d75b7485e59263bdfd8 Add src/wallet/* code to wallet:: namespace (Russell Yanofsky)
90fc8b089d591cabff60ee829a33f96c37fd27ba Add src/node/* code to node:: namespace (Russell Yanofsky)
Pull request description:
There are no code changes, this is just adding `namespace` and `using` declarations and `node::` or `wallet::` qualifiers in some places.
Motivations for this change are:
- To make it easier to see when node and wallet code is being accessed places where it shouldn't be. For example if GUI code is accessing node and wallet internals or if wallet and node code are referencing each other.
- To make source code organization clearer ([#15732](https://github.com/bitcoin/bitcoin/issues/15732)), being able to know that `wallet::` code is in `src/wallet/`, `node::` code is in `src/node/`, `init::` code is in `src/init/`, `util::` code is in `src/util/`, etc.
Reviewing with `git log -p -n1 -U0 --word-diff-regex=.` can be helpful to verify this is only updating declarations, not changing code.
ACKs for top commit:
achow101:
ACK e5b6aef61221b621ad77b5f075a16897e08835bf
MarcoFalke:
Concept ACK e5b6aef61221b621ad77b5f075a16897e08835bf 🍨
Tree-SHA512: 3797745c90246794e2d55a2ee6e8b0ad5c811e4e03a242d3fdfeb68032f8787f0d48ed4097f6b7730f540220c0af99ef423cd9dbe7f76b2ec12e769a757a2c8d
|
|
|
|
742918c8ef353993a07c060f476a160e8272a9ef qt: hide Create Unsigned button behind an expert mode option (Andrew Chow)
5c3b800acd3ceb75ff6bbac8d0e2e1aaa95b0728 qt: Add Create Unsigned button to SendConfirmationDialog (Andrew Chow)
Pull request description:
Instead of having different buttons or changing button behavior for making a PSBT, just have SendConfirmationDialog return whether the user wants a PSBT or a broadcasted transaction. Since this dialog is used by both the bumpFeeAction and the SendCoinsDialog, changes to both to support the different behavior is needed. They will check the return value of the SendConfirmationDialog for whether a PSBT needs to be created instead of checking whether private keys are disabled.
Strings used in this dialog are being slightly modified to work with both private keys enabled and disabled wallets.
Moved from https://github.com/bitcoin/bitcoin/pull/18789
ACKs for top commit:
jarolrod:
ACK 742918c
ryanofsky:
Code review ACK 742918c8ef353993a07c060f476a160e8272a9ef. Just suggested changes since last review. Looks great!
hebasto:
ACK 742918c8ef353993a07c060f476a160e8272a9ef, tested on Linux Mint 20.2 (Qt 5.12.8).
Tree-SHA512: dd29f4364c7b4f15befe8fe63257b26187918786b005e0f8336183270b1a162680b93f6ced60f0285c6e607c084cc0d24950fc68a8f9c056521ede614041be66
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
Instead of having different buttons or changing button behavior for
making a PSBT, just have SendConfirmationDialog return whether the user
wants a PSBT or a broadcasted transaction. Since this dialog is used
by both the bumpFeeAction and the SendCoinsDialog, changes to both
to support the different behavior is needed. They will check
the return value of the SendConfirmationDialog for whether a PSBT
needs to be created instead of checking whether private keys are
disabled.
Strings used in this dialog are being slightly modified to work with
both private keys enabled and disabled wallets.
|
|
shutdown
451ca244db8bc71ffc3cc9982d025f144cc8f3bc qt, refactor: Drop intermediate BitcoinApplication::shutdownResult slot (Hennadii Stepanov)
f3a17bbe5f7d23b6ecc20e363920492b50859dad qt: Do not exit and re-enter main event loop during shutdown (Hennadii Stepanov)
b4e0d2c43181ad97c15b252e95181e2c3f6c1d2a qt, refactor: Allocate SendConfirmationDialog instances on heap (Hennadii Stepanov)
332dea2852d9c68f900ed1f0be99b6cea79c7457 qt, refactor: Keep HelpMessageDialog in the main event loop (Hennadii Stepanov)
c8bae37a7a646badf8e79669bf06ac174e13cd6f qt, refactor: Keep PSBTOperationsDialog in the main event loop (Hennadii Stepanov)
7fa91e831227e556bd8a7ae3da64bd59d4f30d5f qt, refactor: Keep AskPassphraseDialog in the main event loop (Hennadii Stepanov)
6f6fde30e7601185a8f6052b3bf1770407fcc14b qt, refactor: Keep EditAddressDialog in the main event loop (Hennadii Stepanov)
59f7ba4fd7a9e4bc73d784ee74d5b777da9cc436 qt, refactor: Keep CoinControlDialog in the main event loop (Hennadii Stepanov)
7830cd0b35f315570d744f4d2719104c08b33ff1 qt, refactor: Keep OptionsDialog in the main event loop (Hennadii Stepanov)
13f618818dc57673ac0287ad8b28ceb450efb374 qt: Add GUIUtil::ShowModalDialogAndDeleteOnClose (Hennadii Stepanov)
Pull request description:
On master (1ef34ee25ed34b2b092f15bf3dca5c0508092829) during shutdown `QApplication` exits the main event loop, then re-enter again.
This PR streamlines shutdown process by removing the need to interrupt the main event loop, that is required for #59.
Also, blocking [`QDialog::exec()`](https://doc.qt.io/qt-5/qdialog.html#exec) calls are replaced with safer [`QDialog::show()`](https://doc.qt.io/qt-5/qwidget.html#show), except for `SendConfirmationDialog` as that change is not trivial (marked as TODO).
The [`QDialog::open()`](https://doc.qt.io/qt-5/qdialog.html#open) was not used because the actual modality mode (application modal or window modal) of a dialog depends on whether it has a parent.
This PR does not change behavior, and all touched dialogs are still application modal.
As a follow up, a design research could suggest to make some dialogs window modal.
NOTE for reviewers: quitting app while a dialog is open (e.g., via systray icon menu) must work fine.
ACKs for top commit:
laanwj:
Code review and lighly tested ACK 451ca244db8bc71ffc3cc9982d025f144cc8f3bc
promag:
ACK 451ca244db8bc71ffc3cc9982d025f144cc8f3bc, just changed signal to `quitRequested`.
Tree-SHA512: ef01ab6ed803b202e776019a4e1f592e816f7bc786e00574b25a0bf16be2374ddf9db21f0a26da08700df7ef0ab9e879550df46dcfe3b6d940f5ed02ca5f8447
|