highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-15 08:31:49 +01:00
Code Issues Packages Projects Releases Wiki Activity

wallet: Remove COutput::spendable and AvailableCoinsListUnspent

Browse Source 
In descriptor wallets, we consider all outputs to be spendable as we no
longer have mixed mine and watchonly in a wallet. As such,
COutput::spendable is meaningless and can be removed.

Furthermore, CoinFilterParams::only_spendable can be removed as that was
essentially checking for COutput::spendable.

Lastly, AvailableCoinsListUnspent can also be removed as the wrapper is
now only setting the feerate to std::nullopt which is trivial enough that
a dedicated wrapper is not needed.
This commit is contained in:
Ava Chow
2025-05-15 17:57:38 -07:00
parent 620abe985e
commit 6a7aa01574
10 changed files with 31 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/bench/coin_selection.cpp
View File

@@ -74,7 +74,7 @@ static void CoinSelection(benchmark::Bench& bench)
wallet::CoinsResult available_coins;
for (const auto& wtx : wtxs) {
const auto txout = wtx->tx->vout.at(0);
available_coins.coins[OutputType::BECH32].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);
available_coins.coins[OutputType::BECH32].emplace_back(COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0);
}
const CoinEligibilityFilter filter_standard(1, 6, 0);
@@ -105,7 +105,7 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup>
CMutableTransaction tx;
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 0, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ true, /*fees=*/ 0);
COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/0, /*input_bytes=*/-1, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/true, /*fees=*/0);
set.emplace_back();
set.back().Insert(std::make_shared<COutput>(output), /*ancestors=*/ 0, /*descendants=*/ 0);
}

10
src/wallet/coinselection.h
View File

@@ -50,9 +50,6 @@ public:
/** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */
int input_bytes;
/** Whether we have the private keys to spend this output */
bool spendable;
/** Whether we know how to spend this output, ignoring the lack of keys */
bool solvable;
@@ -75,12 +72,11 @@ public:
/** The fee necessary to bump this UTXO's ancestor transactions to the target feerate */
CAmount ancestor_bump_fees{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, const std::optional<CFeeRate> feerate = std::nullopt)
COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool solvable, bool safe, int64_t time, bool from_me, const std::optional<CFeeRate> feerate = std::nullopt)
: outpoint{outpoint},
txout{txout},
depth{depth},
input_bytes{input_bytes},
spendable{spendable},
solvable{solvable},
safe{safe},
time{time},
@@ -93,8 +89,8 @@ public:
}
}
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)
COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool solvable, bool safe, int64_t time, bool from_me, const CAmount fees)
: COutput(outpoint, txout, depth, input_bytes, 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));

7
src/wallet/rpc/coins.cpp
View File

@@ -498,7 +498,7 @@ RPCHelpMan listunspent()
{RPCResult::Type::STR_AMOUNT, "ancestorfees", /*optional=*/true, "The total fees of in-mempool ancestors (including this one) with fee deltas used for mining priority in " + CURRENCY_ATOM + " (if transaction is in the mempool)"},
{RPCResult::Type::STR_HEX, "redeemScript", /*optional=*/true, "The redeem script if the output script is P2SH"},
{RPCResult::Type::STR, "witnessScript", /*optional=*/true, "witness script if the output script is P2WSH or P2SH-P2WSH"},
{RPCResult::Type::BOOL, "spendable", "Whether we have the private keys to spend this output"},
{RPCResult::Type::BOOL, "spendable", "(DEPRECATED) Always true"},
{RPCResult::Type::BOOL, "solvable", "Whether we know how to spend this output, ignoring the lack of keys"},
{RPCResult::Type::BOOL, "reused", /*optional=*/true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"},
{RPCResult::Type::STR, "desc", /*optional=*/true, "(only when solvable) A descriptor for spending this output"},
@@ -598,8 +598,9 @@ RPCHelpMan listunspent()
cctl.m_min_depth = nMinDepth;
cctl.m_max_depth = nMaxDepth;
cctl.m_include_unsafe_inputs = include_unsafe;
filter_coins.check_version_trucness = false;
LOCK(pwallet->cs_wallet);
vecOutputs = AvailableCoinsListUnspent(*pwallet, &cctl, filter_coins).All();
vecOutputs = AvailableCoins(*pwallet, &cctl, /*feerate=*/std::nullopt, filter_coins).All();
}
LOCK(pwallet->cs_wallet);
@@ -672,7 +673,7 @@ RPCHelpMan listunspent()
entry.pushKV("ancestorfees", uint64_t(ancestor_fees));
}
}
entry.pushKV("spendable", out.spendable);
entry.pushKV("spendable", true); // Any coins we list are always spendable
entry.pushKV("solvable", out.solvable);
if (out.solvable) {
std::unique_ptr<SigningProvider> provider = pwallet->GetSolvingProvider(scriptPubKey);

34
src/wallet/spend.cpp
View File

@@ -309,8 +309,8 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
return util::Error{strprintf(_("Not solvable pre-selected input %s"), outpoint.ToString())}; // Not solvable, can't estimate size for fee
}
/* 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, coin_selection_params.m_effective_feerate);
/* Set some defaults for depth, 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, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/false, coin_selection_params.m_effective_feerate);
output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
result.Insert(output, coin_selection_params.m_subtract_fee_outputs);
}
@@ -452,10 +452,6 @@ CoinsResult AvailableCoins(const CWallet& wallet,
// Because CalculateMaximumSignedInputSize infers a solvable descriptor to get the satisfaction size,
// it is safe to assume that this input is solvable if input_bytes is greater than -1.
bool solvable = input_bytes > -1;
bool spendable = (mine & ISMINE_SPENDABLE) != ISMINE_NO;
// Filter by spendable outputs only
if (!spendable && params.only_spendable) continue;
// Obtain script type
std::vector<std::vector<uint8_t>> script_solutions;
@@ -474,7 +470,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
}
auto available_output_type = GetOutputType(type, is_from_p2sh);
auto available_output = COutput(outpoint, output, nDepth, input_bytes, spendable, solvable, tx_safe, wtx.GetTxTime(), tx_from_me, feerate);
auto available_output = COutput(outpoint, output, nDepth, input_bytes, solvable, tx_safe, wtx.GetTxTime(), tx_from_me, feerate);
if (wtx.tx->version == TRUC_VERSION && nDepth == 0 && params.check_version_trucness) {
unconfirmed_truc_coins.emplace_back(available_output_type, available_output);
auto [it, _] = truc_txid_by_value.try_emplace(wtx.tx->GetHash(), 0);
@@ -528,13 +524,6 @@ CoinsResult AvailableCoins(const CWallet& wallet,
return result;
}
CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl* coinControl, CoinFilterParams params)
{
params.only_spendable = false;
params.check_version_trucness = false;
return AvailableCoins(wallet, coinControl, /*feerate=*/ std::nullopt, params);
}
const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& outpoint)
{
AssertLockHeld(wallet.cs_wallet);
@@ -563,21 +552,18 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
CCoinControl coin_control;
CoinFilterParams coins_params;
coins_params.only_spendable = false;
coins_params.skip_locked = false;
for (const COutput& coin : AvailableCoins(wallet, &coin_control, /*feerate=*/std::nullopt, coins_params).All()) {
CTxDestination address;
if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable))) {
if (!ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) {
// For backwards compatibility, we convert P2PK output scripts into PKHash destinations
if (auto pk_dest = std::get_if<PubKeyDestination>(&address)) {
address = PKHash(pk_dest->GetPubKey());
} else {
continue;
}
if (!ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) {
// For backwards compatibility, we convert P2PK output scripts into PKHash destinations
if (auto pk_dest = std::get_if<PubKeyDestination>(&address)) {
address = PKHash(pk_dest->GetPubKey());
} else {
continue;
}
result[address].emplace_back(coin);
}
result[address].emplace_back(coin);
}
return result;
}

8
src/wallet/spend.h
View File

@@ -77,8 +77,6 @@ struct CoinFilterParams {
CAmount min_sum_amount{MAX_MONEY};
// Maximum number of outputs that can be returned
uint64_t max_count{0};
// By default, return only spendable outputs
bool only_spendable{true};
// By default, do not include immature coinbase outputs
bool include_immature_coinbase{false};
// By default, skip locked UTXOs
@@ -96,12 +94,6 @@ CoinsResult AvailableCoins(const CWallet& wallet,
std::optional<CFeeRate> feerate = std::nullopt,
const CoinFilterParams& params = {}) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
/**
* Wrapper function for AvailableCoins which skips the `feerate` and `CoinFilterParams::only_spendable` parameters. Use this function
* to list all available coins (e.g. listunspent RPC) while not intending to fund a transaction.
*/
CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl* coinControl = nullptr, CoinFilterParams params = {}) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
/**
* Find non-change parent output.
*/

2
src/wallet/test/coinselection_tests.cpp
View File

@@ -53,7 +53,7 @@ static OutputGroup MakeCoin(const CAmount& amount, bool is_eff_value = true, Coi
tx.vout[0].nValue = amount + int(is_eff_value) * fees;
tx.nLockTime = next_lock_time++; // so all transactions get different hashes
OutputGroup group(cs_params);
group.Insert(std::make_shared<COutput>(COutPoint(tx.GetHash(), 0), tx.vout.at(0), /*depth=*/1, /*input_bytes=*/custom_spending_vsize, /*spendable=*/true, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/false, /*fees=*/fees), /*ancestors=*/0, /*descendants=*/0);
group.Insert(std::make_shared<COutput>(COutPoint(tx.GetHash(), 0), tx.vout.at(0), /*depth=*/1, /*input_bytes=*/custom_spending_vsize, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/false, /*fees=*/fees), /*ancestors=*/0, /*descendants=*/0);
return group;
}

16
src/wallet/test/coinselector_tests.cpp
View File

@@ -43,7 +43,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, /*fees=*/ 0);
COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/1, /*input_bytes=*/-1, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/false, /*fees=*/ 0);
OutputGroup group;
group.Insert(std::make_shared<COutput>(output), /*ancestors=*/ 0, /*descendants=*/ 0);
result.AddInput(group);
@@ -55,7 +55,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
std::shared_ptr<COutput> coin = std::make_shared<COutput>(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);
std::shared_ptr<COutput> coin = std::make_shared<COutput>(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/1, /*input_bytes=*/148, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/false, fee);
OutputGroup group;
group.Insert(coin, /*ancestors=*/ 0, /*descendants=*/ 0);
coin->long_term_fee = long_term_fee; // group.Insert() will modify long_term_fee, so we need to set it afterwards
@@ -78,7 +78,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun
assert(ret.second);
CWalletTx& wtx = (*ret.first).second;
const auto& txout = wtx.tx->vout.at(nInput);
available_coins.Add(OutputType::BECH32, {COutPoint(wtx.GetHash(), nInput), txout, nAge, custom_size == 0 ? CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr) : custom_size, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate});
available_coins.Add(OutputType::BECH32, {COutPoint(wtx.GetHash(), nInput), txout, nAge, custom_size == 0 ? CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr) : custom_size, /*solvable=*/true, /*safe=*/true, wtx.GetTxTime(), fIsFromMe, feerate});
}
// Helpers
@@ -911,26 +911,26 @@ BOOST_AUTO_TEST_CASE(effective_value_test)
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);
COutput output1(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/1, input_bytes, /*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);
COutput output2(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/1, /*input_bytes=*/-1, /*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));
COutput output3(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/1, input_bytes, /*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);
COutput output4(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/1, input_bytes, /*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);
COutput output5(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/1, /*input_bytes=*/-1, /*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
}

2
src/wallet/test/fuzz/coinselection.cpp
View File

@@ -23,7 +23,7 @@ static void AddCoin(const CAmount& value, int n_input, int n_input_bytes, int lo
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, fee_rate);
coins.emplace_back(COutPoint(tx.GetHash(), n_input), tx.vout.at(n_input), /*depth=*/0, n_input_bytes, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/true, fee_rate);
}
// Randomly distribute coins to instances of OutputGroup

1
src/wallet/test/group_outputs_tests.cpp
View File

@@ -51,7 +51,6 @@ static void addCoin(CoinsResult& coins,
txout,
depth,
CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr),
/*spendable=*/ true,
/*solvable=*/ true,
/*safe=*/ true,
wtx.GetTxTime(),

4
src/wallet/test/wallet_tests.cpp
View File

@@ -424,7 +424,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
// Lock both coins. Confirm number of available coins drops to 0.
{
LOCK(wallet->cs_wallet);
BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).Size(), 2U);
BOOST_CHECK_EQUAL(AvailableCoins(*wallet).Size(), 2U);
}
for (const auto& group : list) {
for (const auto& coin : group.second) {
@@ -434,7 +434,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
}
{
LOCK(wallet->cs_wallet);
BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).Size(), 0U);
BOOST_CHECK_EQUAL(AvailableCoins(*wallet).Size(), 0U);
}
// Confirm ListCoins still returns same result as before, despite coins
// being locked.