Adds non-mempool wallet balance to overview - #952
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste |
Use getBalances when testing the current balance in OverviewPage to take into account non_mempool balances in next commits. Co-authored-by: Anthony Towns <aj@erisian.com.au>
The assert removal is necesary for a next commit to introduce nonmempoolbalance as that balance should always be negative thus making this assert no longer true. Co-authored-by: Anthony Towns <aj@erisian.com.au>
The assert in formatWithPrivacy is removed because m_mine_nonmempool is always negative, making this assert no longer true. Co-authored-by: Anthony Towns <aj@erisian.com.au>
fce681b to
db32a1b
Compare
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
|
Key differences with #911 are:
|
pablomartin4btc
left a comment
pablomartin4btc
left a comment
There was a problem hiding this comment.
Concept ACK
I've suggested some release notes in previous attempt #911 and it'd be nice to include them here if you're up to ofc.
Suggested Qt test snippet for wallettests.cpp...
The PR touches wallettests.cpp but only to fix the existing call — worth adding a UI-level test for the new label visibility and formatting. Since there are no actual non-mempool transactions in the test wallet, calling setBalance() directly with crafted data is the right approach here rather than going through pollBalanceChanged().
// Test non-mempool balance display
interfaces::WalletBalances nonmempool_balances;
nonmempool_balances.balance = walletModel.wallet().getBalances().balance;
nonmempool_balances.nonmempool_balance = -50000;
overviewPage.setBalance(nonmempool_balances);
QVERIFY(overviewPage.findChild<QLabel*>("labelNonMempool")->isVisible());
QVERIFY(overviewPage.findChild<QLabel*>("labelNonMempoolText")->isVisible());
CompareBalance(walletModel, -50000, overviewPage.findChild<QLabel*>("labelNonMempool"));
nonmempool_balances.nonmempool_balance = 0;
overviewPage.setBalance(nonmempool_balances);
QVERIFY(!overviewPage.findChild<QLabel*>("labelNonMempool")->isVisible());
QVERIFY(!overviewPage.findChild<QLabel*>("labelNonMempoolText")->isVisible());
Co-authored-by: Pablo Martin <pablomartin4btc@gmail.com>
|
Added the tests with you as a co-author and added some release notes. |
eee7f87 to
1907923
Compare
|
Could you please weigh in on the UI/UX design choices in this PR? |
4 participants
Just grabbing #911
Copying the motivation from AJ's description:
The wallet can contain transactions that are not accepted into the node's mempool (eg due to containing a too large OP_RETURN output, due to too low a feerate, or due to too many unconfirmed ancestors). In the event you end up in this situation, it can appear as if funds have gone missing from your wallet due to the non-mempool balance not being reported. Correct this by reporting the non-mempool balance.
See bitcoin/bitcoin#33671 for further context.
The changes look like:
How to test
To test, start with
-datacarriersize=10, and runfrom the console. "Non-mempool: " should appear in the Balances pane on the Overview window with the change from the transaction. Restarting with
-datacarriersize=100000should allow the tx to enter the mempool, and move the amount into the Available balance. (Presumably, do this on -regtest after generating 100 blocks; or perhaps signet after grabbing some funds from a faucet)