Merge 73966f75f6 into merged_master (Bitcoin PR bitcoin/bitcoin#25344)

This commit is contained in:
Byron Hambly 2025-04-06 21:07:49 +02:00
commit 54d0589bdc
7 changed files with 99 additions and 35 deletions

View file

@ -145,12 +145,8 @@ void CreatePegInInput(CMutableTransaction& mtx, uint32_t input_idx, Sidechain::B
CreatePegInInputInner(mtx, input_idx, tx_btc, merkle_block, claim_scripts, txData, txOutProofData, active_chain_tip);
}
CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional<bool> rbf, const CBlockIndex* active_chain_tip, std::map<CTxOut, PSBTOutput>* outputs_aux, bool allow_peg_in, bool allow_issuance)
void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, std::optional<bool> rbf, const CBlockIndex* active_chain_tip, bool allow_peg_in, bool allow_issuance)
{
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;
@ -158,17 +154,6 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal
inputs = inputs_in.get_array();
}
const UniValue& outputs = outputs_in.get_array();
CMutableTransaction rawTx;
if (!locktime.isNull()) {
int64_t nLockTime = locktime.getInt<int64_t>();
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();
@ -279,6 +264,16 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal
}
}
}
void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in, std::map<CTxOut, PSBTOutput>* outputs_aux)
{
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();
// Keep track of the fee output so we can add it in the very end of the transaction.
CTxOut fee_out;
@ -383,6 +378,21 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal
outputs_aux->insert(std::make_pair(rawTx.vout[i], psbt_outs[i]));
}
}
}
CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional<bool> rbf, const CBlockIndex* active_chain_tip, std::map<CTxOut, PSBTOutput>* outputs_aux, bool allow_peg_in, bool allow_issuance)
{
CMutableTransaction rawTx;
if (!locktime.isNull()) {
int64_t nLockTime = locktime.getInt<int64_t>();
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, active_chain_tip, allow_peg_in, allow_issuance);
AddOutputs(rawTx, outputs_in, outputs_aux);
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");

View file

@ -47,11 +47,19 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const
*/
void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keystore, std::map<COutPoint, Coin>& coins);
/** Normalize univalue-represented inputs and add them to the transaction */
void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, bool rbf, const CBlockIndex* active_chain_tip, bool allow_peg_in = true, bool allow_issuance = true);
/** Normalize univalue-represented outputs and add them to the transaction */
void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in, std::map<CTxOut, PSBTOutput>* outputs_aux = nullptr);
/** Create a transaction from univalue parameters. If (and only if)
output_pubkeys_out is null, the "nonce hack" of storing Confidential
Assets output pubkeys in nonces will be used. */
CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, std::optional<bool> rbf, const CBlockIndex* active_chain_tip, std::map<CTxOut, PSBTOutput>* outputs_aux = nullptr, bool allow_peg_in = true, bool allow_issuance = true);
// ELEMENTS
/** Create a peg-in input */
void CreatePegInInput(CMutableTransaction& mtx, uint32_t input_idx, CTransactionRef& tx_btc, CMerkleBlock& merkle_block, const std::set<CScript>& claim_scripts, const std::vector<unsigned char>& txData, const std::vector<unsigned char>& txOutProofData, const CBlockIndex* active_chain_tip);
void CreatePegInInput(CMutableTransaction& mtx, uint32_t input_idx, Sidechain::Bitcoin::CTransactionRef& tx_btc, Sidechain::Bitcoin::CMerkleBlock& merkle_block, const std::set<CScript>& claim_scripts, const std::vector<unsigned char>& txData, const std::vector<unsigned char>& txOutProofData, const CBlockIndex* active_chain_tip);

View file

@ -156,7 +156,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
}
Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& 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<CTxOut>& outputs)
{
// We are going to modify coin control later, copy to re-use
CCoinControl new_coin_control(coin_control);

View file

@ -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<CTxOut>& outputs);
//! Sign the new transaction,
//! @return false if the tx couldn't be found or if it was

View file

@ -305,7 +305,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<CTxOut> 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,

View file

@ -1060,6 +1060,26 @@ RPCHelpMan signrawtransactionwithwallet()
};
}
// Definition of allowed formats of specifying transaction outputs in
// `bumpfee`, `psbtbumpfee`, `send` and `walletcreatefundedpsbt` RPCs.
static std::vector<RPCArg> OutputsDoc()
{
return
{
{"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "",
{
{"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the bitcoin address,\n"
"the value (float or string) is the amount in " + CURRENCY_UNIT + ""},
},
},
{"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
{
{"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"},
},
},
};
}
static RPCHelpMan bumpfee_helper(std::string method_name)
{
const bool want_psbt = method_name == "psbtbumpfee";
@ -1096,7 +1116,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"}},
},
@ -1133,6 +1158,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<CTxOut> outputs;
if (!request.params[1].isNull()) {
UniValue options = request.params[1];
@ -1143,6 +1169,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);
@ -1156,6 +1183,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
@ -1173,7 +1210,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:
@ -1248,18 +1285,7 @@ RPCHelpMan send()
{"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The outputs (key-value pairs), where none of the keys are duplicated.\n"
"That is, each address can only appear once and there can only be one 'data' object.\n"
"For convenience, a dictionary, which holds the key-value pairs directly, is also accepted.",
{
{"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "",
{
{"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""},
},
},
{"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
{
{"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"},
},
},
},
OutputsDoc(),
RPCArgOptions{.skip_type_check = true}},
{"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"},
{"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n"

View file

@ -83,7 +83,7 @@ class BumpFeeTest(BitcoinTestFramework):
self.log.info("Running tests")
dest_address = peer_node.getnewaddress()
for mode in ["default", "fee_rate"]:
for mode in ["default", "fee_rate", "new_outputs"]:
test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address)
self.test_invalid_parameters(rbf_node, peer_node, dest_address)
test_segwit_bumpfee_succeeds(self, rbf_node, dest_address)
@ -161,6 +161,14 @@ class BumpFeeTest(BitcoinTestFramework):
assert_raises_rpc_error(-8, 'Invalid estimate_mode parameter, must be one of: "unset", "economical", "conservative"',
rbf_node.bumpfee, rbfid, {"estimate_mode": mode})
self.log.info("Test invalid outputs values")
assert_raises_rpc_error(-8, "Invalid parameter, output argument cannot be an empty array",
rbf_node.bumpfee, rbfid, {"outputs": []})
assert_raises_rpc_error(-8, "Invalid parameter, duplicated address: " + dest_address,
rbf_node.bumpfee, rbfid, {"outputs": [{dest_address: 0.1}, {dest_address: 0.2}]})
assert_raises_rpc_error(-8, "Invalid parameter, duplicate key: data",
rbf_node.bumpfee, rbfid, {"outputs": [{"data": "deadbeef"}, {"data": "deadbeef"}]})
self.clear_mempool()
@ -173,6 +181,10 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
if mode == "fee_rate":
bumped_psbt = rbf_node.psbtbumpfee(rbfid, {"fee_rate": str(NORMAL)})
bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": NORMAL})
elif mode == "new_outputs":
new_address = peer_node.getnewaddress()
bumped_psbt = rbf_node.psbtbumpfee(rbfid, {"outputs": [{new_address: 0.0003}]})
bumped_tx = rbf_node.bumpfee(rbfid, {"outputs": [{new_address: 0.0003}]})
else:
bumped_psbt = rbf_node.psbtbumpfee(rbfid)
bumped_tx = rbf_node.bumpfee(rbfid)
@ -196,6 +208,10 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
bumpedwtx = rbf_node.gettransaction(bumped_tx["txid"])
assert_equal(oldwtx["replaced_by_txid"], bumped_tx["txid"])
assert_equal(bumpedwtx["replaces_txid"], rbfid)
# if this is a new_outputs test, check that outputs were indeed replaced
if mode == "new_outputs":
assert len(bumpedwtx["details"]) == 1
# assert bumpedwtx["details"][0]["address"] == new_address # ELEMENTS FIXME: address differs
self.clear_mempool()
@ -644,12 +660,14 @@ def test_change_script_match(self, rbf_node, dest_address):
self.clear_mempool()
def spend_one_input(node, dest_address, change_size=Decimal("0.0004000")):
def spend_one_input(node, dest_address, change_size=Decimal("0.00040000"), data=None):
tx_input = dict(
sequence=MAX_BIP125_RBF_SEQUENCE, **next(u for u in node.listunspent() if u["amount"] == Decimal("0.00100000")))
destinations = [{dest_address: Decimal("0.00050000")}]
if change_size > 0:
destinations.append({node.getrawchangeaddress(): change_size})
if data:
destinations['data'] = data
destinations.append({"fee": Decimal("0.0001")})
rawtx = node.createrawtransaction([tx_input], destinations)
signedtx = node.signrawtransactionwithwallet(rawtx)