diff options
author | laanwj <126646+laanwj@users.noreply.github.com> | 2022-01-25 17:17:01 +0100 |
---|---|---|
committer | laanwj <126646+laanwj@users.noreply.github.com> | 2022-01-25 17:51:05 +0100 |
commit | b94d0c7af11bd91dad4f180ce2a2ffa09e4b5668 (patch) | |
tree | 540da541c8b7a89fefec9da7ae8afca85459a134 | |
parent | 792d0d8d512cf8ddca200317b74ce550c1a1a428 (diff) | |
parent | 3866272c450cc659207fbc2cff3c690ae8593341 (diff) | |
download | bitcoin-b94d0c7af11bd91dad4f180ce2a2ffa09e4b5668.tar.xz |
Merge bitcoin/bitcoin#23201: wallet: Allow users to specify input weights when funding a transaction
3866272c450cc659207fbc2cff3c690ae8593341 tests: Test specifying input weights (Andrew Chow)
6fa762a37298c4cd3ac063b46b7d1b353d7a658b rpc, wallet: Allow users to specify input weights (Andrew Chow)
808068e90e758b9c74878a5235b2c59731fec3e5 wallet: Allow user specified input size to override (Andrew Chow)
4060c50d7ee31dc8a39229e3553d3d92f8f3516d wallet: add input weights to CCoinControl (Andrew Chow)
Pull request description:
When funding a transaction with external inputs, instead of providing solving data, a user may want to just provide the maximum signed size of that input. This is particularly useful in cases where the input is nonstandard as our dummy signer is unable to handle those inputs.
The input weight can be provided to any input regardless of whether it belongs to the wallet and the provided weight will always be used regardless of any calculated input weight. This allows the user to override the calculated input weight which may overestimate in some circumstances due to missing information (e.g. if the private key is not known, a maximum size signature will be used, but the actual signer may be doing additional work which reduces the size of the signature).
For `send` and `walletcreatefundedpsbt`, the input weight is specified in a `weight` field in an input object. For `fundrawtransaction`, a new `input_weights` field is added to the `options` object. This is an array of objects consisting of a txid, vout, and weight.
Closes #23187
ACKs for top commit:
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/23201/commits/3866272c450cc659207fbc2cff3c690ae8593341
glozow:
reACK 3866272 via range-diff
t-bast:
ACK https://github.com/bitcoin/bitcoin/pull/23201/commits/3866272c450cc659207fbc2cff3c690ae8593341
Tree-SHA512: 2c8b471ee537c62a51389b7c4e86b5ac1c3a223b444195042be8117b3c83e29c0619463610b950cbbd1648d3ed01ecc5bb0b3c4f39640680da9157763b9b9f9f
-rw-r--r-- | src/wallet/coincontrol.h | 19 | ||||
-rw-r--r-- | src/wallet/rpc/spend.cpp | 79 | ||||
-rw-r--r-- | src/wallet/spend.cpp | 10 | ||||
-rw-r--r-- | src/wallet/test/spend_tests.cpp | 51 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 51 | ||||
-rw-r--r-- | src/wallet/wallet.h | 2 | ||||
-rwxr-xr-x | test/functional/rpc_fundrawtransaction.py | 53 | ||||
-rwxr-xr-x | test/functional/rpc_psbt.py | 89 | ||||
-rwxr-xr-x | test/functional/wallet_send.py | 40 |
9 files changed, 376 insertions, 18 deletions
diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 5ef2295c88..65a5c83366 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -115,9 +115,28 @@ public: vOutpoints.assign(setSelected.begin(), setSelected.end()); } + void SetInputWeight(const COutPoint& outpoint, int64_t weight) + { + m_input_weights[outpoint] = weight; + } + + bool HasInputWeight(const COutPoint& outpoint) const + { + return m_input_weights.count(outpoint) > 0; + } + + int64_t GetInputWeight(const COutPoint& outpoint) const + { + auto it = m_input_weights.find(outpoint); + assert(it != m_input_weights.end()); + return it->second; + } + private: std::set<COutPoint> setSelected; std::map<COutPoint, CTxOut> m_external_txouts; + //! Map of COutPoints to the maximum weight for that input + std::map<COutPoint, int64_t> m_input_weights; }; } // namespace wallet diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index cae3542a5e..433b5a1815 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <consensus/validation.h> #include <core_io.h> #include <key_io.h> #include <policy/policy.h> @@ -429,6 +430,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, {"replaceable", UniValueType(UniValue::VBOOL)}, {"conf_target", UniValueType(UniValue::VNUM)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, + {"input_weights", UniValueType(UniValue::VARR)}, }, true, true); @@ -548,6 +550,37 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, } } + if (options.exists("input_weights")) { + for (const UniValue& input : options["input_weights"].get_array().getValues()) { + uint256 txid = ParseHashO(input, "txid"); + + const UniValue& vout_v = find_value(input, "vout"); + if (!vout_v.isNum()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, missing vout key"); + } + int vout = vout_v.get_int(); + if (vout < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout cannot be negative"); + } + + const UniValue& weight_v = find_value(input, "weight"); + if (!weight_v.isNum()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, missing weight key"); + } + int64_t weight = weight_v.get_int64(); + const int64_t min_input_weight = GetTransactionInputWeight(CTxIn()); + CHECK_NONFATAL(min_input_weight == 165); + if (weight < min_input_weight) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, weight cannot be less than 165 (41 bytes (size of outpoint + sequence + empty scriptSig) * 4 (witness scaling factor)) + 1 (empty witness)"); + } + if (weight > MAX_STANDARD_TX_WEIGHT) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, weight cannot be greater than the maximum standard tx weight of %d", MAX_STANDARD_TX_WEIGHT)); + } + + coinControl.SetInputWeight(COutPoint(txid, vout), weight); + } + } + if (tx.vout.size() == 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output"); @@ -585,6 +618,23 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, } } +static void SetOptionsInputWeights(const UniValue& inputs, UniValue& options) +{ + if (options.exists("input_weights")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Input weights should be specified in inputs rather than in options."); + } + if (inputs.size() == 0) { + return; + } + UniValue weights(UniValue::VARR); + for (const UniValue& input : inputs.getValues()) { + if (input.exists("weight")) { + weights.push_back(input); + } + } + options.pushKV("input_weights", weights); +} + RPCHelpMan fundrawtransaction() { return RPCHelpMan{"fundrawtransaction", @@ -626,6 +676,17 @@ RPCHelpMan fundrawtransaction() {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, + {"input_weights", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Inputs and their corresponding weights", + { + {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, + {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output index"}, + {"weight", RPCArg::Type::NUM, RPCArg::Optional::NO, "The maximum weight for this input, " + "including the weight of the outpoint and sequence number. " + "Note that serialized signature sizes are not guaranteed to be consistent, " + "so the maximum DER signatures size of 73 bytes should be used when considering ECDSA signatures." + "Remember to convert serialized sizes to weight units when necessary."}, + }, + }, }, FundTxDoc()), "options"}, @@ -1007,6 +1068,11 @@ RPCHelpMan send() {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"}, {"sequence", RPCArg::Type::NUM, RPCArg::Optional::NO, "The sequence number"}, + {"weight", RPCArg::Type::NUM, RPCArg::DefaultHint{"Calculated from wallet and solving data"}, "The maximum weight for this input, " + "including the weight of the outpoint and sequence number. " + "Note that signature sizes are not guaranteed to be consistent, " + "so the maximum DER signatures size of 73 bytes should be used when considering ECDSA signatures." + "Remember to convert serialized sizes to weight units when necessary."}, }, }, {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"}, @@ -1110,6 +1176,7 @@ RPCHelpMan send() // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; + SetOptionsInputWeights(options["inputs"], options); FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control, /* override_min_fee */ false); bool add_to_wallet = true; @@ -1250,6 +1317,11 @@ RPCHelpMan walletcreatefundedpsbt() {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"}, {"sequence", RPCArg::Type::NUM, RPCArg::DefaultHint{"depends on the value of the 'locktime' and 'options.replaceable' arguments"}, "The sequence number"}, + {"weight", RPCArg::Type::NUM, RPCArg::DefaultHint{"Calculated from wallet and solving data"}, "The maximum weight for this input, " + "including the weight of the outpoint and sequence number. " + "Note that signature sizes are not guaranteed to be consistent, " + "so the maximum DER signatures size of 73 bytes should be used when considering ECDSA signatures." + "Remember to convert serialized sizes to weight units when necessary."}, }, }, }, @@ -1330,10 +1402,12 @@ RPCHelpMan walletcreatefundedpsbt() }, true ); + UniValue options = request.params[3]; + CAmount fee; int change_position; bool rbf{wallet.m_signal_rbf}; - const UniValue &replaceable_arg = request.params[3]["replaceable"]; + const UniValue &replaceable_arg = options["replaceable"]; if (!replaceable_arg.isNull()) { RPCTypeCheckArgument(replaceable_arg, UniValue::VBOOL); rbf = replaceable_arg.isTrue(); @@ -1343,7 +1417,8 @@ RPCHelpMan walletcreatefundedpsbt() // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; - FundTransaction(wallet, rawTx, fee, change_position, request.params[3], coin_control, /* override_min_fee */ true); + SetOptionsInputWeights(request.params[0], options); + FundTransaction(wallet, rawTx, fee, change_position, options, coin_control, /* override_min_fee */ true); // Make a blank psbt PartiallySignedTransaction psbtx(rawTx); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index d87bdc8679..1e47af3f0e 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -455,15 +455,17 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec } input_bytes = GetTxSpendSize(wallet, wtx, outpoint.n, false); txout = wtx.tx->vout.at(outpoint.n); - } - if (input_bytes == -1) { - // The input is external. We either did not find the tx in mapWallet, or we did but couldn't compute the input size with wallet data + } else { + // The input is external. We did not find the tx in mapWallet. if (!coin_control.GetExternalOutput(outpoint, txout)) { - // Not ours, and we don't have solving data. return std::nullopt; } input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /* use_max_sig */ true); } + // If available, override calculated size with coin control specified size + if (coin_control.HasInputWeight(outpoint)) { + input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0); + } CInputCoin coin(outpoint, txout, input_bytes); if (coin.m_input_bytes == -1) { diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index b2a0697c21..334bd5b8bc 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -63,5 +63,56 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) BOOST_CHECK_EQUAL(fee, check_tx(fee + 123)); } +static void TestFillInputToWeight(int64_t additional_weight, std::vector<int64_t> expected_stack_sizes) +{ + static const int64_t EMPTY_INPUT_WEIGHT = GetTransactionInputWeight(CTxIn()); + + CTxIn input; + int64_t target_weight = EMPTY_INPUT_WEIGHT + additional_weight; + BOOST_CHECK(FillInputToWeight(input, target_weight)); + BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), target_weight); + BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), expected_stack_sizes.size()); + for (unsigned int i = 0; i < expected_stack_sizes.size(); ++i) { + BOOST_CHECK_EQUAL(input.scriptWitness.stack[i].size(), expected_stack_sizes[i]); + } +} + +BOOST_FIXTURE_TEST_CASE(FillInputToWeightTest, BasicTestingSetup) +{ + { + // Less than or equal minimum of 165 should not add any witness data + CTxIn input; + BOOST_CHECK(!FillInputToWeight(input, -1)); + BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165); + BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0); + BOOST_CHECK(!FillInputToWeight(input, 0)); + BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165); + BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0); + BOOST_CHECK(!FillInputToWeight(input, 164)); + BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165); + BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0); + BOOST_CHECK(FillInputToWeight(input, 165)); + BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165); + BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0); + } + + // Make sure we can add at least one weight + TestFillInputToWeight(1, {0}); + + // 1 byte compact size uint boundary + TestFillInputToWeight(252, {251}); + TestFillInputToWeight(253, {83, 168}); + TestFillInputToWeight(262, {86, 174}); + TestFillInputToWeight(263, {260}); + + // 3 byte compact size uint boundary + TestFillInputToWeight(65535, {65532}); + TestFillInputToWeight(65536, {21842, 43688}); + TestFillInputToWeight(65545, {21845, 43694}); + TestFillInputToWeight(65546, {65541}); + + // Note: We don't test the next boundary because of memory allocation constraints. +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3fcb086d2d..337a5139e1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1507,6 +1507,49 @@ bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut return true; } +bool FillInputToWeight(CTxIn& txin, int64_t target_weight) +{ + assert(txin.scriptSig.empty()); + assert(txin.scriptWitness.IsNull()); + + int64_t txin_weight = GetTransactionInputWeight(txin); + + // Do nothing if the weight that should be added is less than the weight that already exists + if (target_weight < txin_weight) { + return false; + } + if (target_weight == txin_weight) { + return true; + } + + // Subtract current txin weight, which should include empty witness stack + int64_t add_weight = target_weight - txin_weight; + assert(add_weight > 0); + + // We will want to subtract the size of the Compact Size UInt that will also be serialized. + // However doing so when the size is near a boundary can result in a problem where it is not + // possible to have a stack element size and combination to exactly equal a target. + // To avoid this possibility, if the weight to add is less than 10 bytes greater than + // a boundary, the size will be split so that 2/3rds will be in one stack element, and + // the remaining 1/3rd in another. Using 3rds allows us to avoid additional boundaries. + // 10 bytes is used because that accounts for the maximum size. This does not need to be super precise. + if ((add_weight >= 253 && add_weight < 263) + || (add_weight > std::numeric_limits<uint16_t>::max() && add_weight <= std::numeric_limits<uint16_t>::max() + 10) + || (add_weight > std::numeric_limits<uint32_t>::max() && add_weight <= std::numeric_limits<uint32_t>::max() + 10)) { + int64_t first_weight = add_weight / 3; + add_weight -= first_weight; + + first_weight -= GetSizeOfCompactSize(first_weight); + txin.scriptWitness.stack.emplace(txin.scriptWitness.stack.end(), first_weight, 0); + } + + add_weight -= GetSizeOfCompactSize(add_weight); + txin.scriptWitness.stack.emplace(txin.scriptWitness.stack.end(), add_weight, 0); + assert(GetTransactionInputWeight(txin) == target_weight); + + return true; +} + // Helper for producing a bunch of max-sized low-S low-R signatures (eg 71 bytes) bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, const CCoinControl* coin_control) const { @@ -1515,6 +1558,14 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> for (const auto& txout : txouts) { CTxIn& txin = txNew.vin[nIn]; + // If weight was provided, fill the input to that weight + if (coin_control && coin_control->HasInputWeight(txin.prevout)) { + if (!FillInputToWeight(txin, coin_control->GetInputWeight(txin.prevout))) { + return false; + } + nIn++; + continue; + } // Use max sig if watch only inputs were used or if this particular input is an external input // to ensure a sufficient fee is attained for the requested feerate. const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(txin.prevout)); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 00a1865a0e..e2c5c69c91 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -939,6 +939,8 @@ bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool use_max_sig); + +bool FillInputToWeight(CTxIn& txin, int64_t target_weight); } // namespace wallet #endif // BITCOIN_WALLET_WALLET_H diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index a8e6acea45..759e43194b 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -4,8 +4,10 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the fundrawtransaction RPC.""" + from decimal import Decimal from itertools import product +from math import ceil from test_framework.descriptors import descsum_create from test_framework.key import ECKey @@ -1003,7 +1005,7 @@ class RawTransactionsTest(BitcoinTestFramework): ext_utxo = self.nodes[0].listunspent(addresses=[addr])[0] # An external input without solving data should result in an error - raw_tx = wallet.createrawtransaction([ext_utxo], {self.nodes[0].getnewaddress(): 15}) + raw_tx = wallet.createrawtransaction([ext_utxo], {self.nodes[0].getnewaddress(): ext_utxo["amount"] / 2}) assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, raw_tx) # Error conditions @@ -1011,6 +1013,12 @@ class RawTransactionsTest(BitcoinTestFramework): assert_raises_rpc_error(-5, "'01234567890a0b0c0d0e0f' is not a valid public key", wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["01234567890a0b0c0d0e0f"]}}) assert_raises_rpc_error(-5, "'not a script' is not hex", wallet.fundrawtransaction, raw_tx, {"solving_data": {"scripts":["not a script"]}}) assert_raises_rpc_error(-8, "Unable to parse descriptor 'not a descriptor'", wallet.fundrawtransaction, raw_tx, {"solving_data": {"descriptors":["not a descriptor"]}}) + assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", wallet.fundrawtransaction, raw_tx, {"input_weights": [{"txid": ext_utxo["txid"]}]}) + assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", wallet.fundrawtransaction, raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": -1}]}) + assert_raises_rpc_error(-8, "Invalid parameter, missing weight key", wallet.fundrawtransaction, raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"]}]}) + assert_raises_rpc_error(-8, "Invalid parameter, weight cannot be less than 165", wallet.fundrawtransaction, raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": 164}]}) + assert_raises_rpc_error(-8, "Invalid parameter, weight cannot be less than 165", wallet.fundrawtransaction, raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": -1}]}) + assert_raises_rpc_error(-8, "Invalid parameter, weight cannot be greater than", wallet.fundrawtransaction, raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": 400001}]}) # But funding should work when the solving data is provided funded_tx = wallet.fundrawtransaction(raw_tx, {"solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}}) @@ -1020,10 +1028,45 @@ class RawTransactionsTest(BitcoinTestFramework): assert signed_tx['complete'] funded_tx = wallet.fundrawtransaction(raw_tx, {"solving_data": {"descriptors": [desc]}}) - signed_tx = wallet.signrawtransactionwithwallet(funded_tx['hex']) - assert not signed_tx['complete'] - signed_tx = self.nodes[0].signrawtransactionwithwallet(signed_tx['hex']) - assert signed_tx['complete'] + signed_tx1 = wallet.signrawtransactionwithwallet(funded_tx['hex']) + assert not signed_tx1['complete'] + signed_tx2 = self.nodes[0].signrawtransactionwithwallet(signed_tx1['hex']) + assert signed_tx2['complete'] + + unsigned_weight = self.nodes[0].decoderawtransaction(signed_tx1["hex"])["weight"] + signed_weight = self.nodes[0].decoderawtransaction(signed_tx2["hex"])["weight"] + # Input's weight is difference between weight of signed and unsigned, + # and the weight of stuff that didn't change (prevout, sequence, 1 byte of scriptSig) + input_weight = signed_weight - unsigned_weight + (41 * 4) + low_input_weight = input_weight // 2 + high_input_weight = input_weight * 2 + + # Funding should also work if the input weight is provided + funded_tx = wallet.fundrawtransaction(raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": input_weight}]}) + signed_tx = wallet.signrawtransactionwithwallet(funded_tx["hex"]) + signed_tx = self.nodes[0].signrawtransactionwithwallet(signed_tx["hex"]) + assert_equal(self.nodes[0].testmempoolaccept([signed_tx["hex"]])[0]["allowed"], True) + assert_equal(signed_tx["complete"], True) + # Reducing the weight should have a lower fee + funded_tx2 = wallet.fundrawtransaction(raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": low_input_weight}]}) + assert_greater_than(funded_tx["fee"], funded_tx2["fee"]) + # Increasing the weight should have a higher fee + funded_tx2 = wallet.fundrawtransaction(raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": high_input_weight}]}) + assert_greater_than(funded_tx2["fee"], funded_tx["fee"]) + # The provided weight should override the calculated weight when solving data is provided + funded_tx3 = wallet.fundrawtransaction(raw_tx, {"solving_data": {"descriptors": [desc]}, "input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": high_input_weight}]}) + assert_equal(funded_tx2["fee"], funded_tx3["fee"]) + # The feerate should be met + funded_tx4 = wallet.fundrawtransaction(raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": high_input_weight}], "fee_rate": 10}) + input_add_weight = high_input_weight - (41 * 4) + tx4_weight = wallet.decoderawtransaction(funded_tx4["hex"])["weight"] + input_add_weight + tx4_vsize = int(ceil(tx4_weight / 4)) + assert_fee_amount(funded_tx4["fee"], tx4_vsize, Decimal(0.0001)) + + # Funding with weight at csuint boundaries should not cause problems + funded_tx = wallet.fundrawtransaction(raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": 255}]}) + funded_tx = wallet.fundrawtransaction(raw_tx, {"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": 65539}]}) + self.nodes[2].unloadwallet("extfund") def test_include_unsafe(self): diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 153a201e95..b037807b53 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -606,11 +606,15 @@ class PSBTTest(BitcoinTestFramework): assert_raises_rpc_error(-25, 'Inputs missing or spent', self.nodes[0].walletprocesspsbt, 'cHNidP8BAJoCAAAAAkvEW8NnDtdNtDpsmze+Ht2LH35IJcKv00jKAlUs21RrAwAAAAD/////S8Rbw2cO1020OmybN74e3Ysffkglwq/TSMoCVSzbVGsBAAAAAP7///8CwLYClQAAAAAWABSNJKzjaUb3uOxixsvh1GGE3fW7zQD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XIAAAAAAAEAnQIAAAACczMa321tVHuN4GKWKRncycI22aX3uXgwSFUKM2orjRsBAAAAAP7///9zMxrfbW1Ue43gYpYpGdzJwjbZpfe5eDBIVQozaiuNGwAAAAAA/v///wIA+QKVAAAAABl2qRT9zXUVA8Ls5iVqynLHe5/vSe1XyYisQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAAAAAQEfQM0ClQAAAAAWABRmWQUcjSjghQ8/uH4Bn/zkakwLtAAAAA==') - # Test that we can fund psbts with external inputs specified + self.log.info("Test that we can fund psbts with external inputs specified") + eckey = ECKey() eckey.generate() privkey = bytes_to_wif(eckey.get_bytes()) + self.nodes[1].createwallet("extfund") + wallet = self.nodes[1].get_wallet_rpc("extfund") + # Make a weird but signable script. sh(pkh()) descriptor accomplishes this desc = descsum_create("sh(pkh({}))".format(privkey)) if self.options.descriptors: @@ -622,26 +626,97 @@ class PSBTTest(BitcoinTestFramework): addr_info = self.nodes[0].getaddressinfo(addr) self.nodes[0].sendtoaddress(addr, 10) + self.nodes[0].sendtoaddress(wallet.getnewaddress(), 10) self.generate(self.nodes[0], 6) ext_utxo = self.nodes[0].listunspent(addresses=[addr])[0] # An external input without solving data should result in an error - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[1].walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 10 + ext_utxo['amount']}, 0, {'add_inputs': True}) + assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 15}) # But funding should work when the solving data is provided - psbt = self.nodes[1].walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {'add_inputs': True, "solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}}) - signed = self.nodes[1].walletprocesspsbt(psbt['psbt']) + psbt = wallet.walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {"add_inputs": True, "solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}}) + signed = wallet.walletprocesspsbt(psbt['psbt']) assert not signed['complete'] signed = self.nodes[0].walletprocesspsbt(signed['psbt']) assert signed['complete'] self.nodes[0].finalizepsbt(signed['psbt']) - psbt = self.nodes[1].walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {'add_inputs': True, "solving_data":{"descriptors": [desc]}}) - signed = self.nodes[1].walletprocesspsbt(psbt['psbt']) + psbt = wallet.walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {"add_inputs": True, "solving_data":{"descriptors": [desc]}}) + signed = wallet.walletprocesspsbt(psbt['psbt']) assert not signed['complete'] signed = self.nodes[0].walletprocesspsbt(signed['psbt']) assert signed['complete'] - self.nodes[0].finalizepsbt(signed['psbt']) + final = self.nodes[0].finalizepsbt(signed['psbt'], False) + + dec = self.nodes[0].decodepsbt(signed["psbt"]) + for i, txin in enumerate(dec["tx"]["vin"]): + if txin["txid"] == ext_utxo["txid"] and txin["vout"] == ext_utxo["vout"]: + input_idx = i + break + psbt_in = dec["inputs"][input_idx] + # Calculate the input weight + # (prevout + sequence + length of scriptSig + 2 bytes buffer) * 4 + len of scriptwitness + len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0 + len_scriptwitness = len(psbt_in["final_scriptwitness"]["hex"]) // 2 if "final_scriptwitness" in psbt_in else 0 + input_weight = ((41 + len_scriptsig + 2) * 4) + len_scriptwitness + low_input_weight = input_weight // 2 + high_input_weight = input_weight * 2 + + # Input weight error conditions + assert_raises_rpc_error( + -8, + "Input weights should be specified in inputs rather than in options.", + wallet.walletcreatefundedpsbt, + inputs=[ext_utxo], + outputs={self.nodes[0].getnewaddress(): 15}, + options={"input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": 1000}]} + ) + + # Funding should also work if the input weight is provided + psbt = wallet.walletcreatefundedpsbt( + inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": input_weight}], + outputs={self.nodes[0].getnewaddress(): 15}, + options={"add_inputs": True} + ) + signed = wallet.walletprocesspsbt(psbt["psbt"]) + signed = self.nodes[0].walletprocesspsbt(signed["psbt"]) + final = self.nodes[0].finalizepsbt(signed["psbt"]) + assert self.nodes[0].testmempoolaccept([final["hex"]])[0]["allowed"] + # Reducing the weight should have a lower fee + psbt2 = wallet.walletcreatefundedpsbt( + inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": low_input_weight}], + outputs={self.nodes[0].getnewaddress(): 15}, + options={"add_inputs": True} + ) + assert_greater_than(psbt["fee"], psbt2["fee"]) + # Increasing the weight should have a higher fee + psbt2 = wallet.walletcreatefundedpsbt( + inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": high_input_weight}], + outputs={self.nodes[0].getnewaddress(): 15}, + options={"add_inputs": True} + ) + assert_greater_than(psbt2["fee"], psbt["fee"]) + # The provided weight should override the calculated weight when solving data is provided + psbt3 = wallet.walletcreatefundedpsbt( + inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": high_input_weight}], + outputs={self.nodes[0].getnewaddress(): 15}, + options={'add_inputs': True, "solving_data":{"descriptors": [desc]}} + ) + assert_equal(psbt2["fee"], psbt3["fee"]) + + # Import the external utxo descriptor so that we can sign for it from the test wallet + if self.options.descriptors: + res = wallet.importdescriptors([{"desc": desc, "timestamp": "now"}]) + else: + res = wallet.importmulti([{"desc": desc, "timestamp": "now"}]) + assert res[0]["success"] + # The provided weight should override the calculated weight for a wallet input + psbt3 = wallet.walletcreatefundedpsbt( + inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": high_input_weight}], + outputs={self.nodes[0].getnewaddress(): 15}, + options={"add_inputs": True} + ) + assert_equal(psbt2["fee"], psbt3["fee"]) if __name__ == '__main__': PSBTTest().main() diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index d77d554baa..843a9f52b7 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -518,5 +518,45 @@ class WalletSendTest(BitcoinTestFramework): assert signed["complete"] self.nodes[0].finalizepsbt(signed["psbt"]) + dec = self.nodes[0].decodepsbt(signed["psbt"]) + for i, txin in enumerate(dec["tx"]["vin"]): + if txin["txid"] == ext_utxo["txid"] and txin["vout"] == ext_utxo["vout"]: + input_idx = i + break + psbt_in = dec["inputs"][input_idx] + # Calculate the input weight + # (prevout + sequence + length of scriptSig + 2 bytes buffer) * 4 + len of scriptwitness + len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0 + len_scriptwitness = len(psbt_in["final_scriptwitness"]["hex"]) // 2 if "final_scriptwitness" in psbt_in else 0 + input_weight = ((41 + len_scriptsig + 2) * 4) + len_scriptwitness + + # Input weight error conditions + assert_raises_rpc_error( + -8, + "Input weights should be specified in inputs rather than in options.", + ext_wallet.send, + outputs={self.nodes[0].getnewaddress(): 15}, + options={"inputs": [ext_utxo], "input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": 1000}]} + ) + + # Funding should also work when input weights are provided + res = self.test_send( + from_wallet=ext_wallet, + to_wallet=self.nodes[0], + amount=15, + inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": input_weight}], + add_inputs=True, + psbt=True, + include_watching=True, + fee_rate=10 + ) + signed = ext_wallet.walletprocesspsbt(res["psbt"]) + signed = ext_fund.walletprocesspsbt(res["psbt"]) + assert signed["complete"] + tx = self.nodes[0].finalizepsbt(signed["psbt"]) + testres = self.nodes[0].testmempoolaccept([tx["hex"]])[0] + assert_equal(testres["allowed"], True) + assert_fee_amount(testres["fees"]["base"], testres["vsize"], Decimal(0.0001)) + if __name__ == '__main__': WalletSendTest().main() |