Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/release-notes-25122.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

Wallet
------

- RPC `getreceivedbylabel` now returns an error, "Label not found
in wallet" (-4), if the label is not in the address book. (dash#7550)

6 changes: 6 additions & 0 deletions doc/release-notes-25504.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Updated RPCs
------------

- The `listsinceblock`, `listtransactions` and `gettransaction` output now contain a new
`parent_descs` field for every "receive" entry.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- A new optional `include_change` parameter was added to the `listsinceblock` command.
3 changes: 2 additions & 1 deletion src/Makefile.test.include
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ BITCOIN_TESTS += \
wallet/test/init_tests.cpp \
wallet/test/ismine_tests.cpp \
wallet/test/rpc_util_tests.cpp \
wallet/test/scriptpubkeyman_tests.cpp
wallet/test/scriptpubkeyman_tests.cpp \
wallet/test/walletload_tests.cpp

FUZZ_SUITE_LD_COMMON +=\
$(SQLITE_LIBS) \
Expand Down
36 changes: 17 additions & 19 deletions src/qt/overviewpage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ OverviewPage::OverviewPage(QWidget* parent) :

GUIUtil::updateFonts();

m_balances.balance = -1;

// Recent transactions
ui->listTransactions->setItemDelegate(txdelegate);
// Note: minimum height of listTransactions will be set later in updateAdvancedCJUI() to reflect actual settings
Expand Down Expand Up @@ -202,8 +200,9 @@ void OverviewPage::setPrivacy(bool privacy)
{
m_privacy = privacy;
clientModel->getOptionsModel()->setOption(OptionsModel::OptionID::MaskValues, privacy);
if (m_balances.balance != -1) {
setBalance(m_balances);
const auto& balances = walletModel->getCachedBalance();
if (balances.balance != -1) {
setBalance(balances);
coinJoinStatus(true);
}

Expand All @@ -226,7 +225,6 @@ OverviewPage::~OverviewPage()
void OverviewPage::setBalance(const interfaces::WalletBalances& balances)
{
BitcoinUnit unit = walletModel->getOptionsModel()->getDisplayUnit();
m_balances = balances;
if (walletModel->wallet().isLegacy()) {
if (walletModel->wallet().privateKeysDisabled()) {
ui->labelBalance->setText(BitcoinUnits::floorHtmlWithPrivacy(unit, balances.watch_only_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy));
Expand Down Expand Up @@ -312,12 +310,11 @@ void OverviewPage::setWalletModel(WalletModel *model)
// update the display unit, to not use the default ("DASH")
updateDisplayUnit();
// Keep up to date with wallet
interfaces::Wallet& wallet = model->wallet();
interfaces::WalletBalances balances = wallet.getBalances();
setBalance(balances);
setBalance(model->getCachedBalance());
connect(model, &WalletModel::balanceChanged, this, &OverviewPage::setBalance);

updateWatchOnlyLabels((wallet.haveWatchOnly() && !model->wallet().privateKeysDisabled()) || gArgs.GetBoolArg("-debug-ui", false));
interfaces::Wallet& wallet = model->wallet();
updateWatchOnlyLabels((wallet.haveWatchOnly() && !wallet.privateKeysDisabled()) || gArgs.GetBoolArg("-debug-ui", false));
connect(model, &WalletModel::notifyWatchonlyChanged, [this](bool showWatchOnly) {
updateWatchOnlyLabels(showWatchOnly && !walletModel->wallet().privateKeysDisabled());
});
Expand Down Expand Up @@ -348,11 +345,11 @@ void OverviewPage::setWalletModel(WalletModel *model)

void OverviewPage::updateDisplayUnit()
{
if(walletModel && walletModel->getOptionsModel())
{
if (walletModel && walletModel->getOptionsModel()) {
m_display_bitcoin_unit = walletModel->getOptionsModel()->getDisplayUnit();
if (m_balances.balance != -1) {
setBalance(m_balances);
const auto& balances = walletModel->getCachedBalance();
if (balances.balance != -1) {
setBalance(balances);
}

// Update txdelegate->unit with the current unit
Expand Down Expand Up @@ -404,7 +401,8 @@ void OverviewPage::updateCoinJoinProgress()
QString strAmountAndRounds;
QString strCoinJoinAmount = BitcoinUnits::formatHtmlWithUnit(m_display_bitcoin_unit, clientModel->coinJoinOptions().getAmount() * COIN, false, BitcoinUnits::SeparatorStyle::ALWAYS);

if(m_balances.balance == 0)
const auto& balances = walletModel->getCachedBalance();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh CoinJoin progress from uncached denominated balances

When only denominated_trusted or denominated_untrusted_pending changes, such as after denomination creation/confirmation during mixing, WalletModel::checkBalanceChanged() does not copy the new values because WalletBalances::balanceChanged() ignores those fields. Since this method now uses getCachedBalance() instead of fetching fresh balances, the advanced CoinJoin progress can remain stale until some unrelated tracked balance field changes.

AGENTS.md reference: AGENTS.md:L15-L17

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed as a Dash-specific regression. Before this backport, the advanced CoinJoin path fetched wallet().getBalances() directly. The new cached path depends on WalletBalances::balanceChanged(), which does not compare denominated_trusted or denominated_untrusted_pending, so those fields can remain stale when they change independently. This needs a Dash adaptation before merge.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Refresh CoinJoin progress from uncached denominated balances

The backport replaces the advanced CoinJoin view's direct wallet().getBalances() call with getCachedBalance(), but WalletBalances::balanceChanged() does not compare denominated_trusted or denominated_untrusted_pending. These fields can change without any compared field changing: while CoinJoin is disabled, GetBalance() leaves both denominated fields at zero; after CoinJoin is re-enabled, a wallet containing denominated but not fully mixed outputs repopulates them while its total, pending, immature, and anonymized balances remain unchanged. checkBalanceChanged() then declines to assign the new value, and the advanced progress calculation continues using stale zero denominated balances. Include both denominated fields in cache invalidation, or retain an uncached balance query for the advanced CoinJoin calculation.

source: ['codex']

if(balances.balance == 0)
{
ui->coinJoinProgress->setValue(0);
ui->coinJoinProgress->setToolTip(tr("No inputs detected"));
Expand All @@ -420,7 +418,7 @@ void OverviewPage::updateCoinJoinProgress()

CAmount nAnonymizableBalance = walletModel->wallet().getAnonymizableBalance(false, false);

CAmount nMaxToAnonymize = nAnonymizableBalance + m_balances.anonymized_balance;
CAmount nMaxToAnonymize = nAnonymizableBalance + balances.anonymized_balance;

// If it's more than the anon threshold, limit to that.
if (nMaxToAnonymize > clientModel->coinJoinOptions().getAmount() * COIN) nMaxToAnonymize = clientModel->coinJoinOptions().getAmount() * COIN;
Expand Down Expand Up @@ -451,7 +449,6 @@ void OverviewPage::updateCoinJoinProgress()

if (!fShowAdvancedCJUI) return;

const interfaces::WalletBalances balances = walletModel->wallet().getBalances();
CAmount nDenominatedConfirmedBalance = balances.denominated_trusted;
CAmount nDenominatedUnconfirmedBalance = balances.denominated_untrusted_pending;
CAmount nNormalizedAnonymizedBalance;
Expand All @@ -477,7 +474,7 @@ void OverviewPage::updateCoinJoinProgress()
anonNormPart = anonNormPart > 1 ? 1 : anonNormPart;
anonNormPart *= 100;

anonFullPart = (float)m_balances.anonymized_balance / nMaxToAnonymize;
anonFullPart = (float)balances.anonymized_balance / nMaxToAnonymize;
anonFullPart = anonFullPart > 1 ? 1 : anonFullPart;
anonFullPart *= 100;

Expand Down Expand Up @@ -692,7 +689,7 @@ void OverviewPage::coinJoinStatus(bool fForce)
setWidgetsVisible(true);
}

void OverviewPage::toggleCoinJoin(){
void OverviewPage::toggleCoinJoin() {
QSettings settings;
// Popup some information on first mixing
QString hasMixed = settings.value("hasMixed").toString();
Expand All @@ -707,9 +704,10 @@ void OverviewPage::toggleCoinJoin(){
bool mixing{false};
walletModel->withCoinJoin([&](auto& client) { mixing = client.isMixing(); });
if (!mixing) {
const auto& balances = walletModel->getCachedBalance();
auto& options = walletModel->node().coinJoinOptions();
const CAmount nMinAmount = options.getSmallestDenomination() + options.getMaxCollateralAmount();
if(m_balances.balance < nMinAmount) {
if(balances.balance < nMinAmount) {
QString strMinAmount(BitcoinUnits::formatWithUnit(m_display_bitcoin_unit, nMinAmount));
QMessageBox::warning(this, strCoinJoinName,
tr("%1 requires at least %2 to use.").arg(strCoinJoinName).arg(strMinAmount),
Expand Down
1 change: 0 additions & 1 deletion src/qt/overviewpage.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public Q_SLOTS:
Ui::OverviewPage *ui;
ClientModel* clientModel{nullptr};
WalletModel* walletModel{nullptr};
interfaces::WalletBalances m_balances;
bool m_privacy{false};
BitcoinUnit m_display_bitcoin_unit;
bool fShowAdvancedCJUI;
Expand Down
12 changes: 5 additions & 7 deletions src/qt/sendcoinsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,9 @@ void SendCoinsDialog::setModel(WalletModel *_model)
}
}

interfaces::WalletBalances balances = _model->wallet().getBalances();
setBalance(balances);
connect(_model, &WalletModel::balanceChanged, this, &SendCoinsDialog::setBalance);
connect(_model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &SendCoinsDialog::updateDisplayUnit);
updateDisplayUnit();
connect(_model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &SendCoinsDialog::refreshBalance);
refreshBalance();

// Coin Control
connect(_model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &SendCoinsDialog::coinControlUpdateLabels);
Expand Down Expand Up @@ -822,9 +820,9 @@ void SendCoinsDialog::setBalance(const interfaces::WalletBalances& balances)
}
}

void SendCoinsDialog::updateDisplayUnit()
void SendCoinsDialog::refreshBalance()
{
setBalance(model->wallet().getBalances());
setBalance(model->getCachedBalance());
coinControlUpdateLabels();
ui->customFee->setDisplayUnit(model->getOptionsModel()->getDisplayUnit());
updateSmartFeeLabel();
Expand Down Expand Up @@ -896,7 +894,7 @@ void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry)
m_coin_control->fAllowWatchOnly = model->wallet().privateKeysDisabled() && !model->wallet().hasExternalSigner();

// Calculate available amount to send.
CAmount amount = model->wallet().getAvailableBalance(*m_coin_control);
CAmount amount = model->getAvailableBalance(m_coin_control.get());
for (int i = 0; i < ui->entries->count(); ++i) {
SendCoinsEntry* e = qobject_cast<SendCoinsEntry*>(ui->entries->itemAt(i)->widget());
if (e && !e->isHidden() && e != entry) {
Expand Down
2 changes: 1 addition & 1 deletion src/qt/sendcoinsdialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private Q_SLOTS:
void on_buttonMinimizeFee_clicked();
void removeEntry(SendCoinsEntry* entry);
void useAvailableBalance(SendCoinsEntry* entry);
void updateDisplayUnit();
void refreshBalance();
void coinControlFeatureChanged(bool);
void coinControlButtonClicked();
void coinControlChangeChecked(int);
Expand Down
31 changes: 16 additions & 15 deletions src/qt/test/wallettests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,15 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
return {};
}

void CompareBalance(WalletModel& walletModel, CAmount expected_balance, QLabel* balance_label_to_check, bool use_privacy_formatting)
{
BitcoinUnit unit = walletModel.getOptionsModel()->getDisplayUnit();
QString balanceComparison = use_privacy_formatting
? BitcoinUnits::floorHtmlWithPrivacy(unit, expected_balance, BitcoinUnits::SeparatorStyle::ALWAYS, false)
: BitcoinUnits::formatWithUnit(unit, expected_balance, false/*, BitcoinUnits::SeparatorStyle::ALWAYS*/);
QCOMPARE(balance_label_to_check->text().trimmed(), balanceComparison);
}

//! Simple qt wallet tests.
//
// Test widgets can be debugged interactively calling show() on them and
Expand Down Expand Up @@ -162,15 +171,10 @@ void TestGUI(interfaces::Node& node)
sendCoinsDialog.setModel(&walletModel);
transactionView.setModel(&walletModel);

{
// Check balance in send dialog
QLabel* balanceLabel = sendCoinsDialog.findChild<QLabel*>("labelBalance");
QString balanceText = balanceLabel->text();
BitcoinUnit unit = walletModel.getOptionsModel()->getDisplayUnit();
CAmount balance = walletModel.wallet().getBalance();
QString balanceComparison = BitcoinUnits::formatWithUnit(unit, balance, false /*, BitcoinUnits::SeparatorStyle::ALWAYS*/);
QCOMPARE(balanceText, balanceComparison);
}
// Update walletModel cached balance which will trigger an update for the 'labelBalance' QLabel.
walletModel.pollBalanceChanged();
// Check balance in send dialog
CompareBalance(walletModel, walletModel.wallet().getBalance(), sendCoinsDialog.findChild<QLabel*>("labelBalance"), false);

// Send two transactions, and verify they are added to transaction list.
TransactionTableModel* transactionTableModel = walletModel.getTransactionTableModel();
Expand All @@ -187,12 +191,8 @@ void TestGUI(interfaces::Node& node)
OverviewPage overviewPage;
overviewPage.setClientModel(&clientModel);
overviewPage.setWalletModel(&walletModel);
QLabel* balanceLabel = overviewPage.findChild<QLabel*>("labelBalance");
QString balanceText = balanceLabel->text().trimmed();
BitcoinUnit unit = walletModel.getOptionsModel()->getDisplayUnit();
CAmount balance = walletModel.wallet().getBalance();
QString balanceComparison = BitcoinUnits::floorHtmlWithPrivacy(unit, balance, BitcoinUnits::SeparatorStyle::ALWAYS, false);
QCOMPARE(balanceText, balanceComparison);
walletModel.pollBalanceChanged(); // Manual balance polling update
CompareBalance(walletModel, walletModel.wallet().getBalance(), overviewPage.findChild<QLabel*>("labelBalance"), true);

// Check that each autobackup failure state selects its specific tooltip on the CoinJoin status label
{
Expand Down Expand Up @@ -238,6 +238,7 @@ void TestGUI(interfaces::Node& node)
QPushButton* requestPaymentButton = receiveCoinsDialog.findChild<QPushButton*>("receiveButton");
requestPaymentButton->click();
QString address;
BitcoinUnit unit = walletModel.getOptionsModel()->getDisplayUnit();
for (QWidget* widget : QApplication::topLevelWidgets()) {
if (widget->inherits("ReceiveRequestDialog")) {
ReceiveRequestDialog* receiveRequestDialog = qobject_cast<ReceiveRequestDialog*>(widget);
Expand Down
26 changes: 24 additions & 2 deletions src/qt/walletmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ WalletModel::~WalletModel()

void WalletModel::startPollBalance()
{
// Update the cached balance right away, so every view can make use of it,
// so them don't need to waste resources recalculating it.
pollBalanceChanged();

// This timer will be fired repeatedly to update the balance
// Since the QTimer::timeout is a private signal, it cannot be used
// in the GUIUtil::ExceptionSafeConnect directly.
Expand Down Expand Up @@ -137,12 +141,17 @@ void WalletModel::pollBalanceChanged()

void WalletModel::checkBalanceChanged(const interfaces::WalletBalances& new_balances)
{
if(new_balances.balanceChanged(m_cached_balances)) {
if (new_balances.balanceChanged(m_cached_balances)) {
m_cached_balances = new_balances;
Q_EMIT balanceChanged(new_balances);
}
}

interfaces::WalletBalances WalletModel::getCachedBalance() const
{
return m_cached_balances;
}

void WalletModel::updateTransaction()
{
// Balance and number of transactions might have changed
Expand Down Expand Up @@ -258,7 +267,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
}
}

CAmount nBalance = m_wallet->getAvailableBalance(coinControl);
// If no coin was manually selected, use the cached balance
// Future: can merge this call with 'createTransaction'.
CAmount nBalance = getAvailableBalance(&coinControl);

if(total > nBalance)
{
Expand Down Expand Up @@ -633,3 +644,14 @@ uint256 WalletModel::getLastBlockProcessed() const
{
return m_client_model ? m_client_model->getBestBlockHash() : uint256{};
}

CAmount WalletModel::getAvailableBalance(const CCoinControl* control)
{
if (control && control->HasSelected()) {
return wallet().getAvailableBalance(*control);
}
if (control && control->IsUsingCoinJoin()) {
return getCachedBalance().anonymized_balance;
}
return getCachedBalance().balance;
Comment on lines +648 to +656

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Honor watch-only coin control without manual inputs

SendCoinsDialog sets fAllowWatchOnly for private-keys-disabled wallets before both "Use available balance" and prepareTransaction(). When no inputs are manually selected, this method nevertheless returns getCachedBalance().balance, which contains spendable-key funds and excludes the separately cached watch-only balance. The wallet-level getAvailableBalance(*control) path applies fAllowWatchOnly, filters for solvable watch-only outputs, and was used before this backport. As a result, a watch-only wallet can display funds but receive AmountExceedsBalance before it can create its unsigned PSBT. Delegate to the wallet calculation whenever fAllowWatchOnly is set.

Suggested change
CAmount WalletModel::getAvailableBalance(const CCoinControl* control)
{
if (control && control->HasSelected()) {
return wallet().getAvailableBalance(*control);
}
if (control && control->IsUsingCoinJoin()) {
return getCachedBalance().anonymized_balance;
}
return getCachedBalance().balance;
CAmount WalletModel::getAvailableBalance(const CCoinControl* control)
{
if (control && (control->HasSelected() || control->fAllowWatchOnly)) {
return wallet().getAvailableBalance(*control);
}
if (control && control->IsUsingCoinJoin()) {
return getCachedBalance().anonymized_balance;
}
return getCachedBalance().balance;
}

source: ['codex']

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's backported code as it is from mainstream

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line does match the original gui#598 backport, but upstream later confirmed this exact cached fast path was a bug and fixed it in bitcoin#26699, commit cd98b717398f7b13ace91ea9efac9ce1e60b4d62 (“gui: getAvailableBalance, include watch only balance”). That follow-up explicitly adds the cached watch-only balance for private-keys-disabled wallets; bitcoin#26687 documents the resulting GUI PSBT failure as “The amount exceeds your balance.”

So exactness to gui#598 does not clear this finding—the PR is bringing in gui#598 without its upstream bugfix follow-up. Please backport the relevant bitcoin#26699 commit (or an equivalent fAllowWatchOnly-aware cached path) and its focused regression coverage.

}
Comment on lines +648 to +657

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Use the CoinJoin balance for unselected CoinJoin controls

The CoinJoin send dialog calls UseCoinJoin(true) but normally has no manually selected inputs. The current HasSelected() check therefore returns getCachedBalance().balance, which includes unmixed funds, instead of the available fully mixed amount. As a result, “Use available balance” and prepareTransaction() can accept an amount greater than the mixed balance before transaction creation fails with insufficient mixed funds. Use the cached anonymized balance when CoinJoin is active, while retaining the wallet calculation for manually selected inputs.

Suggested change
CAmount WalletModel::getAvailableBalance(const CCoinControl* control)
{
return control && control->HasSelected() ? wallet().getAvailableBalance(*control) : getCachedBalance().balance;
}
CAmount WalletModel::getAvailableBalance(const CCoinControl* control)
{
if (control && control->HasSelected()) {
return wallet().getAvailableBalance(*control);
}
if (control && control->IsUsingCoinJoin()) {
return getCachedBalance().anonymized_balance;
}
return getCachedBalance().balance;
}

source: ['codex']

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in this update — Use the CoinJoin balance for unselected CoinJoin controls no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

7 changes: 7 additions & 0 deletions src/qt/walletmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,13 @@ class WalletModel : public QObject

uint256 getLastBlockProcessed() const;

// Retrieve the cached wallet balance
interfaces::WalletBalances getCachedBalance() const;

// If coin control has selected outputs, searches the total amount inside the wallet.
// Otherwise, uses the wallet's cached available balance.
CAmount getAvailableBalance(const wallet::CCoinControl* control);

private:
std::unique_ptr<interfaces::Wallet> m_wallet;
std::unique_ptr<interfaces::Handler> m_handler_unload;
Expand Down
1 change: 1 addition & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "listsinceblock", 1, "target_confirmations" },
{ "listsinceblock", 2, "include_watchonly" },
{ "listsinceblock", 3, "include_removed" },
{ "listsinceblock", 4, "include_change" },
{ "sendmany", 1, "amounts" },
{ "sendmany", 2, "minconf" },
{ "sendmany", 3, "addlocked" },
Expand Down
Loading
Loading