From d54c5c8b1b1a38b5b38e6878aea0fa8d6c1ad7e9 Mon Sep 17 00:00:00 2001 From: S3RK <1466284+S3RK@users.noreply.github.com> Date: Mon, 27 Jun 2022 09:11:09 +0200 Subject: wallet: use CCoinControl to estimate signature size --- src/bench/coin_selection.cpp | 2 +- src/wallet/spend.cpp | 18 +++++++++--------- src/wallet/spend.h | 14 +++++--------- src/wallet/test/coinselector_tests.cpp | 4 ++-- src/wallet/wallet.cpp | 14 +++++++------- src/wallet/wallet.h | 2 +- 6 files changed, 25 insertions(+), 29 deletions(-) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index f8914e762c..eaefb9b63a 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -59,7 +59,7 @@ static void CoinSelection(benchmark::Bench& bench) std::vector coins; for (const auto& wtx : wtxs) { const auto txout = wtx->tx->vout.at(0); - coins.emplace_back(COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, CalculateMaximumSignedInputSize(txout, &wallet, false), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0); + coins.emplace_back(COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0); } const CoinEligibilityFilter filter_standard(1, 6, 0); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 5d9cecc398..583c021b8f 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -27,20 +27,20 @@ using interfaces::FoundBlock; namespace wallet { static constexpr size_t OUTPUT_GROUP_MAX_ENTRIES{100}; -int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* provider, bool use_max_sig) +int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* provider, const CCoinControl* coin_control) { CMutableTransaction txn; - txn.vin.push_back(CTxIn(COutPoint())); - if (!provider || !DummySignInput(*provider, txn.vin[0], txout, use_max_sig)) { + txn.vin.push_back(CTxIn(outpoint)); + if (!provider || !DummySignInput(*provider, txn.vin[0], txout, coin_control)) { return -1; } return GetVirtualTransactionInputSize(txn.vin[0]); } -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, bool use_max_sig) +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, const CCoinControl* coin_control) { const std::unique_ptr provider = wallet->GetSolvingProvider(txout.scriptPubKey); - return CalculateMaximumSignedInputSize(txout, provider.get(), use_max_sig); + return CalculateMaximumSignedInputSize(txout, COutPoint(), provider.get(), coin_control); } // txouts needs to be in the order of tx.vin @@ -193,7 +193,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, // Filter by spendable outputs only if (!spendable && only_spendable) continue; - int input_bytes = CalculateMaximumSignedInputSize(output, provider.get(), coinControl && coinControl->fAllowWatchOnly); + int input_bytes = CalculateMaximumSignedInputSize(output, COutPoint(), provider.get(), coinControl); result.coins.emplace_back(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate); result.total_amount += output.nValue; @@ -286,7 +286,7 @@ std::map> ListCoins(const CWallet& wallet) if (ExtractDestination(FindNonChangeParentOutput(wallet, *wtx.tx, output.n).scriptPubKey, address)) { const auto out = wtx.tx->vout.at(output.n); result[address].emplace_back( - COutPoint(wtx.GetHash(), output.n), out, depth, CalculateMaximumSignedInputSize(out, &wallet, false), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ false, wtx.GetTxTime(), CachedTxIsFromMe(wallet, wtx, ISMINE_ALL)); + COutPoint(wtx.GetHash(), output.n), out, depth, CalculateMaximumSignedInputSize(out, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ false, wtx.GetTxTime(), CachedTxIsFromMe(wallet, wtx, ISMINE_ALL)); } } } @@ -443,14 +443,14 @@ std::optional SelectCoins(const CWallet& wallet, const std::vec if (ptr_wtx->tx->vout.size() <= outpoint.n) { return std::nullopt; } - input_bytes = CalculateMaximumSignedInputSize(ptr_wtx->tx->vout[outpoint.n], &wallet, false); txout = ptr_wtx->tx->vout.at(outpoint.n); + input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control); } else { // The input is external. We did not find the tx in mapWallet. if (!coin_control.GetExternalOutput(outpoint, txout)) { return std::nullopt; } - input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /*use_max_sig=*/true); + input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control); } // If available, override calculated size with coin control specified size if (coin_control.HasInputWeight(outpoint)) { diff --git a/src/wallet/spend.h b/src/wallet/spend.h index a7d91eb9c4..21f0095e77 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -14,20 +14,16 @@ namespace wallet { /** Get the marginal bytes if spending the specified output from this transaction. - * use_max_sig indicates whether to use the maximum sized, 72 byte signature when calculating the - * size of the input spend. This should only be set when watch-only outputs are allowed */ -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, bool use_max_sig = false); -int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* pwallet, bool use_max_sig = false); - + * Use CoinControl to determine whether to expect signature grinding when calculating the size of the input spend. */ +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, const CCoinControl* coin_control = nullptr); +int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* pwallet, const CCoinControl* coin_control = nullptr); struct TxSize { int64_t vsize{-1}; int64_t weight{-1}; }; -/** Calculate the size of the transaction assuming all signatures are max size -* Use DummySignatureCreator, which inserts 71 byte signatures everywhere. -* NOTE: this requires that all inputs must be in mapWallet (eg the tx should -* be AllInputsMine). */ +/** Calculate the size of the transaction using CoinControl to determine + * whether to expect signature grinding when calculating the size of the input spend. */ TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* wallet, const std::vector& txouts, const CCoinControl* coin_control = nullptr); TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* wallet, const CCoinControl* coin_control = nullptr) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 0664b64244..e85bf58d4e 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -86,8 +86,8 @@ static void add_coin(std::vector& coins, CWallet& wallet, const CAmount auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateInactive{})); assert(ret.second); CWalletTx& wtx = (*ret.first).second; - const auto txout = wtx.tx->vout.at(nInput); - coins.emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, false), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate); + const auto& txout = wtx.tx->vout.at(nInput); + coins.emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate); } /** Check if SelectionResult a is equivalent to SelectionResult b. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d0b093bbb7..b90ef98850 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1504,13 +1504,16 @@ bool CWallet::AddWalletFlags(uint64_t flags) } // Helper for producing a max-sized low-S low-R signature (eg 71 bytes) -// or a max-sized low-S signature (e.g. 72 bytes) if use_max_sig is true -bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool use_max_sig) +// or a max-sized low-S signature (e.g. 72 bytes) depending on coin_control +bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, const CCoinControl* coin_control) { // Fill in dummy signatures for fee calculation. const CScript& scriptPubKey = txout.scriptPubKey; SignatureData sigdata; + // Use max sig if watch only inputs were used or if this particular input is an external input + // to ensure a sufficient fee is attained for the requested feerate. + const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(tx_in.prevout)); if (!ProduceSignature(provider, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) { return false; } @@ -1577,12 +1580,9 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector nIn++; continue; } - // Use max sig if watch only inputs were used or if this particular input is an external input - // to ensure a sufficient fee is attained for the requested feerate. - const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(txin.prevout)); const std::unique_ptr provider = GetSolvingProvider(txout.scriptPubKey); - if (!provider || !DummySignInput(*provider, txin, txout, use_max_sig)) { - if (!coin_control || !DummySignInput(coin_control->m_external_provider, txin, txout, use_max_sig)) { + if (!provider || !DummySignInput(*provider, txin, txout, coin_control)) { + if (!coin_control || !DummySignInput(coin_control->m_external_provider, txin, txout, coin_control)) { return false; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bf42b3da9e..1cf91d6ef6 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -932,7 +932,7 @@ bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); //! Remove wallet name from persistent configuration so it will not be loaded on startup. bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); -bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool use_max_sig); +bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, const CCoinControl* coin_control = nullptr); bool FillInputToWeight(CTxIn& txin, int64_t target_weight); } // namespace wallet -- cgit v1.2.3