Skip to content

Commit

Permalink
Move BnB SFFO restriction test
Browse files Browse the repository at this point in the history
  • Loading branch information
@murchandamus
murchandamus committed Dec 12, 2023
1 parent 8cf032f commit ad7e31c
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 38 deletions.
54 changes: 54 additions & 0 deletions src/wallet/test/coinselection_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,35 @@ static CoinSelectionParams init_default_params()

static const CoinSelectionParams default_cs_params = init_default_params();

static void AddCoinToWallet(CoinsResult& available_coins, CWallet& wallet, const CAmount& nValue, CFeeRate feerate = default_cs_params.m_effective_feerate, int nAge = 6*24, bool fIsFromMe = false, int nInput =0, bool spendable = false, int custom_size = 0)
{
CMutableTransaction tx;
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
if (spendable) {
tx.vout[nInput].scriptPubKey = GetScriptForDestination(*Assert(wallet.GetNewDestination(OutputType::BECH32, "")));
}
uint256 txid = tx.GetHash();

LOCK(wallet.cs_wallet);
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;
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});
}

static std::unique_ptr<CWallet> NewWallet(const node::NodeContext& m_node, const std::string& wallet_name = "")
{
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), wallet_name, CreateMockableWalletDatabase());
BOOST_CHECK(wallet->LoadWallet() == DBErrors::LOAD_OK);
LOCK(wallet->cs_wallet);
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
wallet->SetupDescriptorScriptPubKeyMans();
return wallet;
}

/** Check if SelectionResult a is equivalent to SelectionResult b.
* Equivalent means same input values, but maybe different inputs (i.e. same value, different prevout) */
static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b)
Expand Down Expand Up @@ -375,6 +404,31 @@ BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/ 10 * CENT, /*expected_input_amounts=*/ {10 * CENT}, CFeeRate{25'000});
}

BOOST_AUTO_TEST_CASE(tx_creation_bnb_sffo_restriction)
{
// Verify the transaction creation process does not produce a BnB solution when SFFO is enabled.
// This is currently problematic because it could require a change output. And BnB is specialized on changeless solutions.
std::unique_ptr<CWallet> wallet = NewWallet(m_node);
WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(300, uint256{})); // set a high block so internal UTXOs are selectable

CoinSelectionParams params = init_default_params();
params.m_long_term_feerate = CFeeRate(1000); // LFTRE is less than Feerate, thrifty mode
params.m_subtract_fee_outputs = true;

// Add spendable coin at the BnB selection upper bound
CoinsResult available_coins;
AddCoinToWallet(available_coins, *wallet, COIN + params.m_cost_of_change, /*feerate=*/params.m_effective_feerate, /*nAge=*/6*24, /*fIsFromMe=*/true, /*nInput=*/0, /*spendable=*/true);
AddCoinToWallet(available_coins, *wallet, 0.7 * COIN, /*feerate=*/params.m_effective_feerate, /*nAge=*/6*24, /*fIsFromMe=*/true, /*nInput=*/0, /*spendable=*/true);
AddCoinToWallet(available_coins, *wallet, 0.6 * COIN, /*feerate=*/params.m_effective_feerate, /*nAge=*/6*24, /*fIsFromMe=*/true, /*nInput=*/0, /*spendable=*/true);
// Now verify coin selection does not produce BnB result
auto result = WITH_LOCK(wallet->cs_wallet, return SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, COIN, /*coin_control=*/{}, params));
BOOST_CHECK(result.has_value());
BOOST_CHECK_NE(result->GetAlgo(), SelectionAlgorithm::BNB);
// Knapsack will only find a changeless solution on an exact match to the satoshi, SRD doesn’t look for changeless
BOOST_CHECK(result->GetInputSet().size() == 2);
BOOST_CHECK(result->GetAlgo() == SelectionAlgorithm::SRD || result->GetAlgo() == SelectionAlgorithm::KNAPSACK);
}

//TODO: SelectCoins Test
// TODO: Test at `SelectCoins`/spend.cpp level that changeless solutions can be achieved by combining preset inputs with BnB solutions
// TODO: Test that immature coins are not considered
Expand Down
38 changes: 0 additions & 38 deletions src/wallet/test/coinselector_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,44 +72,6 @@ static std::unique_ptr<CWallet> NewWallet(const node::NodeContext& m_node, const
return wallet;
}

BOOST_AUTO_TEST_CASE(bnb_sffo_restriction)
{
// Verify the coin selection process does not produce a BnB solution when SFFO is enabled.
// This is currently problematic because it could require a change output. And BnB is specialized on changeless solutions.
std::unique_ptr<CWallet> wallet = NewWallet(m_node);
WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(300, uint256{})); // set a high block so internal UTXOs are selectable

FastRandomContext rand{};
CoinSelectionParams params{
rand,
/*change_output_size=*/ 31, // unused value, p2wpkh output size (wallet default change type)
/*change_spend_size=*/ 68, // unused value, p2wpkh input size (high-r signature)
/*min_change_target=*/ 0, // dummy, set later
/*effective_feerate=*/ CFeeRate(3000),
/*long_term_feerate=*/ CFeeRate(1000),
/*discard_feerate=*/ CFeeRate(1000),
/*tx_noinputs_size=*/ 0,
/*avoid_partial=*/ false,
};
params.m_subtract_fee_outputs = true;
params.m_change_fee = params.m_effective_feerate.GetFee(params.change_output_size);
params.m_cost_of_change = params.m_discard_feerate.GetFee(params.change_spend_size) + params.m_change_fee;
params.m_min_change_target = params.m_cost_of_change + 1;
// Add spendable coin at the BnB selection upper bound
CoinsResult available_coins;
add_coin(available_coins, *wallet, COIN + params.m_cost_of_change, /*feerate=*/params.m_effective_feerate, /*nAge=*/6, /*fIsFromMe=*/true, /*nInput=*/0, /*spendable=*/true);
add_coin(available_coins, *wallet, 0.5 * COIN + params.m_cost_of_change, /*feerate=*/params.m_effective_feerate, /*nAge=*/6, /*fIsFromMe=*/true, /*nInput=*/0, /*spendable=*/true);
add_coin(available_coins, *wallet, 0.5 * COIN, /*feerate=*/params.m_effective_feerate, /*nAge=*/6, /*fIsFromMe=*/true, /*nInput=*/0, /*spendable=*/true);
// Knapsack will only find a changeless solution on an exact match to the satoshi, SRD doesn’t look for changeless
// If BnB were run, it would produce a single input solution with the best waste score
auto result = WITH_LOCK(wallet->cs_wallet, return SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, COIN, /*coin_control=*/{}, params));
BOOST_CHECK(result.has_value());
BOOST_CHECK_NE(result->GetAlgo(), SelectionAlgorithm::BNB);
BOOST_CHECK(result->GetInputSet().size() == 2);
// We have only considered BnB, SRD, and Knapsack. Test needs to be reevaluated if new algo is added
BOOST_CHECK(result->GetAlgo() == SelectionAlgorithm::SRD || result->GetAlgo() == SelectionAlgorithm::KNAPSACK);
}

// Tests that with the ideal conditions, the coin selector will always be able to find a solution that can pay the target value
BOOST_AUTO_TEST_CASE(SelectCoins_test)
{
Expand Down

0 comments on commit ad7e31c

Please sign in to comment.