Skip to content

Commit ed70e65

Browse files
dergoeggedergoegge
committed
Introduce types for txids & wtxids
1 parent cdb14d7 commit ed70e65

File tree

9 files changed

+109
-33
lines changed

9 files changed

+109
-33
lines changed

‎src/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ BITCOIN_CORE_H = \
328328
util/time.h \
329329
util/tokenpipe.h \
330330
util/trace.h \
331+
util/transaction_identifier.h \
331332
util/translation.h \
332333
util/types.h \
333334
util/ui_change_type.h \

‎src/net_processing.cpp‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1851,9 +1851,9 @@ void PeerManagerImpl::BlockConnected(const std::shared_ptr<const CBlock>& pblock
18511851
{
18521852
LOCK(m_recent_confirmed_transactions_mutex);
18531853
for (const auto& ptx : pblock->vtx) {
1854-
m_recent_confirmed_transactions.insert(ptx->GetHash());
1854+
m_recent_confirmed_transactions.insert(ptx->GetHash().ToUint256());
18551855
if (ptx->HasWitness()) {
1856-
m_recent_confirmed_transactions.insert(ptx->GetWitnessHash());
1856+
m_recent_confirmed_transactions.insert(ptx->GetWitnessHash().ToUint256());
18571857
}
18581858
}
18591859
}
@@ -2967,7 +2967,7 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer)
29672967
// See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034
29682968
// for concerns around weakening security of unupgraded nodes
29692969
// if we start doing this too early.
2970-
m_recent_rejects.insert(porphanTx->GetWitnessHash());
2970+
m_recent_rejects.insert(porphanTx->GetWitnessHash().ToUint256());
29712971
// If the transaction failed for TX_INPUTS_NOT_STANDARD,
29722972
// then we know that the witness was irrelevant to the policy
29732973
// failure, since this check depends only on the txid
@@ -2979,7 +2979,7 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer)
29792979
if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && porphanTx->HasWitness()) {
29802980
// We only add the txid if it differs from the wtxid, to
29812981
// avoid wasting entries in the rolling bloom filter.
2982-
m_recent_rejects.insert(porphanTx->GetHash());
2982+
m_recent_rejects.insert(porphanTx->GetHash().ToUint256());
29832983
}
29842984
}
29852985
m_orphanage.EraseTx(orphanHash);
@@ -4230,8 +4230,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
42304230
// regardless of what witness is provided, we will not accept
42314231
// this, so we don't need to allow for redownload of this txid
42324232
// from any of our non-wtxidrelay peers.
4233-
m_recent_rejects.insert(tx.GetHash());
4234-
m_recent_rejects.insert(tx.GetWitnessHash());
4233+
m_recent_rejects.insert(tx.GetHash().ToUint256());
4234+
m_recent_rejects.insert(tx.GetWitnessHash().ToUint256());
42354235
m_txrequest.ForgetTxHash(tx.GetHash());
42364236
m_txrequest.ForgetTxHash(tx.GetWitnessHash());
42374237
}
@@ -4250,7 +4250,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
42504250
// See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034
42514251
// for concerns around weakening security of unupgraded nodes
42524252
// if we start doing this too early.
4253-
m_recent_rejects.insert(tx.GetWitnessHash());
4253+
m_recent_rejects.insert(tx.GetWitnessHash().ToUint256());
42544254
m_txrequest.ForgetTxHash(tx.GetWitnessHash());
42554255
// If the transaction failed for TX_INPUTS_NOT_STANDARD,
42564256
// then we know that the witness was irrelevant to the policy
@@ -4261,7 +4261,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
42614261
// transactions are later received (resulting in
42624262
// parent-fetching by txid via the orphan-handling logic).
42634263
if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && tx.HasWitness()) {
4264-
m_recent_rejects.insert(tx.GetHash());
4264+
m_recent_rejects.insert(tx.GetHash().ToUint256());
42654265
m_txrequest.ForgetTxHash(tx.GetHash());
42664266
}
42674267
if (RecursiveDynamicUsage(*ptx) < 100000) {
@@ -5694,17 +5694,22 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
56945694
LOCK(tx_relay->m_bloom_filter_mutex);
56955695

56965696
for (const auto& txinfo : vtxinfo) {
5697-
const uint256& hash = peer->m_wtxid_relay ? txinfo.tx->GetWitnessHash() : txinfo.tx->GetHash();
5698-
CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash);
5699-
tx_relay->m_tx_inventory_to_send.erase(hash);
5697+
CInv inv{
5698+
peer->m_wtxid_relay ? MSG_WTX : MSG_TX,
5699+
peer->m_wtxid_relay ?
5700+
txinfo.tx->GetWitnessHash().ToUint256() :
5701+
txinfo.tx->GetHash().ToUint256(),
5702+
};
5703+
tx_relay->m_tx_inventory_to_send.erase(inv.hash);
5704+
57005705
// Don't send transactions that peers will not put into their mempool
57015706
if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) {
57025707
continue;
57035708
}
57045709
if (tx_relay->m_bloom_filter) {
57055710
if (!tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue;
57065711
}
5707-
tx_relay->m_tx_inventory_known_filter.insert(hash);
5712+
tx_relay->m_tx_inventory_known_filter.insert(inv.hash);
57085713
vInv.push_back(inv);
57095714
if (vInv.size() == MAX_INV_SZ) {
57105715
m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));

‎src/primitives/transaction.cpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <tinyformat.h>
1313
#include <uint256.h>
1414
#include <util/strencodings.h>
15+
#include <util/transaction_identifier.h>
1516
#include <version.h>
1617

1718
#include <cassert>
@@ -65,22 +66,23 @@ std::string CTxOut::ToString() const
6566
CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {}
6667
CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime) {}
6768

68-
uint256 CMutableTransaction::GetHash() const
69+
Txid CMutableTransaction::GetHash() const
6970
{
70-
return (CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash();
71+
return Txid::FromUint256((CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash());
7172
}
7273

73-
uint256 CTransaction::ComputeHash() const
74+
Txid CTransaction::ComputeHash() const
7475
{
75-
return (CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash();
76+
return Txid::FromUint256((CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash());
7677
}
7778

78-
uint256 CTransaction::ComputeWitnessHash() const
79+
Wtxid CTransaction::ComputeWitnessHash() const
7980
{
8081
if (!HasWitness()) {
81-
return hash;
82+
return Wtxid::FromUint256(hash.ToUint256());
8283
}
83-
return (CHashWriter{0} << *this).GetHash();
84+
85+
return Wtxid::FromUint256((CHashWriter{0} << *this).GetHash());
8486
}
8587

8688
CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}

‎src/primitives/transaction.h‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <script/script.h>
1212
#include <serialize.h>
1313
#include <uint256.h>
14+
#include <util/transaction_identifier.h> // IWYU pragma: export
1415

1516
#include <cstddef>
1617
#include <cstdint>
@@ -309,11 +310,11 @@ class CTransaction
309310

310311
private:
311312
/** Memory only. */
312-
const uint256 hash;
313-
const uint256 m_witness_hash;
313+
const Txid hash;
314+
const Wtxid m_witness_hash;
314315

315-
uint256 ComputeHash() const;
316-
uint256 ComputeWitnessHash() const;
316+
Txid ComputeHash() const;
317+
Wtxid ComputeWitnessHash() const;
317318

318319
public:
319320
/** Convert a CMutableTransaction into a CTransaction. */
@@ -334,8 +335,8 @@ class CTransaction
334335
return vin.empty() && vout.empty();
335336
}
336337

337-
const uint256& GetHash() const { return hash; }
338-
const uint256& GetWitnessHash() const { return m_witness_hash; };
338+
const Txid& GetHash() const { return hash; }
339+
const Wtxid& GetWitnessHash() const { return m_witness_hash; };
339340

340341
// Return sum of txouts.
341342
CAmount GetValueOut() const;
@@ -405,7 +406,7 @@ struct CMutableTransaction
405406
/** Compute the hash of this CMutableTransaction. This is computed on the
406407
* fly, as opposed to GetHash() in CTransaction, which uses a cached result.
407408
*/
408-
uint256 GetHash() const;
409+
Txid GetHash() const;
409410

410411
bool HasWitness() const
411412
{

‎src/test/fuzz/package_eval.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
238238
}
239239
if (fuzzed_data_provider.ConsumeBool()) {
240240
const auto& txid = fuzzed_data_provider.ConsumeBool() ?
241-
txs.back()->GetHash() :
241+
txs.back()->GetHash().ToUint256() :
242242
PickValue(fuzzed_data_provider, mempool_outpoints).hash;
243243
const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN);
244244
tx_pool.PrioritiseTransaction(txid, delta);

‎src/test/fuzz/tx_pool.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
227227
}
228228
if (fuzzed_data_provider.ConsumeBool()) {
229229
const auto& txid = fuzzed_data_provider.ConsumeBool() ?
230-
tx->GetHash() :
230+
tx->GetHash().ToUint256() :
231231
PickValue(fuzzed_data_provider, outpoints_rbf).hash;
232232
const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN);
233233
tx_pool.PrioritiseTransaction(txid, delta);
@@ -344,7 +344,7 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
344344
}
345345
if (fuzzed_data_provider.ConsumeBool()) {
346346
const auto& txid = fuzzed_data_provider.ConsumeBool() ?
347-
mut_tx.GetHash() :
347+
mut_tx.GetHash().ToUint256() :
348348
PickValue(fuzzed_data_provider, txids);
349349
const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN);
350350
tx_pool.PrioritiseTransaction(txid, delta);

‎src/util/transaction_identifier.h‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
#ifndef BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H
2+
#define BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H
3+
4+
#include <uint256.h>
5+
#include <util/types.h>
6+
7+
/** transaction_identifier represents the two canonical transaction identifier
8+
* types (txid, wtxid).*/
9+
template <bool has_witness>
10+
class transaction_identifier
11+
{
12+
uint256 m_wrapped;
13+
14+
// Note: Use FromUint256 externally instead.
15+
transaction_identifier(const uint256& wrapped) : m_wrapped{wrapped} {}
16+
17+
// TODO: Comparisons with uint256 should be disallowed once we have
18+
// converted most of the code to using the new txid types.
19+
constexpr int Compare(const uint256& other) const { return m_wrapped.Compare(other); }
20+
constexpr int Compare(const transaction_identifier<has_witness>& other) const { return m_wrapped.Compare(other.m_wrapped); }
21+
template <typename Other>
22+
constexpr int Compare(const Other& other) const
23+
{
24+
static_assert(ALWAYS_FALSE<Other>, "Forbidden comparison type");
25+
return 0;
26+
}
27+
28+
public:
29+
transaction_identifier() : m_wrapped{} {}
30+
31+
template <typename Other>
32+
bool operator==(const Other& other) const { return Compare(other) == 0; }
33+
template <typename Other>
34+
bool operator!=(const Other& other) const { return Compare(other) != 0; }
35+
template <typename Other>
36+
bool operator<(const Other& other) const { return Compare(other) < 0; }
37+
38+
uint256 ToUint256() const { return m_wrapped; }
39+
static transaction_identifier FromUint256(const uint256& id) { return {id}; }
40+
41+
/** Wrapped `uint256` methods. */
42+
constexpr bool IsNull() const { return m_wrapped.IsNull(); }
43+
constexpr void SetNull() { m_wrapped.SetNull(); }
44+
std::string GetHex() const { return m_wrapped.GetHex(); }
45+
std::string ToString() const { return m_wrapped.ToString(); }
46+
constexpr const std::byte* data() const { return reinterpret_cast<const std::byte*>(m_wrapped.data()); }
47+
constexpr const std::byte* begin() const { return reinterpret_cast<const std::byte*>(m_wrapped.begin()); }
48+
constexpr const std::byte* end() const { return reinterpret_cast<const std::byte*>(m_wrapped.end()); }
49+
template <typename Stream> void Serialize(Stream& s) const { m_wrapped.Serialize(s); }
50+
template <typename Stream> void Unserialize(Stream& s) { m_wrapped.Unserialize(s); }
51+
52+
/** Conversion function to `uint256`.
53+
*
54+
* Note: new code should use `ToUint256`.
55+
*
56+
* TODO: This should be removed once the majority of the code has switched
57+
* to using the Txid and Wtxid types. Until then it makes for a smoother
58+
* transition to allow this conversion. */
59+
operator uint256() const { return m_wrapped; }
60+
};
61+
62+
/** Txid commits to all transaction fields except the witness. */
63+
using Txid = transaction_identifier<false>;
64+
/** Wtxid commits to all transaction fields including the witness. */
65+
using Wtxid = transaction_identifier<true>;
66+
67+
#endif // BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H

‎src/validation.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ class MemPoolAccept
616616

617617
const CTransactionRef& m_ptx;
618618
/** Txid. */
619-
const uint256& m_hash;
619+
const Txid& m_hash;
620620
TxValidationState m_state;
621621
/** A temporary cache containing serialized transaction data for signature verification.
622622
* Reused across PolicyScriptChecks and ConsensusScriptChecks. */
@@ -1862,7 +1862,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
18621862
// transaction).
18631863
uint256 hashCacheEntry;
18641864
CSHA256 hasher = g_scriptExecutionCacheHasher;
1865-
hasher.Write(tx.GetWitnessHash().begin(), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin());
1865+
hasher.Write(UCharCast(tx.GetWitnessHash().begin()), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin());
18661866
AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks
18671867
if (g_scriptExecutionCache.contains(hashCacheEntry, !cacheFullScriptStore)) {
18681868
return true;

‎src/wallet/transaction.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,8 @@ class CWalletTx
319319
bool isInactive() const { return state<TxStateInactive>(); }
320320
bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); }
321321
bool isConfirmed() const { return state<TxStateConfirmed>(); }
322-
const uint256& GetHash() const { return tx->GetHash(); }
323-
const uint256& GetWitnessHash() const { return tx->GetWitnessHash(); }
322+
const Txid& GetHash() const { return tx->GetHash(); }
323+
const Wtxid& GetWitnessHash() const { return tx->GetWitnessHash(); }
324324
bool IsCoinBase() const { return tx->IsCoinBase(); }
325325

326326
// Disable copying of CWalletTx objects to prevent bugs where instances get

0 commit comments

Comments
 (0)