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

806b0052c3b45415862f74f20ba5f389e5b673de [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 806b0052c3b45415862f74f20ba5f389e5b673de

Tree-SHA512: bee95811711cdab100b614d2347921407af3b400aea613ca156953ed3f60b924ad29a1d335bd0e240c0b7c0fbb360226bab03294d226a5560cdf2a3f21e6d406

7 files changed, 10 insertions, 13 deletions

diff --git a/src/policy/fees.h b/src/policy/fees.h
index 6e61f76178..16683bf5ad 100644
--- a/src/policy/fees.h
+++ b/src/policy/fees.h
@@ -43,7 +43,6 @@ enum class FeeReason {
     PAYTXFEE,
     FALLBACK,
     REQUIRED,
-    MAXTXFEE,
 };
 
 /* Used to determine type of fee estimation requested */
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index a2b295df21..c1eba61749 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -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;
     }
diff --git a/src/util/error.cpp b/src/util/error.cpp
index 68ffd8b046..9331a92ad7 100644
--- a/src/util/error.cpp
+++ b/src/util/error.cpp
@@ -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);
diff --git a/src/util/error.h b/src/util/error.h
index d93309551b..0fd474b962 100644
--- a/src/util/error.h
+++ b/src/util/error.h
@@ -27,6 +27,7 @@ enum class TransactionError {
     INVALID_PSBT,
     PSBT_MISMATCH,
     SIGHASH_MISMATCH,
+    MAX_FEE_EXCEEDED,
 };
 
 std::string TransactionErrorString(const TransactionError error);
diff --git a/src/util/fees.cpp b/src/util/fees.cpp
index 5fdaa1284c..cf16d5e44f 100644
--- a/src/util/fees.cpp
+++ b/src/util/fees.cpp
@@ -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);
 
diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp
index ad69e84358..2792058f2a 100644
--- a/src/wallet/fees.cpp
+++ b/src/wallet/fees.cpp
@@ -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)
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 16366a0c23..8807acb6b7 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -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;
 }