aboutsummaryrefslogtreecommitdiff
path: root/src/qt
AgeCommit message (Collapse)Author
2023-02-21Pass all characters to SecureString including nullsJohn Moffett
`SecureString` is a `std::string` specialization with a secure allocator. However, it's treated like a C- string (no explicit length and null-terminated). This can cause unexpected behavior. For instance, if a user enters a passphrase with an embedded null character (which is possible through Qt and the JSON-RPC), it will ignore any characters after the null, giving the user a false sense of security. Instead of assigning `SecureString` via `std::string::c_str()`, assign it via a `std::string_view` of the original. This explicitly captures the size and doesn't make any extraneous copies in memory.
2023-02-17Merge bitcoin/bitcoin#25619: net: avoid overriding non-virtual ToString() in ↵Andrew Chow
CService and use better naming c9d548c91fb12fba516dee896f1f97692cfa2104 net: remove CService::ToStringPort() (Vasil Dimov) fd4f0f41e915d99c9b0eac1afd21c5628222e368 gui: simplify OptionsDialog::updateDefaultProxyNets() (Vasil Dimov) 96c791dd20fea54c17d224000dee677bc158f66a net: remove CService::ToString() use ToStringAddrPort() instead (Vasil Dimov) 944a9de08a00f8273e73cd28b40e46cc0eb0bad1 net: remove CNetAddr::ToString() and use ToStringAddr() instead (Vasil Dimov) 043b9de59aec88ae5e29daac7dc2a8b51a9414ce scripted-diff: rename ToStringIP[Port]() to ToStringAddr[Port]() (Vasil Dimov) Pull request description: Before this PR we had the somewhat confusing combination of methods: `CNetAddr::ToStringIP()` `CNetAddr::ToString()` (duplicate of the above) `CService::ToStringIPPort()` `CService::ToString()` (duplicate of the above, overrides a non-virtual method from `CNetAddr`) `CService::ToStringPort()` Avoid [overriding non-virtual methods](https://github.com/bitcoin/bitcoin/pull/25349/#issuecomment-1185226396). "IP" stands for "Internet Protocol" and while sometimes "IP addresses" are called just "IPs", it is incorrect to call Tor or I2P addresses "IPs". Thus use "Addr" instead of "IP". Change the above to: `CNetAddr::ToStringAddr()` `CService::ToStringAddrPort()` The changes touch a lot of files, but are mostly mechanical. ACKs for top commit: sipa: utACK c9d548c91fb12fba516dee896f1f97692cfa2104 achow101: ACK c9d548c91fb12fba516dee896f1f97692cfa2104 jonatack: re-ACK c9d548c91fb12fba516dee896f1f97692cfa2104 only change since my previous reviews is rebase, but as a sanity check rebased to current master and at each commit quickly re-reviewed and re-verified clean build and green unit tests LarryRuane: ACK c9d548c91fb12fba516dee896f1f97692cfa2104 Tree-SHA512: 633fb044bdecf9f551b5e3314c385bf10e2b78e8027dc51ec324b66b018da35e5b01f3fbe6295bbc455ea1bcd1a3629de1918d28de510693afaf6a52693f2157
2023-02-17Merge bitcoin/bitcoin#26889: refactor: wallet, remove global 'ArgsManager' ↵Andrew Chow
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
2023-02-15Merge bitcoin-core/gui#711: refactor: Disable unused special members ↵Andrew Chow
functions in `UnlockContext` 9fa43b5af6b180f4b5f76726f443ee60259d2cd0 refactor: Disable unused special members functions in `UnlockContext` (Hennadii Stepanov) Pull request description: Also `UnlockContext::valid` and `UnlockContext::relock` are `const` now. ACKs for top commit: achow101: ACK 9fa43b5af6b180f4b5f76726f443ee60259d2cd0 john-moffett: ACK 9fa43b5af6b180f4b5f76726f443ee60259d2cd0 furszy: ACK 9fa43b5a Tree-SHA512: 6d9fa8208676b9bd5d85b73cb2d3136e7f28ef59e68ee34915ec598458868e302a80b9ef1384c0bf7a4c42f936830c3add9662ca0bae73860a55a25cc374b699
2023-02-15refactor: remove <util/system.h> include from wallet.hfurszy
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.
2023-02-15refactor: wallet, remove global 'ArgsManager' accessfurszy
we are not using it anymore
2023-02-15Merge bitcoin-core/gui#603: Add settings.json prune-prev, proxy-prev, ↵Hennadii Stepanov
onion-prev settings 9d3127b11e34131409dab7c08bde5b444d90b2cb Add settings.json prune-prev, proxy-prev, onion-prev settings (Ryan Ofsky) Pull request description: With #602, if proxy and pruning settings are disabled in the GUI and the GUI is restarted, proxy and prune values are not stored anywhere. So if these settings are enabled in the future, default values will be shown, not previous values. This PR stores previous values so they will preserved across restarts. I'm not sure I like this behavior because showing default values seems simpler and safer to me. Previous values may just have been set temporarily and may have never actually worked, and it adds some code complexity to store them. This PR is one way of resolving #596. Other solutions are possible and could be implemented as alternatives. ACKs for top commit: hebasto: ACK 9d3127b11e34131409dab7c08bde5b444d90b2cb, tested on Ubuntu 22.04. vasild: ACK 9d3127b11e34131409dab7c08bde5b444d90b2cb jarolrod: tACK 9d3127b11e34131409dab7c08bde5b444d90b2cb Tree-SHA512: 1778d1819443490c880cfd5c1711d9c5ac75ea3ee8440e2f0ced81d293247163a78ae8aba6027215110aec6533bd7dc6472aeead6796bfbd51bf2354e28f24a9
2023-02-14refactor: Disable unused special members functions in `UnlockContext`Hennadii Stepanov
2023-02-09Merge bitcoin-core/gui#701: Persist Mask Values optionHennadii Stepanov
4de02def844102c08b65bf1311a333e7aca482b9 qt: Persist Mask Values option (Andrew Chow) Pull request description: The mask values option is memory only. If a user has enabled this option, it's reasonable to expect that they would want to have it enabled on the next start. ACKs for top commit: RandyMcMillan: tACK https://github.com/bitcoin-core/gui/commit/4de02def844102c08b65bf1311a333e7aca482b9 jarolrod: tACK 4de02def844102c08b65bf1311a333e7aca482b9 pablomartin4btc: > tACK [4de02de](https://github.com/bitcoin-core/gui/commit/4de02def844102c08b65bf1311a333e7aca482b9) john-moffett: tACK 4de02def844102c08b65bf1311a333e7aca482b9 Tree-SHA512: 247deb78df4911516625bf8b25d752feb480ce30eb31335cf9baeb07b7c6c225fcc37d5c45de62d6e6895ec10c7eefabb15527e3c9723a3b8ddda1e12ebbf46b
2023-02-07Remove reindex special case from the progress bar labelMarcoFalke
2023-02-03Merge bitcoin-core/gui#653: Show watchonly balance only for Legacy walletsHennadii Stepanov
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
2023-02-03Merge bitcoin-core/gui#705: doc: Fix comment about how wallet txs are sortedHennadii Stepanov
c497a198db6f417d2612078a9fbc101e259fab33 Fix comment about how wallet txs are sorted (John Moffett) Pull request description: The wallet transactions in the node are not sorted by txid (or any hash) since https://github.com/bitcoin/bitcoin/pull/24699. This is how they're stored in memory now: https://github.com/bitcoin-core/gui/blob/835212cd1d8f8fc7f19775f5ff8cc21c099122b2/src/wallet/wallet.h#L397-L399 ACKs for top commit: achow101: ACK c497a198db6f417d2612078a9fbc101e259fab33 jarolrod: ACK c497a198db6f417d2612078a9fbc101e259fab33 Tree-SHA512: e72559991688452ef254474d4235dc75fac655bce04909c3a0eece907360f4c6f57707db9b4373a4bd2271b23c57e863684c33e0728adf48e477c5499cdfdad7
2023-02-02Merge bitcoin-core/gui#695: Fix misleading RPC console wallet messageHennadii Stepanov
576f7b86147447215566f0b15ef0b56cd1282929 Fix misleading RPC console wallet message (John Moffett) Pull request description: ## Misleading message from RPCConsole window ## In certain circumstances, the GUI console will display the message 'Executing command without any wallet' when it is, in fact, using the currently loaded wallet. For instance: ![scr3](https://user-images.githubusercontent.com/116917595/211404066-d49a6cbf-d3c3-4e89-8720-3583c6acf521.gif) In RPC calls, if no wallet is explicitly selected and there is exactly one wallet loaded, the [default](https://github.com/bitcoin-core/gui/blob/39363a4b945114f5e4718f75098f3036e8fe6a1d/src/wallet/rpc/util.cpp#L71-L93) is to act on that loaded wallet. The GUI console acts that way in reality, but sometimes erroneously reports that it's not acting on any particular wallet. The root issue is due to the logic that prevents changing the selected wallet if the RPCConsole is visible: https://github.com/bitcoin-core/gui/blob/39363a4b945114f5e4718f75098f3036e8fe6a1d/src/qt/rpcconsole.cpp#L783-L786 This PR removes that unnecessary logic. This does have some ramifications. Prior to this PR, if a user opened the console window without any wallets loaded, then opened two or more wallets, the RPC console would select "None" of the wallets and any wallet-specific RPCs would fail. However, the behavior was different if the user hadn't had the console window open. In that case, if they opened the RPC Console window _after_ loading at least the first wallet, it would select the first-loaded wallet. This context-dependent behavior is (IMO) undesirable, and this PR changes it to be consistent. ACKs for top commit: hebasto: ACK 576f7b86147447215566f0b15ef0b56cd1282929, tested on Ubuntu 22.04 (Qt 5.15.3). Tree-SHA512: 627da186025ba4f4e8df7fdd1b10363f923c4ecc50f023bbf2aece6e2593da65c45147c933effaca9040f813a6e46f034fc2d1ee2fb0f401000a3a6221a0e36e
2023-02-02Merge bitcoin-core/gui#704: Correctly limit overview transaction listHennadii Stepanov
08209c039ff4ca5be4982da7a2ab7a624117ce1a Correctly limit overview transaction list (John Moffett) Pull request description: Fixes #703 The way the main overview page limits the number of transactions displayed (currently 5) is not an appropriate use of Qt. Our subclassed transaction sort/filter proxy model returns a maximum of `5` in `rowCount()`. However, the model itself actually may hold significantly more. While this has _worked_, it breaks the contract of `rowCount()`. If `bitcoin-qt` is run with a DEBUG build of Qt, it'll result in an assert-crash in certain relatively common situations (see #703 for details). Instead of artificially limiting the `rowCount()` in the subclassed filter, we can hide/unhide the rows in the displaying `QListView` upon any changes in the sorted proxy filter. I loaded a wallet with 20,000 transactions and did not notice any performance differences between master and this branch. For reference, this is the list I'm referring to: <img width="934" alt="image" src="https://user-images.githubusercontent.com/116917595/214947304-3f289380-3510-487b-80e8-d19428cf2f0f.png"> ACKs for top commit: Sjors: tACK 08209c039ff4ca5be4982da7a2ab7a624117ce1a hebasto: ACK 08209c039ff4ca5be4982da7a2ab7a624117ce1a, tested on Ubuntu 22.04. Tree-SHA512: c2a7b1a2a6e6ff30694830d7c722274c4c47494a81ce9ef25f8e5587c24871b02343969f4437507693d4fd40ba7a212702b159cf54b3357d8d76c02bc8245113
2023-02-01Fix clang-tidy readability-const-return-type violationsMarcoFalke
2023-01-27Fix comment about how wallet txs are sortedJohn Moffett
The wallet transactions in the node are not sorted by hash. Also make comment Doxygen compatible.
2023-01-26Correctly limit overview transaction listJohn Moffett
The way that the main overview page limits the number of transactions displayed (currently 5) is not an appropriate use of Qt. If it's run with a DEBUG build of Qt, it'll result in a segfault in certain relatively common situations. Instead of artificially limiting the rowCount() in the subclassed proxy filter, we hide/unhide the rows in the displaying QListView upon any changes in the sorted proxy filter.
2023-01-26Use DataStream where possibleMarcoFalke
2023-01-23qt: Persist Mask Values optionAndrew Chow
The mask values option is memory only. If a user has enabled this option, it's reasonable to expect that they would want to have it enabled on the next start.
2023-01-17Merge bitcoin-core/gui#686: clang-tidy: Force checks for headers in `src/qt`Hennadii Stepanov
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
2023-01-15Merge bitcoin-core/gui#690: Catch invalid networks combination crashHennadii Stepanov
f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e gui: bugfix, catch invalid networks combination crash (furszy) Pull request description: The app currently crashes if a network is set inside bitcoin.conf and another one is provided as param. The reason is an uncaught runtime_error. ACKs for top commit: jarolrod: tACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e johnny9: tACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e john-moffett: ACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e pablomartin4btc: Tested ACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e. hebasto: ACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e, tested on Ubuntu 22.04 (Qt 5.15.3). Tree-SHA512: fc5e26ae0a361e37d53d904cc122d07f064f261b309629c6386cb046ab1b3d2c805cbfe0db8ed3e934af52c6cf0ebb0bef9df9117b4330d9b0ea40c76f9270f9
2023-01-11Merge bitcoin/bitcoin#26758: refactor: Add `performance-no-automatic-move` ↵MarcoFalke
clang-tidy check 9567bfeab95cc0932073641dd162903850987d43 clang-tidy: Add `performance-no-automatic-move` check (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#26642 as [requested](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1054673201). For the problem description see https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html. The following types are affected: - `std::pair<CAddress, NodeSeconds>` - `std::vector<CAddress>` - `UniValue`, also see bitcoin/bitcoin#25429 - `QColor` - `CBlock` - `MempoolAcceptResult` - `std::shared_ptr<CWallet>` - `std::optional<SelectionResult>` - `CTransactionRef`, which is `std::shared_ptr<const CTransaction>` ACKs for top commit: andrewtoth: ACK 9567bfeab95cc0932073641dd162903850987d43 aureleoules: ACK 9567bfeab95cc0932073641dd162903850987d43 Tree-SHA512: 9b6a5d539205b41d2c86402d384318ed2e1d89e66333ebd200a48fd7df3ce6f6c60a3e989eda5cc503fb34b8d82526f95e56776e1af51e63b49e3a1fef72dbcb
2023-01-09Fix misleading RPC console wallet messageJohn Moffett
In certain circumstances, the GUI console will display the message 'Executing command without any wallet' when it is, in fact, using the default wallet. In RPC calls, if no wallet is explicitly selected and there is exactly one wallet loaded, the default is to act on that loaded wallet. The GUI console acts that way in reality, but erroneously reports that it's not acting on any particular wallet.
2023-01-08Fix segfault when shutdown during wallet openJohn Moffett
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.
2023-01-03Merge bitcoin/bitcoin#26661: wallet: Coin Selection, return accurate error ↵Andrew Chow
messages 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 gui: create tx, launch error dialog if backend throws runtime_error (furszy) f4d79477ff0946b0bd340ade9251fa38e3b95dd7 wallet: coin selection, add duplicated inputs checks (furszy) 0aa065b14e67592d5be8f46ebbe5d59a083ff0a5 wallet: return accurate error messages from Coin Selection (furszy) 7e8340ab1a970a14e180b1fcf420b46a5657b062 wallet: make SelectCoins flow return util::Result (furszy) e5e147fe97f706e82bc51358f8bdc355f355be57 wallet: refactor eight consecutive 'AttemptSelection' calls into a loop (furszy) Pull request description: Work decoupled from #25806, which cleanup and improves the Coin Selection flow further. Adding the capability to propagate specific error messages from the Coin Selection process to the user. Instead of always returning the general "Insufficient funds" message which is not always accurate to what happened internally. Letting us instruct the user how to proceed under certain circumstances. The following error messages were added: 1) If the selection result exceeds the maximum transaction weight, we now will return: -> "The inputs size exceeds the maximum weight. Please try sending a smaller amount or manually consolidating your wallet's UTXOs". 2) If the user pre-selected inputs and disallowed the automatic coin selection process (no other inputs are allowed), we now will return: -> "The preselected coins total amount does not cover the transaction target. Please allow other inputs to be automatically selected or include more coins manually". 3) The double-counted preset inputs during Coin Selection error will now throw an "internal bug detected" message instead of crashing the node. The essence of this work comes from several comments: 1. https://github.com/bitcoin/bitcoin/pull/26560#discussion_r1037395665 2. https://github.com/bitcoin/bitcoin/pull/25729#discussion_r940619491 3. https://github.com/bitcoin/bitcoin/pull/25269#pullrequestreview-1135240825 4. https://github.com/bitcoin/bitcoin/issues/23144 (which is connected to #24845) ACKs for top commit: ishaanam: crACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 achow101: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 aureleoules: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 theStack: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 :city_sunrise: Tree-SHA512: 9de30792d7a5849cae77747aa978e70390b66ee9d082779a56088a024f82e725b0af050e6603aece0ac8229f6d73bc471ba97b4ab69dc7eddf419f5f56ae89a5
2022-12-27clang-tidy: Add `performance-no-automatic-move` checkHennadii Stepanov
https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html
2022-12-26gui: bugfix, catch invalid networks combination crashfurszy
We shouldn't crash if a network is set inside bitcoin.conf and another one is provided as param.
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-21gui: create tx, launch error dialog if backend throws runtime_errorfurszy
only will ever happen if something unexpected happened.
2022-12-21Merge bitcoin-core/gui#687: Load PSBTs using istreambuf_iterator rather than ↵MarcoFalke
istream_iterator bb5ea1d9a954b7b9f443ee8fbbb04549cd0b08a7 qt: Load PSBTs using istreambuf_iterator rather than istream_iterator (Andrew Chow) Pull request description: `istream_iterator` eats whitespace charactesr which causes parsing failures for PSBTs that contain the bytes corresponding to those characters. `istreambuf_iterator` is the correct thing to use here. This is a regression in 24.0. https://github.com/bitcoin/bitcoin/pull/25001 accidentally changed the original `istreambuf_iterator` to `istream_iterator`. ACKs for top commit: furszy: Tested ACK bb5ea1d9 MarcoFalke: review ACK bb5ea1d9a954b7b9f443ee8fbbb04549cd0b08a7 🍇 Tree-SHA512: 35d90eee3efdcb6a360af69ac1727f9f2837ea621297196de3136299f5de6d9975df4e425e1fc5b8813c1ddb2a4d60c3969e1d5d968953a4628ca45e37d3bf05
2022-12-20Merge bitcoin/bitcoin#21576: rpc, gui: bumpfee signer supportAndrew Chow
2c07cfacd1745844a1d3c57f2e8617549b9815d7 gui: bumpfee signer support (Sjors Provoost) 7e02a3329797211ed5d35e5f5e7b919c099b78ba rpc: bumpfee signer support (Sjors Provoost) 304ece994504220c355577170409b9200941f2af rpc: document bools in FillPSBT() calls (Sjors Provoost) Pull request description: The `bumpfee` RPC call and GUI fee bump interface now work with an external signer. ACKs for top commit: achow101: ACK 2c07cfacd1745844a1d3c57f2e8617549b9815d7 furszy: code review ACK 2c07cfac jarolrod: tACK 2c07cfa Tree-SHA512: 0c7b931f76fac67c9e33b9b935f29af6f69ac67a5ffcc586ed2f1676feac427735b1d971723b29ef332bb6fb5762949598ebbf728587e8f0ded95a9bfbb3e7a4
2022-12-20Merge bitcoin-core/gui#605: Delete splash screen widget earlyHennadii Stepanov
1b228497fa729c512a15bdfa80f61a610abfe8a5 qt: Drop no longer used `SplashScreen::finish()` slot (Hennadii Stepanov) 10811afff40efed1fda7eecab89884eaadd7146c qt: Drop no longer used `BitcoinApplication::splashFinished()` signal (Hennadii Stepanov) 5299cfe371ddad806b1c4538d17cde069e25b1c1 qt: Delete splash screen widget explicitly (Hennadii Stepanov) Pull request description: Fixes bitcoin-core/gui#604. Fixes bitcoin/bitcoin#25146. Fixes bitcoin/bitcoin#26340. `SplashScreen::deleteLater()` [does not guarantee](https://doc.qt.io/qt-5/qobject.html#deleteLater) deletion of the `m_splash` object prior to the wallet context deletion. If the latter happens first, the [segfault](https://github.com/bitcoin-core/gui/issues/604#issuecomment-1133907013) follows. ACKs for top commit: dooglus: ACK https://github.com/bitcoin-core/gui/commit/1b228497fa729c512a15bdfa80f61a610abfe8a5 furszy: ACK 1b228497 john-moffett: ACK 1b228497fa729c512a15bdfa80f61a610abfe8a5 Tree-SHA512: bb01d0bf2051f5b184dc415c4f5d32dfb7b8bd772feff7ec7754ded4c6482de27f004b9712df7d53c5ee82e153f48aef4372e4a49d7bcbbb137f73e9b4947962
2022-12-18qt: Load PSBTs using istreambuf_iterator rather than istream_iteratorAndrew Chow
istream_iterator eats whitespace charactesr which causes parsing failures for PSBTs that contain the bytes corresponding to those characters.
2022-12-16clang-tidy, qt: Fix `modernize-use-default-member-init` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html
2022-12-15clang-tidy: Fix `readability-redundant-string-init` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-string-init.html
2022-12-13Merge bitcoin-core/gui#682: Don't directly delete abandoned txes from GUIHennadii Stepanov
e75d2276324d54a01971afdf531df91748275bd5 Minor fix: Don't directly delete abandoned txes (John Moffett) Pull request description: This fully closes bitcoin/bitcoin#12179. Currently, when a user abandons a transaction by clicking "Abandon Transaction" in the context menu, a call is made to remove it from the GUI view: `model->getTransactionTableModel()->updateTransaction(hashQStr, CT_UPDATED, false);` (The `false` parameter is for `bool showTransaction`) This behavior is probably unwanted, as the transaction is not actually removed from the wallet and would show up again if the node is restarted. However, the previous line, `model->wallet().abandonTransaction(hash);`, changes the underlying model and calls `NotifyTransactionChanged(wtx.GetHash(), CT_UPDATED);`, which queues a signal that eventually calls back to `updateTransaction`, this time with `showTransaction` set to `true`. This runs on a separate thread, so it gets called *after* the 'subsequent' `updateTransaction`. The transaction gets removed from the GUI and immediately added back. In a nutshell, `updateTransaction` gets called twice. The first (direct) call deletes the transaction from the GUI. The second (sent via a queued signal) brings it back to the GUI. The first direct call is redundant and unwanted. Worse, if the `abandonTransaction` call fails for any reason, the transaction still gets removed from the GUI. (This is what caused bitcoin#12179. It can still be triggered if, eg., a user clicks "Abandon Transaction" the moment after a new block is found.) There are no conditions (to my knowledge) where an abandoned transaction should be directly removed from the GUI. If the underlying model changes, the deletion should be reflected anyway by the queued signal to `updateTransaction`. The behavior is borne out by the QT logs. To reproduce, send a transaction with RBF enabled, then bump the fee, then 'abandon transaction' on the first transaction. The logs will show something like this: ``` 2022-11-28T14:48:00Z [qt] GUI: "NotifyTransactionChanged: 2c5811484f1adec92a739a5e70b453b03eaed0f7cc0538fbd0ee1589e586b951 status= 1" 2022-11-28T14:48:00Z [qt] GUI: "TransactionTablePriv::updateWallet: 2c5811484f1adec92a739a5e70b453b03eaed0f7cc0538fbd0ee1589e586b951 1" 2022-11-28T14:48:00Z [qt] GUI: " inModel=1 Index=381-382 showTransaction=0 derivedStatus=2" 2022-11-28T14:48:00Z [qt] GUI: "TransactionTablePriv::updateWallet: 2c5811484f1adec92a739a5e70b453b03eaed0f7cc0538fbd0ee1589e586b951 1" 2022-11-28T14:48:00Z [qt] GUI: " inModel=0 Index=381-381 showTransaction=1 derivedStatus=0" ``` Notice the duplicate `updateWallet` calls with different `showTransaction` values. ACKs for top commit: hebasto: ACK e75d2276324d54a01971afdf531df91748275bd5 jarolrod: tACK e75d2276324d54a01971afdf531df91748275bd5 Tree-SHA512: 00f150f747c2ee1605af861a21d5c3b9773a4a9985e8dab62e48bd32885b1bfa4e8cbf805ad61af77aec9d3ccefaed3f4311a29086aa8c22d55d5326ba68ece6
2022-12-12gui: simplify OptionsDialog::updateDefaultProxyNets()Vasil Dimov
Do not create strings and compare them to check if one `addr:port` equals another. Use `CService::operator==()` instead. `strDefaultProxyGUI` was assigned the same value 3 times. Instead save it in `const CService ui_proxy` at the beginning of the function.
2022-12-12scripted-diff: rename ToStringIP[Port]() to ToStringAddr[Port]()Vasil Dimov
"IP" stands for "Internet Protocol". "IP address" is sometimes shortened to just "IP" or "address". However, Tor or I2P addresses are not "IP addresses", nor "IPs". Thus, use "Addr" instead of "IP" for addresses that could be IP, Tor or I2P addresses: `CService::ToStringIPPort()` -> `CService::ToStringAddrPort()` `CNetAddr::ToStringIP()` -> `CNetAddr::ToStringAddr()` -BEGIN VERIFY SCRIPT- sed -i 's/ToStringIPPort/ToStringAddrPort/g' -- $(git grep -l ToStringIPPort src) sed -i 's/ToStringIP/ToStringAddr/g' -- $(git grep -l ToStringIP src) -END VERIFY SCRIPT-
2022-12-06Merge bitcoin-core/gui#683: doc: Drop no longer relevant commentHennadii Stepanov
5d332da2cfdc67d69a165119c6ff598e29b28afa doc: Drop no longer relevant comment (Hennadii Stepanov) Pull request description: The comment was introduced in 4cf3411056f6a59fc5fe07784b6b6a512d76b046, and since 7e4bd19785ff9120b7242ff9f15231868aaf7db4 it has been no longer relevant. ACKs for top commit: jarolrod: ACK 5d332da2cfdc67d69a165119c6ff598e29b28afa Tree-SHA512: 6d32561336993b1ff7d7c524d090ac52aefb40078ed706ca4c6d5026cc3f63244c49c0e00e45ff192ba0e9f1527faf63249aa18bc8aa677b9e053d387e0f4027
2022-12-05Fixup clang-tidy named argument commentsfanquake
Fix comments so they are checked/consistent. Fix incorrect arguments.
2022-12-02doc: Drop no longer relevant commentHennadii Stepanov
The comment was introduced in 4cf3411056f6a59fc5fe07784b6b6a512d76b046, and since 7e4bd19785ff9120b7242ff9f15231868aaf7db4 it has been no longer relevant.
2022-11-25Minor fix: Don't directly delete abandoned txesJohn Moffett
This fully closes bitcoin#12179. Currently, in the GUI, when a user abandons a transaction, a call is made to remove it from the list, and another signal fires (eventually) that adds it back to the GUI with a trash can icon. There are no conditions where the abandoned transaction should be directly removed from the GUI. If the underlying model changes, the deletion will be reflected anyway.
2022-11-17Merge bitcoin-core/gui#680: Fixes MacOS 13 segfault by preventing certain ↵Hennadii Stepanov
notifications after main window is destroyed 8a5014cd8a05b3ab86ae34a47653a82ce11bdf17 Fixes bitcoin#26490 by preventing notifications (John Moffett) Pull request description: This is a PR to address https://github.com/bitcoin/bitcoin/issues/26490 The menu bar currently subscribes to window focus change notifications to enable or disable certain menu options in response to the window status. Notifications are automatically unsubscribed (disconnected in Qt parlance) if the sender is deleted -- in this case, the sender is the QTApplication object (`qApp`). However, MacOS 13 sends a window focus change notification *after* the main window has been destroyed but *before* `qApp` has been fully destroyed. Since the menu bar is deleted in the main window's destructor, it no longer exists when it receives these notifications (in two different places via lambda expressions). The solution is to pass the main window (`this`) as context when subscribing to the notifications. In this [overloaded version](https://doc.qt.io/qt-5/qobject.html#connect-1) of `connect`, Qt automatically unsubscribes to notifications if the sender OR context (here the main window object) is destroyed. Since the spurious notifications are sent after the main window object is destroyed, this change prevents them from being sent. Tested on Mac OS 13 and 12 only. ACKs for top commit: hebasto: ACK 8a5014cd8a05b3ab86ae34a47653a82ce11bdf17 Tree-SHA512: 3dff0a252fe0e93dd68cf5503135ecf6a72bcf385ba38407d6021ab77cca323f8bbe58aeca90ec124aa2a22ab9d35b706946179ac3b5d171c96a7010de51a090
2022-11-16Fix Transaction Relay tooltip text in Peers details windowJon Atack
as a value of N/A could occur due to a lock or a disconnection race but not during connection setup.
2022-11-15Fixes bitcoin#26490 by preventing notificationsJohn Moffett
MacOS 13 sends a window focus change notification after the main window has been destroyed but before the QTApplication has been destroyed. This results in the menu bar receiving a notification despite it no longer existing. The solution is to pass the main window as context when subscribing to the notifications. Qt automatically unsubscribes to notifications if the sender OR context is destroyed.
2022-11-01Merge bitcoin/bitcoin#25548: gui: Check for readlink buffer overflow and ↵fanquake
handle gracefully e049fd76f0d57c1e6400fbfbaf4cc6ebe540f16f Bugfix: Check for readlink buffer overflow and handle gracefully (Luke Dashjr) Pull request description: If readlink returns the size of the buffer, an overflow may have (safely) occurred. Pass a buffer size of MAX_PATH+1 (the size of the actual buffer) to detect this scenario. ACKs for top commit: hebasto: ACK e049fd76f0d57c1e6400fbfbaf4cc6ebe540f16f. Tree-SHA512: 188bace79cbe556efe7782e46b870c02729b07b104a9316b0f7d50013504972e85baf507403d2d6060bb2bf3e13f40d735bddd18255d97a60810208c3de87691
2022-10-31refactor: move url.h/cpp from lib util to lib commonfanquake
2022-10-27Merge bitcoin/bitcoin#25685: wallet: Faster transaction creation by removing ↵Andrew Chow
pre-set-inputs fetching responsibility from Coin Selection 3fcb545ab26be3e785b5e5654be0bdc77099d827 bench: benchmark transaction creation process (furszy) a8a75346d7e7247596c8a580d65ceaad49c97b97 wallet: SelectCoins, return early if target is covered by preset-inputs (furszy) f41712a734dc119f8a5e053a9cfa1f0411b5e8f1 wallet: simplify preset inputs selection target check (furszy) 5baedc33519661af9d19efcefd23dca8998d2547 wallet: remove fetch pre-selected-inputs responsibility from SelectCoins (furszy) 295852f61998a025b0b28a0671e6e1cf0dc08d0d wallet: encapsulate pre-selected-inputs lookup into its own function (furszy) 37e7887cb4bfd7db6eb462ed0741c45aea22a990 wallet: skip manually selected coins from 'AvailableCoins' result (furszy) 94c0766b0cd1990c1399a745c88c2ba4c685d8d1 wallet: skip available coins fetch if "other inputs" are disallowed (furszy) Pull request description: #### # Context (Current Flow on Master) In the transaction creation process, in order to select which coins the new transaction will spend, we first obtain all the available coins known by the wallet, which means walking-through the wallet txes map, gathering the ones that fulfill certain spendability requirements in a vector. This coins vector is then provided to the Coin Selection process, which first checks if the user has manually selected any input (which could be internal, aka known by the wallet, or external), and if it does, it fetches them by searching each of them inside the wallet and/or inside the Coin Control external tx data. Then, after finding the pre-selected-inputs and gathering them in a vector, the Coin Selection process walks-through the entire available coins vector once more just to erase coins that are in both vectors. So the Coin Selection process doesn’t pick them twice (duplicate inputs inside the same transaction). #### # Process Workflow Changes Now, a new method, `FetchCoins` will be responsible for: 1) Lookup the user pre-selected-inputs (which can be internal or external). 2) And, fetch the available coins in the wallet (excluding the already fetched ones). Which will occur prior to the Coin Selection process. Which allows us to never include the pre-selected-inputs inside the available coins vector in the first place, as well as doing other nice improvements (written below). So, Coin Selection can perform its main responsibility without mixing it with having to fetch internal/external coins nor any slow and unneeded duplicate coins verification. #### # Summarizing the Improvements: 1) If any pre-selected-input lookup fail, the process will return the error right away. (before, the wallet was fetching all the wallet available coins, walking through the entire txes map, and then failing for an invalid pre-selected-input inside SelectCoins) 2) The pre-selected-inputs lookup failure causes are properly described on the return error. (before, we were returning an "Insufficient Funds" error for everything, even if the failure was due a not solvable external input) 3) **Faster Coin Selection**: no longer need to "remove the pre-set inputs from the available coins vector so that Coin Selection doesn't pick them" (which meant to loop-over the entire available coins vector at Coin Selection time, erasing duplicate coins that were pre-selected). Now, the available coins vector, which is built after the pre-selected-inputs fetching, doesn’t include the already selected inputs in the first place. 4) **Faster transaction creation** for transactions that only use manually selected inputs. We now will return early, as soon as we finish fetching the pre-selected-inputs and not perform the resources expensive calculation of walking-through the entire wallet txes map to obtain the available coins (coins that we will not use). --------------------------- Added a new bench (f6d0bb2) measuring the transaction creation process, for a wallet with ~250k UTXO, only using the pre-selected-inputs inside coin control. Setting `m_allow_other_inputs=false` to disallow the wallet to include coins automatically. #### Result on this PR (tip f6d0bb2d): | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 1,048,675.00 | 953.58 | 0.3% | 0.06 | `WalletCreateTransaction` vs #### Result on master (tip 4a4289e2): | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 96,373,458.20 | 10.38 | 0.2% | 5.30 | `WalletCreateTransaction` The benchmark took to run in master: **96.37 milliseconds**, while in this PR: **1 millisecond** 🚀 . ACKs for top commit: S3RK: Code Review ACK 3fcb545ab26be3e785b5e5654be0bdc77099d827 achow101: ACK 3fcb545ab26be3e785b5e5654be0bdc77099d827 aureleoules: reACK 3fcb545ab26be3e785b5e5654be0bdc77099d827 Tree-SHA512: 42f833e92f40c348007ca565a4c98039e6f1ff25d8322bc2b27115824744779baf0b0a38452e4e2cdcba45076473f1028079bbd0f670020481ec5d3db42e4731
2022-10-27Merge bitcoin-core/gui#665: Switch to the selected wallet after loadingHennadii Stepanov
b8b59ff9fea69f4f25d98005e3ac9172a7a11c12 gui: update the screen after loading wallet (w0xlt) Pull request description: Currently, the user loads a wallet and the screen does not switch to the selected wallet after loading (File -> Open Wallet -> wallet name). This PR changes that by making the `OpenWalletActivity::opened` signal connection a `Qt::QueuedConnection` type. ACKs for top commit: jarolrod: ACK b8b59ff9fea69f4f25d98005e3ac9172a7a11c12 hebasto: ACK b8b59ff9fea69f4f25d98005e3ac9172a7a11c12, tested on Ubuntu 22.04. Tree-SHA512: 43cd755638b643f481014a7933a0af25df2d109e859cb5f878bc04e562950d550716fa38465140060e28526b2441688580cbcbe4ec6819566b4f95162ca5e527
2022-10-27qt: Drop no longer used `SplashScreen::finish()` slotHennadii Stepanov
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-17 03:17:06 +0000