Skip to content

Commit

Permalink
refactor: Rename CTransaction::nVersion to version
Browse files Browse the repository at this point in the history
In order to ensure that the change of nVersion to a uint32_t in the
previous commit has no effect, rename nVersion to version in this commit
so that reviewers can easily spot if a spot was missed or if there is a
check somewhere whose semantics have changed.
  • Loading branch information
achow101 committed Jan 26, 2024
1 parent 7dfe6dc commit 3a41d8c
Show file tree
Hide file tree
Showing 34 changed files with 108 additions and 108 deletions.
4 changes: 2 additions & 2 deletions contrib/signet/miner
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ def signet_txs(block, challenge):
sd += struct.pack("<I", block.nTime)

to_spend = CTransaction()
to_spend.nVersion = 0
to_spend.version = 0
to_spend.nLockTime = 0
to_spend.vin = [CTxIn(COutPoint(0, 0xFFFFFFFF), b"\x00" + CScriptOp.encode_op_pushdata(sd), 0)]
to_spend.vout = [CTxOut(0, challenge)]
to_spend.rehash()

spend = CTransaction()
spend.nVersion = 0
spend.version = 0
spend.nLockTime = 0
spend.vin = [CTxIn(COutPoint(to_spend.sha256, 0), b"", 0)]
spend.vout = [CTxOut(0, b"\x6a")]
Expand Down
2 changes: 1 addition & 1 deletion src/bitcoin-tx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ static void MutateTxVersion(CMutableTransaction& tx, const std::string& cmdVal)
throw std::runtime_error("Invalid TX version requested: '" + cmdVal + "'");
}

tx.nVersion = (int) newVersion;
tx.version = (int) newVersion;
}

static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal)
Expand Down
2 changes: 1 addition & 1 deletion src/consensus/tx_verify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags
int nMinHeight = -1;
int64_t nMinTime = -1;

bool fEnforceBIP68 = tx.nVersion >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE;
bool fEnforceBIP68 = tx.version >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE;

// Do not enforce sequence numbers as a relative lock time
// unless we have been instructed to
Expand Down
2 changes: 1 addition & 1 deletion src/core_write.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry

entry.pushKV("txid", tx.GetHash().GetHex());
entry.pushKV("hash", tx.GetWitnessHash().GetHex());
entry.pushKV("version", tx.nVersion);
entry.pushKV("version", tx.version);
entry.pushKV("size", tx.GetTotalSize());
entry.pushKV("vsize", (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR);
entry.pushKV("weight", GetTransactionWeight(tx));
Expand Down
2 changes: 1 addition & 1 deletion src/kernel/chainparams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
static CBlock CreateGenesisBlock(const char* pszTimestamp, const CScript& genesisOutputScript, uint32_t nTime, uint32_t nNonce, uint32_t nBits, int32_t nVersion, const CAmount& genesisReward)
{
CMutableTransaction txNew;
txNew.nVersion = 1;
txNew.version = 1;
txNew.vin.resize(1);
txNew.vout.resize(1);
txNew.vin[0].scriptSig = CScript() << 486604799 << CScriptNum(4) << std::vector<unsigned char>((const unsigned char*)pszTimestamp, (const unsigned char*)pszTimestamp + strlen(pszTimestamp));
Expand Down
2 changes: 1 addition & 1 deletion src/policy/policy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_

bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason)
{
if (tx.nVersion > TX_MAX_STANDARD_VERSION || tx.nVersion < 1) {
if (tx.version > TX_MAX_STANDARD_VERSION || tx.version < 1) {
reason = "version";
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/policy/policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_
// Changing the default transaction version requires a two step process: first
// adapting relay policy by bumping TX_MAX_STANDARD_VERSION, and then later
// allowing the new transaction version in the wallet/RPC.
static constexpr decltype(CTransaction::nVersion) TX_MAX_STANDARD_VERSION{2};
static constexpr decltype(CTransaction::version) TX_MAX_STANDARD_VERSION{2};

/**
* Check for standard transaction types
Expand Down
10 changes: 5 additions & 5 deletions src/primitives/transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ std::string CTxOut::ToString() const
return strprintf("CTxOut(nValue=%d.%08d, scriptPubKey=%s)", nValue / COIN, nValue % COIN, HexStr(scriptPubKey).substr(0, 30));
}

CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {}
CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime) {}
CMutableTransaction::CMutableTransaction() : version(CTransaction::CURRENT_VERSION), nLockTime(0) {}
CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), version(tx.version), nLockTime(tx.nLockTime) {}

Txid CMutableTransaction::GetHash() const
{
Expand Down Expand Up @@ -92,8 +92,8 @@ Wtxid CTransaction::ComputeWitnessHash() const
return Wtxid::FromUint256((HashWriter{} << TX_WITH_WITNESS(*this)).GetHash());
}

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()} {}
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()} {}
CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), version(tx.version), nLockTime(tx.nLockTime), m_has_witness{ComputeHasWitness()}, hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), version(tx.version), nLockTime(tx.nLockTime), m_has_witness{ComputeHasWitness()}, hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}

CAmount CTransaction::GetValueOut() const
{
Expand All @@ -117,7 +117,7 @@ std::string CTransaction::ToString() const
std::string str;
str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%u, vout.size=%u, nLockTime=%u)\n",
GetHash().ToString().substr(0,10),
nVersion,
version,
vin.size(),
vout.size(),
nLockTime);
Expand Down
12 changes: 6 additions & 6 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:
* - uint32_t nVersion
* - uint32_t version
* - std::vector<CTxIn> vin
* - std::vector<CTxOut> vout
* - uint32_t nLockTime
*
* Extended transaction serialization format:
* - uint32_t nVersion
* - uint32_t version
* - unsigned char dummy = 0x00
* - unsigned char flags (!= 0)
* - std::vector<CTxIn> vin
Expand All @@ -217,7 +217,7 @@ void UnserializeTransaction(TxType& tx, Stream& s, const TransactionSerParams& p
{
const bool fAllowWitness = params.allow_witness;

s >> tx.nVersion;
s >> tx.version;
unsigned char flags = 0;
tx.vin.clear();
tx.vout.clear();
Expand Down Expand Up @@ -257,7 +257,7 @@ void SerializeTransaction(const TxType& tx, Stream& s, const TransactionSerParam
{
const bool fAllowWitness = params.allow_witness;

s << tx.nVersion;
s << tx.version;
unsigned char flags = 0;
// Consistency check
if (fAllowWitness) {
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 uint32_t nVersion;
const uint32_t version;
const uint32_t nLockTime;

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

explicit CMutableTransaction();
Expand Down
8 changes: 4 additions & 4 deletions src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1744,8 +1744,8 @@ static RPCHelpMan joinpsbts()
}
psbtxs.push_back(psbtx);
// Choose the highest version number
if (psbtx.tx->nVersion > best_version) {
best_version = psbtx.tx->nVersion;
if (psbtx.tx->version > best_version) {
best_version = psbtx.tx->version;
}
// Choose the lowest lock time
if (psbtx.tx->nLockTime < best_locktime) {
Expand All @@ -1756,7 +1756,7 @@ static RPCHelpMan joinpsbts()
// Create a blank psbt where everything will be added
PartiallySignedTransaction merged_psbt;
merged_psbt.tx = CMutableTransaction();
merged_psbt.tx->nVersion = best_version;
merged_psbt.tx->version = best_version;
merged_psbt.tx->nLockTime = best_locktime;

// Merge
Expand Down Expand Up @@ -1791,7 +1791,7 @@ static RPCHelpMan joinpsbts()

PartiallySignedTransaction shuffled_psbt;
shuffled_psbt.tx = CMutableTransaction();
shuffled_psbt.tx->nVersion = merged_psbt.tx->nVersion;
shuffled_psbt.tx->version = merged_psbt.tx->version;
shuffled_psbt.tx->nLockTime = merged_psbt.tx->nLockTime;
for (int i : input_indices) {
shuffled_psbt.AddInput(merged_psbt.tx->vin[i], merged_psbt.inputs[i]);
Expand Down
10 changes: 5 additions & 5 deletions src/script/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1321,8 +1321,8 @@ class CTransactionSignatureSerializer
/** Serialize txTo */
template<typename S>
void Serialize(S &s) const {
// Serialize nVersion
::Serialize(s, txTo.nVersion);
// Serialize version
::Serialize(s, txTo.version);
// Serialize vin
unsigned int nInputs = fAnyoneCanPay ? 1 : txTo.vin.size();
::WriteCompactSize(s, nInputs);
Expand Down Expand Up @@ -1512,7 +1512,7 @@ bool SignatureHashSchnorr(uint256& hash_out, ScriptExecutionData& execdata, cons
ss << hash_type;

// Transaction level data
ss << tx_to.nVersion;
ss << tx_to.version;
ss << tx_to.nLockTime;
if (input_type != SIGHASH_ANYONECANPAY) {
ss << cache.m_prevouts_single_hash;
Expand Down Expand Up @@ -1594,7 +1594,7 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn

HashWriter ss{};
// Version
ss << txTo.nVersion;
ss << txTo.version;
// Input prevouts/nSequence (none/all, depending on flags)
ss << hashPrevouts;
ss << hashSequence;
Expand Down 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 (txTo->nVersion < 2)
if (txTo->version < 2)
return false;

// Sequence numbers with their most significant bit set are not
Expand Down
4 changes: 2 additions & 2 deletions src/signet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ static uint256 ComputeModifiedMerkleRoot(const CMutableTransaction& cb, const CB
std::optional<SignetTxs> SignetTxs::Create(const CBlock& block, const CScript& challenge)
{
CMutableTransaction tx_to_spend;
tx_to_spend.nVersion = 0;
tx_to_spend.version = 0;
tx_to_spend.nLockTime = 0;
tx_to_spend.vin.emplace_back(COutPoint(), CScript(OP_0), 0);
tx_to_spend.vout.emplace_back(0, challenge);

CMutableTransaction tx_spending;
tx_spending.nVersion = 0;
tx_spending.version = 0;
tx_spending.nLockTime = 0;
tx_spending.vin.emplace_back(COutPoint(), CScript(), 0);
tx_spending.vout.emplace_back(0, CScript(OP_RETURN));
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/package_eval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
// Create transaction to add to the mempool
const CTransactionRef tx = [&] {
CMutableTransaction tx_mut;
tx_mut.nVersion = CTransaction::CURRENT_VERSION;
tx_mut.version = CTransaction::CURRENT_VERSION;
tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>();
// Last tx will sweep all outpoints in package
const auto num_in = last_tx ? package_outpoints.size() : fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size());
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 @@ -227,7 +227,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
// Create transaction to add to the mempool
const CTransactionRef tx = [&] {
CMutableTransaction tx_mut;
tx_mut.nVersion = CTransaction::CURRENT_VERSION;
tx_mut.version = CTransaction::CURRENT_VERSION;
tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>();
const auto num_in = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, outpoints_rbf.size());
const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, outpoints_rbf.size() * 2);
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ CMutableTransaction ConsumeTransaction(FuzzedDataProvider& fuzzed_data_provider,
{
CMutableTransaction tx_mut;
const auto p2wsh_op_true = fuzzed_data_provider.ConsumeBool();
tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ?
tx_mut.version = fuzzed_data_provider.ConsumeBool() ?
CTransaction::CURRENT_VERSION :
fuzzed_data_provider.ConsumeIntegral<int32_t>();
tx_mut.nLockTime = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
Expand Down
4 changes: 2 additions & 2 deletions src/test/hash_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ BOOST_AUTO_TEST_CASE(siphash)

HashWriter ss{};
CMutableTransaction tx;
// Note these tests were originally written with tx.nVersion=1
// Note these tests were originally written with tx.version=1
// and the test would be affected by default tx version bumps if not fixed.
tx.nVersion = 1;
tx.version = 1;
ss << TX_WITH_WITNESS(tx);
BOOST_CHECK_EQUAL(SipHashUint256(1, 2, ss.GetHash()), 0x79751e980c2a0a35ULL);

Expand Down
4 changes: 2 additions & 2 deletions src/test/miner_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
std::vector<int> prevheights;

// relative height locked
tx.nVersion = 2;
tx.version = 2;
tx.vin.resize(1);
prevheights.resize(1);
tx.vin[0].prevout.hash = txFirst[0]->GetHash(); // only 1 transaction
Expand Down Expand Up @@ -623,7 +623,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
pblock->nVersion = VERSIONBITS_TOP_BITS;
pblock->nTime = m_node.chainman->ActiveChain().Tip()->GetMedianTimePast()+1;
CMutableTransaction txCoinbase(*pblock->vtx[0]);
txCoinbase.nVersion = 1;
txCoinbase.version = 1;
txCoinbase.vin[0].scriptSig = CScript{} << (m_node.chainman->ActiveChain().Height() + 1) << bi.extranonce;
txCoinbase.vout.resize(1); // Ignore the (optional) segwit commitment added by CreateNewBlock (as the hardcoded nonces don't account for this)
txCoinbase.vout[0].scriptPubKey = CScript();
Expand Down
2 changes: 1 addition & 1 deletion src/test/sighash_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void static RandomScript(CScript &script) {

void static RandomTransaction(CMutableTransaction& tx, bool fSingle)
{
tx.nVersion = InsecureRand32();
tx.version = InsecureRand32();
tx.vin.clear();
tx.vout.clear();
tx.nLockTime = (InsecureRandBool()) ? InsecureRand32() : 0;
Expand Down
4 changes: 2 additions & 2 deletions src/test/sigopcount_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,15 @@ static ScriptError VerifyWithFlag(const CTransaction& output, const CMutableTran
*/
static void BuildTxs(CMutableTransaction& spendingTx, CCoinsViewCache& coins, CMutableTransaction& creationTx, const CScript& scriptPubKey, const CScript& scriptSig, const CScriptWitness& witness)
{
creationTx.nVersion = 1;
creationTx.version = 1;
creationTx.vin.resize(1);
creationTx.vin[0].prevout.SetNull();
creationTx.vin[0].scriptSig = CScript();
creationTx.vout.resize(1);
creationTx.vout[0].nValue = 1;
creationTx.vout[0].scriptPubKey = scriptPubKey;

spendingTx.nVersion = 1;
spendingTx.version = 1;
spendingTx.vin.resize(1);
spendingTx.vin[0].prevout.hash = creationTx.GetHash();
spendingTx.vin[0].prevout.n = 0;
Expand Down
20 changes: 10 additions & 10 deletions src/test/transaction_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ BOOST_AUTO_TEST_CASE(test_Get)
static void CreateCreditAndSpend(const FillableSigningProvider& keystore, const CScript& outscript, CTransactionRef& output, CMutableTransaction& input, bool success = true)
{
CMutableTransaction outputm;
outputm.nVersion = 1;
outputm.version = 1;
outputm.vin.resize(1);
outputm.vin[0].prevout.SetNull();
outputm.vin[0].scriptSig = CScript();
Expand All @@ -428,7 +428,7 @@ static void CreateCreditAndSpend(const FillableSigningProvider& keystore, const
assert(output->vout[0] == outputm.vout[0]);

CMutableTransaction inputm;
inputm.nVersion = 1;
inputm.version = 1;
inputm.vin.resize(1);
inputm.vin[0].prevout.hash = output->GetHash();
inputm.vin[0].prevout.n = 0;
Expand Down Expand Up @@ -485,7 +485,7 @@ static void ReplaceRedeemScript(CScript& script, const CScript& redeemScript)
BOOST_AUTO_TEST_CASE(test_big_witness_transaction)
{
CMutableTransaction mtx;
mtx.nVersion = 1;
mtx.version = 1;

CKey key = GenerateRandomKey(); // Need to use compressed keys in segwit or the signing will fail
FillableSigningProvider keystore;
Expand Down Expand Up @@ -779,21 +779,21 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
t.vout[0].nValue = nDustThreshold;
CheckIsStandard(t);

// Disallowed nVersion
t.nVersion = static_cast<uint32_t>(-1);
// Disallowed version
t.version = static_cast<uint32_t>(-1);
CheckIsNotStandard(t, "version");

t.nVersion = 0;
t.version = 0;
CheckIsNotStandard(t, "version");

t.nVersion = 3;
t.version = 3;
CheckIsNotStandard(t, "version");

// Allowed nVersion
t.nVersion = 1;
// Allowed version
t.version = 1;
CheckIsStandard(t);

t.nVersion = 2;
t.version = 2;
CheckIsStandard(t);

// Check dust with odd relay fee to verify rounding:
Expand Down
4 changes: 2 additions & 2 deletions src/test/txpackage_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
CKey child_key = GenerateRandomKey();
CScript child_locking_script = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey()));
CMutableTransaction mtx_child1;
mtx_child1.nVersion = 1;
mtx_child1.version = 1;
mtx_child1.vin.resize(1);
mtx_child1.vin[0].prevout.hash = ptx_parent->GetHash();
mtx_child1.vin[0].prevout.n = 0;
Expand Down Expand Up @@ -555,7 +555,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
CTransactionRef ptx_grandparent2 = MakeTransactionRef(mtx_grandparent2);

CMutableTransaction mtx_parent2_v1;
mtx_parent2_v1.nVersion = 1;
mtx_parent2_v1.version = 1;
mtx_parent2_v1.vin.resize(1);
mtx_parent2_v1.vin[0].prevout.hash = ptx_grandparent2->GetHash();
mtx_parent2_v1.vin[0].prevout.n = 0;
Expand Down
Loading

0 comments on commit 3a41d8c

Please sign in to comment.