kernel: expose transaction version #36283

nervana21 commented at 1:54 AM on September 17, 2026: contributor

This PR is a follow-up to #36194 and #36207. It completes the existing set of transaction accessors allowing clients to read the transaction version without slicing serialized bytes.

Tests cover a range of versions.

DrahtBot added the label Validation on Sep 17, 2026

DrahtBot commented at 1:55 AM on September 17, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36283.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	yuvicc

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

nervana21 commented at 1:55 AM on September 17, 2026: contributor

cc: @lucasdbr05 @KY-U

in src/test/kernel/test_kernel.cpp:483 in 4397e8647a

 475 | @@ -475,6 +476,15 @@ BOOST_AUTO_TEST_CASE(btck_transaction_tests)
 476 |      check_equal(script_pubkey_roundtrip.ToBytes(), script_pubkey.ToBytes());
 477 |  }
 478 |  
 479 | +BOOST_AUTO_TEST_CASE(btck_transaction_version_tests)
 480 | +{
 481 | +    auto tx_data{hex_string_to_byte_vec("02000000013f7cebd65c27431a90bba7f796914fe8cc2ddfc3f2cbd6f7e5f2fc854534da95000000006b483045022100de1ac3bcdfb0332207c4a91f3832bd2c2915840165f876ab47c5f8996b971c3602201c6c053d750fadde599e6f5c4e1963df0f01fc0d97815e8157e3d59fe09ca30d012103699b464d1d8bc9e47d4fb1cdaa89a1c5783d68363c4dbc4b524ed3d857148617feffffff02836d3c01000000001976a914fc25d6d5c94003bf5b0c7b640a248e2c637fcfb088ac7ada8202000000001976a914fbed3d9b11183209a57999d54d59f67c019e756c88ac6acb0700")};
 482 | +    for (const uint32_t version : {0u, 1u, 2u, 3u, 0xffffffffu}) {
 483 | +        WriteLE32(tx_data.data(), version);

stickies-v commented at 9:40 AM on September 17, 2026:

We try to keep dependencies for test_kernel on internal headers minimal, so in that light including crypto/common.h just to write a byte seems excessive. Perhaps concatenating strings or something like tx_data[i] = std::byte(version >> (8 * i)) could be an alternative?

Also, transaction version is a scalar member like locktime etc. I think this can just be folded into btck_transaction_tests.

stickies-v commented at 9:40 AM on September 17, 2026: contributor

Code LGTM 4397e8647a27d40fbc26d29b17bbedded5692009 but test should be improved a bit

yuvicc commented at 12:02 PM on September 17, 2026: contributor

Concept ACK

KY-U commented at 3:01 PM on September 17, 2026: contributor

GetVersion() is provided by TransactionApi to both Transaction and TransactionView, but the current test only exercises the owning type. Would it be usefull to also check it on the existing transaction view, so both wrapper instantiations are covered?

<details> <summary>diff testing both wrappers, moving the test to btck_transaction_tests and without crypto/common.h as suggested by [@stickies-v](/bitcoin-bitcoin/contributor/stickies-v/)</summary>

diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
index 08c7c1e5f3..1df9b9fbd1 100644
--- a/src/test/kernel/test_kernel.cpp
+++ b/src/test/kernel/test_kernel.cpp
@@ -2,7 +2,6 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <crypto/common.h>
 #include <kernel/bitcoinkernel.h>
 #include <kernel/bitcoinkernel_wrapper.h>
 #include <util/byte_units.h>
@@ -410,6 +409,19 @@ BOOST_AUTO_TEST_CASE(btck_transaction_tests)
     BOOST_CHECK_EQUAL(tx.CountOutputs(), 2);
     BOOST_CHECK_EQUAL(tx.CountInputs(), 1);
     BOOST_CHECK_EQUAL(tx.GetLocktime(), 510826);
+
+    auto versioned_tx_data{tx_data};
+    for (const uint32_t version : {0u, 1u, 2u, 3u, 0xffffffffu}) {
+        for (size_t i = 0; i < 4; ++i) {
+            versioned_tx_data[i] = std::byte((version >> (8 * i)) & 0xff);
+        }
+
+        Transaction versioned_tx{versioned_tx_data};
+        TransactionView versioned_tx_view{versioned_tx.get()};
+
+        BOOST_CHECK_EQUAL(versioned_tx.GetVersion(), version);
+        BOOST_CHECK_EQUAL(versioned_tx_view.GetVersion(), version);
+    }
     auto broken_tx_data{std::span<std::byte>{tx_data.begin(), tx_data.begin() + 10}};
     BOOST_CHECK_THROW(Transaction{broken_tx_data}, std::runtime_error);
     auto input{tx.GetInput(0)};
@@ -476,14 +488,6 @@ BOOST_AUTO_TEST_CASE(btck_transaction_tests)
     check_equal(script_pubkey_roundtrip.ToBytes(), script_pubkey.ToBytes());
 }
 
-BOOST_AUTO_TEST_CASE(btck_transaction_version_tests)
-{
-    auto tx_data{hex_string_to_byte_vec("02000000013f7cebd65c27431a90bba7f796914fe8cc2ddfc3f2cbd6f7e5f2fc854534da95000000006b483045022100de1ac3bcdfb0332207c4a91f3832bd2c2915840165f876ab47c5f8996b971c3602201c6c053d750fadde599e6f5c4e1963df0f01fc0d97815e8157e3d59fe09ca30d012103699b464d1d8bc9e47d4fb1cdaa89a1c5783d68363c4dbc4b524ed3d857148617feffffff02836d3c01000000001976a914fc25d6d5c94003bf5b0c7b640a248e2c637fcfb088ac7ada8202000000001976a914fbed3d9b11183209a57999d54d59f67c019e756c88ac6acb0700")};
-    for (const uint32_t version : {0u, 1u, 2u, 3u, 0xffffffffu}) {
-        WriteLE32(tx_data.data(), version);
-        BOOST_CHECK_EQUAL(Transaction{tx_data}.GetVersion(), version);
-    }
-}
 
 BOOST_AUTO_TEST_CASE(btck_transaction_id_tests)
 {

</details>

I've added an extra & 0xff on byte conversion to make it explicit that only the least significant byte of the shifted value is used, though this is purely stylistic.

kernel: expose transaction version

Expose CTransaction::version, allowing clients to read the transaction
version without slicing serialized bytes.

Test the accessor against a range of transaction versions.

a40947d1c5

nervana21 force-pushed on Sep 17, 2026

nervana21 commented at 4:28 PM on September 17, 2026: contributor

Thanks for the reviews. I've updated the code to address feedback on the testing logic.