From adc6ab25bba42ce9e7ed6bd7599aabb6dead6987 Mon Sep 17 00:00:00 2001 From: josibake Date: Sun, 5 May 2024 14:16:08 +0200 Subject: wallet: use CRecipient instead of CTxOut Now that a CRecipient holds a CTxDestination, we can get the serialized size and determine if the output is dust using the CRecipient directly. This does not change any current behavior, but provides a nice generalization that can be used to apply special logic to a CTxDestination serialization and dust calculations in the future. Specifically, in a later PR when support for `V0SilentPayment` destinations is added, we need to use `WitnessV1Taproot` as the scriptPubKey for serialized size calcuations whenever the `CRecipient` destination is a `V0SilentPayment` destination. --- src/wallet/spend.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 4cbcfdb60f..39744f714a 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -976,6 +976,16 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng } } +size_t GetSerializeSizeForRecipient(const CRecipient& recipient) +{ + return ::GetSerializeSize(CTxOut(recipient.nAmount, GetScriptForDestination(recipient.dest))); +} + +bool IsDust(const CRecipient& recipient, const CFeeRate& dustRelayFee) +{ + return ::IsDust(CTxOut(recipient.nAmount, GetScriptForDestination(recipient.dest)), dustRelayFee); +} + static util::Result CreateTransactionInternal( CWallet& wallet, const std::vector& vecSend, @@ -1097,9 +1107,9 @@ static util::Result CreateTransactionInternal( CTxOut txout(recipient.nAmount, GetScriptForDestination(recipient.dest)); // Include the fee cost for outputs. - coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout); + coin_selection_params.tx_noinputs_size += GetSerializeSizeForRecipient(recipient); - if (IsDust(txout, wallet.chain().relayDustFee())) { + if (IsDust(recipient, wallet.chain().relayDustFee())) { return util::Error{_("Transaction amount too small")}; } txNew.vout.push_back(txout); -- cgit v1.2.3 From a9c7300135f0188daa5bce5491e2daf2dd8da8ae Mon Sep 17 00:00:00 2001 From: josibake Date: Sun, 5 May 2024 14:17:17 +0200 Subject: move-only: refactor CreateTransactionInternal Move the output serialization size and dust calculation into the loop where the outputs are iterated over to calculate the total sum. Move the code for adding the the txoutputs to the transaction to after coin selection. While this code structure generally follows a more logical flow, the primary motivation for moving the code for adding outputs to the transaction sets us up nicely for silent payments (in a future PR): we need to know the input set before generating the final output scriptPubKeys. --- src/wallet/spend.cpp | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 39744f714a..2e4d2d4334 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1008,12 +1008,20 @@ static util::Result CreateTransactionInternal( // Set the long term feerate estimate to the wallet's consolidate feerate coin_selection_params.m_long_term_feerate = wallet.m_consolidate_feerate; + // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size) + coin_selection_params.tx_noinputs_size = 10 + GetSizeOfCompactSize(vecSend.size()); // bytes for output count CAmount recipients_sum = 0; const OutputType change_type = wallet.TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : wallet.m_default_change_type, vecSend); ReserveDestination reservedest(&wallet, change_type); unsigned int outputs_to_subtract_fee_from = 0; // The number of outputs which we are subtracting the fee from for (const auto& recipient : vecSend) { + if (IsDust(recipient, wallet.chain().relayDustFee())) { + return util::Error{_("Transaction amount too small")}; + } + + // Include the fee cost for outputs. + coin_selection_params.tx_noinputs_size += GetSerializeSizeForRecipient(recipient); recipients_sum += recipient.nAmount; if (recipient.fSubtractFeeFromAmount) { @@ -1098,23 +1106,6 @@ static util::Result CreateTransactionInternal( const auto change_spend_fee = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size); coin_selection_params.min_viable_change = std::max(change_spend_fee + 1, dust); - // Static vsize overhead + outputs vsize. 4 version, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size) - coin_selection_params.tx_noinputs_size = 10 + GetSizeOfCompactSize(vecSend.size()); // bytes for output count - - // vouts to the payees - for (const auto& recipient : vecSend) - { - CTxOut txout(recipient.nAmount, GetScriptForDestination(recipient.dest)); - - // Include the fee cost for outputs. - coin_selection_params.tx_noinputs_size += GetSerializeSizeForRecipient(recipient); - - if (IsDust(recipient, wallet.chain().relayDustFee())) { - return util::Error{_("Transaction amount too small")}; - } - txNew.vout.push_back(txout); - } - // Include the fees for things that aren't inputs, excluding the change output const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.m_subtract_fee_outputs ? 0 : coin_selection_params.tx_noinputs_size); CAmount selection_target = recipients_sum + not_input_fees; @@ -1155,6 +1146,11 @@ static util::Result CreateTransactionInternal( result.GetWaste(), result.GetSelectedValue()); + // vouts to the payees + for (const auto& recipient : vecSend) + { + txNew.vout.emplace_back(recipient.nAmount, GetScriptForDestination(recipient.dest)); + } const CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee); if (change_amount > 0) { CTxOut newTxOut(change_amount, scriptChange); -- cgit v1.2.3