Skip to content

Commit

Permalink
Merge bitcoin#28740: refactor: Add LIFETIMEBOUND to all (w)txid getters
Browse files Browse the repository at this point in the history
faec889 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 faec889
  stickies-v:
    ACK faec889
  pablomartin4btc:
    cr ACK faec889

Tree-SHA512: 0c2a151f39d0e007b4d33b0b85ad578cc220f3e9dd94890e812b3181c3901545b039325707731cc39a5e89557f59c1154c6320525f78f5de95f119a514d2d23f
  • Loading branch information
fanquake committed Oct 29, 2023
2 parents e789b30 + faec889 commit 42b0d5f
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 17 deletions.
8 changes: 4 additions & 4 deletions src/primitives/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -335,8 +335,8 @@ class CTransaction
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;
Expand Down Expand Up @@ -433,7 +433,7 @@ class GenTxid
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); }
};
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/tx_pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/util/transaction_identifier.h
Original file line number Diff line number Diff line change
@@ -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>

Expand Down Expand Up @@ -35,7 +36,7 @@ class transaction_identifier
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. */
Expand All @@ -56,7 +57,7 @@ class transaction_identifier
* 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. */
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/test/group_outputs_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/test/wallet_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 9 additions & 8 deletions src/wallet/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -330,8 +331,8 @@ class CWalletTx
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:
Expand Down

0 comments on commit 42b0d5f

Please sign in to comment.