Skip to content

Commit 27e70f1

Browse files
achow101achow101
committed
consensus: Store transaction nVersion as uint32_t
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.
1 parent 6e4d18f commit 27e70f1

File tree

16 files changed

+47
-39
lines changed

16 files changed

+47
-39
lines changed

‎src/bitcoin-tx.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,12 @@ static CAmount ExtractAndValidateValue(const std::string& strValue)
203203

204204
static void MutateTxVersion(CMutableTransaction& tx, const std::string& cmdVal)
205205
{
206-
int64_t newVersion;
207-
if (!ParseInt64(cmdVal, &newVersion) || newVersion < 1 || newVersion > TX_MAX_STANDARD_VERSION) {
206+
uint32_t newVersion;
207+
if (!ParseUInt32(cmdVal, &newVersion) || newVersion < 1 || newVersion > TX_MAX_STANDARD_VERSION) {
208208
throw std::runtime_error("Invalid TX version requested: '" + cmdVal + "'");
209209
}
210210

211-
tx.nVersion = (int) newVersion;
211+
tx.nVersion = newVersion;
212212
}
213213

214214
static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal)

‎src/consensus/tx_verify.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,7 @@ std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags
4848
int nMinHeight = -1;
4949
int64_t nMinTime = -1;
5050

51-
// tx.nVersion is signed integer so requires cast to unsigned otherwise
52-
// we would be doing a signed comparison and half the range of nVersion
53-
// wouldn't support BIP 68.
54-
bool fEnforceBIP68 = static_cast<uint32_t>(tx.nVersion) >= 2
55-
&& flags & LOCKTIME_VERIFY_SEQUENCE;
51+
bool fEnforceBIP68 = tx.nVersion >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE;
5652

5753
// Do not enforce sequence numbers as a relative lock time
5854
// unless we have been instructed to

‎src/core_write.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,7 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
174174

175175
entry.pushKV("txid", tx.GetHash().GetHex());
176176
entry.pushKV("hash", tx.GetWitnessHash().GetHex());
177-
// Transaction version is actually unsigned in consensus checks, just signed in memory,
178-
// so cast to unsigned before giving it to the user.
179-
entry.pushKV("version", static_cast<int64_t>(static_cast<uint32_t>(tx.nVersion)));
177+
entry.pushKV("version", tx.nVersion);
180178
entry.pushKV("size", tx.GetTotalSize());
181179
entry.pushKV("vsize", (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR);
182180
entry.pushKV("weight", GetTransactionWeight(tx));

‎src/primitives/transaction.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ std::string CTxOut::ToString() const
6363
return strprintf("CTxOut(nValue=%d.%08d, scriptPubKey=%s)", nValue / COIN, nValue % COIN, HexStr(scriptPubKey).substr(0, 30));
6464
}
6565

66-
CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {}
67-
CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime) {}
66+
CMutableTransaction::CMutableTransaction() : nVersion{CTransaction::CURRENT_VERSION}, nLockTime{0} {}
67+
CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion{tx.nVersion}, nLockTime{tx.nLockTime} {}
6868

6969
Txid CMutableTransaction::GetHash() const
7070
{
@@ -92,8 +92,8 @@ Wtxid CTransaction::ComputeWitnessHash() const
9292
return Wtxid::FromUint256((HashWriter{} << TX_WITH_WITNESS(*this)).GetHash());
9393
}
9494

95-
CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), m_has_witness{ComputeHasWitness()}, hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
96-
CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nLockTime(tx.nLockTime), m_has_witness{ComputeHasWitness()}, hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
95+
CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion{tx.nVersion}, nLockTime{tx.nLockTime}, m_has_witness{ComputeHasWitness()}, hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
96+
CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion{tx.nVersion}, nLockTime{tx.nLockTime}, m_has_witness{ComputeHasWitness()}, hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
9797

9898
CAmount CTransaction::GetValueOut() const
9999
{
@@ -115,7 +115,7 @@ unsigned int CTransaction::GetTotalSize() const
115115
std::string CTransaction::ToString() const
116116
{
117117
std::string str;
118-
str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%u, vout.size=%u, nLockTime=%u)\n",
118+
str += strprintf("CTransaction(hash=%s, ver=%u, vin.size=%u, vout.size=%u, nLockTime=%u)\n",
119119
GetHash().ToString().substr(0,10),
120120
nVersion,
121121
vin.size(),

‎src/primitives/transaction.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,13 @@ static constexpr TransactionSerParams TX_NO_WITNESS{.allow_witness = false};
197197

198198
/**
199199
* Basic transaction serialization format:
200-
* - int32_t nVersion
200+
* - uint32_t nVersion
201201
* - std::vector<CTxIn> vin
202202
* - std::vector<CTxOut> vout
203203
* - uint32_t nLockTime
204204
*
205205
* Extended transaction serialization format:
206-
* - int32_t nVersion
206+
* - uint32_t nVersion
207207
* - unsigned char dummy = 0x00
208208
* - unsigned char flags (!= 0)
209209
* - std::vector<CTxIn> vin
@@ -296,7 +296,7 @@ class CTransaction
296296
{
297297
public:
298298
// Default transaction version.
299-
static const int32_t CURRENT_VERSION=2;
299+
static const uint32_t CURRENT_VERSION{2};
300300

301301
// The local variables are made const to prevent unintended modification
302302
// without updating the cached hash value. However, CTransaction is not
@@ -305,7 +305,7 @@ class CTransaction
305305
// structure, including the hash.
306306
const std::vector<CTxIn> vin;
307307
const std::vector<CTxOut> vout;
308-
const int32_t nVersion;
308+
const uint32_t nVersion;
309309
const uint32_t nLockTime;
310310

311311
private:
@@ -378,7 +378,7 @@ struct CMutableTransaction
378378
{
379379
std::vector<CTxIn> vin;
380380
std::vector<CTxOut> vout;
381-
int32_t nVersion;
381+
uint32_t nVersion;
382382
uint32_t nLockTime;
383383

384384
explicit CMutableTransaction();

‎src/rpc/rawtransaction.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,8 +1748,8 @@ static RPCHelpMan joinpsbts()
17481748
}
17491749
psbtxs.push_back(psbtx);
17501750
// Choose the highest version number
1751-
if (static_cast<uint32_t>(psbtx.tx->nVersion) > best_version) {
1752-
best_version = static_cast<uint32_t>(psbtx.tx->nVersion);
1751+
if (psbtx.tx->nVersion > best_version) {
1752+
best_version = psbtx.tx->nVersion;
17531753
}
17541754
// Choose the lowest lock time
17551755
if (psbtx.tx->nLockTime < best_locktime) {
@@ -1760,7 +1760,7 @@ static RPCHelpMan joinpsbts()
17601760
// Create a blank psbt where everything will be added
17611761
PartiallySignedTransaction merged_psbt;
17621762
merged_psbt.tx = CMutableTransaction();
1763-
merged_psbt.tx->nVersion = static_cast<int32_t>(best_version);
1763+
merged_psbt.tx->nVersion = best_version;
17641764
merged_psbt.tx->nLockTime = best_locktime;
17651765

17661766
// Merge

‎src/script/interpreter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1743,7 +1743,7 @@ bool GenericTransactionSignatureChecker<T>::CheckSequence(const CScriptNum& nSeq
17431743

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

17491749
// Sequence numbers with their most significant bit set are not

‎src/test/fuzz/util.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ CMutableTransaction ConsumeTransaction(FuzzedDataProvider& fuzzed_data_provider,
4545
const auto p2wsh_op_true = fuzzed_data_provider.ConsumeBool();
4646
tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ?
4747
CTransaction::CURRENT_VERSION :
48-
fuzzed_data_provider.ConsumeIntegral<int32_t>();
48+
fuzzed_data_provider.ConsumeIntegral<uint32_t>();
4949
tx_mut.nLockTime = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
5050
const auto num_in = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, max_num_in);
5151
const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, max_num_out);

‎src/test/sighash_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ void static RandomScript(CScript &script) {
9292

9393
void static RandomTransaction(CMutableTransaction& tx, bool fSingle)
9494
{
95-
tx.nVersion = int(InsecureRand32());
95+
tx.nVersion = InsecureRand32();
9696
tx.vin.clear();
9797
tx.vout.clear();
9898
tx.nLockTime = (InsecureRandBool()) ? InsecureRand32() : 0;

‎src/test/transaction_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -780,7 +780,7 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
780780
CheckIsStandard(t);
781781

782782
// Disallowed nVersion
783-
t.nVersion = -1;
783+
t.nVersion = std::numeric_limits<uint32_t>::max();
784784
CheckIsNotStandard(t, "version");
785785

786786
t.nVersion = 0;

0 commit comments

Comments
 (0)