Merge a8b0892 into merged_master (Bitcoin PR #20536)

This PR adds a new functional test which unfortunately we need to disable
because it triggers an unrelated Elements bug (assertation failure when
attempting a large fundrawtransaction).

See #880

src/wallet/feebumper.cpp

@@ -207,7 +207,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
     if (coin_control.m_feerate) {
         // The user provided a feeRate argument.
         // We calculate this here to avoid compiler warning on the cs_wallet lock
-        const int64_t maxTxSize = CalculateMaximumSignedTxSize(*wtx.tx, &wallet);
+        const int64_t maxTxSize = CalculateMaximumSignedTxSize(*wtx.tx, &wallet).first;
         Result res = CheckFeeRate(wallet, wtx, *new_coin_control.m_feerate, maxTxSize, errors);
         if (res != Result::OK) {
             return res;

src/wallet/wallet.cpp

@@ -1658,7 +1658,8 @@ bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set<CScri
     return true;
 }
 
-int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const CCoinControl* coin_control)
+// Returns pair of vsize and weight
+std::pair<int64_t, int64_t> CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const CCoinControl* coin_control)
 {
     std::vector<CTxOut> txouts;
     // Look up the inputs. The inputs are either in the wallet, or in coin_control.
@@ -1670,24 +1671,27 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall
         } else if (coin_control) {
             CTxOut txout;
             if (!coin_control->GetExternalOutput(input.prevout, txout)) {
-                return -1;
+                return std::make_pair(-1, -1);
             }
             txouts.emplace_back(txout);
         } else {
-            return -1;
+            return std::make_pair(-1, -1);
         }
     }
     return CalculateMaximumSignedTxSize(tx, wallet, txouts, coin_control);
 }
 
 // txouts needs to be in the order of tx.vin
-int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, const CCoinControl* coin_control)
+std::pair<int64_t, int64_t> CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, const CCoinControl* coin_control)
 {
     CMutableTransaction txNew(tx);
     if (!wallet->DummySignTx(txNew, txouts, coin_control)) {
-        return -1;
+        return std::make_pair(-1, -1);
     }
-    return GetVirtualTransactionSize(CTransaction(txNew));
+    CTransaction ctx(txNew);
+    int64_t vsize = GetVirtualTransactionSize(ctx);
+    int64_t weight = GetTransactionWeight(ctx);
+    return std::make_pair(vsize, weight);
 }
 
 int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* provider, bool use_max_sig) {
@@ -3273,6 +3277,7 @@ bool CWallet::CreateTransactionInternal(
     CMutableTransaction txNew;
     FeeCalculation feeCalc;
     CAmount nFeeNeeded;
+    std::pair<int64_t, int64_t> tx_sizes;
     int nBytes;
     {
         std::set<CInputCoin> setCoins;
@@ -3712,7 +3717,8 @@ bool CWallet::CreateTransactionInternal(
                     }
                 }
 
-                nBytes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, &coin_control);
+                tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, &coin_control);
+                nBytes = tx_sizes.first;
                 if (nBytes < 0) {
                     error = _("Missing solving data for estimating transaction size");
                     return false;
@@ -3960,7 +3966,8 @@ bool CWallet::CreateTransactionInternal(
         tx = MakeTransactionRef(std::move(txNew));
 
         // Limit size
-        if (GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT)
+        if ((sign && GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT) ||
+            (!sign && tx_sizes.second > MAX_STANDARD_TX_WEIGHT))
         {
             error = _("Transaction too large");
             return false;

src/wallet/wallet.h

@@ -1490,8 +1490,10 @@ class WalletRescanReserver
 
 // Calculate the size of the transaction assuming all signatures are max size
 // Use DummySignatureCreator, which inserts 71 byte signatures everywhere.
-int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const CCoinControl* coin_control = nullptr) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet);
-int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, const CCoinControl* coin_control = nullptr);
+// NOTE: this requires that all inputs must be in mapWallet (eg the tx should
+// be IsAllFromMe).
+std::pair<int64_t, int64_t> CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const CCoinControl* coin_control = nullptr) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet);
+std::pair<int64_t, int64_t> CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, const CCoinControl* coin_control = nullptr);
 
 //! Add wallet name to persistent configuration so it will be loaded on startup.
 bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);

test/functional/rpc_fundrawtransaction.py

@@ -95,6 +95,7 @@ def run_test(self):
         self.test_address_reuse()
         self.test_option_subtract_fee_from_outputs()
         self.test_subtract_fee_with_presets()
+        self.test_transaction_too_large()
 
     def test_change_position(self):
         """Ensure setting changePosition in fundraw with an exact match is handled properly."""
@@ -955,5 +956,26 @@ def test_subtract_fee_with_presets(self):
         signedtx = self.nodes[0].signrawtransactionwithwallet(blindedtx)
         self.nodes[0].sendrawtransaction(signedtx['hex'])
 
+    def test_transaction_too_large(self):
+        self.log.info("Test fundrawtx where BnB solution would result in a too large transaction, but Knapsack would not")
+# ELEMENTS: FIXME we need to disable this test currently as we cannot have more than 256 inputs
+#  in a transaction. See https://github.com/ElementsProject/elements/issues/880
+#
+#        self.nodes[0].createwallet("large")
+#        wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+#        recipient = self.nodes[0].get_wallet_rpc("large")
+#        outputs = {}
+#        rawtx = recipient.createrawtransaction([], {wallet.getnewaddress(): 147.99899260})
+#
+#        # Make 1500 0.1 BTC outputs
+#        # The amount that we target for funding is in the BnB range when these outputs are used.
+#        # However if these outputs are selected, the transaction will end up being too large, so it shouldn't use BnB and instead fallback to Knapsack
+#        # but that behavior is not implemented yet. For now we just check that we get an error.
+#        for i in range(0, 1500):
+#            outputs[recipient.getnewaddress()] = 0.1
+#        wallet.sendmany("", outputs)
+#        self.nodes[0].generate(10)
+#        assert_raises_rpc_error(-4, "Transaction too large", recipient.fundrawtransaction, rawtx)
+
 if __name__ == '__main__':
     RawTransactionsTest().main()