Skip to content

Commit

Permalink
consensus: Store transaction nVersion as uin32_t
Browse files Browse the repository at this point in the history
Given that the use of a transaction's nVersion is always as an unsigned
int, it doesn't make sense to store it as signed and then cast it to
unsigned.
  • Loading branch information
achow101 committed Jan 25, 2024
1 parent 3672099 commit 4bcceb3
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/core_write.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
entry.pushKV("hash", tx.GetWitnessHash().GetHex());
// Transaction version is actually unsigned in consensus checks, just signed in memory,
// so cast to unsigned before giving it to the user.
entry.pushKV("version", static_cast<int64_t>(static_cast<uint32_t>(tx.nVersion)));
entry.pushKV("version", static_cast<int64_t>(tx.nVersion));
entry.pushKV("size", tx.GetTotalSize());
entry.pushKV("vsize", (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR);
entry.pushKV("weight", GetTransactionWeight(tx));
Expand Down
8 changes: 4 additions & 4 deletions src/primitives/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,13 @@ static constexpr TransactionSerParams TX_NO_WITNESS{.allow_witness = false};

/**
* Basic transaction serialization format:
* - int32_t nVersion
* - uint32_t nVersion
* - std::vector<CTxIn> vin
* - std::vector<CTxOut> vout
* - uint32_t nLockTime
*
* Extended transaction serialization format:
* - int32_t nVersion
* - uint32_t nVersion
* - unsigned char dummy = 0x00
* - unsigned char flags (!= 0)
* - std::vector<CTxIn> vin
Expand Down Expand Up @@ -305,7 +305,7 @@ class CTransaction
// structure, including the hash.
const std::vector<CTxIn> vin;
const std::vector<CTxOut> vout;
const int32_t nVersion;
const uint32_t nVersion;
const uint32_t nLockTime;

private:
Expand Down Expand Up @@ -378,7 +378,7 @@ struct CMutableTransaction
{
std::vector<CTxIn> vin;
std::vector<CTxOut> vout;
int32_t nVersion;
uint32_t nVersion;
uint32_t nLockTime;

explicit CMutableTransaction();
Expand Down
2 changes: 1 addition & 1 deletion src/script/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1743,7 +1743,7 @@ bool GenericTransactionSignatureChecker<T>::CheckSequence(const CScriptNum& nSeq

// Fail if the transaction's version number is not set high
// enough to trigger BIP 68 rules.
if (static_cast<uint32_t>(txTo->nVersion) < 2)
if (txTo->nVersion < 2)
return false;

// Sequence numbers with their most significant bit set are not
Expand Down
6 changes: 3 additions & 3 deletions test/functional/test_framework/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ def __init__(self, tx=None):
self.wit = copy.deepcopy(tx.wit)

def deserialize(self, f):
self.nVersion = struct.unpack("<i", f.read(4))[0]
self.nVersion = struct.unpack("<I", f.read(4))[0]
self.vin = deser_vector(f, CTxIn)
flags = 0
if len(self.vin) == 0:
Expand All @@ -598,7 +598,7 @@ def deserialize(self, f):

def serialize_without_witness(self):
r = b""
r += struct.pack("<i", self.nVersion)
r += struct.pack("<I", self.nVersion)
r += ser_vector(self.vin)
r += ser_vector(self.vout)
r += struct.pack("<I", self.nLockTime)
Expand All @@ -610,7 +610,7 @@ def serialize_with_witness(self):
if not self.wit.is_null():
flags |= 1
r = b""
r += struct.pack("<i", self.nVersion)
r += struct.pack("<I", self.nVersion)
if flags:
dummy = []
r += ser_vector(dummy)
Expand Down

0 comments on commit 4bcceb3

Please sign in to comment.