aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/coinselection.cpp18
-rw-r--r--src/wallet/coinselection.h47
-rw-r--r--src/wallet/rpc/addresses.cpp4
-rw-r--r--src/wallet/rpc/backup.cpp2
-rw-r--r--src/wallet/rpc/coins.cpp31
-rw-r--r--src/wallet/rpc/spend.cpp2
-rw-r--r--src/wallet/spend.cpp27
-rw-r--r--src/wallet/spend.h8
-rw-r--r--src/wallet/test/coinselector_tests.cpp116
-rw-r--r--src/wallet/test/fuzz/coinselection.cpp6
-rw-r--r--src/wallet/test/wallet_tests.cpp4
-rw-r--r--src/wallet/wallet.cpp13
12 files changed, 201 insertions, 77 deletions
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 0b236e2e48..07df8d9fc8 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -307,7 +307,7 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups,
}
}
- if (LogAcceptCategory(BCLog::SELECTCOINS)) {
+ if (LogAcceptCategory(BCLog::SELECTCOINS, BCLog::Level::Debug)) {
std::string log_message{"Coin selection best subset: "};
for (unsigned int i = 0; i < applicable_groups.size(); i++) {
if (vfBest[i]) {
@@ -328,24 +328,18 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups,
******************************************************************************/
void OutputGroup::Insert(const COutput& output, size_t ancestors, size_t descendants, bool positive_only) {
- // Compute the effective value first
- const CAmount coin_fee = output.input_bytes < 0 ? 0 : m_effective_feerate.GetFee(output.input_bytes);
- const CAmount ev = output.txout.nValue - coin_fee;
-
// Filter for positive only here before adding the coin
- if (positive_only && ev <= 0) return;
+ if (positive_only && output.GetEffectiveValue() <= 0) return;
m_outputs.push_back(output);
COutput& coin = m_outputs.back();
- coin.fee = coin_fee;
- fee += coin.fee;
+ fee += coin.GetFee();
coin.long_term_fee = coin.input_bytes < 0 ? 0 : m_long_term_feerate.GetFee(coin.input_bytes);
long_term_fee += coin.long_term_fee;
- coin.effective_value = ev;
- effective_value += coin.effective_value;
+ effective_value += coin.GetEffectiveValue();
m_from_me &= coin.from_me;
m_value += coin.txout.nValue;
@@ -380,8 +374,8 @@ CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost,
CAmount waste = 0;
CAmount selected_effective_value = 0;
for (const COutput& coin : inputs) {
- waste += coin.fee - coin.long_term_fee;
- selected_effective_value += use_effective_value ? coin.effective_value : coin.txout.nValue;
+ waste += coin.GetFee() - coin.long_term_fee;
+ selected_effective_value += use_effective_value ? coin.GetEffectiveValue() : coin.txout.nValue;
}
if (change_cost) {
diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index 25c672eda0..9135e48104 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -20,6 +20,14 @@ static constexpr CAmount CHANGE_UPPER{1000000};
/** A UTXO under consideration for use in funding a new transaction. */
struct COutput {
+private:
+ /** The output's value minus fees required to spend it.*/
+ std::optional<CAmount> effective_value;
+
+ /** The fee required to spend this output at the transaction's target feerate. */
+ std::optional<CAmount> fee;
+
+public:
/** The outpoint identifying this UTXO */
COutPoint outpoint;
@@ -55,16 +63,10 @@ struct COutput {
/** Whether the transaction containing this output is sent from the owning wallet */
bool from_me;
- /** The output's value minus fees required to spend it. Initialized as the output's absolute value. */
- CAmount effective_value;
-
- /** The fee required to spend this output at the transaction's target feerate. */
- CAmount fee{0};
-
/** The fee required to spend this output at the consolidation feerate. */
CAmount long_term_fee{0};
- COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me)
+ COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me, const std::optional<CFeeRate> feerate = std::nullopt)
: outpoint{outpoint},
txout{txout},
depth{depth},
@@ -73,9 +75,22 @@ struct COutput {
solvable{solvable},
safe{safe},
time{time},
- from_me{from_me},
- effective_value{txout.nValue}
- {}
+ from_me{from_me}
+ {
+ if (feerate) {
+ fee = input_bytes < 0 ? 0 : feerate.value().GetFee(input_bytes);
+ effective_value = txout.nValue - fee.value();
+ }
+ }
+
+ COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me, const CAmount fees)
+ : COutput(outpoint, txout, depth, input_bytes, spendable, solvable, safe, time, from_me)
+ {
+ // if input_bytes is unknown, then fees should be 0, if input_bytes is known, then the fees should be a positive integer or 0 (input_bytes known and fees = 0 only happens in the tests)
+ assert((input_bytes < 0 && fees == 0) || (input_bytes > 0 && fees >= 0));
+ fee = fees;
+ effective_value = txout.nValue - fee.value();
+ }
std::string ToString() const;
@@ -83,6 +98,18 @@ struct COutput {
{
return outpoint < rhs.outpoint;
}
+
+ CAmount GetFee() const
+ {
+ assert(fee.has_value());
+ return fee.value();
+ }
+
+ CAmount GetEffectiveValue() const
+ {
+ assert(effective_value.has_value());
+ return effective_value.value();
+ }
};
/** Parameters for one iteration of Coin Selection. */
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index d5444f5051..f25ad59528 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -302,11 +302,11 @@ RPCHelpMan addmultisigaddress()
result.pushKV("descriptor", descriptor->ToString());
UniValue warnings(UniValue::VARR);
- if (!request.params[3].isNull() && OutputTypeFromDestination(dest) != output_type) {
+ if (descriptor->GetOutputType() != output_type) {
// Only warns if the user has explicitly chosen an address type we cannot generate
warnings.push_back("Unable to make chosen address type, please ensure no uncompressed public keys are present.");
}
- if (warnings.size()) result.pushKV("warnings", warnings);
+ if (!warnings.empty()) result.pushKV("warnings", warnings);
return result;
},
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index c289c05f2c..aa9f23c886 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -1621,7 +1621,7 @@ RPCHelpMan importdescriptors()
},
RPCExamples{
HelpExampleCli("importdescriptors", "'[{ \"desc\": \"<my descriptor>\", \"timestamp\":1455191478, \"internal\": true }, "
- "{ \"desc\": \"<my desccriptor 2>\", \"label\": \"example 2\", \"timestamp\": 1455191480 }]'") +
+ "{ \"desc\": \"<my descriptor 2>\", \"label\": \"example 2\", \"timestamp\": 1455191480 }]'") +
HelpExampleCli("importdescriptors", "'[{ \"desc\": \"<my descriptor>\", \"timestamp\":1455191478, \"active\": true, \"range\": [0,100], \"label\": \"<my bech32 wallet>\" }]'")
},
[&](const RPCHelpMan& self, const JSONRPCRequest& main_request) -> UniValue
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index 95296b4c9d..2649fa586c 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -18,28 +18,31 @@
namespace wallet {
static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
- std::set<CScript> output_scripts;
-
+ std::set<CTxDestination> addresses;
if (by_label) {
// Get the set of addresses assigned to label
- std::string label = LabelFromValue(params[0]);
- for (const auto& address : wallet.GetLabelAddresses(label)) {
- auto output_script{GetScriptForDestination(address)};
- if (wallet.IsMine(output_script)) {
- output_scripts.insert(output_script);
- }
- }
+ addresses = wallet.GetLabelAddresses(LabelFromValue(params[0]));
+ if (addresses.empty()) throw JSONRPCError(RPC_WALLET_ERROR, "Label not found in wallet");
} else {
// Get the address
CTxDestination dest = DecodeDestination(params[0].get_str());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
}
- CScript script_pub_key = GetScriptForDestination(dest);
- if (!wallet.IsMine(script_pub_key)) {
- throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet");
+ addresses.insert(dest);
+ }
+
+ // Filter by own scripts only
+ std::set<CScript> output_scripts;
+ for (const auto& address : addresses) {
+ auto output_script{GetScriptForDestination(address)};
+ if (wallet.IsMine(output_script)) {
+ output_scripts.insert(output_script);
}
- output_scripts.insert(script_pub_key);
+ }
+
+ if (output_scripts.empty()) {
+ throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet");
}
// Minimum confirmations
@@ -635,7 +638,7 @@ RPCHelpMan listunspent()
cctl.m_max_depth = nMaxDepth;
cctl.m_include_unsafe_inputs = include_unsafe;
LOCK(pwallet->cs_wallet);
- AvailableCoins(*pwallet, vecOutputs, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
+ AvailableCoinsListUnspent(*pwallet, vecOutputs, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
}
LOCK(pwallet->cs_wallet);
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index c7b57ba4be..d1a0ba50f6 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -1398,7 +1398,7 @@ RPCHelpMan sendall()
total_input_value += tx->tx->vout[input.prevout.n].nValue;
}
} else {
- AvailableCoins(*pwallet, all_the_utxos, &coin_control, /*nMinimumAmount=*/0);
+ AvailableCoins(*pwallet, all_the_utxos, &coin_control, fee_rate, /*nMinimumAmount=*/0);
for (const COutput& output : all_the_utxos) {
CHECK_NONFATAL(output.input_bytes > 0);
if (send_max && fee_rate.GetFee(output.input_bytes) > output.txout.nValue) {
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index e5fd7b0eb4..6f79ae4e9b 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -84,7 +84,7 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *walle
return CalculateMaximumSignedTxSize(tx, wallet, txouts, coin_control);
}
-void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount)
+void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const CCoinControl* coinControl, std::optional<CFeeRate> feerate, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount)
{
AssertLockHeld(wallet.cs_wallet);
@@ -192,7 +192,7 @@ void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const C
bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
int input_bytes = GetTxSpendSize(wallet, wtx, i, (coinControl && coinControl->fAllowWatchOnly));
- vCoins.emplace_back(COutPoint(wtx.GetHash(), i), wtx.tx->vout.at(i), nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me);
+ vCoins.emplace_back(COutPoint(wtx.GetHash(), i), wtx.tx->vout.at(i), nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate);
// Checks the sum amount of all UTXO's.
if (nMinimumSumAmount != MAX_MONEY) {
@@ -211,13 +211,18 @@ void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const C
}
}
+void AvailableCoinsListUnspent(const CWallet& wallet, std::vector<COutput>& vCoins, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount)
+{
+ AvailableCoins(wallet, vCoins, coinControl, /*feerate=*/ std::nullopt, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
+}
+
CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinControl)
{
LOCK(wallet.cs_wallet);
CAmount balance = 0;
std::vector<COutput> vCoins;
- AvailableCoins(wallet, vCoins, coinControl);
+ AvailableCoinsListUnspent(wallet, vCoins, coinControl);
for (const COutput& out : vCoins) {
if (out.spendable) {
balance += out.txout.nValue;
@@ -257,7 +262,7 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
std::map<CTxDestination, std::vector<COutput>> result;
std::vector<COutput> availableCoins;
- AvailableCoins(wallet, availableCoins);
+ AvailableCoinsListUnspent(wallet, availableCoins);
for (const COutput& coin : availableCoins) {
CTxDestination address;
@@ -477,12 +482,11 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
}
/* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */
- COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false);
- output.effective_value = output.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(output.input_bytes);
+ COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate);
if (coin_selection_params.m_subtract_fee_outputs) {
value_to_select -= output.txout.nValue;
} else {
- value_to_select -= output.effective_value;
+ value_to_select -= output.GetEffectiveValue();
}
preset_coins.insert(outpoint);
/* Set ancestors and descendants to 0 as they don't matter for preset inputs since no actual selection is being done.
@@ -788,7 +792,7 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
// Get available coins
std::vector<COutput> vAvailableCoins;
- AvailableCoins(wallet, vAvailableCoins, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0);
+ AvailableCoins(wallet, vAvailableCoins, &coin_control, coin_selection_params.m_effective_feerate, 1, MAX_MONEY, MAX_MONEY, 0);
// Choose coins to use
std::optional<SelectionResult> result = SelectCoins(wallet, vAvailableCoins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
@@ -993,12 +997,13 @@ std::optional<CreatedTransactionResult> CreateTransaction(
tmp_cc.m_avoid_partial_spends = true;
bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results
std::optional<CreatedTransactionResult> txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, error2, tmp_cc, fee_calc_out, sign);
+ // if fee of this alternative one is within the range of the max fee, we use this one
+ const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped->fee + wallet.m_max_aps_fee) : false};
+ TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(),
+ txr_grouped.has_value() ? txr_grouped->fee : 0, txr_grouped.has_value() ? txr_grouped->change_pos : 0);
if (txr_grouped) {
- // if fee of this alternative one is within the range of the max fee, we use this one
- const bool use_aps = txr_grouped->fee <= txr_ungrouped->fee + wallet.m_max_aps_fee;
wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n",
txr_ungrouped->fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped");
- TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, true, txr_grouped->fee, txr_grouped->change_pos);
if (use_aps) return txr_grouped;
}
}
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 8af712110d..988058a25a 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -37,7 +37,13 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* walle
/**
* populate vCoins with vector of available COutputs.
*/
-void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+void AvailableCoins(const CWallet& wallet, std::vector<COutput>& vCoins, const CCoinControl* coinControl = nullptr, std::optional<CFeeRate> feerate = std::nullopt, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+
+/**
+ * Wrapper function for AvailableCoins which skips the `feerate` parameter. Use this function
+ * to list all available coins (e.g. listunspent RPC) while not intending to fund a transaction.
+ */
+void AvailableCoinsListUnspent(const CWallet& wallet, std::vector<COutput>& vCoins, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinControl = nullptr);
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 72e749477b..76f28917a4 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -41,7 +41,7 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<COutput>& se
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
- set.emplace_back(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false);
+ set.emplace_back(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, /*fees=*/ 0);
}
static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result)
@@ -50,7 +50,7 @@ static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result)
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
- COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false);
+ COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, /*fees=*/ 0);
OutputGroup group;
group.Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ true);
result.AddInput(group);
@@ -62,14 +62,12 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fe
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
- COutput coin(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false);
- coin.effective_value = nValue - fee;
- coin.fee = fee;
+ COutput coin(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ 148, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, fee);
coin.long_term_fee = long_term_fee;
set.insert(coin);
}
-static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false)
+static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount& nValue, CFeeRate feerate = CFeeRate(0), int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false)
{
CMutableTransaction tx;
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
@@ -88,7 +86,7 @@ static void add_coin(std::vector<COutput>& 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;
- coins.emplace_back(COutPoint(wtx.GetHash(), nInput), wtx.tx->vout.at(nInput), nAge, GetTxSpendSize(wallet, wtx, nInput), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe);
+ coins.emplace_back(COutPoint(wtx.GetHash(), nInput), wtx.tx->vout.at(nInput), nAge, GetTxSpendSize(wallet, wtx, nInput), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate);
}
/** Check if SelectionResult a is equivalent to SelectionResult b.
@@ -311,13 +309,13 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
std::vector<COutput> coins;
- add_coin(coins, *wallet, 1);
+ add_coin(coins, *wallet, 1, coin_selection_params_bnb.m_effective_feerate);
coins.at(0).input_bytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change));
// Test fees subtracted from output:
coins.clear();
- add_coin(coins, *wallet, 1 * CENT);
+ add_coin(coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate);
coins.at(0).input_bytes = 40;
coin_selection_params_bnb.m_subtract_fee_outputs = true;
const auto result9 = SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change);
@@ -334,9 +332,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
std::vector<COutput> coins;
- add_coin(coins, *wallet, 5 * CENT, 6 * 24, false, 0, true);
- add_coin(coins, *wallet, 3 * CENT, 6 * 24, false, 0, true);
- add_coin(coins, *wallet, 2 * CENT, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 5 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 3 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 2 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
CCoinControl coin_control;
coin_control.fAllowOtherInputs = true;
coin_control.Select(coins.at(0).outpoint);
@@ -344,6 +342,61 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
const auto result10 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb);
BOOST_CHECK(result10);
}
+ {
+ std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase());
+ wallet->LoadWallet();
+ LOCK(wallet->cs_wallet);
+ wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+ wallet->SetupDescriptorScriptPubKeyMans();
+
+ std::vector<COutput> coins;
+
+ // single coin should be selected when effective fee > long term fee
+ coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000);
+ coin_selection_params_bnb.m_long_term_feerate = CFeeRate(3000);
+
+ add_coin(coins, *wallet, 10 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 9 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+
+ expected_result.Clear();
+ add_coin(10 * CENT, 2, expected_result);
+ CCoinControl coin_control;
+ const auto result11 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb);
+ BOOST_CHECK(EquivalentResult(expected_result, *result11));
+ coins.clear();
+
+ // more coins should be selected when effective fee < long term fee
+ coin_selection_params_bnb.m_effective_feerate = CFeeRate(3000);
+ coin_selection_params_bnb.m_long_term_feerate = CFeeRate(5000);
+
+ add_coin(coins, *wallet, 10 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 9 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+
+ expected_result.Clear();
+ add_coin(9 * CENT, 2, expected_result);
+ add_coin(1 * CENT, 2, expected_result);
+ const auto result12 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb);
+ BOOST_CHECK(EquivalentResult(expected_result, *result12));
+ coins.clear();
+
+ // pre selected coin should be selected even if disadvantageous
+ coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000);
+ coin_selection_params_bnb.m_long_term_feerate = CFeeRate(3000);
+
+ add_coin(coins, *wallet, 10 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 9 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+ add_coin(coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
+
+ expected_result.Clear();
+ add_coin(9 * CENT, 2, expected_result);
+ add_coin(1 * CENT, 2, expected_result);
+ coin_control.fAllowOtherInputs = true;
+ coin_control.Select(coins.at(1).outpoint); // pre select 9 coin
+ const auto result13 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb);
+ BOOST_CHECK(EquivalentResult(expected_result, *result13));
+ }
}
BOOST_AUTO_TEST_CASE(knapsack_solver_test)
@@ -367,7 +420,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
// with an empty wallet we can't even pay one cent
BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1 * CENT, CENT));
- add_coin(coins, *wallet, 1*CENT, 4); // add a new 1 cent coin
+ add_coin(coins, *wallet, 1*CENT, CFeeRate(0), 4); // add a new 1 cent coin
// with a new 1 cent coin, we still can't find a mature 1 cent
BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1 * CENT, CENT));
@@ -388,7 +441,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 3 * CENT);
add_coin(coins, *wallet, 5*CENT); // add a mature 5 cent coin,
- add_coin(coins, *wallet, 10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses
+ add_coin(coins, *wallet, 10*CENT, CFeeRate(0), 3, true); // a new 10 cent coin sent from one of our own addresses
add_coin(coins, *wallet, 20*CENT); // and a mature 20 cent coin
// now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38
@@ -815,5 +868,40 @@ BOOST_AUTO_TEST_CASE(waste_test)
BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /* change cost */ 0, new_target));
}
+BOOST_AUTO_TEST_CASE(effective_value_test)
+{
+ const int input_bytes = 148;
+ const CFeeRate feerate(1000);
+ const CAmount nValue = 10000;
+ const int nInput = 0;
+
+ CMutableTransaction tx;
+ tx.vout.resize(1);
+ tx.vout[nInput].nValue = nValue;
+
+ // standard case, pass feerate in constructor
+ COutput output1(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, feerate);
+ const CAmount expected_ev1 = 9852; // 10000 - 148
+ BOOST_CHECK_EQUAL(output1.GetEffectiveValue(), expected_ev1);
+
+ // input bytes unknown (input_bytes = -1), pass feerate in constructor
+ COutput output2(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, feerate);
+ BOOST_CHECK_EQUAL(output2.GetEffectiveValue(), nValue); // The effective value should be equal to the absolute value if input_bytes is -1
+
+ // negative effective value, pass feerate in constructor
+ COutput output3(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, CFeeRate(100000));
+ const CAmount expected_ev3 = -4800; // 10000 - 14800
+ BOOST_CHECK_EQUAL(output3.GetEffectiveValue(), expected_ev3);
+
+ // standard case, pass fees in constructor
+ const CAmount fees = 148;
+ COutput output4(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, fees);
+ BOOST_CHECK_EQUAL(output4.GetEffectiveValue(), expected_ev1);
+
+ // input bytes unknown (input_bytes = -1), pass fees in constructor
+ COutput output5(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, /*fees=*/ 0);
+ BOOST_CHECK_EQUAL(output5.GetEffectiveValue(), nValue); // The effective value should be equal to the absolute value if input_bytes is -1
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace wallet
diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp
index 2693b68cca..3465f2f331 100644
--- a/src/wallet/test/fuzz/coinselection.cpp
+++ b/src/wallet/test/fuzz/coinselection.cpp
@@ -14,13 +14,13 @@
namespace wallet {
-static void AddCoin(const CAmount& value, int n_input, int n_input_bytes, int locktime, std::vector<COutput>& coins)
+static void AddCoin(const CAmount& value, int n_input, int n_input_bytes, int locktime, std::vector<COutput>& coins, CFeeRate fee_rate)
{
CMutableTransaction tx;
tx.vout.resize(n_input + 1);
tx.vout[n_input].nValue = value;
tx.nLockTime = locktime; // all transactions get different hashes
- coins.emplace_back(COutPoint(tx.GetHash(), n_input), tx.vout.at(n_input), /*depth=*/0, n_input_bytes, /*spendable=*/true, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/true);
+ coins.emplace_back(COutPoint(tx.GetHash(), n_input), tx.vout.at(n_input), /*depth=*/0, n_input_bytes, /*spendable=*/true, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/true, fee_rate);
}
// Randomly distribute coins to instances of OutputGroup
@@ -70,7 +70,7 @@ FUZZ_TARGET(coinselection)
if (total_balance + amount >= MAX_MONEY) {
break;
}
- AddCoin(amount, n_input, n_input_bytes, ++next_locktime, utxo_pool);
+ AddCoin(amount, n_input, n_input_bytes, ++next_locktime, utxo_pool, coin_params.m_effective_feerate);
total_balance += amount;
}
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index d4406fd5dd..70863f5464 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -584,7 +584,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
{
LOCK(wallet->cs_wallet);
std::vector<COutput> available;
- AvailableCoins(*wallet, available);
+ AvailableCoinsListUnspent(*wallet, available);
BOOST_CHECK_EQUAL(available.size(), 2U);
}
for (const auto& group : list) {
@@ -596,7 +596,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
{
LOCK(wallet->cs_wallet);
std::vector<COutput> available;
- AvailableCoins(*wallet, available);
+ AvailableCoinsListUnspent(*wallet, available);
BOOST_CHECK_EQUAL(available.size(), 0U);
}
// Confirm ListCoins still returns same result as before, despite coins
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 7d9f8b05bb..910562e669 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2106,7 +2106,7 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
// Add tx to wallet, because if it has change it's also ours,
// otherwise just for transaction history.
- AddToWallet(tx, TxStateInactive{}, [&](CWalletTx& wtx, bool new_tx) {
+ CWalletTx* wtx = AddToWallet(tx, TxStateInactive{}, [&](CWalletTx& wtx, bool new_tx) {
CHECK_NONFATAL(wtx.mapValue.empty());
CHECK_NONFATAL(wtx.vOrderForm.empty());
wtx.mapValue = std::move(mapValue);
@@ -2116,6 +2116,11 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
return true;
});
+ // wtx can only be null if the db write failed.
+ if (!wtx) {
+ throw std::runtime_error(std::string(__func__) + ": Wallet db error, transaction commit failed");
+ }
+
// Notify that old coins are spent
for (const CTxIn& txin : tx->vin) {
CWalletTx &coin = mapWallet.at(txin.prevout.hash);
@@ -2123,17 +2128,13 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
NotifyTransactionChanged(coin.GetHash(), CT_UPDATED);
}
- // Get the inserted-CWalletTx from mapWallet so that the
- // wtx cached mempool state is updated correctly
- CWalletTx& wtx = mapWallet.at(tx->GetHash());
-
if (!fBroadcastTransactions) {
// Don't submit tx to the mempool
return;
}
std::string err_string;
- if (!SubmitTxMemoryPoolAndRelay(wtx, err_string, true)) {
+ if (!SubmitTxMemoryPoolAndRelay(*wtx, err_string, true)) {
WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string);
// TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
}
generated by cgit v1.2.3 (git 2.26.2) at 2025-02-12 04:14:35 +0000