diff options
| -rw-r--r-- | src/rpc/client.cpp | 1 | ||||
| -rw-r--r-- | src/wallet/rpcwallet.cpp | 114 | ||||
| -rwxr-xr-x | test/functional/rpc_deprecated.py | 37 | ||||
| -rwxr-xr-x | test/functional/wallet_bumpfee.py | 8 |
4 files changed, 109 insertions, 51 deletions
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 97d1c00369..d61e02aee2 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -151,6 +151,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getmempoolancestors", 1, "verbose" }, { "getmempooldescendants", 1, "verbose" }, { "bumpfee", 1, "options" }, + { "psbtbumpfee", 1, "options" }, { "logging", 0, "include" }, { "logging", 1, "exclude" }, { "disconnectnode", 1, "nodeid" }, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 58eaf54175..17460a2932 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3222,59 +3222,73 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) static UniValue bumpfee(const JSONRPCRequest& request) { - RPCHelpMan{"bumpfee", - "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" - "An opt-in RBF transaction with the given txid must be in the wallet.\n" - "The command will pay the additional fee by reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist.\n" - "All inputs in the original transaction will be included in the replacement transaction.\n" - "The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n" - "By default, the new fee will be calculated automatically using estimatesmartfee.\n" - "The user can specify a confirmation target for estimatesmartfee.\n" - "Alternatively, the user can specify a fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction.\n" - "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" - "returned by getnetworkinfo) to enter the node's mempool.\n", + bool want_psbt = request.strMethod == "psbtbumpfee"; + + RPCHelpMan{request.strMethod, + "\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n" + + std::string(want_psbt ? "Returns a PSBT instead of creating and signing a new transaction.\n" : "") + + "An opt-in RBF transaction with the given txid must be in the wallet.\n" + "The command will pay the additional fee by reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist.\n" + "All inputs in the original transaction will be included in the replacement transaction.\n" + "The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n" + "By default, the new fee will be calculated automatically using estimatesmartfee.\n" + "The user can specify a confirmation target for estimatesmartfee.\n" + "Alternatively, the user can specify a fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction.\n" + "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" + "returned by getnetworkinfo) to enter the node's mempool.\n", + { + {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { - {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", - { - {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, - {"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'conf_target'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n" - " Specify a fee rate instead of relying on the built-in fee estimator.\n" - "Must be at least 0.0001 " + CURRENCY_UNIT + " per kB higher than the current transaction fee rate.\n"}, - {"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n" - " marked bip-125 replaceable. If true, the sequence numbers in the transaction will\n" - " be left unchanged from the original. If false, any input sequence numbers in the\n" - " original transaction that were less than 0xfffffffe will be increased to 0xfffffffe\n" - " so the new transaction will not be explicitly bip-125 replaceable (though it may\n" - " still be replaceable in practice, for example if it has unconfirmed ancestors which\n" - " are replaceable)."}, - {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" - " \"" + FeeModes("\"\n\"") + "\""}, - }, - "options"}, - }, - RPCResult{ - RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "psbt", "The base64-encoded unsigned PSBT of the new transaction. Only returned when wallet private keys are disabled."}, - {RPCResult::Type::STR_HEX, "txid", "The id of the new transaction. Only returned when wallet private keys are enabled."}, - {RPCResult::Type::STR_AMOUNT, "origfee", "The fee of the replaced transaction."}, - {RPCResult::Type::STR_AMOUNT, "fee", "The fee of the new transaction."}, - {RPCResult::Type::ARR, "errors", "Errors encountered during processing (may be empty).", - { - {RPCResult::Type::STR, "", ""}, - }}, - } - }, - RPCExamples{ - "\nBump the fee, get the new transaction\'s txid\n" + - HelpExampleCli("bumpfee", "<txid>") + {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"}, + {"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'conf_target'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n" + " Specify a fee rate instead of relying on the built-in fee estimator.\n" + "Must be at least 0.0001 " + CURRENCY_UNIT + " per kB higher than the current transaction fee rate.\n"}, + {"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n" + " marked bip-125 replaceable. If true, the sequence numbers in the transaction will\n" + " be left unchanged from the original. If false, any input sequence numbers in the\n" + " original transaction that were less than 0xfffffffe will be increased to 0xfffffffe\n" + " so the new transaction will not be explicitly bip-125 replaceable (though it may\n" + " still be replaceable in practice, for example if it has unconfirmed ancestors which\n" + " are replaceable)."}, + {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" + " \"" + FeeModes("\"\n\"") + "\""}, }, - }.Check(request); + "options"}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>( + { + {RPCResult::Type::STR, "psbt", "The base64-encoded unsigned PSBT of the new transaction." + std::string(want_psbt ? "" : " Only returned when wallet private keys are disabled. (DEPRECATED)")}, + }, + want_psbt ? std::vector<RPCResult>{} : std::vector<RPCResult>{{RPCResult::Type::STR_HEX, "txid", "The id of the new transaction. Only returned when wallet private keys are enabled."}} + ), + { + {RPCResult::Type::STR_AMOUNT, "origfee", "The fee of the replaced transaction."}, + {RPCResult::Type::STR_AMOUNT, "fee", "The fee of the new transaction."}, + {RPCResult::Type::ARR, "errors", "Errors encountered during processing (may be empty).", + { + {RPCResult::Type::STR, "", ""}, + }}, + }) + }, + RPCExamples{ + "\nBump the fee, get the new transaction\'s" + std::string(want_psbt ? "psbt" : "txid") + "\n" + + HelpExampleCli(request.strMethod, "<txid>") + }, + }.Check(request); std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !want_psbt) { + if (!pwallet->chain().rpcEnableDeprecated("bumpfee")) { + throw JSONRPCError(RPC_METHOD_DEPRECATED, "Using bumpfee with wallets that have private keys disabled is deprecated. Use psbtbumpfee instead or restart bitcoind with -deprecatedrpc=bumpfee. This functionality will be removed in 0.22"); + } + want_psbt = true; + } + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ}); uint256 hash(ParseHashV(request.params[0], "txid")); @@ -3359,7 +3373,7 @@ static UniValue bumpfee(const JSONRPCRequest& request) // If wallet private keys are enabled, return the new transaction id, // otherwise return the base64-encoded unsigned PSBT of the new transaction. - if (!pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + if (!want_psbt) { if (!feebumper::SignTransaction(*pwallet, mtx)) { throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction."); } @@ -3392,6 +3406,11 @@ static UniValue bumpfee(const JSONRPCRequest& request) return result; } +static UniValue psbtbumpfee(const JSONRPCRequest& request) +{ + return bumpfee(request); +} + UniValue rescanblockchain(const JSONRPCRequest& request) { RPCHelpMan{"rescanblockchain", @@ -4137,6 +4156,7 @@ static const CRPCCommand commands[] = { "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","label","address_type"} }, { "wallet", "backupwallet", &backupwallet, {"destination"} }, { "wallet", "bumpfee", &bumpfee, {"txid", "options"} }, + { "wallet", "psbtbumpfee", &psbtbumpfee, {"txid", "options"} }, { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse", "descriptors"} }, { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py index 9a21998d11..b71854d234 100755 --- a/test/functional/rpc_deprecated.py +++ b/test/functional/rpc_deprecated.py @@ -4,13 +4,13 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test deprecation of RPC calls.""" from test_framework.test_framework import BitcoinTestFramework -# from test_framework.util import assert_raises_rpc_error +from test_framework.util import assert_raises_rpc_error, find_vout_for_address class DeprecatedRpcTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True - self.extra_args = [[], []] + self.extra_args = [[], ['-deprecatedrpc=bumpfee']] def run_test(self): # This test should be used to verify correct behaviour of deprecated @@ -23,7 +23,38 @@ class DeprecatedRpcTest(BitcoinTestFramework): # self.log.info("Test generate RPC") # assert_raises_rpc_error(-32, 'The wallet generate rpc method is deprecated', self.nodes[0].rpc.generate, 1) # self.nodes[1].generate(1) - self.log.info("No tested deprecated RPC methods") + + if self.is_wallet_compiled(): + self.log.info("Test bumpfee RPC") + self.nodes[0].generate(101) + self.nodes[0].createwallet(wallet_name='nopriv', disable_private_keys=True) + noprivs0 = self.nodes[0].get_wallet_rpc('nopriv') + w0 = self.nodes[0].get_wallet_rpc('') + self.nodes[1].createwallet(wallet_name='nopriv', disable_private_keys=True) + noprivs1 = self.nodes[1].get_wallet_rpc('nopriv') + + address = w0.getnewaddress() + desc = w0.getaddressinfo(address)['desc'] + change_addr = w0.getrawchangeaddress() + change_desc = w0.getaddressinfo(change_addr)['desc'] + txid = w0.sendtoaddress(address=address, amount=10) + vout = find_vout_for_address(w0, txid, address) + self.nodes[0].generate(1) + rawtx = w0.createrawtransaction([{'txid': txid, 'vout': vout}], {w0.getnewaddress(): 5}, 0, True) + rawtx = w0.fundrawtransaction(rawtx, {'changeAddress': change_addr}) + signed_tx = w0.signrawtransactionwithwallet(rawtx['hex'])['hex'] + + noprivs0.importmulti([{'desc': desc, 'timestamp': 0}, {'desc': change_desc, 'timestamp': 0, 'internal': True}]) + noprivs1.importmulti([{'desc': desc, 'timestamp': 0}, {'desc': change_desc, 'timestamp': 0, 'internal': True}]) + + txid = w0.sendrawtransaction(signed_tx) + self.sync_all() + + assert_raises_rpc_error(-32, 'Using bumpfee with wallets that have private keys disabled is deprecated. Use psbtbumpfee instead or restart bitcoind with -deprecatedrpc=bumpfee. This functionality will be removed in 0.22', noprivs0.bumpfee, txid) + bumped_psbt = noprivs1.bumpfee(txid) + assert 'psbt' in bumped_psbt + else: + self.log.info("No tested deprecated RPC methods") if __name__ == '__main__': DeprecatedRpcTest().main() diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 0ef78b0e1c..53496084ef 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -123,13 +123,19 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address): self.sync_mempools((rbf_node, peer_node)) assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool() if mode == "fee_rate": + bumped_psbt = rbf_node.psbtbumpfee(rbfid, {"fee_rate": NORMAL}) bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": NORMAL}) else: + bumped_psbt = rbf_node.psbtbumpfee(rbfid) bumped_tx = rbf_node.bumpfee(rbfid) assert_equal(bumped_tx["errors"], []) assert bumped_tx["fee"] > -rbftx["fee"] assert_equal(bumped_tx["origfee"], -rbftx["fee"]) assert "psbt" not in bumped_tx + assert_equal(bumped_psbt["errors"], []) + assert bumped_psbt["fee"] > -rbftx["fee"] + assert_equal(bumped_psbt["origfee"], -rbftx["fee"]) + assert "psbt" in bumped_psbt # check that bumped_tx propagates, original tx was evicted and has a wallet conflict self.sync_mempools((rbf_node, peer_node)) assert bumped_tx["txid"] in rbf_node.getrawmempool() @@ -391,7 +397,7 @@ def test_watchonly_psbt(self, peer_node, rbf_node, dest_address): assert_equal(len(watcher.decodepsbt(psbt)["tx"]["vin"]), 1) # Bump fee, obnoxiously high to add additional watchonly input - bumped_psbt = watcher.bumpfee(original_txid, {"fee_rate": HIGH}) + bumped_psbt = watcher.psbtbumpfee(original_txid, {"fee_rate": HIGH}) assert_greater_than(len(watcher.decodepsbt(bumped_psbt['psbt'])["tx"]["vin"]), 1) assert "txid" not in bumped_psbt assert_equal(bumped_psbt["origfee"], -watcher.gettransaction(original_txid)["fee"]) |