diff options
author | fanquake <fanquake@gmail.com> | 2022-03-24 20:22:51 +0000 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-03-24 20:46:43 +0000 |
commit | 3740cdd12551fab2cc9ef12afff8680c34086343 (patch) | |
tree | c3356a68d8ba8f98c49c9d75f97f03cd799b2f67 /src/wallet/test | |
parent | f0c9ba2b48c3aabdf445076574be7eeb77fe2f48 (diff) | |
parent | 049003fe68a4183f6f20da16f58f10079d1e02df (diff) | |
download | bitcoin-3740cdd12551fab2cc9ef12afff8680c34086343.tar.xz |
Merge bitcoin/bitcoin#24091: wallet: Consolidate CInputCoin and COutput
049003fe68a4183f6f20da16f58f10079d1e02df coinselection: Remove COutput operators == and != (Andrew Chow)
f6c39c6adb6cbf9c87f04d3d667701905ef5c0a0 coinselection: Remove CInputCoin (Andrew Chow)
70f31f1a81710aa59e95770de9a84bf58cbce1e8 coinselection: Use COutput instead of CInputCoin (Andrew Chow)
14fbb57b79c664090f6a4e60d7bdfc9759ff4307 coinselection: Add effective value and fees to COutput (Andrew Chow)
f0821230b8de2eec21a869d1edf9e2b9f502de25 moveonly: move COutput to coinselection.h (Andrew Chow)
42e974e15c6deba1d9395a4da9341c9ebec6e8e5 wallet: Remove CWallet and CWalletTx from COutput's constructor (Andrew Chow)
14d04d5ad15ae56df56edee7ca9a202b52037889 wallet: Replace CWalletTx in COutput with COutPoint and CTxOut (Andrew Chow)
0ba4d1916e26e2a5d603edcdb7625463989d25b6 wallet: Provide input bytes to COutput (Andrew Chow)
d51f27d3bb0d6e3ca55bcd23ce53e4fe413a9360 wallet: Store whether a COutput is from the wallet (Andrew Chow)
b799814bbd53736b79495072f3c9e05989a465e8 wallet: Store tx time in COutput (Andrew Chow)
46022953ee2e8113167bafd1fd48a383a578b13c wallet: Remove use_max_sig default value (Andrew Chow)
10379f007fd2c18f4cd24d0a0783d6d929f45556 scripted-diff: Rename COutput member variables (Andrew Chow)
c7c64db41e1718584aa2f30ff27f60ab0966de62 wallet: cleanup COutput constructor (Andrew Chow)
Pull request description:
While working on coin selection code, it occurred to me that `CInputCoin` is really a subset of `COutput` and the conversion of a `COutput` to a `CInputCoin` does not appear to be all that useful. So this PR adds fields that are present in `CInputCoin` to `COutput` and replaces the usage of `CInputCoin` with `COutput`.
`COutput` is also moved to coinselection.h. As part of this move, the usage of `CWalletTx` is removed from `COutput`. It is instead replaced by storing a `COutPoint` and the `CTxOut` rather than the entire `CWalletTx` as coin selection does not really need the full `CWalletTx`. The `CWalletTx` was only used for figuring out whether the transaction containing the output was from the current wallet, and for the transaction's time. These are now parameters to `COutput`'s constructor.
ACKs for top commit:
ryanofsky:
Code review ACK 049003fe68a4183f6f20da16f58f10079d1e02df, just adding comments and removing == operators since last review
w0xlt:
reACK 049003f
Xekyo:
reACK 049003fe68a4183f6f20da16f58f10079d1e02df
Tree-SHA512: 048b4cd620a0415e1d9fe8597257ee4bc64656566e1d28a9bdd147d6d72dc87c3f34a3339fa9ab6acf42c388df7901fc4ee900ccaabc3de790ffad162b544c15
Diffstat (limited to 'src/wallet/test')
-rw-r--r-- | src/wallet/test/coinselector_tests.cpp | 57 | ||||
-rw-r--r-- | src/wallet/test/wallet_tests.cpp | 2 |
2 files changed, 20 insertions, 39 deletions
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f0c799b43e..14bec8ca4c 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -28,20 +28,20 @@ BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup) // we repeat those tests this many times and only complain if all iterations of the test fail #define RANDOM_REPEATS 5 -typedef std::set<CInputCoin> CoinSet; +typedef std::set<COutput> CoinSet; static const CoinEligibilityFilter filter_standard(1, 6, 0); static const CoinEligibilityFilter filter_confirmed(1, 1, 0); static const CoinEligibilityFilter filter_standard_extra(6, 6, 0); static int nextLockTime = 0; -static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) +static void add_coin(const CAmount& nValue, int nInput, std::vector<COutput>& set) { CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; tx.nLockTime = nextLockTime++; // so all transactions get different hashes - set.emplace_back(MakeTransactionRef(tx), nInput); + 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); } static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result) @@ -50,9 +50,9 @@ 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 - CInputCoin coin(MakeTransactionRef(tx), nInput); + 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); OutputGroup group; - group.Insert(coin, 1, false, 0, 0, true); + group.Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ true); result.AddInput(group); } @@ -62,10 +62,10 @@ 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 - CInputCoin coin(MakeTransactionRef(tx), nInput); + 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.m_fee = fee; - coin.m_long_term_fee = long_term_fee; + coin.fee = fee; + coin.long_term_fee = long_term_fee; set.insert(coin); } @@ -82,24 +82,13 @@ static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount assert(destination_ok); tx.vout[nInput].scriptPubKey = GetScriptForDestination(dest); } - if (fIsFromMe) { - // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), - // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() - tx.vin.resize(1); - } 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; - if (fIsFromMe) - { - wtx.m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); - wtx.m_is_cache_empty = false; - } - COutput output(wallet, wtx, nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); - coins.push_back(output); + 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); } /** Check if SelectionResult a is equivalent to SelectionResult b. @@ -124,11 +113,14 @@ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b) /** Check if this selection is equal to another one. Equal means same inputs (i.e same value and prevout) */ static bool EqualResult(const SelectionResult& a, const SelectionResult& b) { - std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin()); + std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin(), + [](const COutput& a, const COutput& b) { + return a.outpoint == b.outpoint; + }); return ret.first == a.GetInputSet().end() && ret.second == b.GetInputSet().end(); } -static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) +static CAmount make_hard_case(int utxos, std::vector<COutput>& utxo_pool) { utxo_pool.clear(); CAmount target = 0; @@ -140,24 +132,13 @@ static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) return target; } -inline std::vector<OutputGroup>& GroupCoins(const std::vector<CInputCoin>& coins) -{ - static std::vector<OutputGroup> static_groups; - static_groups.clear(); - for (auto& coin : coins) { - static_groups.emplace_back(); - static_groups.back().Insert(coin, 0, true, 0, 0, false); - } - return static_groups; -} - inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& coins) { static std::vector<OutputGroup> static_groups; static_groups.clear(); for (auto& coin : coins) { static_groups.emplace_back(); - static_groups.back().Insert(coin.GetInputCoin(), coin.nDepth, coin.tx->m_amounts[CWalletTx::DEBIT].m_cached[ISMINE_SPENDABLE] && coin.tx->m_amounts[CWalletTx::DEBIT].m_value[ISMINE_SPENDABLE] == 1 /* HACK: we can't figure out the is_me flag so we use the conditions defined above; perhaps set safe to false for !fIsFromMe in add_coin() */, 0, 0, false); + static_groups.back().Insert(coin, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false); } return static_groups; } @@ -185,7 +166,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) { FastRandomContext rand{}; // Setup - std::vector<CInputCoin> utxo_pool; + std::vector<COutput> utxo_pool; SelectionResult expected_result(CAmount(0)); ///////////////////////// @@ -329,13 +310,13 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) std::vector<COutput> coins; add_coin(coins, *wallet, 1); - coins.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 + 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); - coins.at(0).nInputBytes = 40; + 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); BOOST_CHECK(result9); @@ -356,7 +337,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(coins, *wallet, 2 * CENT, 6 * 24, false, 0, true); CCoinControl coin_control; coin_control.fAllowOtherInputs = true; - coin_control.Select(COutPoint(coins.at(0).tx->GetHash(), coins.at(0).i)); + coin_control.Select(coins.at(0).outpoint); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); const auto result10 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(result10); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7d2b769ae6..c1fa2c3c60 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -588,7 +588,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) for (const auto& group : list) { for (const auto& coin : group.second) { LOCK(wallet->cs_wallet); - wallet->LockCoin(COutPoint(coin.tx->GetHash(), coin.i)); + wallet->LockCoin(coin.outpoint); } } { |