Merge #16373: bumpfee: Return PSBT when wallet has privkeys disabled
091a876664
Test watchonly wallet bumpfee with PSBT return (Gregory Sanders)e9b4f9419c
bumpfee: Return PSBT when wallet has privkeys disabled (Gregory Sanders)75a5e478b6
Change bumpfee to use watch-only funds for legacy watchonly wallets (Gregory Sanders) Pull request description: The main use-case here is for using with watch-only wallets with PSBT-signing cold wallets of all kinds. ACKs for top commit: achow101: ACK091a876664
Sjors: Tested ACK091a876664
meshcollider: utACK091a876664
Tree-SHA512: f7cf663e1af0b029e5c99eac88c5fdc3bc9e9a3841da8a608e8a9957e9bcf6a78864b8c2706fcaf78a480ffe11badd80c4fad29f97c0bb929e0470fafda5c22e
This commit is contained in:
commit
45f151913e
3 changed files with 123 additions and 19 deletions
|
@ -47,7 +47,8 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet
|
|||
|
||||
// check that original tx consists entirely of our inputs
|
||||
// if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
|
||||
if (!wallet.IsAllFromMe(*wtx.tx, ISMINE_SPENDABLE)) {
|
||||
isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
|
||||
if (!wallet.IsAllFromMe(*wtx.tx, filter)) {
|
||||
errors.push_back("Transaction contains inputs that don't belong to this wallet");
|
||||
return feebumper::Result::WALLET_ERROR;
|
||||
}
|
||||
|
@ -78,7 +79,8 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wt
|
|||
CFeeRate incrementalRelayFee = std::max(wallet.chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE));
|
||||
|
||||
// Given old total fee and transaction size, calculate the old feeRate
|
||||
CAmount old_fee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut();
|
||||
isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
|
||||
CAmount old_fee = wtx.GetDebit(filter) - wtx.tx->GetValueOut();
|
||||
const int64_t txSize = GetVirtualTransactionSize(*(wtx.tx));
|
||||
CFeeRate nOldFeeRate(old_fee, txSize);
|
||||
// Min total fee is old fee + relay fee
|
||||
|
@ -195,7 +197,8 @@ Result CreateTotalBumpTransaction(const CWallet* wallet, const uint256& txid, co
|
|||
}
|
||||
|
||||
// calculate the old fee and fee-rate
|
||||
old_fee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut();
|
||||
isminefilter filter = wallet->GetLegacyScriptPubKeyMan() && wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
|
||||
old_fee = wtx.GetDebit(filter) - wtx.tx->GetValueOut();
|
||||
CFeeRate nOldFeeRate(old_fee, txSize);
|
||||
// The wallet uses a conservative WALLET_INCREMENTAL_RELAY_FEE value to
|
||||
// future proof against changes to network wide policy for incremental relay
|
||||
|
@ -308,7 +311,8 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
|
|||
}
|
||||
}
|
||||
|
||||
old_fee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut();
|
||||
isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
|
||||
old_fee = wtx.GetDebit(filter) - wtx.tx->GetValueOut();
|
||||
|
||||
if (coin_control.m_feerate) {
|
||||
// The user provided a feeRate argument.
|
||||
|
|
|
@ -3365,10 +3365,11 @@ static UniValue bumpfee(const JSONRPCRequest& request)
|
|||
},
|
||||
RPCResult{
|
||||
"{\n"
|
||||
" \"txid\": \"value\", (string) The id of the new transaction\n"
|
||||
" \"origfee\": n, (numeric) Fee of the replaced transaction\n"
|
||||
" \"fee\": n, (numeric) Fee of the new transaction\n"
|
||||
" \"errors\": [ str... ] (json array of strings) Errors encountered during processing (may be empty)\n"
|
||||
" \"psbt\": \"psbt\", (string) The base64-encoded unsigned PSBT of the new transaction. Only returned when wallet private keys are disabled.\n"
|
||||
" \"txid\": \"value\", (string) The id of the new transaction. Only returned when wallet private keys are enabled.\n"
|
||||
" \"origfee\": n, (numeric) The fee of the replaced transaction.\n"
|
||||
" \"fee\": n, (numeric) The fee of the new transaction.\n"
|
||||
" \"errors\": [ str... ] (json array of strings) Errors encountered during processing (may be empty).\n"
|
||||
"}\n"
|
||||
},
|
||||
RPCExamples{
|
||||
|
@ -3380,10 +3381,12 @@ static UniValue bumpfee(const JSONRPCRequest& request)
|
|||
RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ});
|
||||
uint256 hash(ParseHashV(request.params[0], "txid"));
|
||||
|
||||
CCoinControl coin_control;
|
||||
coin_control.fAllowWatchOnly = pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
|
||||
// optional parameters
|
||||
CAmount totalFee = 0;
|
||||
CCoinControl coin_control;
|
||||
coin_control.m_signal_bip125_rbf = true;
|
||||
|
||||
if (!request.params[1].isNull()) {
|
||||
UniValue options = request.params[1];
|
||||
RPCTypeCheckObj(options,
|
||||
|
@ -3468,17 +3471,32 @@ static UniValue bumpfee(const JSONRPCRequest& request)
|
|||
}
|
||||
}
|
||||
|
||||
// sign bumped transaction
|
||||
if (!feebumper::SignTransaction(*pwallet, mtx)) {
|
||||
throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction.");
|
||||
}
|
||||
// commit the bumped transaction
|
||||
uint256 txid;
|
||||
if (feebumper::CommitTransaction(*pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {
|
||||
throw JSONRPCError(RPC_WALLET_ERROR, errors[0]);
|
||||
}
|
||||
UniValue result(UniValue::VOBJ);
|
||||
result.pushKV("txid", txid.GetHex());
|
||||
|
||||
// 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 (!feebumper::SignTransaction(*pwallet, mtx)) {
|
||||
throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction.");
|
||||
}
|
||||
|
||||
uint256 txid;
|
||||
if (feebumper::CommitTransaction(*pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {
|
||||
throw JSONRPCError(RPC_WALLET_ERROR, errors[0]);
|
||||
}
|
||||
|
||||
result.pushKV("txid", txid.GetHex());
|
||||
} else {
|
||||
PartiallySignedTransaction psbtx(mtx);
|
||||
bool complete = false;
|
||||
const TransactionError err = FillPSBT(pwallet, psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */);
|
||||
CHECK_NONFATAL(err == TransactionError::OK);
|
||||
CHECK_NONFATAL(!complete);
|
||||
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
|
||||
ssTx << psbtx;
|
||||
result.pushKV("psbt", EncodeBase64(ssTx.str()));
|
||||
}
|
||||
|
||||
result.pushKV("origfee", ValueFromAmount(old_fee));
|
||||
result.pushKV("fee", ValueFromAmount(new_fee));
|
||||
UniValue result_errors(UniValue::VARR);
|
||||
|
|
|
@ -78,6 +78,7 @@ class BumpFeeTest(BitcoinTestFramework):
|
|||
test_small_output_fails(rbf_node, dest_address)
|
||||
test_dust_to_fee(rbf_node, dest_address)
|
||||
test_settxfee(rbf_node, dest_address)
|
||||
test_watchonly_psbt(self, peer_node, rbf_node, dest_address)
|
||||
test_rebumping(rbf_node, dest_address)
|
||||
test_rebumping_not_replaceable(rbf_node, dest_address)
|
||||
test_unconfirmed_not_spendable(rbf_node, rbf_node_address)
|
||||
|
@ -103,6 +104,7 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
|
|||
assert_equal(bumped_tx["errors"], [])
|
||||
assert bumped_tx["fee"] > -rbftx["fee"]
|
||||
assert_equal(bumped_tx["origfee"], -rbftx["fee"])
|
||||
assert "psbt" not in bumped_tx
|
||||
# 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()
|
||||
|
@ -280,6 +282,86 @@ def test_maxtxfee_fails(test, rbf_node, dest_address):
|
|||
test.restart_node(1, test.extra_args[1])
|
||||
rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
|
||||
|
||||
def test_watchonly_psbt(test, peer_node, rbf_node, dest_address):
|
||||
priv_rec_desc = "wpkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/0/*)#rweraev0"
|
||||
pub_rec_desc = rbf_node.getdescriptorinfo(priv_rec_desc)["descriptor"]
|
||||
priv_change_desc = "wpkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/*)#j6uzqvuh"
|
||||
pub_change_desc = rbf_node.getdescriptorinfo(priv_change_desc)["descriptor"]
|
||||
# Create a wallet with private keys that can sign PSBTs
|
||||
rbf_node.createwallet(wallet_name="signer", disable_private_keys=False, blank=True)
|
||||
signer = rbf_node.get_wallet_rpc("signer")
|
||||
assert signer.getwalletinfo()['private_keys_enabled']
|
||||
result = signer.importmulti([{
|
||||
"desc": priv_rec_desc,
|
||||
"timestamp": 0,
|
||||
"range": [0,1],
|
||||
"internal": False,
|
||||
"keypool": False # Keys can only be imported to the keypool when private keys are disabled
|
||||
},
|
||||
{
|
||||
"desc": priv_change_desc,
|
||||
"timestamp": 0,
|
||||
"range": [0, 0],
|
||||
"internal": True,
|
||||
"keypool": False
|
||||
}])
|
||||
assert_equal(result, [{'success': True}, {'success': True}])
|
||||
|
||||
# Create another wallet with just the public keys, which creates PSBTs
|
||||
rbf_node.createwallet(wallet_name="watcher", disable_private_keys=True, blank=True)
|
||||
watcher = rbf_node.get_wallet_rpc("watcher")
|
||||
assert not watcher.getwalletinfo()['private_keys_enabled']
|
||||
|
||||
result = watcher.importmulti([{
|
||||
"desc": pub_rec_desc,
|
||||
"timestamp": 0,
|
||||
"range": [0,10],
|
||||
"internal": False,
|
||||
"keypool": True,
|
||||
"watchonly": True
|
||||
},
|
||||
{
|
||||
"desc": pub_change_desc,
|
||||
"timestamp": 0,
|
||||
"range": [0, 10],
|
||||
"internal": True,
|
||||
"keypool": True,
|
||||
"watchonly": True
|
||||
}])
|
||||
assert_equal(result, [{'success': True}, {'success': True}])
|
||||
|
||||
funding_address1 = watcher.getnewaddress(address_type='bech32')
|
||||
funding_address2 = watcher.getnewaddress(address_type='bech32')
|
||||
peer_node.sendmany("", {funding_address1: 0.001, funding_address2: 0.001})
|
||||
peer_node.generate(1)
|
||||
test.sync_all()
|
||||
|
||||
# Create single-input PSBT for transaction to be bumped
|
||||
psbt = watcher.walletcreatefundedpsbt([], {dest_address:0.0005}, 0, {"feeRate": 0.00001}, True)['psbt']
|
||||
psbt_signed = signer.walletprocesspsbt(psbt=psbt, sign=True, sighashtype="ALL", bip32derivs=True)
|
||||
psbt_final = watcher.finalizepsbt(psbt_signed["psbt"])
|
||||
original_txid = watcher.sendrawtransaction(psbt_final["hex"])
|
||||
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":0.005})
|
||||
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"])
|
||||
assert not watcher.finalizepsbt(bumped_psbt["psbt"])["complete"]
|
||||
|
||||
# Sign bumped transaction
|
||||
bumped_psbt_signed = signer.walletprocesspsbt(psbt=bumped_psbt["psbt"], sign=True, sighashtype="ALL", bip32derivs=True)
|
||||
bumped_psbt_final = watcher.finalizepsbt(bumped_psbt_signed["psbt"])
|
||||
assert bumped_psbt_final["complete"]
|
||||
|
||||
# Broadcast bumped transaction
|
||||
bumped_txid = watcher.sendrawtransaction(bumped_psbt_final["hex"])
|
||||
assert bumped_txid in rbf_node.getrawmempool()
|
||||
assert original_txid not in rbf_node.getrawmempool()
|
||||
|
||||
rbf_node.unloadwallet("watcher")
|
||||
rbf_node.unloadwallet("signer")
|
||||
|
||||
def test_rebumping(rbf_node, dest_address):
|
||||
# check that re-bumping the original tx fails, but bumping the bumper succeeds
|
||||
|
|
Loading…
Reference in a new issue