wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction

promag · promag · commit 5c1b9714cb0a · 2019-07-02T16:13:39.000+01:00
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
@@ -221,9 +221,12 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
             return TransactionCreationFailed;
         }
 
-        // Reject absurdly high fee
-        if (nFeeRequired > m_wallet->getDefaultMaxTxFee())
+        // Reject absurdly high fee. (This can never happen because the
+        // wallet never creates transactions with fee greater than
+        // m_default_max_tx_fee. This merely a belt-and-suspenders check).
+        if (nFeeRequired > m_wallet->getDefaultMaxTxFee()) {
             return AbsurdFee;
+        }
     }
 
     return SendCoinsReturn(OK);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -2694,11 +2694,6 @@ 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;
 }
 
@@ -3135,6 +3130,11 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
         }
     }
 
+    if (nFeeRet > m_default_max_tx_fee) {
+        strFailReason = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);
+        return false;
+    }
+
     if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) {
         // Lastly, ensure this tx will pass the mempool's chain limits
         if (!chain().checkChainLimits(tx)) {
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
@@ -136,7 +136,7 @@ def run_test(self):
         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
+        # previously this was silently 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
diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py
@@ -80,6 +80,7 @@ def run_test(self):
         test_bumpfee_metadata(rbf_node, dest_address)
         test_locked_wallet_fails(rbf_node, dest_address)
         test_change_script_match(rbf_node, dest_address)
+        test_maxtxfee_fails(self, rbf_node, dest_address)
         # These tests wipe out a number of utxos that are expected in other tests
         test_small_output_with_feerate_succeeds(rbf_node, dest_address)
         test_no_more_inputs_fails(rbf_node, dest_address)
@@ -248,6 +249,15 @@ def test_settxfee(rbf_node, dest_address):
     rbf_node.settxfee(Decimal("0.00000000"))  # unset paytxfee
 
 
+def test_maxtxfee_fails(test, rbf_node, dest_address):
+    test.restart_node(1, ['-maxtxfee=0.00003'] + test.extra_args[1])
+    rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
+    rbfid = spend_one_input(rbf_node, dest_address)
+    assert_raises_rpc_error(-4, "Unable to create transaction: Fee exceeds maximum configured by -maxtxfee", rbf_node.bumpfee, rbfid)
+    test.restart_node(1, test.extra_args[1])
+    rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
+
+
 def test_rebumping(rbf_node, dest_address):
     # check that re-bumping the original tx fails, but bumping the bumper succeeds
     rbfid = spend_one_input(rbf_node, dest_address)

Original file line number	Diff line number	Diff line change
`@@ -221,9 +221,12 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact`
`221`	`221`	`return TransactionCreationFailed;`
`222`	`222`	`}`
`223`	`223`
`224`		`- // Reject absurdly high fee`
`225`		`- if (nFeeRequired > m_wallet->getDefaultMaxTxFee())`
	`224`	`+ // Reject absurdly high fee. (This can never happen because the`
	`225`	`+ // wallet never creates transactions with fee greater than`
	`226`	`+ // m_default_max_tx_fee. This merely a belt-and-suspenders check).`
	`227`	`+ if (nFeeRequired > m_wallet->getDefaultMaxTxFee()) {`
`226`	`228`	`return AbsurdFee;`
	`229`	`+ }`
`227`	`230`	`}`
`228`	`231`
`229`	`232`	`return SendCoinsReturn(OK);`
Original file line number	Diff line number	Diff line change
`@@ -2694,11 +2694,6 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC`
`2694`	`2694`	`}`
`2695`	`2695`	`}`
`2696`	`2696`
`2697`		`- if (nFeeRet > this->m_default_max_tx_fee) {`
`2698`		`- strFailReason = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);`
`2699`		`- return false;`
`2700`		`- }`
`2701`		`-`
`2702`	`2697`	`return true;`
`2703`	`2698`	`}`
`2704`	`2699`
`@@ -3135,6 +3130,11 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std`
`3135`	`3130`	`}`
`3136`	`3131`	`}`
`3137`	`3132`
	`3133`	`+ if (nFeeRet > m_default_max_tx_fee) {`
	`3134`	`+ strFailReason = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);`
	`3135`	`+ return false;`
	`3136`	`+ }`
	`3137`	`+`
`3138`	`3138`	`if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) {`
`3139`	`3139`	`// Lastly, ensure this tx will pass the mempool's chain limits`
`3140`	`3140`	`if (!chain().checkChainLimits(tx)) {`