Skip to content

Commit b94d0c7

Browse files
committed
Merge bitcoin/bitcoin#23201: wallet: Allow users to specify input weights when funding a transaction
3866272 tests: Test specifying input weights (Andrew Chow) 6fa762a rpc, wallet: Allow users to specify input weights (Andrew Chow) 808068e wallet: Allow user specified input size to override (Andrew Chow) 4060c50 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 bitcoin/bitcoin@3866272 glozow: reACK 3866272 via range-diff t-bast: ACK bitcoin/bitcoin@3866272 Tree-SHA512: 2c8b471ee537c62a51389b7c4e86b5ac1c3a223b444195042be8117b3c83e29c0619463610b950cbbd1648d3ed01ecc5bb0b3c4f39640680da9157763b9b9f9f
2 parents 792d0d8 + 3866272 commit b94d0c7

File tree

9 files changed

+376
-18
lines changed

9 files changed

+376
-18
lines changed

src/wallet/coincontrol.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,28 @@ class CCoinControl
115115
vOutpoints.assign(setSelected.begin(), setSelected.end());
116116
}
117117

118+
void SetInputWeight(const COutPoint& outpoint, int64_t weight)
119+
{
120+
m_input_weights[outpoint] = weight;
121+
}
122+
123+
bool HasInputWeight(const COutPoint& outpoint) const
124+
{
125+
return m_input_weights.count(outpoint) > 0;
126+
}
127+
128+
int64_t GetInputWeight(const COutPoint& outpoint) const
129+
{
130+
auto it = m_input_weights.find(outpoint);
131+
assert(it != m_input_weights.end());
132+
return it->second;
133+
}
134+
118135
private:
119136
std::set<COutPoint> setSelected;
120137
std::map<COutPoint, CTxOut> m_external_txouts;
138+
//! Map of COutPoints to the maximum weight for that input
139+
std::map<COutPoint, int64_t> m_input_weights;
121140
};
122141
} // namespace wallet
123142

src/wallet/rpc/spend.cpp

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

5+
#include <consensus/validation.h>
56
#include <core_io.h>
67
#include <key_io.h>
78
#include <policy/policy.h>
@@ -429,6 +430,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
429430
{"replaceable", UniValueType(UniValue::VBOOL)},
430431
{"conf_target", UniValueType(UniValue::VNUM)},
431432
{"estimate_mode", UniValueType(UniValue::VSTR)},
433+
{"input_weights", UniValueType(UniValue::VARR)},
432434
},
433435
true, true);
434436

@@ -548,6 +550,37 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
548550
}
549551
}
550552

553+
if (options.exists("input_weights")) {
554+
for (const UniValue& input : options["input_weights"].get_array().getValues()) {
555+
uint256 txid = ParseHashO(input, "txid");
556+
557+
const UniValue& vout_v = find_value(input, "vout");
558+
if (!vout_v.isNum()) {
559+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, missing vout key");
560+
}
561+
int vout = vout_v.get_int();
562+
if (vout < 0) {
563+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout cannot be negative");
564+
}
565+
566+
const UniValue& weight_v = find_value(input, "weight");
567+
if (!weight_v.isNum()) {
568+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, missing weight key");
569+
}
570+
int64_t weight = weight_v.get_int64();
571+
const int64_t min_input_weight = GetTransactionInputWeight(CTxIn());
572+
CHECK_NONFATAL(min_input_weight == 165);
573+
if (weight < min_input_weight) {
574+
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)");
575+
}
576+
if (weight > MAX_STANDARD_TX_WEIGHT) {
577+
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, weight cannot be greater than the maximum standard tx weight of %d", MAX_STANDARD_TX_WEIGHT));
578+
}
579+
580+
coinControl.SetInputWeight(COutPoint(txid, vout), weight);
581+
}
582+
}
583+
551584
if (tx.vout.size() == 0)
552585
throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output");
553586

@@ -585,6 +618,23 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
585618
}
586619
}
587620

621+
static void SetOptionsInputWeights(const UniValue& inputs, UniValue& options)
622+
{
623+
if (options.exists("input_weights")) {
624+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Input weights should be specified in inputs rather than in options.");
625+
}
626+
if (inputs.size() == 0) {
627+
return;
628+
}
629+
UniValue weights(UniValue::VARR);
630+
for (const UniValue& input : inputs.getValues()) {
631+
if (input.exists("weight")) {
632+
weights.push_back(input);
633+
}
634+
}
635+
options.pushKV("input_weights", weights);
636+
}
637+
588638
RPCHelpMan fundrawtransaction()
589639
{
590640
return RPCHelpMan{"fundrawtransaction",
@@ -626,6 +676,17 @@ RPCHelpMan fundrawtransaction()
626676
{"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."},
627677
},
628678
},
679+
{"input_weights", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Inputs and their corresponding weights",
680+
{
681+
{"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"},
682+
{"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output index"},
683+
{"weight", RPCArg::Type::NUM, RPCArg::Optional::NO, "The maximum weight for this input, "
684+
"including the weight of the outpoint and sequence number. "
685+
"Note that serialized signature sizes are not guaranteed to be consistent, "
686+
"so the maximum DER signatures size of 73 bytes should be used when considering ECDSA signatures."
687+
"Remember to convert serialized sizes to weight units when necessary."},
688+
},
689+
},
629690
},
630691
FundTxDoc()),
631692
"options"},
@@ -1007,6 +1068,11 @@ RPCHelpMan send()
10071068
{"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"},
10081069
{"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"},
10091070
{"sequence", RPCArg::Type::NUM, RPCArg::Optional::NO, "The sequence number"},
1071+
{"weight", RPCArg::Type::NUM, RPCArg::DefaultHint{"Calculated from wallet and solving data"}, "The maximum weight for this input, "
1072+
"including the weight of the outpoint and sequence number. "
1073+
"Note that signature sizes are not guaranteed to be consistent, "
1074+
"so the maximum DER signatures size of 73 bytes should be used when considering ECDSA signatures."
1075+
"Remember to convert serialized sizes to weight units when necessary."},
10101076
},
10111077
},
10121078
{"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"},
@@ -1110,6 +1176,7 @@ RPCHelpMan send()
11101176
// Automatically select coins, unless at least one is manually selected. Can
11111177
// be overridden by options.add_inputs.
11121178
coin_control.m_add_inputs = rawTx.vin.size() == 0;
1179+
SetOptionsInputWeights(options["inputs"], options);
11131180
FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control, /* override_min_fee */ false);
11141181

11151182
bool add_to_wallet = true;
@@ -1250,6 +1317,11 @@ RPCHelpMan walletcreatefundedpsbt()
12501317
{"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"},
12511318
{"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"},
12521319
{"sequence", RPCArg::Type::NUM, RPCArg::DefaultHint{"depends on the value of the 'locktime' and 'options.replaceable' arguments"}, "The sequence number"},
1320+
{"weight", RPCArg::Type::NUM, RPCArg::DefaultHint{"Calculated from wallet and solving data"}, "The maximum weight for this input, "
1321+
"including the weight of the outpoint and sequence number. "
1322+
"Note that signature sizes are not guaranteed to be consistent, "
1323+
"so the maximum DER signatures size of 73 bytes should be used when considering ECDSA signatures."
1324+
"Remember to convert serialized sizes to weight units when necessary."},
12531325
},
12541326
},
12551327
},
@@ -1330,10 +1402,12 @@ RPCHelpMan walletcreatefundedpsbt()
13301402
}, true
13311403
);
13321404

1405+
UniValue options = request.params[3];
1406+
13331407
CAmount fee;
13341408
int change_position;
13351409
bool rbf{wallet.m_signal_rbf};
1336-
const UniValue &replaceable_arg = request.params[3]["replaceable"];
1410+
const UniValue &replaceable_arg = options["replaceable"];
13371411
if (!replaceable_arg.isNull()) {
13381412
RPCTypeCheckArgument(replaceable_arg, UniValue::VBOOL);
13391413
rbf = replaceable_arg.isTrue();
@@ -1343,7 +1417,8 @@ RPCHelpMan walletcreatefundedpsbt()
13431417
// Automatically select coins, unless at least one is manually selected. Can
13441418
// be overridden by options.add_inputs.
13451419
coin_control.m_add_inputs = rawTx.vin.size() == 0;
1346-
FundTransaction(wallet, rawTx, fee, change_position, request.params[3], coin_control, /* override_min_fee */ true);
1420+
SetOptionsInputWeights(request.params[0], options);
1421+
FundTransaction(wallet, rawTx, fee, change_position, options, coin_control, /* override_min_fee */ true);
13471422

13481423
// Make a blank psbt
13491424
PartiallySignedTransaction psbtx(rawTx);

src/wallet/spend.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -455,15 +455,17 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
455455
}
456456
input_bytes = GetTxSpendSize(wallet, wtx, outpoint.n, false);
457457
txout = wtx.tx->vout.at(outpoint.n);
458-
}
459-
if (input_bytes == -1) {
460-
// 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
458+
} else {
459+
// The input is external. We did not find the tx in mapWallet.
461460
if (!coin_control.GetExternalOutput(outpoint, txout)) {
462-
// Not ours, and we don't have solving data.
463461
return std::nullopt;
464462
}
465463
input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /* use_max_sig */ true);
466464
}
465+
// If available, override calculated size with coin control specified size
466+
if (coin_control.HasInputWeight(outpoint)) {
467+
input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0);
468+
}
467469

468470
CInputCoin coin(outpoint, txout, input_bytes);
469471
if (coin.m_input_bytes == -1) {

src/wallet/test/spend_tests.cpp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,5 +63,56 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
6363
BOOST_CHECK_EQUAL(fee, check_tx(fee + 123));
6464
}
6565

66+
static void TestFillInputToWeight(int64_t additional_weight, std::vector<int64_t> expected_stack_sizes)
67+
{
68+
static const int64_t EMPTY_INPUT_WEIGHT = GetTransactionInputWeight(CTxIn());
69+
70+
CTxIn input;
71+
int64_t target_weight = EMPTY_INPUT_WEIGHT + additional_weight;
72+
BOOST_CHECK(FillInputToWeight(input, target_weight));
73+
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), target_weight);
74+
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), expected_stack_sizes.size());
75+
for (unsigned int i = 0; i < expected_stack_sizes.size(); ++i) {
76+
BOOST_CHECK_EQUAL(input.scriptWitness.stack[i].size(), expected_stack_sizes[i]);
77+
}
78+
}
79+
80+
BOOST_FIXTURE_TEST_CASE(FillInputToWeightTest, BasicTestingSetup)
81+
{
82+
{
83+
// Less than or equal minimum of 165 should not add any witness data
84+
CTxIn input;
85+
BOOST_CHECK(!FillInputToWeight(input, -1));
86+
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165);
87+
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0);
88+
BOOST_CHECK(!FillInputToWeight(input, 0));
89+
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165);
90+
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0);
91+
BOOST_CHECK(!FillInputToWeight(input, 164));
92+
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165);
93+
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0);
94+
BOOST_CHECK(FillInputToWeight(input, 165));
95+
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165);
96+
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0);
97+
}
98+
99+
// Make sure we can add at least one weight
100+
TestFillInputToWeight(1, {0});
101+
102+
// 1 byte compact size uint boundary
103+
TestFillInputToWeight(252, {251});
104+
TestFillInputToWeight(253, {83, 168});
105+
TestFillInputToWeight(262, {86, 174});
106+
TestFillInputToWeight(263, {260});
107+
108+
// 3 byte compact size uint boundary
109+
TestFillInputToWeight(65535, {65532});
110+
TestFillInputToWeight(65536, {21842, 43688});
111+
TestFillInputToWeight(65545, {21845, 43694});
112+
TestFillInputToWeight(65546, {65541});
113+
114+
// Note: We don't test the next boundary because of memory allocation constraints.
115+
}
116+
66117
BOOST_AUTO_TEST_SUITE_END()
67118
} // namespace wallet

src/wallet/wallet.cpp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,6 +1507,49 @@ bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut
15071507
return true;
15081508
}
15091509

1510+
bool FillInputToWeight(CTxIn& txin, int64_t target_weight)
1511+
{
1512+
assert(txin.scriptSig.empty());
1513+
assert(txin.scriptWitness.IsNull());
1514+
1515+
int64_t txin_weight = GetTransactionInputWeight(txin);
1516+
1517+
// Do nothing if the weight that should be added is less than the weight that already exists
1518+
if (target_weight < txin_weight) {
1519+
return false;
1520+
}
1521+
if (target_weight == txin_weight) {
1522+
return true;
1523+
}
1524+
1525+
// Subtract current txin weight, which should include empty witness stack
1526+
int64_t add_weight = target_weight - txin_weight;
1527+
assert(add_weight > 0);
1528+
1529+
// We will want to subtract the size of the Compact Size UInt that will also be serialized.
1530+
// However doing so when the size is near a boundary can result in a problem where it is not
1531+
// possible to have a stack element size and combination to exactly equal a target.
1532+
// To avoid this possibility, if the weight to add is less than 10 bytes greater than
1533+
// a boundary, the size will be split so that 2/3rds will be in one stack element, and
1534+
// the remaining 1/3rd in another. Using 3rds allows us to avoid additional boundaries.
1535+
// 10 bytes is used because that accounts for the maximum size. This does not need to be super precise.
1536+
if ((add_weight >= 253 && add_weight < 263)
1537+
|| (add_weight > std::numeric_limits<uint16_t>::max() && add_weight <= std::numeric_limits<uint16_t>::max() + 10)
1538+
|| (add_weight > std::numeric_limits<uint32_t>::max() && add_weight <= std::numeric_limits<uint32_t>::max() + 10)) {
1539+
int64_t first_weight = add_weight / 3;
1540+
add_weight -= first_weight;
1541+
1542+
first_weight -= GetSizeOfCompactSize(first_weight);
1543+
txin.scriptWitness.stack.emplace(txin.scriptWitness.stack.end(), first_weight, 0);
1544+
}
1545+
1546+
add_weight -= GetSizeOfCompactSize(add_weight);
1547+
txin.scriptWitness.stack.emplace(txin.scriptWitness.stack.end(), add_weight, 0);
1548+
assert(GetTransactionInputWeight(txin) == target_weight);
1549+
1550+
return true;
1551+
}
1552+
15101553
// Helper for producing a bunch of max-sized low-S low-R signatures (eg 71 bytes)
15111554
bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, const CCoinControl* coin_control) const
15121555
{
@@ -1515,6 +1558,14 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut>
15151558
for (const auto& txout : txouts)
15161559
{
15171560
CTxIn& txin = txNew.vin[nIn];
1561+
// If weight was provided, fill the input to that weight
1562+
if (coin_control && coin_control->HasInputWeight(txin.prevout)) {
1563+
if (!FillInputToWeight(txin, coin_control->GetInputWeight(txin.prevout))) {
1564+
return false;
1565+
}
1566+
nIn++;
1567+
continue;
1568+
}
15181569
// Use max sig if watch only inputs were used or if this particular input is an external input
15191570
// to ensure a sufficient fee is attained for the requested feerate.
15201571
const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(txin.prevout));

src/wallet/wallet.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -939,6 +939,8 @@ bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);
939939
bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);
940940

941941
bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool use_max_sig);
942+
943+
bool FillInputToWeight(CTxIn& txin, int64_t target_weight);
942944
} // namespace wallet
943945

944946
#endif // BITCOIN_WALLET_WALLET_H

0 commit comments

Comments
 (0)