Skip to content

Commit

Permalink
Merge bitcoin#21910: refactor: remove redundant fOnlySafe argument
Browse files Browse the repository at this point in the history 
c30dd02 refactor: remove redundant fOnlySafe argument (t-bast)

Pull request description:

  The `fOnlySafe` argument to `AvailableCoins` is now redundant, since bitcoin#21359 added a similar field inside the `CCoinControl` struct (see bitcoin#21359 (comment)).

  Not all code paths create a `CCoinControl` instance, but when it's missing we can default to using only safe inputs which is backwards-compatible.

ACKs for top commit:
  instagibbs:
    utACK c30dd02
  promag:
    Code review ACK c30dd02.
  achow101:
    ACK c30dd02
  meshcollider:
    Code review + test run ACK c30dd02

Tree-SHA512: af3cb598d06f233fc48a7c9c45bb14da92b5cf4168b8dbd4f134dc3e0c2b615c6590238ddb1eaf380aea5bbdd3386d2ac8ecd7d22dfc93579adc39248542839b
  • Loading branch information
@meshcollider @vijaydasmp
meshcollider authored and vijaydasmp committed May 7, 2024
1 parent 21efcfa commit 3b82cdd
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/coinjoin/client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1538,7 +1538,7 @@ bool CCoinJoinClientSession::CreateCollateralTransaction(CMutableTransaction& tx
CCoinControl coin_control;
coin_control.nCoinType = CoinType::ONLY_COINJOIN_COLLATERAL;

m_wallet.AvailableCoins(vCoins, true, &coin_control);
m_wallet.AvailableCoins(vCoins, &coin_control);

if (vCoins.empty()) {
strReason = strprintf("%s requires a collateral transaction and could not locate an acceptable input!", gCoinJoinName);
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/masternode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ static UniValue masternode_outputs(const JSONRPCRequest& request)
coin_control.nCoinType = CoinType::ONLY_MASTERNODE_COLLATERAL;
{
LOCK(wallet->cs_wallet);
wallet->AvailableCoins(vPossibleCoins, true, &coin_control);
wallet->AvailableCoins(vPossibleCoins, &coin_control);
}
UniValue outputsArr(UniValue::VARR);
for (const auto& out : vPossibleCoins) {
Expand Down
3 changes: 2 additions & 1 deletion src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3262,9 +3262,10 @@ static RPCHelpMan listunspent()
coinControl.m_avoid_address_reuse = false;
coinControl.m_min_depth = nMinDepth;
coinControl.m_max_depth = nMaxDepth;
coinControl.m_include_unsafe_inputs = include_unsafe;

LOCK(pwallet->cs_wallet);
pwallet->AvailableCoins(vecOutputs, !include_unsafe, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
pwallet->AvailableCoins(vecOutputs, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
}

LOCK(pwallet->cs_wallet);
Expand Down
15 changes: 8 additions & 7 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2556,7 +2556,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const

CAmount balance = 0;
std::vector<COutput> vCoins;
AvailableCoins(vCoins, true, coinControl);
AvailableCoins(vCoins, coinControl);
for (const COutput& out : vCoins) {
if (out.fSpendable) {
balance += out.tx->tx->vout[out.i].nValue;
Expand All @@ -2565,7 +2565,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const
return balance;
}

void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount &nMinimumSumAmount, const uint64_t nMaximumCount) const
void CWallet::AvailableCoins(std::vector<COutput> &vCoins, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount &nMinimumSumAmount, const uint64_t nMaximumCount) const
{
AssertLockHeld(cs_wallet);

Expand All @@ -2578,6 +2578,7 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
bool allow_used_addresses = !IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) || (coinControl && !coinControl->m_avoid_address_reuse);
const int min_depth = {coinControl ? coinControl->m_min_depth : DEFAULT_MIN_DEPTH};
const int max_depth = {coinControl ? coinControl->m_max_depth : DEFAULT_MAX_DEPTH};
const bool only_safe = {coinControl ? !coinControl->m_include_unsafe_inputs : true};

std::set<uint256> trusted_parents;
for (auto pcoin : GetSpendableTXs()) {
Expand All @@ -2598,7 +2599,7 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const

bool safeTx = IsTrusted(*pcoin, trusted_parents);

if (fOnlySafe && !safeTx) {
if (only_safe && !safeTx) {
continue;
}

Expand Down Expand Up @@ -3089,7 +3090,7 @@ bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::ve

CCoinControl coin_control;
coin_control.nCoinType = CoinType::ONLY_READY_TO_MIX;
AvailableCoins(vCoins, true, &coin_control);
AvailableCoins(vCoins, &coin_control);
WalletCJLogPrint((*this), "CWallet::%s -- vCoins.size(): %d\n", __func__, vCoins.size());

Shuffle(vCoins.rbegin(), vCoins.rend(), FastRandomContext());
Expand Down Expand Up @@ -3289,7 +3290,7 @@ bool CWallet::SelectDenominatedAmounts(CAmount nValueMax, std::set<CAmount>& set
std::vector<COutput> vCoins;
CCoinControl coin_control;
coin_control.nCoinType = CoinType::ONLY_READY_TO_MIX;
AvailableCoins(vCoins, true, &coin_control);
AvailableCoins(vCoins, &coin_control);
// larger denoms first
std::sort(vCoins.rbegin(), vCoins.rend(), CompareByPriority());

Expand Down Expand Up @@ -3329,7 +3330,7 @@ bool CWallet::HasCollateralInputs(bool fOnlyConfirmed) const
std::vector<COutput> vCoins;
CCoinControl coin_control;
coin_control.nCoinType = CoinType::ONLY_COINJOIN_COLLATERAL;
AvailableCoins(vCoins, fOnlyConfirmed, &coin_control);
AvailableCoins(vCoins, &coin_control);

return !vCoins.empty();
}
Expand Down Expand Up @@ -3401,7 +3402,7 @@ bool CWallet::CreateTransactionInternal(
{
CAmount nAmountAvailable{0};
std::vector<COutput> vAvailableCoins;
AvailableCoins(vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0);
AvailableCoins(vAvailableCoins, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0);
CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy
coin_selection_params.use_bnb = false; // never use BnB

Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
/**
* populate vCoins with vector of available COutputs.
*/
void AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe = true, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void AvailableCoins(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) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);

/**
* Return list of available coins and locked coins grouped by non-change output address.
Expand Down

0 comments on commit 3b82cdd

Please sign in to comment.