diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -901,7 +901,8 @@ bool IsUsedDestination(const TxId &txid, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetUsedDestinationState(WalletBatch &batch, const TxId &txid, - unsigned int n, bool used) + unsigned int n, bool used, + std::set &tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::vector GroupOutputs(const std::vector &outputs, @@ -1134,6 +1135,13 @@ std::set GetLabelAddresses(const std::string &label) const; + /** + * Marks all outputs in each one of the destinations dirty, so their cache + * is reset and does not return outdated information. + */ + void MarkDestinationsDirty(const std::set &destinations) + EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool GetNewDestination(const OutputType type, const std::string label, CTxDestination &dest, std::string &error); bool GetNewChangeDestination(const OutputType type, CTxDestination &dest, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -779,8 +779,9 @@ } } -void CWallet::SetUsedDestinationState(WalletBatch &batch, const TxId &txid, - unsigned int n, bool used) { +void CWallet::SetUsedDestinationState( + WalletBatch &batch, const TxId &txid, unsigned int n, bool used, + std::set &tx_destinations) { AssertLockHeld(cs_wallet); const CWalletTx *srctx = GetWalletTx(txid); if (!srctx) { @@ -792,7 +793,9 @@ if (IsMine(dst)) { if (used && !GetDestData(dst, "used", nullptr)) { // p for "present", opposite of absent (null) - AddDestData(batch, dst, "used", "p"); + if (AddDestData(batch, dst, "used", "p")) { + tx_destinations.insert(dst); + } } else if (!used && GetDestData(dst, "used", nullptr)) { EraseDestData(batch, dst, "used"); } @@ -830,10 +833,15 @@ if (IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { // Mark used destinations + std::set tx_destinations; + for (const CTxIn &txin : wtxIn.tx->vin) { const COutPoint &op = txin.prevout; - SetUsedDestinationState(batch, op.GetTxId(), op.GetN(), true); + SetUsedDestinationState(batch, op.GetTxId(), op.GetN(), true, + tx_destinations); } + + MarkDestinationsDirty(tx_destinations); } // Inserts only if not already there, returns tx inserted or tx found. @@ -3492,6 +3500,23 @@ return oldestKey; } +void CWallet::MarkDestinationsDirty( + const std::set &destinations) { + for (auto &entry : mapWallet) { + CWalletTx &wtx = entry.second; + + for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { + CTxDestination dst; + + if (ExtractDestination(wtx.tx->vout[i].scriptPubKey, dst) && + destinations.count(dst)) { + wtx.MarkDirty(); + break; + } + } + } +} + std::map CWallet::GetAddressBalances(interfaces::Chain::Lock &locked_chain) { std::map balances; diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -97,6 +97,8 @@ self.test_fund_send_fund_senddirty() reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) self.test_fund_send_fund_send() + reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) + self.test_getbalances_used() def test_persistence(self): '''Test that wallet files persist the avoid_reuse flag.''' @@ -318,6 +320,41 @@ assert_approx(self.nodes[1].getbalance(), 1, 0.001) assert_approx(self.nodes[1].getbalance(avoid_reuse=False), 11, 0.001) + def test_getbalances_used(self): + ''' + getbalances and listunspent should pick up on reused addresses + immediately, even for address reusing outputs created before the first + transaction was spending from that address + ''' + self.log.info("Test getbalances used category") + + # node under test should be completely empty + assert_equal(self.nodes[1].getbalance(avoid_reuse=False), 0) + + new_addr = self.nodes[1].getnewaddress() + ret_addr = self.nodes[0].getnewaddress() + + # send multiple transactions, reusing one address + for _ in range(11): + self.nodes[0].sendtoaddress(new_addr, 1) + + self.nodes[0].generate(1) + self.sync_all() + + # send transaction that should not use all the available outputs + # per the current coin selection algorithm + self.nodes[1].sendtoaddress(ret_addr, 5) + + # getbalances and listunspent should show the remaining outputs + # in the reused address as used/reused + assert_unspent( + self.nodes[1], + total_count=2, + total_sum=6, + reused_count=1, + reused_sum=1) + assert_balances(self.nodes[1], mine={"used": 1, "trusted": 5}) + if __name__ == '__main__': AvoidReuseTest().main()