aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/rpc/spend.cpp5
-rw-r--r--src/wallet/spend.cpp46
-rw-r--r--src/wallet/spend.h2
3 files changed, 33 insertions, 20 deletions
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 7ab4044bf5..fb5e9a425e 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -1651,11 +1651,8 @@ RPCHelpMan walletcreatefundedpsbt()
CAmount fee;
int change_position;
- bool rbf{wallet.m_signal_rbf};
const UniValue &replaceable_arg = options["replaceable"];
- if (!replaceable_arg.isNull()) {
- rbf = replaceable_arg.isTrue();
- }
+ const bool rbf{replaceable_arg.isNull() ? wallet.m_signal_rbf : replaceable_arg.get_bool()};
CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf);
CCoinControl coin_control;
// Automatically select coins, unless at least one is manually selected. Can
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 3ced3ebeb5..33289c1d2f 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -8,6 +8,7 @@
#include <interfaces/chain.h>
#include <numeric>
#include <policy/policy.h>
+#include <primitives/transaction.h>
#include <script/signingprovider.h>
#include <util/check.h>
#include <util/fees.h>
@@ -524,8 +525,9 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
if (results.size() > 0) return *std::min_element(results.begin(), results.end());
// If we can't fund the transaction from any individual OutputType, run coin selection one last time
- // over all available coins, which would allow mixing
- if (allow_mixed_output_types) {
+ // over all available coins, which would allow mixing.
+ // If TypesCount() <= 1, there is nothing to mix.
+ if (allow_mixed_output_types && available_coins.TypesCount() > 1) {
if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params)}) {
return result;
}
@@ -778,7 +780,6 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
AssertLockHeld(wallet.cs_wallet);
// out variables, to be packed into returned result structure
- CAmount nFeeRet;
int nChangePosInOut = change_pos;
FastRandomContext rng_fast;
@@ -960,24 +961,28 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
return util::Error{_("Missing solving data for estimating transaction size")};
}
CAmount fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes);
- nFeeRet = result->GetSelectedValue() - recipients_sum - change_amount;
+ const CAmount output_value = CalculateOutputValue(txNew);
+ Assume(recipients_sum + change_amount == output_value);
+ CAmount current_fee = result->GetSelectedValue() - output_value;
- // The only time that fee_needed should be less than the amount available for fees is when
- // we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug.
- if (!coin_selection_params.m_subtract_fee_outputs && fee_needed > nFeeRet) {
- return util::Error{Untranslated(STR_INTERNAL_BUG("Fee needed > fee paid"))};
+ // Sanity check that the fee cannot be negative as that means we have more output value than input value
+ if (current_fee < 0) {
+ return util::Error{Untranslated(STR_INTERNAL_BUG("Fee paid < 0"))};
}
// If there is a change output and we overpay the fees then increase the change to match the fee needed
- if (nChangePosInOut != -1 && fee_needed < nFeeRet) {
+ if (nChangePosInOut != -1 && fee_needed < current_fee) {
auto& change = txNew.vout.at(nChangePosInOut);
- change.nValue += nFeeRet - fee_needed;
- nFeeRet = fee_needed;
+ change.nValue += current_fee - fee_needed;
+ current_fee = result->GetSelectedValue() - CalculateOutputValue(txNew);
+ if (fee_needed != current_fee) {
+ return util::Error{Untranslated(STR_INTERNAL_BUG("Change adjustment: Fee needed != fee paid"))};
+ }
}
// Reduce output values for subtractFeeFromAmount
if (coin_selection_params.m_subtract_fee_outputs) {
- CAmount to_reduce = fee_needed - nFeeRet;
+ CAmount to_reduce = fee_needed - current_fee;
int i = 0;
bool fFirst = true;
for (const auto& recipient : vecSend)
@@ -1008,7 +1013,16 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
}
++i;
}
- nFeeRet = fee_needed;
+ current_fee = result->GetSelectedValue() - CalculateOutputValue(txNew);
+ if (fee_needed != current_fee) {
+ return util::Error{Untranslated(STR_INTERNAL_BUG("SFFO: Fee needed != fee paid"))};
+ }
+ }
+
+ // fee_needed should now always be less than or equal to the current fees that we pay.
+ // If it is not, it is a bug.
+ if (fee_needed > current_fee) {
+ return util::Error{Untranslated(STR_INTERNAL_BUG("Fee needed > fee paid"))};
}
// Give up if change keypool ran out and change is required
@@ -1030,7 +1044,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
return util::Error{_("Transaction too large")};
}
- if (nFeeRet > wallet.m_default_max_tx_fee) {
+ if (current_fee > wallet.m_default_max_tx_fee) {
return util::Error{TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED)};
}
@@ -1046,14 +1060,14 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
reservedest.KeepDestination();
wallet.WalletLogPrintf("Fee Calculation: Fee:%d Bytes:%u Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
- nFeeRet, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay,
+ current_fee, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay,
feeCalc.est.pass.start, feeCalc.est.pass.end,
(feeCalc.est.pass.totalConfirmed + feeCalc.est.pass.inMempool + feeCalc.est.pass.leftMempool) > 0.0 ? 100 * feeCalc.est.pass.withinTarget / (feeCalc.est.pass.totalConfirmed + feeCalc.est.pass.inMempool + feeCalc.est.pass.leftMempool) : 0.0,
feeCalc.est.pass.withinTarget, feeCalc.est.pass.totalConfirmed, feeCalc.est.pass.inMempool, feeCalc.est.pass.leftMempool,
feeCalc.est.fail.start, feeCalc.est.fail.end,
(feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) > 0.0 ? 100 * feeCalc.est.fail.withinTarget / (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) : 0.0,
feeCalc.est.fail.withinTarget, feeCalc.est.fail.totalConfirmed, feeCalc.est.fail.inMempool, feeCalc.est.fail.leftMempool);
- return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut, feeCalc);
+ return CreatedTransactionResult(tx, current_fee, nChangePosInOut, feeCalc);
}
util::Result<CreatedTransactionResult> CreateTransaction(
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 2b861c2361..46144d4c92 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -46,6 +46,8 @@ struct CoinsResult {
/** The following methods are provided so that CoinsResult can mimic a vector,
* i.e., methods can work with individual OutputType vectors or on the entire object */
size_t Size() const;
+ /** Return how many different output types this struct stores */
+ size_t TypesCount() const { return coins.size(); }
void Clear();
void Erase(const std::unordered_set<COutPoint, SaltedOutpointHasher>& coins_to_remove);
void Shuffle(FastRandomContext& rng_fast);
generated by cgit v1.2.3 (git 2.26.2) at 2025-02-03 06:20:48 +0000