bumpfee: be able to bump fee of a tx with external inputs

In some cases, notably psbtbumpfee, it is okay, and potentially desired,
to be able to bump the fee of a transaction which contains external
inputs.
This commit is contained in:
Andrew Chow 2021-10-05 22:06:19 -04:00
parent 31dd3dc9e5
commit 1bc8106d4c
4 changed files with 28 additions and 16 deletions

View File

@ -20,7 +20,7 @@
namespace wallet { namespace wallet {
//! Check whether transaction has descendant in wallet or mempool, or has been //! Check whether transaction has descendant in wallet or mempool, or has been
//! mined, or conflicts with a mined transaction. Return a feebumper::Result. //! mined, or conflicts with a mined transaction. Return a feebumper::Result.
static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, bool require_mine, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{ {
if (wallet.HasWalletSpend(wtx.tx)) { if (wallet.HasWalletSpend(wtx.tx)) {
errors.push_back(Untranslated("Transaction has descendants in the wallet")); errors.push_back(Untranslated("Transaction has descendants in the wallet"));
@ -49,15 +49,16 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet
return feebumper::Result::WALLET_ERROR; return feebumper::Result::WALLET_ERROR;
} }
// check that original tx consists entirely of our inputs if (require_mine) {
// 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) // check that original tx consists entirely of our inputs
isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; // 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 (!AllInputsMine(wallet, *wtx.tx, filter)) { isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet")); if (!AllInputsMine(wallet, *wtx.tx, filter)) {
return feebumper::Result::WALLET_ERROR; errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet"));
return feebumper::Result::WALLET_ERROR;
}
} }
return feebumper::Result::OK; return feebumper::Result::OK;
} }
@ -149,12 +150,12 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
if (wtx == nullptr) return false; if (wtx == nullptr) return false;
std::vector<bilingual_str> errors_dummy; std::vector<bilingual_str> errors_dummy;
feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy); feebumper::Result res = PreconditionChecks(wallet, *wtx, /* require_mine=*/ true, errors_dummy);
return res == feebumper::Result::OK; return res == feebumper::Result::OK;
} }
Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors, Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx) CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine)
{ {
// We are going to modify coin control later, copy to re-use // We are going to modify coin control later, copy to re-use
CCoinControl new_coin_control(coin_control); CCoinControl new_coin_control(coin_control);
@ -216,7 +217,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
} }
} }
Result result = PreconditionChecks(wallet, wtx, errors); Result result = PreconditionChecks(wallet, wtx, require_mine, errors);
if (result != Result::OK) { if (result != Result::OK) {
return result; return result;
} }
@ -309,7 +310,7 @@ Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransacti
const CWalletTx& oldWtx = it->second; const CWalletTx& oldWtx = it->second;
// make sure the transaction still has no descendants and hasn't been mined in the meantime // make sure the transaction still has no descendants and hasn't been mined in the meantime
Result result = PreconditionChecks(wallet, oldWtx, errors); Result result = PreconditionChecks(wallet, oldWtx, /* require_mine=*/ false, errors);
if (result != Result::OK) { if (result != Result::OK) {
return result; return result;
} }

View File

@ -33,14 +33,25 @@ enum class Result
//! Return whether transaction can be bumped. //! Return whether transaction can be bumped.
bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid); bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
//! Create bumpfee transaction based on feerate estimates. /** Create bumpfee transaction based on feerate estimates.
*
* @param[in] wallet The wallet to use for this bumping
* @param[in] txid The txid of the transaction to bump
* @param[in] coin_control A CCoinControl object which provides feerates and other information used for coin selection
* @param[out] errors Errors
* @param[out] old_fee The fee the original transaction pays
* @param[out] new_fee the fee that the bump transaction pays
* @param[out] mtx The bump transaction itself
* @param[in] require_mine Whether the original transaction must consist of inputs that can be spent by the wallet
*/
Result CreateRateBumpTransaction(CWallet& wallet, Result CreateRateBumpTransaction(CWallet& wallet,
const uint256& txid, const uint256& txid,
const CCoinControl& coin_control, const CCoinControl& coin_control,
std::vector<bilingual_str>& errors, std::vector<bilingual_str>& errors,
CAmount& old_fee, CAmount& old_fee,
CAmount& new_fee, CAmount& new_fee,
CMutableTransaction& mtx); CMutableTransaction& mtx,
bool require_mine);
//! Sign the new transaction, //! Sign the new transaction,
//! @return false if the tx couldn't be found or if it was //! @return false if the tx couldn't be found or if it was

View File

@ -291,7 +291,7 @@ public:
CAmount& new_fee, CAmount& new_fee,
CMutableTransaction& mtx) override CMutableTransaction& mtx) override
{ {
return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) == feebumper::Result::OK; return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true) == feebumper::Result::OK;
} }
bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); } bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
bool commitBumpTransaction(const uint256& txid, bool commitBumpTransaction(const uint256& txid,

View File

@ -1045,7 +1045,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
CMutableTransaction mtx; CMutableTransaction mtx;
feebumper::Result res; feebumper::Result res;
// Targeting feerate bump. // Targeting feerate bump.
res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx); res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt);
if (res != feebumper::Result::OK) { if (res != feebumper::Result::OK) {
switch(res) { switch(res) {
case feebumper::Result::INVALID_ADDRESS_OR_KEY: case feebumper::Result::INVALID_ADDRESS_OR_KEY: