Skip to content

Commit

Permalink
Merge bitcoin#23201: wallet: Allow users to specify input weights whe…
Browse files Browse the repository at this point in the history
…n 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 bitcoin#23187

ACKs for top commit:
  instagibbs:
    reACK bitcoin@3866272
  glozow:
    reACK 3866272 via range-diff
  t-bast:
    ACK bitcoin@3866272

Tree-SHA512: 2c8b471ee537c62a51389b7c4e86b5ac1c3a223b444195042be8117b3c83e29c0619463610b950cbbd1648d3ed01ecc5bb0b3c4f39640680da9157763b9b9f9f
  • Loading branch information
laanwj committed Jan 25, 2022
2 parents 792d0d8 + 3866272 commit b94d0c7
Show file tree
Hide file tree
Showing 9 changed files with 376 additions and 18 deletions.
19 changes: 19 additions & 0 deletions src/wallet/coincontrol.h
Expand Up @@ -115,9 +115,28 @@ class CCoinControl
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

Expand Down
79 changes: 77 additions & 2 deletions src/wallet/rpc/spend.cpp
Expand Up @@ -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>
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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."},
},
},
},
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down
10 changes: 6 additions & 4 deletions src/wallet/spend.cpp
Expand Up @@ -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) {
Expand Down
51 changes: 51 additions & 0 deletions src/wallet/test/spend_tests.cpp
Expand Up @@ -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
51 changes: 51 additions & 0 deletions src/wallet/wallet.cpp
Expand Up @@ -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
{
Expand All @@ -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));
Expand Down
2 changes: 2 additions & 0 deletions src/wallet/wallet.h
Expand Up @@ -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

0 comments on commit b94d0c7

Please sign in to comment.