diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 5e319d4f959..08adf09df4d 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -190,7 +190,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; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 08e480225d5..9e0befac43b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1611,14 +1611,15 @@ bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig) { std::vector txouts; for (const CTxIn& input : tx.vin) { const auto mi = wallet->mapWallet.find(input.prevout.hash); // Can not estimate size without knowing the input details if (mi == wallet->mapWallet.end()) { - return -1; + return std::make_pair(-1, -1); } assert(input.prevout.n < mi->second.tx->vout.size()); txouts.emplace_back(mi->second.tx->vout[input.prevout.n]); @@ -1627,13 +1628,16 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall } // txouts needs to be in the order of tx.vin -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts, bool use_max_sig) +std::pair CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts, bool use_max_sig) { CMutableTransaction txNew(tx); if (!wallet->DummySignTx(txNew, txouts, use_max_sig)) { - 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 CWallet* wallet, bool use_max_sig) @@ -2779,6 +2783,7 @@ bool CWallet::CreateTransactionInternal( CMutableTransaction txNew; FeeCalculation feeCalc; CAmount nFeeNeeded; + std::pair tx_sizes; int nBytes; { std::set setCoins; @@ -2962,7 +2967,8 @@ bool CWallet::CreateTransactionInternal( txNew.vin.push_back(CTxIn(coin.outpoint,CScript())); } - nBytes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, coin_control.fAllowWatchOnly); + tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, coin_control.fAllowWatchOnly); + nBytes = tx_sizes.first; if (nBytes < 0) { error = _("Signing transaction failed"); return false; @@ -3072,7 +3078,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; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index eb797938cd4..3a76163dd21 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1334,8 +1334,8 @@ class WalletRescanReserver // Use DummySignatureCreator, which inserts 71 byte signatures everywhere. // NOTE: this requires that all inputs must be in mapWallet (eg the tx should // be IsAllFromMe). -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig = false) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet); -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts, bool use_max_sig = false); +std::pair CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig = false) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet); +std::pair CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts, bool use_max_sig = false); //! Add wallet name to persistent configuration so it will be loaded on startup. bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 569471dc873..6b300e72316 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -94,6 +94,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.""" @@ -907,5 +908,24 @@ def test_subtract_fee_with_presets(self): signedtx = self.nodes[0].signrawtransactionwithwallet(fundedtx['hex']) 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") + + 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()