a06e845e82 BlockTip struct created and connected to notifyHeaderTip and notifyBlockTip signals. (furszy)
2f867203b0 Added best block hash to the NotifyHeaderTip and NotifyBlockTip signals. (furszy)
Pull request description:
Rationale:
The height based polling in the GUI is an issue on chain reorgs. Any new tip signal with the same height as the one that it's cached in the model was not triggering the GUI update (interpreting it as the same same block when it could receive a different one). Ending up with bad information presented in the GUI.
This PR essentially changes the last cached height to be a last cached block hash.
---
Old historical information of this PR.
As the tip height is cached and updated via signaling in clientModel, there is no need to continue locking `cs_main` on every balance poll (`m_node.getNumBlocks()` method call).
Extra topic:
Would suggest to change the `cachedNumBlocks` field inside `walletModel` to a more understandable name, maybe `nLastBalanceUpdateHeight`.
And finally, this will have the equal height reorg issue mentioned [here](https://github.com/bitcoin/bitcoin/pull/17905#issuecomment-577324304), whatever is presented to fix it, this should use the same flow too.
**[Edit - 24/01/2020]**
Have added #[17905](https://github.com/bitcoin/bitcoin/pull/17905#issuecomment-577324304) comment fix here too.
ACKs for top commit:
jonasschnelli:
utACK a06e845e82 - it would be great to have QT unit tests (in this case for a reorg) that either automatically inspect the window content based on accessibility and tests for expected values or at least allow for quick manual re-testing (screenshots, automatically create UI situations).
hebasto:
re-ACK a06e845e82, suggested style changes implemented since the [previous](https://github.com/bitcoin/bitcoin/pull/17993#pullrequestreview-417249705) review.
ryanofsky:
Code review ACK a06e845e82. A lot of changes since the last review: rebase after sync_state introduction #18152 and tryGetBalances revert #18587, reverting getLastBlockTime change, fixing spacing and initializations and renaming some variables
Tree-SHA512: 835e587a8296df9899cccd7b3e598a5970942b640e432e6a32de0b4eaea5b40f9271258f089ec033595311707b74a0f7187ecf8ed397c713e1153e2714072975
c4ea501e96 qt: Hide non PKHash-Addresses in signing address book (Emil Engler)
Pull request description:
[Video Demo](https://www.youtube.com/watch?v=T-Rp2pFRmzY)
This PR hides all non PKHash addresses in the signing GUI in the Address Book when it is opened through the signing dialog, as non PKHash addresses are useless there.
ACKs for top commit:
jonasschnelli:
Code Review ACK c4ea501e96
Tree-SHA512: e321d45e15534b2d68da5a1297b1c7551cdd784f03203f54c9385c2ce0bb2b7316c09f9e8c3eb41bfa1e7207ecc94c8ed08f012e2d6c117b803996ade26feb2f
Tweak of #17905 to make gui display of transactions and balances more
consistent. This change shouldn't cause visible effects in normal cases, just
make GUI wallet code more internally correct and consistent.
96cb597325 gui: Avoid redundant tx status updates (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
In `TransactionTablePriv::index`, avoid calling `interfaces::Wallet::tryGetTxStatus` if the status is up to date as of the most recent `NotifyBlockTip` notification. Store height from the most recent notification in a new `ClientModel::cachedNumBlocks` variable in order to check this.
This avoids floods of IPC traffic from `tryGetTxStatus` with #10102 when there are a lot of transactions. It might also make the GUI a little more efficient even when there is no IPC.
ACKs for top commit:
promag:
Code review ACK 96cb597325.
hebasto:
ACK 96cb597325
Tree-SHA512: fce597bf52a813ad4923110d0a39229ea09e1631e0d580ea18cffb09e58cdbb4b111a40a9a9270ff16d8163cd47b0bd9f1fe7e3a6c7ebb19198f049f8dd1aa46
Replace by privateKeysDisabled method to avoid need for GUI to reference
internal wallet flags.
Also remove adjacent WalletModel canGetAddresses wrapper that serves no purpose
and make Wallet::canGetAddresses non-const so it can be implemented by IPC
classes in #10102.
In TransactionTablePriv::index, avoid calling
interfaces::Wallet::tryGetTxStatus if the status is up to date as of the most
recent NotifyBlockTip notification. Store height from the most recent
notification in a new ClientModel::cachedNumBlocks variable in order to check
this.
This avoids floods of IPC traffic from tryGetTxStatus with #10102 when there
are a lot of transactions. It might also make the GUI a little more efficient
even when there is no IPC.
9e95931865 [wallet] Remove `state` argument from CWallet::CommitTransaction (John Newbery)
d1734f9a3b [wallet] Remove return value from CommitTransaction() (John Newbery)
b6f486a02b [wallet] Add doxygen comment to CWallet::CommitTransaction() (John Newbery)
8bba91b22d [wallet] Fix whitespace in CWallet::CommitTransaction() (John Newbery)
Pull request description:
`CommitTransaction()` returns a bool to indicate success, but since commit
b3a7410 (#9302) it only returns true, even if the transaction was not
successfully broadcast. This commit changes CommitTransaction() to return
void.
All dead code in `if (!CommitTransaction())` branches has been removed.
Two additional commits fix up the idiosyncratic whitespace in `CommitTransaction` and add a doxygen comment for the function.
ACKs for top commit:
laanwj:
ACK 9e95931865
Tree-SHA512: a55a2c20369a45222fc0e02d0891495655a926e71c4f52cb72624768dd7b9c1dca716ea67d38420afb90f40c6e0fd448caa60c18fd693bb10ecb110b641820e6
CommitTransaction returns a bool to indicate success, but since commit
b3a74100b8 it only returns true, even if the transaction was not
successfully broadcast. This commit changes CommitTransaction() to return
void.
All dead code in `if (!CommitTransaction())` branches has been removed.
Whenever the keypool changes (new keys generated, new seed set,
keypool runs out, etc.), notify the GUI that the keypool has changed. The
receive button can then be enabled and disabled as necessary.
fa2510d5c1 Use C++11 default member initializers (MarcoFalke)
Pull request description:
Changes:
* Remove unused constructors that leave some members uninitialized
* Remove manual initialization in each constructor and prefer C++11 default member initializers
This is not a stylistic change, but a change that avoids bugs such as:
* fix uninitialized read when stringifying an addrLocal #14728
* qt: Initialize members in WalletModel #12426
* net: correctly initialize nMinPingUsecTime #6636
* ...
Tree-SHA512: 0f896f3b9fcc464d5fc7525f7c86343ef9ce9fb13425fbc68e9a9728fd8710c2b4e2fd039ee08279ea41ff20fd92b7185cf5cca95a0bcb6a5340a1e6f03cae6b
This patch adds a --disable-bip70 configure option that disables BIP70
payment request support. When disabled, this removes the dependency of
the GUI on OpenSSL and Protobuf.
d795c610d3 [qt] TransactionView: highlight replacement tx after fee bump (Sjors Provoost)
Pull request description:
Consistent with #12421 which highlights the transaction after send.
<img width="747" alt="1" src="https://user-images.githubusercontent.com/10217/38036280-a7358ea4-32a6-11e8-8f92-417e9e1e3e8b.png">
<img width="685" alt="2" src="https://user-images.githubusercontent.com/10217/38036289-aac87040-32a6-11e8-9f94-81745ff6c592.png">
~I'm not too proud of the `QTimer::singleShot(10` bit; any suggestions on how to properly wait for the transactions table to become aware of the new transaction?~
Although I could have called `focusTransaction()` directly from `TransactionView::bumpFee()` I'm using the same signal as the send screen. This should make it easier to move fee bump / transaction replacement functionality around later.
Tree-SHA512: 242055b7c3d32c7b2cf871f5ceda2581221902fd53fa29e0b092713fc16d3191adbe8cbb28417d522dda9febec8cc05e07afe3489cd7caaecd33460c1dde6fbc
779c5f984 Qt: hide RPCConsole wallet selector when no wallets are present (Jonas Schnelli)
dc6f150f3 Qt: show wallet name in request dlg in case of multiwallet (Jonas Schnelli)
4826ca4b8 Qt: show wallet name in send confirmation dlg in case of multiwallet (Jonas Schnelli)
cfa4133ce GUI: RPCConsole: Log wallet changes (Luke Dashjr)
b6d04fc7c Qt: Get wallet name from WalletModel rather than passing it around (Luke Dashjr)
12d8d2681 Qt: When multiple wallets are used, include in notifications the name (Jonas Schnelli)
d1ec34a76 Qt: QComboBox::setVisible doesn't work in toolbars, so defer adding it at all until needed (Luke Dashjr)
d49cc70e6 Qt: Add wallet selector to debug console (Jonas Schnelli)
d558f44c5 Bugfix: RPC: Add missing UnregisterHTTPHandler for /wallet/ (Luke Dashjr)
85d531971 Qt: Ensure UI updates only come from the currently selected walletView (Luke Dashjr)
e449f9a9e Qt: Add a combobox to toolbar to select from multiple wallets (Luke Dashjr)
3dba3c3ac Qt: Load all wallets into WalletModels (Luke Dashjr)
Pull request description:
This is an overhaul of #11383 (plus some additions).
It avoids unnecessary coupling of httpserver/jsonrpc and the wallet as well as it avoids pointer pure passing (and pointer deletion) of `CWallet` (plus other minor design changes).
Additionally it adds the wallet name to the sendconfirmation and request dialog (in case multiwallet is active)
Tree-SHA512: 3d06e18badbc5d1821e488bf1dae463bb0be544cf11b2b618e025812bfdd13c5f39604bb93b4c705313930e7dc4e66f4848b9469ba14871bade58e7a027246a1
GUI wallet uses RBF by default, regardless of -walletrbf.
RPC and debug console in the GUI remain unchanged; they don't
use RBF by default, unless launched with -walletrbf=1.
