highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-06-29 18:20:58 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge #20536: wallet: Error with "Transaction too large" if the funded tx will end up being too large after signing

Browse Source 
48a0319bab Add a test that selects too large if BnB is used (Andrew Chow)
3e69939b78 Fail if maximum weight is too large (Andrew Chow)
51e2cd322c Have CalculateMaximumSignedTxSize also compute tx weight (Andrew Chow)

Pull request description:

  Currently the `Transaction too large` is calculated on the transaction that is returned from `CreateTransaction`. This does not make sense for when `CreateTransaction` is being used for `fundrawtransaction` as no signing occurs so the final returned transaction is missing signatures. Thus users may successfully fund a transaction but fail to broadcast it after it has been fully signed.

  So instead we should figure out whether the transaction we are funding will be too large after it is signed. We can do this by having `CalculateMaximumSignedTxSize` also return the transaction weight and then comparing that weight against the maximum weight.

ACKs for top commit:
  instagibbs:
    ACK 48a0319bab
  meshcollider:
    utACK 48a0319bab
  Xekyo:
    utACK with nits 48a0319bab

Tree-SHA512: 1700c60b07f67e2d5c591c5ccd131ac9f1861fab3def961c3c9c4b3281ec1063fe8e4f0f7f1038cac72692340856406bcee8fb45c8104d2ad34357a0ec878ac7
This commit is contained in:
Samuel Dobson
2021-03-09 10:04:55 +13:00
parent 2067f9e5e8 48a0319bab
commit a8b0892b74
4 changed files with 37 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/wallet/feebumper.cpp
View File

@ -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;

21
src/wallet/wallet.cpp
View File

@ -1611,14 +1611,15 @@ bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set<CScri
return true;
}
int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig)
// Returns pair of vsize and weight
std::pair<int64_t, int64_t> CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig)
{
std::vector<CTxOut> 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<CTxOut>& txouts, bool use_max_sig)
std::pair<int64_t, int64_t> CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& 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<int64_t, int64_t> tx_sizes;
int nBytes;
{
std::set<CInputCoin> 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;

4
src/wallet/wallet.h
View File

@ -1334,8 +1334,8 @@ public:
// 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<CTxOut>& txouts, bool use_max_sig = false);
std::pair<int64_t, int64_t> CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig = false) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet);
std::pair<int64_t, int64_t> CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& 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);

20
test/functional/rpc_fundrawtransaction.py
View File

@ -94,6 +94,7 @@ class RawTransactionsTest(BitcoinTestFramework):
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 @@ class RawTransactionsTest(BitcoinTestFramework):
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()