From c0ebb9838252fb187db8719755801758d89651f7 Mon Sep 17 00:00:00 2001 From: Seibart Nedor Date: Sat, 11 Jun 2022 21:54:52 +0300 Subject: [PATCH] wallet: add `outputs` arguments to `bumpfee` and `psbtbumpfee` --- src/rpc/rawtransaction_util.cpp | 43 ++++++++++++++++++++------------- src/rpc/rawtransaction_util.h | 7 ++++++ src/wallet/feebumper.cpp | 21 ++++++++++------ src/wallet/feebumper.h | 3 ++- src/wallet/interfaces.cpp | 3 ++- src/wallet/rpc/spend.cpp | 23 +++++++++++++++--- 6 files changed, 70 insertions(+), 30 deletions(-) diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 15b8e1dcd0..3ba930f84f 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -21,12 +21,8 @@ #include #include -CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional rbf) +void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, std::optional rbf) { - if (outputs_in.isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument must be non-null"); - } - UniValue inputs; if (inputs_in.isNull()) { inputs = UniValue::VARR; @@ -34,18 +30,6 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal inputs = inputs_in.get_array(); } - const bool outputs_is_obj = outputs_in.isObject(); - UniValue outputs = outputs_is_obj ? outputs_in.get_obj() : outputs_in.get_array(); - - CMutableTransaction rawTx; - - if (!locktime.isNull()) { - int64_t nLockTime = locktime.getInt(); - if (nLockTime < 0 || nLockTime > LOCKTIME_MAX) - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, locktime out of range"); - rawTx.nLockTime = nLockTime; - } - for (unsigned int idx = 0; idx < inputs.size(); idx++) { const UniValue& input = inputs[idx]; const UniValue& o = input.get_obj(); @@ -84,6 +68,16 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal rawTx.vin.push_back(in); } +} + +void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) +{ + if (outputs_in.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument must be non-null"); + } + + const bool outputs_is_obj = outputs_in.isObject(); + UniValue outputs = outputs_is_obj ? outputs_in.get_obj() : outputs_in.get_array(); if (!outputs_is_obj) { // Translate array of key-value pairs into dict @@ -132,6 +126,21 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal rawTx.vout.push_back(out); } } +} + +CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional rbf) +{ + CMutableTransaction rawTx; + + if (!locktime.isNull()) { + int64_t nLockTime = locktime.getInt(); + if (nLockTime < 0 || nLockTime > LOCKTIME_MAX) + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, locktime out of range"); + rawTx.nLockTime = nLockTime; + } + + AddInputs(rawTx, inputs_in, rbf); + AddOutputs(rawTx, outputs_in); if (rbf.has_value() && rbf.value() && rawTx.vin.size() > 0 && !SignalsOptInRBF(CTransaction(rawTx))) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter combination: Sequence number(s) contradict replaceable option"); diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index 0c3823bc1e..a863432b7a 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -38,6 +38,13 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const */ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keystore, std::map& coins); + +/** Normalize univalue-represented inputs and add them to the transaction */ +void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, bool rbf); + +/** Normalize univalue-represented outputs and add them to the transaction */ +void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in); + /** Create a transaction from univalue parameters */ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional rbf); diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index bd158b5985..37a704bfa4 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -155,7 +155,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid) } Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector& errors, - CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine) + CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine, const std::vector& outputs) { // We are going to modify coin control later, copy to re-use CCoinControl new_coin_control(coin_control); @@ -222,11 +222,19 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo return result; } - // Fill in recipients(and preserve a single change key if there is one) - // While we're here, calculate the output amount - std::vector recipients; + // Calculate the old output amount. CAmount output_value = 0; - for (const auto& output : wtx.tx->vout) { + for (const auto& old_output : wtx.tx->vout) { + output_value += old_output.nValue; + } + + old_fee = input_value - output_value; + + // Fill in recipients (and preserve a single change key if there + // is one). If outputs vector is non-empty, replace original + // outputs with its contents, otherwise use original outputs. + std::vector recipients; + for (const auto& output : outputs.empty() ? wtx.tx->vout : outputs) { if (!OutputIsChange(wallet, output)) { CRecipient recipient = {output.scriptPubKey, output.nValue, false}; recipients.push_back(recipient); @@ -235,11 +243,8 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo ExtractDestination(output.scriptPubKey, change_dest); new_coin_control.destChange = change_dest; } - output_value += output.nValue; } - old_fee = input_value - output_value; - if (coin_control.m_feerate) { // The user provided a feeRate argument. // We calculate this here to avoid compiler warning on the cs_wallet lock diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h index a96871b26f..53cf16e0f1 100644 --- a/src/wallet/feebumper.h +++ b/src/wallet/feebumper.h @@ -51,7 +51,8 @@ Result CreateRateBumpTransaction(CWallet& wallet, CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, - bool require_mine); + bool require_mine, + const std::vector& outputs); //! Sign the new transaction, //! @return false if the tx couldn't be found or if it was diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 68dd3da9b5..1a76e46c54 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -291,7 +291,8 @@ public: CAmount& new_fee, CMutableTransaction& mtx) override { - return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true) == feebumper::Result::OK; + std::vector outputs; // just an empty list of new recipients for now + return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true, outputs) == feebumper::Result::OK; } bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); } bool commitBumpTransaction(const uint256& txid, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 0c2994706e..860b1459a5 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -958,7 +958,7 @@ RPCHelpMan signrawtransactionwithwallet() } // Definition of allowed formats of specifying transaction outputs in -// `send` and `walletcreatefundedpsbt` RPCs. +// `bumpfee`, `psbtbumpfee`, `send` and `walletcreatefundedpsbt` RPCs. static std::vector OutputsDoc() { return @@ -1013,7 +1013,12 @@ static RPCHelpMan bumpfee_helper(std::string method_name) "still be replaceable in practice, for example if it has unconfirmed ancestors which\n" "are replaceable).\n"}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" - "\"" + FeeModes("\"\n\"") + "\""}, + "\"" + FeeModes("\"\n\"") + "\""}, + {"outputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "New outputs (key-value pairs) which will replace\n" + "the original ones, if provided. Each address can only appear once and there can\n" + "only be one \"data\" object.\n", + OutputsDoc(), + RPCArgOptions{.skip_type_check = true}}, }, RPCArgOptions{.oneline_description="options"}}, }, @@ -1050,6 +1055,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) coin_control.fAllowWatchOnly = pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); // optional parameters coin_control.m_signal_bip125_rbf = true; + std::vector outputs; if (!request.params[1].isNull()) { UniValue options = request.params[1]; @@ -1060,6 +1066,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) {"fee_rate", UniValueType()}, // will be checked by AmountFromValue() in SetFeeEstimateMode() {"replaceable", UniValueType(UniValue::VBOOL)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, + {"outputs", UniValueType()}, // will be checked by AddOutputs() }, true, true); @@ -1073,6 +1080,16 @@ static RPCHelpMan bumpfee_helper(std::string method_name) coin_control.m_signal_bip125_rbf = options["replaceable"].get_bool(); } SetFeeEstimateMode(*pwallet, coin_control, conf_target, options["estimate_mode"], options["fee_rate"], /*override_min_fee=*/false); + + // Prepare new outputs by creating a temporary tx and calling AddOutputs(). + if (!options["outputs"].isNull()) { + if (options["outputs"].isArray() && options["outputs"].empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument cannot be an empty array"); + } + CMutableTransaction tempTx; + AddOutputs(tempTx, options["outputs"]); + outputs = tempTx.vout; + } } // Make sure the results are valid at least up to the most recent block @@ -1090,7 +1107,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) CMutableTransaction mtx; feebumper::Result res; // Targeting feerate bump. - res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt); + res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt, outputs); if (res != feebumper::Result::OK) { switch(res) { case feebumper::Result::INVALID_ADDRESS_OR_KEY: