diff options
author | fanquake <fanquake@gmail.com> | 2023-10-29 10:05:13 +0100 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2023-10-29 10:10:53 +0100 |
commit | 42b0d5f59b868e8ed5ae6c7df3bc133f79e3bfb3 (patch) | |
tree | bd32913df89db000f46022e311540dffbdbea257 | |
parent | e789b30b2565c8bdbf48a45f2c5a7b92e5d61d25 (diff) | |
parent | faec889f938f90e0b887426db27a15ec0d169399 (diff) | |
download | bitcoin-42b0d5f59b868e8ed5ae6c7df3bc133f79e3bfb3.tar.xz |
Merge bitcoin/bitcoin#28740: refactor: Add LIFETIMEBOUND to all (w)txid getters
faec889f938f90e0b887426db27a15ec0d169399 refactor: Add LIFETIMEBOUND to all (w)txid getters (MarcoFalke)
Pull request description:
Currently some getters return a reference, some don't. Fix this by returning a reference everywhere. Also, add `LIFETIMEBOUND` to all. Then, use the compiler warnings to create copies only where needed.
Also, fix iwyu includes while touching the includes.
ACKs for top commit:
dergoegge:
Code review ACK faec889f938f90e0b887426db27a15ec0d169399
stickies-v:
ACK faec889f938f90e0b887426db27a15ec0d169399
pablomartin4btc:
cr ACK faec889f938f90e0b887426db27a15ec0d169399
Tree-SHA512: 0c2a151f39d0e007b4d33b0b85ad578cc220f3e9dd94890e812b3181c3901545b039325707731cc39a5e89557f59c1154c6320525f78f5de95f119a514d2d23f
-rw-r--r-- | src/primitives/transaction.h | 8 | ||||
-rw-r--r-- | src/test/fuzz/tx_pool.cpp | 2 | ||||
-rw-r--r-- | src/util/transaction_identifier.h | 5 | ||||
-rw-r--r-- | src/wallet/test/group_outputs_tests.cpp | 2 | ||||
-rw-r--r-- | src/wallet/test/wallet_tests.cpp | 2 | ||||
-rw-r--r-- | src/wallet/transaction.h | 17 |
6 files changed, 19 insertions, 17 deletions
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 2516647a84..89deb9de4d 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -6,8 +6,8 @@ #ifndef BITCOIN_PRIMITIVES_TRANSACTION_H #define BITCOIN_PRIMITIVES_TRANSACTION_H +#include <attributes.h> #include <consensus/amount.h> -#include <prevector.h> #include <script/script.h> #include <serialize.h> #include <uint256.h> @@ -335,8 +335,8 @@ public: return vin.empty() && vout.empty(); } - const Txid& GetHash() const { return hash; } - const Wtxid& GetWitnessHash() const { return m_witness_hash; }; + const Txid& GetHash() const LIFETIMEBOUND { return hash; } + const Wtxid& GetWitnessHash() const LIFETIMEBOUND { return m_witness_hash; }; // Return sum of txouts. CAmount GetValueOut() const; @@ -433,7 +433,7 @@ public: static GenTxid Txid(const uint256& hash) { return GenTxid{false, hash}; } static GenTxid Wtxid(const uint256& hash) { return GenTxid{true, hash}; } bool IsWtxid() const { return m_is_wtxid; } - const uint256& GetHash() const { return m_hash; } + const uint256& GetHash() const LIFETIMEBOUND { return m_hash; } friend bool operator==(const GenTxid& a, const GenTxid& b) { return a.m_is_wtxid == b.m_is_wtxid && a.m_hash == b.m_hash; } friend bool operator<(const GenTxid& a, const GenTxid& b) { return std::tie(a.m_is_wtxid, a.m_hash) < std::tie(b.m_is_wtxid, b.m_hash); } }; diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 1ba0f17cc2..5ec3e89d1e 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -343,7 +343,7 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool) tx_pool.RollingFeeUpdate(); } if (fuzzed_data_provider.ConsumeBool()) { - const auto& txid = fuzzed_data_provider.ConsumeBool() ? + const auto txid = fuzzed_data_provider.ConsumeBool() ? mut_tx.GetHash().ToUint256() : PickValue(fuzzed_data_provider, txids); const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN); diff --git a/src/util/transaction_identifier.h b/src/util/transaction_identifier.h index 77f4b8367d..4fb9b49966 100644 --- a/src/util/transaction_identifier.h +++ b/src/util/transaction_identifier.h @@ -1,6 +1,7 @@ #ifndef BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H #define BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H +#include <attributes.h> #include <uint256.h> #include <util/types.h> @@ -35,7 +36,7 @@ public: template <typename Other> bool operator<(const Other& other) const { return Compare(other) < 0; } - uint256 ToUint256() const { return m_wrapped; } + const uint256& ToUint256() const LIFETIMEBOUND { return m_wrapped; } static transaction_identifier FromUint256(const uint256& id) { return {id}; } /** Wrapped `uint256` methods. */ @@ -56,7 +57,7 @@ public: * TODO: This should be removed once the majority of the code has switched * to using the Txid and Wtxid types. Until then it makes for a smoother * transition to allow this conversion. */ - operator uint256() const { return m_wrapped; } + operator const uint256&() const LIFETIMEBOUND { return m_wrapped; } }; /** Txid commits to all transaction fields except the witness. */ diff --git a/src/wallet/test/group_outputs_tests.cpp b/src/wallet/test/group_outputs_tests.cpp index e6b25cc216..b32dc8ed5a 100644 --- a/src/wallet/test/group_outputs_tests.cpp +++ b/src/wallet/test/group_outputs_tests.cpp @@ -40,7 +40,7 @@ static void addCoin(CoinsResult& coins, tx.vout[0].nValue = nValue; tx.vout[0].scriptPubKey = GetScriptForDestination(dest); - const uint256& txid = tx.GetHash(); + const auto txid{tx.GetHash().ToUint256()}; 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); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index dea7be03a6..bcbc31ed3e 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -963,7 +963,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup) mtx.vin.clear(); mtx.vin.emplace_back(tx_id_to_spend, 0); wallet.transactionAddedToMempool(MakeTransactionRef(mtx)); - const uint256& good_tx_id = mtx.GetHash(); + const auto good_tx_id{mtx.GetHash().ToUint256()}; { // Verify balance update for the new tx and the old one diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 2969ba7fdb..db858fa5ba 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -5,19 +5,20 @@ #ifndef BITCOIN_WALLET_TRANSACTION_H #define BITCOIN_WALLET_TRANSACTION_H -#include <bitset> -#include <cstdint> +#include <attributes.h> #include <consensus/amount.h> #include <primitives/transaction.h> -#include <serialize.h> -#include <wallet/types.h> -#include <threadsafety.h> #include <tinyformat.h> +#include <uint256.h> #include <util/overloaded.h> #include <util/strencodings.h> #include <util/string.h> +#include <wallet/types.h> -#include <list> +#include <bitset> +#include <cstdint> +#include <map> +#include <utility> #include <variant> #include <vector> @@ -330,8 +331,8 @@ public: bool isInactive() const { return state<TxStateInactive>(); } bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); } bool isConfirmed() const { return state<TxStateConfirmed>(); } - const Txid& GetHash() const { return tx->GetHash(); } - const Wtxid& GetWitnessHash() const { return tx->GetWitnessHash(); } + const Txid& GetHash() const LIFETIMEBOUND { return tx->GetHash(); } + const Wtxid& GetWitnessHash() const LIFETIMEBOUND { return tx->GetWitnessHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } private: |