Merge #16257: [wallet] abort when attempting to fund a transaction above -maxtxfee

806b0052c3 [wallet] abort when attempting to fund a transaction above maxtxfee (Sjors Provoost)

Pull request description:

  `FundTransaction` calls `GetMinimumFee` which, when the fee rate is absurdly high, quietly reduces the fee to `-maxtxfee`.

  Becaue an absurdly high fee rate is usually the result of a fat finger, aborting seems safer behavior.

  Before:
  ```
  bitcoin-cli walletcreatefundedpsbt '[]' '[{"tb1q...": 0.01}]' 0 '{"feeRate": 10}' true
  {
    "psbt": "cHNidP8...gAA=",
    "fee": 0.10000000,
    "changepos": 1
  }

  ```

  After:
  ```
  bitcoin-cli walletcreatefundedpsbt '[]' '[{"tb1q...": 0.01}]' 0 '{"feeRate": 10}' true
  error code: -25
  error message:
  Fee exceeds maximum configured by -maxtxfee
  ```

  QT still checks the max fee rate as expected:
  <img width="566" alt="Schermafbeelding 2019-06-20 om 19 52 00" src="https://user-images.githubusercontent.com/10217/59888424-a2aa7100-9395-11e9-8ae6-8a3c1f7de585.png">

ACKs for top commit:
  laanwj:
    Code review ACK 806b0052c3

Tree-SHA512: bee95811711cdab100b614d2347921407af3b400aea613ca156953ed3f60b924ad29a1d335bd0e240c0b7c0fbb360226bab03294d226a5560cdf2a3f21e6d406
This commit is contained in:
Wladimir J. van der Laan 2019-07-01 16:00:33 +02:00
commit 1212808762
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
10 changed files with 27 additions and 13 deletions

View file

@ -0,0 +1,6 @@
Wallet changes
--------------
When creating a transaction with a fee above `-maxtxfee` (default 0.1 BTC),
the RPC commands `walletcreatefundedpsbt` and `fundrawtransaction` will now fail
instead of rounding down the fee. Beware that the `feeRate` argument is specified
in BTC per kilobyte, not satoshi per byte.

View file

@ -43,7 +43,6 @@ enum class FeeReason {
PAYTXFEE,
FALLBACK,
REQUIRED,
MAXTXFEE,
};
/* Used to determine type of fee estimation requested */

View file

@ -221,9 +221,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
return TransactionCreationFailed;
}
// reject absurdly high fee. (This can never happen because the
// wallet caps the fee at m_default_max_tx_fee. This merely serves as a
// belt-and-suspenders check)
// Reject absurdly high fee
if (nFeeRequired > m_wallet->getDefaultMaxTxFee())
return AbsurdFee;
}

View file

@ -27,6 +27,8 @@ std::string TransactionErrorString(const TransactionError err)
return "PSBTs not compatible (different transactions)";
case TransactionError::SIGHASH_MISMATCH:
return "Specified sighash value does not match existing value";
case TransactionError::MAX_FEE_EXCEEDED:
return "Fee exceeds maximum configured by -maxtxfee";
// no default case, so the compiler can warn about missing cases
}
assert(false);

View file

@ -27,6 +27,7 @@ enum class TransactionError {
INVALID_PSBT,
PSBT_MISMATCH,
SIGHASH_MISMATCH,
MAX_FEE_EXCEEDED,
};
std::string TransactionErrorString(const TransactionError error);

View file

@ -18,7 +18,6 @@ std::string StringForFeeReason(FeeReason reason) {
{FeeReason::PAYTXFEE, "PayTxFee set"},
{FeeReason::FALLBACK, "Fallback fee"},
{FeeReason::REQUIRED, "Minimum Required Fee"},
{FeeReason::MAXTXFEE, "MaxTxFee limit"}
};
auto reason_string = fee_reason_strings.find(reason);

View file

@ -18,14 +18,7 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes)
CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, FeeCalculation* feeCalc)
{
CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes);
// Always obey the maximum
const CAmount max_tx_fee = wallet.m_default_max_tx_fee;
if (fee_needed > max_tx_fee) {
fee_needed = max_tx_fee;
if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE;
}
return fee_needed;
return GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes);
}
CFeeRate GetRequiredFeeRate(const CWallet& wallet)

View file

@ -2694,6 +2694,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
}
}
if (nFeeRet > this->m_default_max_tx_fee) {
strFailReason = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);
return false;
}
return true;
}

View file

@ -662,6 +662,7 @@ class RawTransactionsTest(BitcoinTestFramework):
result = self.nodes[3].fundrawtransaction(rawtx) # uses min_relay_tx_fee (set by settxfee)
result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee})
result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10*min_relay_tx_fee})
assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[3].fundrawtransaction, rawtx, {"feeRate": 1})
result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex'])
assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate)
assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate)

View file

@ -9,6 +9,7 @@ from decimal import Decimal
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_greater_than,
assert_raises_rpc_error,
connect_nodes_bi,
disconnect_nodes,
@ -129,6 +130,15 @@ class PSBTTest(BitcoinTestFramework):
assert_equal(walletprocesspsbt_out['complete'], True)
self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex'])
# feeRate of 0.1 BTC / KB produces a total fee slightly below -maxtxfee (~0.05280000):
res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 0.1})
assert_greater_than(res["fee"], 0.05)
assert_greater_than(0.06, res["fee"])
# feeRate of 10 BTC / KB produces a total fee well above -maxtxfee
# previously this was silenty capped at -maxtxfee
assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 10})
# partially sign multisig things with node 1
psbtx = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wsh_pos},{"txid":txid,"vout":p2sh_pos},{"txid":txid,"vout":p2sh_p2wsh_pos}], {self.nodes[1].getnewaddress():29.99})['psbt']
walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(psbtx)