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

6 files changed, 206 insertions, 6 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