Merge bitcoin/bitcoin#25679: wallet: Correctly identify external inputs that are also in the wallet

ef8e2a5b09 tests: Test that external inputs of txs in wallet is handled correctly (Andrew Chow)
eb879634db wallet: Try estimating input size with external data if wallet fails (Andrew Chow)
a537d7aaa0 wallet: SelectExternal actually external inputs (Andrew Chow)
f2d00bfe1a wallet: Add CWallet::IsMine(COutPoint) (Andrew Chow)

Pull request description:

  if a transaction is being funded that has an external input, and that input's parent is also in the wallet, we will fail to detect that and fail to fund the transaction. In order to correctly detect such inputs, we need to be doing `IsMine` on all specified inputs in order to use `Select` and `SelectExternal` correctly. Additionally `SelectCoins` needs to call `CalculateMaximumSignedInputSize` with the correct parameters which depends on whether the wallet is able to solve for the input. Because there are some situations where the wallet could find an external input to belong to it (e.g. watching an address - unable to solve, but will be ISMINE_WATCHONLY), instead of switching which `CalculateMaximumSignedInputSize` to use, we should call the one that uses the wallet, and if that fails, try again with the one that uses external solving data.

  Also adds a test for this case.

ACKs for top commit:
  instagibbs:
    ACK ef8e2a5b09
  furszy:
    ACK ef8e2a5b
  ishaanam:
    reACK ef8e2a5b09

Tree-SHA512: a43c4aefeed4605f33a36ce87ebb916e2c153fea6d415b02c9a89275e84a7e3bf12840b33c296d2d2bde46350390da48d9262f9567338e3f21d5936aae4caa1e
This commit is contained in:
fanquake 2022-08-19 08:50:39 +01:00
commit 0425ce577f
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
4 changed files with 44 additions and 11 deletions

View file

@ -569,8 +569,12 @@ std::optional<SelectionResult> 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);
@ -1127,12 +1131,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);
}
}

View file

@ -1428,6 +1428,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);

View file

@ -685,6 +685,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;

View file

@ -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."""
@ -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")