highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-12 23:18:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Set effective_value when initializing a COutput

Browse Source 
Previously in COutput, effective_value was initialized as the absolute
value of the txout, and fee as 0. effective_value along with fee were
calculated outside of the COutput constructor and set after the
object had been initialized. These changes will allow either the fee
or the feerate to be passed in a COutput constructor. If either are
provided, fee and effective_value are calculated and set in the
constructor. As a result, AvailableCoins also needs to be passed the
feerate when utxos are being spent. When balance is calculated or the
coins are being listed and feerate is neither available nor required,
AvailableCoinsListUnspent is used instead, which runs AvailableCoins
while providing the default value for feerate. Unit tests for the
calculation of effective value have also been added.
This commit is contained in:
ishaanam
2022-04-24 18:01:58 -04:00
parent 640eb772e5
commit 6fbb0edac2
9 changed files with 137 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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);
@@ -353,36 +351,49 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
std::vector<COutput> coins;
add_coin(coins, *wallet, 10 * CENT, 6 * 24, false, 0, true);
add_coin(coins, *wallet, 9 * CENT, 6 * 24, false, 0, true);
add_coin(coins, *wallet, 1 * CENT, 6 * 24, false, 0, true);
// 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;
coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000);
coin_selection_params_bnb.m_long_term_feerate = CFeeRate(3000);
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);
coin_selection_params_bnb.m_effective_feerate = CFeeRate(3000);
coin_selection_params_bnb.m_long_term_feerate = CFeeRate(5000);
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
coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000);
coin_selection_params_bnb.m_long_term_feerate = CFeeRate(3000);
const auto result13 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb);
BOOST_CHECK(EquivalentResult(expected_result, *result13));
}
@@ -409,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));
@@ -430,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
@@ -857,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

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

@@ -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