From f2d00bfe1a32a11c0d88e8c1d3bae6a6b01db15e Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 22 Jul 2022 14:49:10 -0400 Subject: [PATCH 1/4] wallet: Add CWallet::IsMine(COutPoint) It is useful to have an IsMine function that can take an outpoint. --- src/wallet/wallet.cpp | 13 +++++++++++++ src/wallet/wallet.h | 1 + 2 files changed, 14 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 45f49954a3..02fe6fba5b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1421,6 +1421,19 @@ bool CWallet::IsMine(const CTransaction& tx) const return false; } +isminetype CWallet::IsMine(const COutPoint& outpoint) const +{ + AssertLockHeld(cs_wallet); + auto wtx = GetWalletTx(outpoint.hash); + if (!wtx) { + return ISMINE_NO; + } + if (outpoint.n >= wtx->tx->vout.size()) { + return ISMINE_NO; + } + return IsMine(wtx->tx->vout[outpoint.n]); +} + bool CWallet::IsFromMe(const CTransaction& tx) const { return (GetDebit(tx, ISMINE_ALL) > 0); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 247c172da3..d8c54bdb01 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -684,6 +684,7 @@ public: CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const; isminetype IsMine(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsMine(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + isminetype IsMine(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** should probably be renamed to IsRelevantToMe */ bool IsFromMe(const CTransaction& tx) const; CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; From a537d7aaa069bc216aeab381bbc4d312b5ffedf1 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 22 Jul 2022 15:30:10 -0400 Subject: [PATCH 2/4] wallet: SelectExternal actually external inputs If an external input's utxo was created by a transaction that the wallet knows about, then it would not be selected using SelectExternal. This results in either funding failure or incorrect weight calculation. --- src/wallet/spend.cpp | 12 ++++++++---- test/functional/rpc_fundrawtransaction.py | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index d6362c1f14..bffb665fa8 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1127,12 +1127,16 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, wallet.chain().findCoins(coins); for (const CTxIn& txin : tx.vin) { - // if it's not in the wallet and corresponding UTXO is found than select as external output const auto& outPoint = txin.prevout; - if (wallet.mapWallet.find(outPoint.hash) == wallet.mapWallet.end() && !coins[outPoint].out.IsNull()) { - coinControl.SelectExternal(outPoint, coins[outPoint].out); - } else { + if (wallet.IsMine(outPoint)) { + // The input was found in the wallet, so select as internal coinControl.Select(outPoint); + } else if (coins[outPoint].out.IsNull()) { + error = _("Unable to find UTXO for external input"); + return false; + } else { + // The input was not in the wallet, but is in the UTXO set, so select as external + coinControl.SelectExternal(outPoint, coins[outPoint].out); } } diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index cf9ad3f458..d07c31a532 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -408,7 +408,7 @@ class RawTransactionsTest(BitcoinTestFramework): inputs = [ {'txid' : "1c7f966dab21119bac53213a2bc7532bff1fa844c124fd750a7d0b1332440bd1", 'vout' : 0} ] #invalid vin! outputs = { self.nodes[0].getnewaddress() : 1.0} rawtx = self.nodes[2].createrawtransaction(inputs, outputs) - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx) + assert_raises_rpc_error(-4, "Unable to find UTXO for external input", self.nodes[2].fundrawtransaction, rawtx) def test_fee_p2pkh(self): """Compare fee of a standard pubkeyhash transaction.""" From eb879634db2116b23e884dab21318743f974f1f3 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 22 Jul 2022 15:47:06 -0400 Subject: [PATCH 3/4] wallet: Try estimating input size with external data if wallet fails Instead of choosing whether to use the wallet or external data when estimating the size of an input, first use the wallet, then try external data if that failed. --- src/wallet/spend.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index bffb665fa8..001acd04e2 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -569,8 +569,12 @@ std::optional SelectCoins(const CWallet& wallet, CoinsResult& a if (!coin_control.GetExternalOutput(outpoint, txout)) { return std::nullopt; } + } + + if (input_bytes == -1) { input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control); } + // If available, override calculated size with coin control specified size if (coin_control.HasInputWeight(outpoint)) { input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0); From ef8e2a5b09dea73de8d57e6b976d77edbde5f8ff Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 22 Jul 2022 15:26:40 -0400 Subject: [PATCH 4/4] tests: Test that external inputs of txs in wallet is handled correctly --- test/functional/rpc_fundrawtransaction.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index d07c31a532..5a870de6c7 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -1079,17 +1079,28 @@ class RawTransactionsTest(BitcoinTestFramework): self.nodes[2].createwallet("test_weight_calculation") wallet = self.nodes[2].get_wallet_rpc("test_weight_calculation") - addr = wallet.getnewaddress() - txid = self.nodes[0].sendtoaddress(addr, 5) + addr = wallet.getnewaddress(address_type="bech32") + ext_addr = self.nodes[0].getnewaddress(address_type="bech32") + txid = self.nodes[0].send([{addr: 5}, {ext_addr: 5}])["txid"] vout = find_vout_for_address(self.nodes[0], txid, addr) + ext_vout = find_vout_for_address(self.nodes[0], txid, ext_addr) - self.nodes[0].sendtoaddress(wallet.getnewaddress(), 5) + self.nodes[0].sendtoaddress(wallet.getnewaddress(address_type="bech32"), 5) self.generate(self.nodes[0], 1) - rawtx = wallet.createrawtransaction([{'txid': txid, 'vout': vout}], [{self.nodes[0].getnewaddress(): 9.999}]) - fundedtx = wallet.fundrawtransaction(rawtx, {'fee_rate': 10}) + rawtx = wallet.createrawtransaction([{'txid': txid, 'vout': vout}], [{self.nodes[0].getnewaddress(address_type="bech32"): 8}]) + fundedtx = wallet.fundrawtransaction(rawtx, {'fee_rate': 10, "change_type": "bech32"}) # with 71-byte signatures we should expect following tx size - tx_size = 10 + 41*2 + 31*2 + (2 + 107*2)/4 + # tx overhead (10) + 2 inputs (41 each) + 2 p2wpkh (31 each) + (segwit marker and flag (2) + 2 p2wpkh 71 byte sig witnesses (107 each)) / witness scaling factor (4) + tx_size = ceil(10 + 41*2 + 31*2 + (2 + 107*2)/4) + assert_equal(fundedtx['fee'] * COIN, tx_size * 10) + + # Using the other output should have 72 byte sigs + rawtx = wallet.createrawtransaction([{'txid': txid, 'vout': ext_vout}], [{self.nodes[0].getnewaddress(): 13}]) + ext_desc = self.nodes[0].getaddressinfo(ext_addr)["desc"] + fundedtx = wallet.fundrawtransaction(rawtx, {'fee_rate': 10, "change_type": "bech32", "solving_data": {"descriptors": [ext_desc]}}) + # tx overhead (10) + 3 inputs (41 each) + 2 p2wpkh(31 each) + (segwit marker and flag (2) + 2 p2wpkh 71 bytes sig witnesses (107 each) + p2wpkh 72 byte sig witness (108)) / witness scaling factor (4) + tx_size = ceil(10 + 41*3 + 31*2 + (2 + 107*2 + 108)/4) assert_equal(fundedtx['fee'] * COIN, tx_size * 10) self.nodes[2].unloadwallet("test_weight_calculation")