diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 154146f08d3083..84ab6c9a6a7215 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -51,8 +51,7 @@ std::pair CalculateSequenceLocks(const CTransaction &tx, int flags // tx.nVersion is signed integer so requires cast to unsigned otherwise // we would be doing a signed comparison and half the range of nVersion // wouldn't support BIP 68. - bool fEnforceBIP68 = static_cast(tx.nVersion) >= 2 - && flags & LOCKTIME_VERIFY_SEQUENCE; + bool fEnforceBIP68 = tx.nVersion >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE; // Do not enforce sequence numbers as a relative lock time // unless we have been instructed to diff --git a/src/core_write.cpp b/src/core_write.cpp index 63f2c36b5af238..412869a7027739 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -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(static_cast(tx.nVersion))); + entry.pushKV("version", static_cast(tx.nVersion)); entry.pushKV("size", tx.GetTotalSize()); entry.pushKV("vsize", (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR); entry.pushKV("weight", GetTransactionWeight(tx)); diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index ccbeb3ec49bb6c..36f8306439a2f6 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -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 vin * - std::vector vout * - uint32_t nLockTime * * Extended transaction serialization format: - * - int32_t nVersion + * - uint32_t nVersion * - unsigned char dummy = 0x00 * - unsigned char flags (!= 0) * - std::vector vin @@ -305,7 +305,7 @@ class CTransaction // structure, including the hash. const std::vector vin; const std::vector vout; - const int32_t nVersion; + const uint32_t nVersion; const uint32_t nLockTime; private: @@ -378,7 +378,7 @@ struct CMutableTransaction { std::vector vin; std::vector vout; - int32_t nVersion; + uint32_t nVersion; uint32_t nLockTime; explicit CMutableTransaction(); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 634be2f7fb9d6b..255b9a2441afb5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1744,8 +1744,8 @@ static RPCHelpMan joinpsbts() } psbtxs.push_back(psbtx); // Choose the highest version number - if (static_cast(psbtx.tx->nVersion) > best_version) { - best_version = static_cast(psbtx.tx->nVersion); + if (psbtx.tx->nVersion > best_version) { + best_version = psbtx.tx->nVersion; } // Choose the lowest lock time if (psbtx.tx->nLockTime < best_locktime) { @@ -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 = static_cast(best_version); + merged_psbt.tx->nVersion = best_version; merged_psbt.tx->nLockTime = best_locktime; // Merge diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index c969ce45f12496..1ebf5425a5c611 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1743,7 +1743,7 @@ bool GenericTransactionSignatureChecker::CheckSequence(const CScriptNum& nSeq // Fail if the transaction's version number is not set high // enough to trigger BIP 68 rules. - if (static_cast(txTo->nVersion) < 2) + if (txTo->nVersion < 2) return false; // Sequence numbers with their most significant bit set are not diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py index e85541d0ec2580..bff886308f90e1 100755 --- a/test/functional/feature_taproot.py +++ b/test/functional/feature_taproot.py @@ -1410,7 +1410,7 @@ def test_spenders(self, node, spenders, input_counts): while left: # Construct CTransaction with random nVersion, nLocktime tx = CTransaction() - tx.nVersion = random.choice([1, 2, random.randint(-0x80000000, 0x7fffffff)]) + tx.nVersion = random.choice([1, 2, random.randrange(0xffffffff)]) min_sequence = (tx.nVersion != 1 and tx.nVersion != 0) * 0x80000000 # The minimum sequence number to disable relative locktime if random.choice([True, False]): tx.nLockTime = random.randrange(LOCKTIME_THRESHOLD, self.lastblocktime - 7200) # all absolute locktimes in the past diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 664f2df3f19a0d..638514aa1ba04e 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -346,7 +346,7 @@ def test_submitpackage(self): peer = node.add_p2p_connection(P2PTxInvStore()) txs = self.wallet.create_self_transfer_chain(chain_length=2) bad_child = tx_from_hex(txs[1]["hex"]) - bad_child.nVersion = -1 + bad_child.nVersion = 0xffffffff hex_partial_acceptance = [txs[0]["hex"], bad_child.serialize().hex()] res = node.submitpackage(hex_partial_acceptance) assert_equal(res["package_msg"], "transaction failed") diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index c12865b5e39b6f..8ac6c232fb5627 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -464,9 +464,9 @@ def transaction_version_number_tests(self): self.log.info("Test transaction version numbers") # Test the minimum transaction version number that fits in a signed 32-bit integer. - # As transaction version is unsigned, this should convert to its unsigned equivalent. + # As transaction version is serialized unsigned, this should convert to its unsigned equivalent. tx = CTransaction() - tx.nVersion = -0x80000000 + tx.nVersion = 0x80000000 rawtx = tx.serialize().hex() decrawtx = self.nodes[0].decoderawtransaction(rawtx) assert_equal(decrawtx['version'], 0x80000000) @@ -478,6 +478,20 @@ def transaction_version_number_tests(self): decrawtx = self.nodes[0].decoderawtransaction(rawtx) assert_equal(decrawtx['version'], 0x7fffffff) + # Test the minimum transaction version number that fits in an unsigned 32-bit integer. + tx = CTransaction() + tx.nVersion = 0 + rawtx = tx.serialize().hex() + decrawtx = self.nodes[0].decoderawtransaction(rawtx) + assert_equal(decrawtx['version'], 0) + + # Test the maximum transaction version number that fits in an unsigned 32-bit integer. + tx = CTransaction() + tx.nVersion = 0xffffffff + rawtx = tx.serialize().hex() + decrawtx = self.nodes[0].decoderawtransaction(rawtx) + assert_equal(decrawtx['version'], 0xffffffff) + def raw_multisig_transaction_legacy_tests(self): self.log.info("Test raw multisig transactions (legacy)") # The traditional multisig workflow does not work with descriptor wallets so these are legacy only. diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index d008cb39aa22ec..0150e6156a760f 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -575,7 +575,7 @@ def __init__(self, tx=None): self.wit = copy.deepcopy(tx.wit) def deserialize(self, f): - self.nVersion = struct.unpack("