Skip to content

Commit db15e71

Browse files
achow101achow101
committed
Use BnB when preset inputs are selected
1 parent cfec3e0 commit db15e71

File tree

2 files changed

+58
-29
lines changed

2 files changed

+58
-29
lines changed

‎src/wallet/test/coinselector_tests.cpp‎

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,29 +53,39 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set)
5353
set.emplace(MakeTransactionRef(tx), nInput);
5454
}
5555

56-
static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0)
56+
static void add_coin(CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false)
5757
{
5858
balance += nValue;
5959
static int nextLockTime = 0;
6060
CMutableTransaction tx;
6161
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
6262
tx.vout.resize(nInput + 1);
6363
tx.vout[nInput].nValue = nValue;
64+
if (spendable) {
65+
CTxDestination dest;
66+
std::string error;
67+
assert(wallet.GetNewDestination(OutputType::BECH32, "", dest, error));
68+
tx.vout[nInput].scriptPubKey = GetScriptForDestination(dest);
69+
}
6470
if (fIsFromMe) {
6571
// IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(),
6672
// so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe()
6773
tx.vin.resize(1);
6874
}
69-
std::unique_ptr<CWalletTx> wtx = MakeUnique<CWalletTx>(&testWallet, MakeTransactionRef(std::move(tx)));
75+
std::unique_ptr<CWalletTx> wtx = MakeUnique<CWalletTx>(&wallet, MakeTransactionRef(std::move(tx)));
7076
if (fIsFromMe)
7177
{
7278
wtx->m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1);
7379
}
7480
COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */);
7581
vCoins.push_back(output);
76-
testWallet.AddToWallet(*wtx.get());
82+
wallet.AddToWallet(*wtx.get());
7783
wtxn.emplace_back(std::move(wtx));
7884
}
85+
static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false)
86+
{
87+
add_coin(testWallet, nValue, nAge, fIsFromMe, nInput, spendable);
88+
}
7989

8090
static void empty_wallet(void)
8191
{
@@ -250,17 +260,24 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
250260
vCoins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail
251261
BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used));
252262

253-
// Make sure that we aren't using BnB when there are preset inputs
263+
// Make sure that can use BnB when there are preset inputs
254264
empty_wallet();
255-
add_coin(5 * CENT);
256-
add_coin(3 * CENT);
257-
add_coin(2 * CENT);
258-
CCoinControl coin_control;
259-
coin_control.fAllowOtherInputs = true;
260-
coin_control.Select(COutPoint(vCoins.at(0).tx->GetHash(), vCoins.at(0).i));
261-
BOOST_CHECK(testWallet.SelectCoins(vCoins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used));
262-
BOOST_CHECK(!bnb_used);
263-
BOOST_CHECK(!coin_selection_params_bnb.use_bnb);
265+
{
266+
std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock());
267+
bool firstRun;
268+
wallet->LoadWallet(firstRun);
269+
LOCK(wallet->cs_wallet);
270+
add_coin(*wallet, 5 * CENT, 6 * 24, false, 0, true);
271+
add_coin(*wallet, 3 * CENT, 6 * 24, false, 0, true);
272+
add_coin(*wallet, 2 * CENT, 6 * 24, false, 0, true);
273+
CCoinControl coin_control;
274+
coin_control.fAllowOtherInputs = true;
275+
coin_control.Select(COutPoint(vCoins.at(0).tx->GetHash(), vCoins.at(0).i));
276+
coin_selection_params_bnb.effective_fee = CFeeRate(0);
277+
BOOST_CHECK(wallet->SelectCoins(vCoins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used));
278+
BOOST_CHECK(bnb_used);
279+
BOOST_CHECK(coin_selection_params_bnb.use_bnb);
280+
}
264281
}
265282

266283
BOOST_AUTO_TEST_CASE(knapsack_solver_test)

‎src/wallet/wallet.cpp‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2674,6 +2674,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
26742674
bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const
26752675
{
26762676
std::vector<COutput> vCoins(vAvailableCoins);
2677+
CAmount value_to_select = nTargetValue;
26772678

26782679
// coin control -> return all selected outputs (we want all selected to go into the transaction for sure)
26792680
if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs)
@@ -2699,22 +2700,33 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
26992700
coin_control.ListSelected(vPresetInputs);
27002701
for (const COutPoint& outpoint : vPresetInputs)
27012702
{
2702-
// For now, don't use BnB if preset inputs are selected. TODO: Enable this later
2703-
bnb_used = false;
2704-
coin_selection_params.use_bnb = false;
2705-
27062703
std::map<uint256, CWalletTx>::const_iterator it = mapWallet.find(outpoint.hash);
27072704
if (it != mapWallet.end())
27082705
{
27092706
const CWalletTx& wtx = it->second;
27102707
// Clearly invalid input, fail
2711-
if (wtx.tx->vout.size() <= outpoint.n)
2708+
if (wtx.tx->vout.size() <= outpoint.n) {
2709+
bnb_used = false;
27122710
return false;
2711+
}
27132712
// Just to calculate the marginal byte size
2714-
nValueFromPresetInputs += wtx.tx->vout[outpoint.n].nValue;
2715-
setPresetCoins.insert(CInputCoin(wtx.tx, outpoint.n));
2716-
} else
2713+
CInputCoin coin(wtx.tx, outpoint.n, wtx.GetSpendSize(outpoint.n, false));
2714+
nValueFromPresetInputs += coin.txout.nValue;
2715+
if (coin.m_input_bytes <= 0) {
2716+
bnb_used = false;
2717+
return false; // Not solvable, can't estimate size for fee
2718+
}
2719+
coin.effective_value = coin.txout.nValue - coin_selection_params.effective_fee.GetFee(coin.m_input_bytes);
2720+
if (coin_selection_params.use_bnb) {
2721+
value_to_select -= coin.effective_value;
2722+
} else {
2723+
value_to_select -= coin.txout.nValue;
2724+
}
2725+
setPresetCoins.insert(coin);
2726+
} else {
2727+
bnb_used = false;
27172728
return false; // TODO: Allow non-wallet inputs
2729+
}
27182730
}
27192731

27202732
// remove preset inputs from vCoins
@@ -2743,14 +2755,14 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
27432755
size_t max_descendants = (size_t)std::max<int64_t>(1, limit_descendant_count);
27442756
bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS);
27452757

2746-
bool res = nTargetValue <= nValueFromPresetInputs ||
2747-
SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 6, 0), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used) ||
2748-
SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 1, 0), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used) ||
2749-
(m_spend_zero_conf_change && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, 2), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
2750-
(m_spend_zero_conf_change && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
2751-
(m_spend_zero_conf_change && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
2752-
(m_spend_zero_conf_change && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
2753-
(m_spend_zero_conf_change && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max()), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used));
2758+
bool res = value_to_select <= 0 ||
2759+
SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used) ||
2760+
SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used) ||
2761+
(m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
2762+
(m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
2763+
(m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
2764+
(m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
2765+
(m_spend_zero_conf_change && !fRejectLongChains && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max()), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used));
27542766

27552767
// because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset
27562768
util::insert(setCoinsRet, setPresetCoins);

0 commit comments

Comments
 (0)