diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2b1f9f375..fe004a862 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2924,7 +2924,7 @@ static UniValue listunspent(const JSONRPCRequest& request) CTxDestination address; const CScript& scriptPubKey = out.tx->tx->vout[out.i].scriptPubKey; bool fValidAddress = ExtractDestination(scriptPubKey, address); - bool reused = avoid_reuse && pwallet->IsUsedDestination(address); + bool reused = avoid_reuse && pwallet->IsUsedDestination(out.tx->GetHash(), out.i); if (destinations.size() && (!fValidAddress || !destinations.count(address))) continue; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 64c7623bf..2dfd60a91 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1073,17 +1073,31 @@ void CWallet::SetUsedDestinationState(const uint256& hash, unsigned int n, bool } } -bool CWallet::IsUsedDestination(const CTxDestination& dst) const -{ - LOCK(cs_wallet); - return ::IsMine(*this, dst) && GetDestData(dst, "used", nullptr); -} - bool CWallet::IsUsedDestination(const uint256& hash, unsigned int n) const { + AssertLockHeld(cs_wallet); CTxDestination dst; const CWalletTx* srctx = GetWalletTx(hash); - return srctx && ExtractDestination(srctx->tx->vout[n].scriptPubKey, dst) && IsUsedDestination(dst); + if (srctx) { + assert(srctx->tx->vout.size() > n); + // When descriptor wallets arrive, these additional checks are + // likely superfluous and can be optimized out + for (const auto& keyid : GetAffectedKeys(srctx->tx->vout[n].scriptPubKey, *this)) { + WitnessV0KeyHash wpkh_dest(keyid); + if (GetDestData(wpkh_dest, "used", nullptr)) { + return true; + } + ScriptHash sh_wpkh_dest(wpkh_dest); + if (GetDestData(sh_wpkh_dest, "used", nullptr)) { + return true; + } + PKHash pkh_dest(keyid); + if (GetDestData(pkh_dest, "used", nullptr)) { + return true; + } + } + } + return false; } bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 789a2d924..ee4ac9158 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1004,9 +1004,8 @@ public: bool IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - // Whether this or any UTXO with the same CTxDestination has been spent. - bool IsUsedDestination(const CTxDestination& dst) const; - bool IsUsedDestination(const uint256& hash, unsigned int n) const; + // Whether this or any known UTXO with the same single key has been spent. + bool IsUsedDestination(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetUsedDestinationState(const uint256& hash, unsigned int n, bool used); std::vector GroupOutputs(const std::vector& outputs, bool single_coin) const; diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 3c8064ea2..55b30afde 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -83,7 +83,12 @@ class AvoidReuseTest(BitcoinTestFramework): reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) self.test_fund_send_fund_senddirty() reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) - self.test_fund_send_fund_send() + self.test_fund_send_fund_send("legacy") + reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) + self.test_fund_send_fund_send("p2sh-segwit") + reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) + self.test_fund_send_fund_send("bech32") + def test_persistence(self): '''Test that wallet files persist the avoid_reuse flag.''' @@ -174,7 +179,7 @@ class AvoidReuseTest(BitcoinTestFramework): assert_approx(self.nodes[1].getbalance(), 5, 0.001) assert_approx(self.nodes[1].getbalance(avoid_reuse=False), 5, 0.001) - def test_fund_send_fund_send(self): + def test_fund_send_fund_send(self, second_addr_type): ''' Test the simple case where [1] generates a new address A, then [0] sends 10 BTC to A. @@ -184,7 +189,7 @@ class AvoidReuseTest(BitcoinTestFramework): [1] tries to spend 4 BTC (succeeds; change address sufficient) ''' - fundaddr = self.nodes[1].getnewaddress() + fundaddr = self.nodes[1].getnewaddress(label="", address_type="legacy") retaddr = self.nodes[0].getnewaddress() self.nodes[0].sendtoaddress(fundaddr, 10) @@ -205,7 +210,19 @@ class AvoidReuseTest(BitcoinTestFramework): # getbalances should show no used, 5 btc trusted assert_balances(self.nodes[1], mine={"used": 0, "trusted": 5}) - self.nodes[0].sendtoaddress(fundaddr, 10) + # For the second send, we transmute it to a related single-key address + # to make sure it's also detected as re-use + fund_spk = self.nodes[0].getaddressinfo(fundaddr)["scriptPubKey"] + fund_decoded = self.nodes[0].decodescript(fund_spk) + if second_addr_type == "p2sh-segwit": + new_fundaddr = fund_decoded["segwit"]["p2sh-segwit"] + elif second_addr_type == "bech32": + new_fundaddr = fund_decoded["segwit"]["addresses"][0] + else: + new_fundaddr = fundaddr + assert_equal(second_addr_type, "legacy") + + self.nodes[0].sendtoaddress(new_fundaddr, 10) self.nodes[0].generate(1) self.sync_all()