Repository: bitcoin/bitcoin (language: cpp)

Issue:
Update nChainTx to 64bit type
https://github.com/bitcoin/bitcoin/blob/8106b268cde8e97a7c330afdda39b6bb55e5574a/src/chain.h#L178-L186

Hey, it's 2024! `nChainTx` seems to currently be 953368191 (height 826100), which is hex `38d3 3e7f`, so we're only 22% of the way to an overflow, and would still need another 200k blocks (~ four years) full of 60 byte transactions to get there.


Discussion / hints:
Background: https://github.com/bitcoin/bitcoin/pull/13875 and https://github.com/bitcoin/bitcoin/pull/1677
Would this be a hard fork because it would be loosening the conditions for validation?
> Would this be a hard fork because it would be loosening the conditions for validation?

I don't think so, since I don't think the value of `nChainTx` is used in validation (or do you see a spot where it is?). It's used for logging (progress estimation), and whether it is 0 is used in `HaveNumChainTxs`. The latter could become problematic in case of an overflow, since if the `unsigned int` would overflow exactly to zero (which is a bit unlikely given that there are a few thousand txns in each block but ceratainly possible) the node would crash in https://github.com/bitcoin/bitcoin/blob/478ac185be49feffd1231366c07e06068683f941/src/validation.cpp#L3017.
^^^ The above was the line that I was referencing. Thank you. 
There was also the idea of dropping this number entirely: https://github.com/bitcoin/bitcoin/pull/13875#issuecomment-410472259

Though see also https://github.com/bitcoin/bitcoin/pull/13875#issuecomment-750715082
> Though see also https://github.com/bitcoin/bitcoin/pull/13875#issuecomment-750715082

This is `nTx`, not `nChainTx`.
For reference, I removed `nChainTx` from consensus in https://github.com/bitcoin/bitcoin/pull/29349. The next steps would be to add a member to `CChain` to account for the sum of `nTx`, and update it when `SetTip` is called. Then, update the RPC, wallet and estimate functions to derive the `nChainTx` they need from the `nChainTx` at the tip, walking the relevant portion of the chain.

This is possible, but I am not sure if it is worth it, because:

* It is more code overall.
* The RPC, wallet, and logging code is slower when it has to walk the chain.

The benefit mainly seems to be to reduce the memory footprint? On 64-bit platforms, `sizeof(CBlockIndex)` is 144. Adding 4 bytes or removing 4 bytes is less than 10% of cost or savings. Ref: https://godbolt.org/z/rKEPYTh4j

Removing `nTx` is even harder, so less worth it. Especially, given that it could be reduced to 16 bits in the memory layout, if needed.

This leads to an alternative solution to not increase the memory footprint: Move the 32+32 bits used for nTx and nChainTx into a single 64-bit blob, where the 16 bit are for nTx and the remaining for nChainTx. But, again, the overall heap memory usage shouldn't change much, either way.
> This leads to an alternative solution to not increase the memory footprint: Move the 32+32 bits used for nTx and nChainTx into a single 64-bit blob, where the 16 bit are for nTx and the remaining for nChainTx. But, again, the overall heap memory usage shouldn't change much, either way.

This does seem like an nice and easy solution. Would be good to incorporate in #29331

As a tangent, I was thinking along the same lines last week before implementing #29370 and came up with worse solution, taking advantage of the fact that when `nChainTx` is set, you can derive `nTx` by subtracting `nChainTx - pprev->nChainTx`. In case it's interesting:

<details><summary>nChainTx hack</summary>
<p>

```c++
class CBlockIndex
{
public:
    CBlockIndex* pprev{nullptr};

    // Indicate whether num_tx_value contains total number of transactions in
    // this block, or total number of transactions in the chain up and including
    // this block.
    enum NumTxType { UNSET = 0, BLOCK_TX = 1, CHAIN_TX = 2 };
    NumTxType num_tx_type : 4 {UNSET};
    uint64_t num_tx_value : 60;

    uint64_t NumBlockTx()
    {
        if (num_tx_type == BLOCK_TX) return num_tx_value;
        if (num_tx_type == CHAIN_TX) {
            assert(pprev && pprev->num_tx_type == CHAIN_TX);
            return num_tx_value - pprev->num_tx_value;
        }
        return 0;
    }

    uint64_t NumChainTx()
    {
        return num_tx_type == CHAIN_TX ? num_tx_value : 0;
    }
};
```


> This does seem like an nice and easy solution. Would be good to incorporate in #29331

Not sure if it so easy. On 32-bit platforms, `nTx` currently starts at byte 60, so making it 16-bit will still eat 32-bit with padding. So forcing a 64-bit blob of both values is needed, but that requires changing all code that uses either field. Not sure if worth it for a +-5% memory difference?
> Not sure if it so easy. On 32-bit platforms, `nTx` currently starts at byte 60, so making it 16-bit will still eat 32-bit with padding.

I didn't test on a 32-bit platform, but aren't bitfields a good solution to this? The following seems to work for me, increasing `nChainTx` range without increasing `CBlockIndex` size on x86_64 (144 bytes):


<details><summary>diff</summary>
<p>

```diff
--- a/src/chain.h
+++ b/src/chain.h
@@ -178,7 +178,7 @@ public:
     //! Note: this value is faked during UTXO snapshot load to ensure that
     //! LoadBlockIndex() will load index entries for blocks that we lack data for.
     //! @sa ActivateSnapshot
-    unsigned int nTx{0};
+    uint16_t nTx : 16 {0};
 
     //! (memory only) Number of transactions in the chain up to and including this block.
     //! This value will be non-zero only if and only if transactions for this block and all its parents are available.
@@ -188,7 +188,7 @@ public:
     //! have the underlying block data available during snapshot load.
     //! @sa AssumeutxoData
     //! @sa ActivateSnapshot
-    unsigned int nChainTx{0};
+    uint64_t nChainTx : 48 {0};
 
     //! Verification status of this block. See enum BlockStatus
     //!
@@ -412,7 +412,12 @@ public:
 
         READWRITE(VARINT_MODE(obj.nHeight, VarIntMode::NONNEGATIVE_SIGNED));
         READWRITE(VARINT(obj.nStatus));
-        READWRITE(VARINT(obj.nTx));
+
+        unsigned int num_tx;
+        SER_WRITE(obj, num_tx = obj.nTx);
+        READWRITE(VARINT(num_tx));
+        SER_READ(obj, obj.nTx = num_tx);
+
         if (obj.nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO)) READWRITE(VARINT_MODE(obj.nFile, VarIntMode::NONNEGATIVE_SIGNED));
         if (obj.nStatus & BLOCK_HAVE_DATA) READWRITE(VARINT(obj.nDataPos));
         if (obj.nStatus & BLOCK_HAVE_UNDO) READWRITE(VARINT(obj.nUndoPos));
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5119,7 +5119,7 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin
 
     double fTxTotal;
 
-    if (pindex->nChainTx <= data.nTxCount) {
+    if (static_cast<int64_t>(pindex->nChainTx) <= data.nTxCount) {
         fTxTotal = data.nTxCount + (nNow - data.nTime) * data.dTxRate;
     } else {
         fTxTotal = pindex->nChainTx + (nNow - pindex->GetBlockTime()) * data.dTxRate;
```


> bitfields

Jup, that didn't work for me. Or maybe I did something wrong. See https://godbolt.org/z/z6EoWKbjY

```
   60:0-15 |   int16_t nTx
   64:0-47 |   int64_t nChainTx
```

(nTx eats byte 60-64, even though it is only a two byte bitfield)
> Jup, that didn't work for me. Or maybe I did something wrong. See https://godbolt.org/z/z6EoWKbjY

It's not that surprising that the compiler adds padding so the struct member with uint64_t type is inside a field aligned to 8 bytes. But if you move the two bitfield members to begin on an 8 byte boundary (like between the nDataPos and nUndoPos fields, or to the beginning of the struct, it does get packed without extra padding). So I think the diff above is an easy way to avoid consuming extra memory on 64 bit platforms, even though we could go further with more changes to get the same optimization on 32 bit.
> > bitfields
> 
> Jup, that didn't work for me. Or maybe I did something wrong. See https://godbolt.org/z/z6EoWKbjY
> 
> ```
>    60:0-15 |   int16_t nTx
>    64:0-47 |   int64_t nChainTx
> ```
> 
> (nTx eats byte 60-64, even though it is only a two byte bitfield)

I think the problem here is you have an odd number of 32-bit pointers prior to `nTx`, so it's misaligned to be combined with `nChainTx`. So you end up with `nTx [4B-6B padding] nChainTx nStatus [4B padding]`. Moving `nStatus` prior to `nTx` gives an even number of 32-bit objects prior to `nTx`, so drops the size back down.
When updating this should also update the type of the `AssumeutxoData::nChainTx` field:

https://github.com/bitcoin/bitcoin/blob/1cd2e29870e4ad3b7c57b1d567df0e6df56572b0/src/kernel/chainparams.h#L57

Fix patch:
diff --git a/src/chain.h b/src/chain.h
index e8a8c066c8e1b..c46392c5355c5 100644
--- a/src/chain.h
+++ b/src/chain.h
@@ -102,7 +102,7 @@ enum BlockStatus : uint32_t {
      *
      * If a block's validity is at least VALID_TRANSACTIONS, CBlockIndex::nTx will be set. If a block and all previous
      * blocks back to the genesis block or an assumeutxo snapshot block are at least VALID_TRANSACTIONS,
-     * CBlockIndex::nChainTx will be set.
+     * CBlockIndex::m_chain_tx_count will be set.
      */
     BLOCK_VALID_TRANSACTIONS =    3,
 
@@ -173,8 +173,7 @@ class CBlockIndex
     //! This value will be non-zero if this block and all previous blocks back
     //! to the genesis block or an assumeutxo snapshot block have reached the
     //! VALID_TRANSACTIONS level.
-    //! Change to 64-bit type before 2024 (assuming worst case of 60 byte transactions).
-    unsigned int nChainTx{0};
+    uint64_t m_chain_tx_count{0};
 
     //! Verification status of this block. See enum BlockStatus
     //!
@@ -254,10 +253,10 @@ class CBlockIndex
      * Does not imply the transactions are consensus-valid (ConnectTip might fail)
      * Does not imply the transactions are still stored on disk. (IsBlockPruned might return true)
      *
-     * Note that this will be true for the snapshot base block, if one is loaded, since its nChainTx value will have
+     * Note that this will be true for the snapshot base block, if one is loaded, since its m_chain_tx_count value will have
      * been set manually based on the related AssumeutxoData entry.
      */
-    bool HaveNumChainTxs() const { return nChainTx != 0; }
+    bool HaveNumChainTxs() const { return m_chain_tx_count != 0; }
 
     NodeSeconds Time() const
     {
diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp
index 2b729e3b7a9b4..aa6d80556a073 100644
--- a/src/kernel/chainparams.cpp
+++ b/src/kernel/chainparams.cpp
@@ -180,7 +180,7 @@ class CMainParams : public CChainParams {
         chainTxData = ChainTxData{
             // Data from RPC: getchaintxstats 4096 000000000000000000026811d149d4d261995ec5b3f64f439a0a10e1a464af9a
             .nTime    = 1704194835,
-            .nTxCount = 946728933,
+            .tx_count = 946728933,
             .dTxRate  = 6.569290261471664,
         };
     }
@@ -272,7 +272,7 @@ class CTestNetParams : public CChainParams {
             {
                 .height = 2'500'000,
                 .hash_serialized = AssumeutxoHash{uint256S("0xf841584909f68e47897952345234e37fcd9128cd818f41ee6c3ca68db8071be7")},
-                .nChainTx = 66484552,
+                .m_chain_tx_count = 66484552,
                 .blockhash = uint256S("0x0000000000000093bcb68c03a9a168ae252572d348a2eaeba2cdf9231d73206f")
             }
         };
@@ -280,7 +280,7 @@ class CTestNetParams : public CChainParams {
         chainTxData = ChainTxData{
             // Data from RPC: getchaintxstats 4096 000000000001323071f38f21ea5aae529ece491eadaccce506a59bcc2d968917
             .nTime    = 1703579240,
-            .nTxCount = 67845391,
+            .tx_count = 67845391,
             .dTxRate  = 1.464436832560951,
         };
     }
@@ -312,7 +312,7 @@ class SigNetParams : public CChainParams {
             chainTxData = ChainTxData{
                 // Data from RPC: getchaintxstats 4096 0000000870f15246ba23c16e370a7ffb1fc8a3dcf8cb4492882ed4b0e3d4cd26
                 .nTime    = 1706331472,
-                .nTxCount = 2425380,
+                .tx_count = 2425380,
                 .dTxRate  = 0.008277759863833788,
             };
         } else {
@@ -382,7 +382,7 @@ class SigNetParams : public CChainParams {
             {
                 .height = 160'000,
                 .hash_serialized = AssumeutxoHash{uint256S("0xfe0a44309b74d6b5883d246cb419c6221bcccf0b308c9b59b7d70783dbdf928a")},
-                .nChainTx = 2289496,
+                .m_chain_tx_count = 2289496,
                 .blockhash = uint256S("0x0000003ca3c99aff040f2563c2ad8f8ec88bd0fd6b8f0895cfaf1ef90353a62c")
             }
         };
@@ -498,21 +498,21 @@ class CRegTestParams : public CChainParams
             {   // For use by unit tests
                 .height = 110,
                 .hash_serialized = AssumeutxoHash{uint256S("0x6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1")},
-                .nChainTx = 111,
+                .m_chain_tx_count = 111,
                 .blockhash = uint256S("0x696e92821f65549c7ee134edceeeeaaa4105647a3c4fd9f298c0aec0ab50425c")
             },
             {
                 // For use by fuzz target src/test/fuzz/utxo_snapshot.cpp
                 .height = 200,
                 .hash_serialized = AssumeutxoHash{uint256S("0x4f34d431c3e482f6b0d67b64609ece3964dc8d7976d02ac68dd7c9c1421738f2")},
-                .nChainTx = 201,
+                .m_chain_tx_count = 201,
                 .blockhash = uint256S("0x5e93653318f294fb5aa339d00bbf8cf1c3515488ad99412c37608b139ea63b27"),
             },
             {
                 // For use by test/functional/feature_assumeutxo.py
                 .height = 299,
                 .hash_serialized = AssumeutxoHash{uint256S("0xa4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27")},
-                .nChainTx = 334,
+                .m_chain_tx_count = 334,
                 .blockhash = uint256S("0x3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0")
             },
         };
diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h
index 5d45a1fa9c07f..e367de2f93982 100644
--- a/src/kernel/chainparams.h
+++ b/src/kernel/chainparams.h
@@ -50,11 +50,11 @@ struct AssumeutxoData {
     //! The expected hash of the deserialized UTXO set.
     AssumeutxoHash hash_serialized;
 
-    //! Used to populate the nChainTx value, which is used during BlockManager::LoadBlockIndex().
+    //! Used to populate the m_chain_tx_count value, which is used during BlockManager::LoadBlockIndex().
     //!
     //! We need to hardcode the value here because this is computed cumulatively using block data,
     //! which we do not necessarily have at the time of snapshot load.
-    unsigned int nChainTx;
+    uint64_t m_chain_tx_count;
 
     //! The hash of the base block for this snapshot. Used to refer to assumeutxo data
     //! prior to having a loaded blockindex.
@@ -69,7 +69,7 @@ struct AssumeutxoData {
  */
 struct ChainTxData {
     int64_t nTime;    //!< UNIX timestamp of last known number of transactions
-    int64_t nTxCount; //!< total number of transactions between genesis and that timestamp
+    uint64_t tx_count; //!< total number of transactions between genesis and that timestamp
     double dTxRate;   //!< estimated number of transactions per second after that timestamp
 };
 
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 29b624cb12f57..bfb11e9fcd4f4 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -410,11 +410,11 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
         m_snapshot_height = au_data.height;
         CBlockIndex* base{LookupBlockIndex(*snapshot_blockhash)};
 
-        // Since nChainTx (responsible for estimated progress) isn't persisted
+        // Since m_chain_tx_count (responsible for estimated progress) isn't persisted
         // to disk, we must bootstrap the value for assumedvalid chainstates
         // from the hardcoded assumeutxo chainparams.
-        base->nChainTx = au_data.nChainTx;
-        LogPrintf("[snapshot] set nChainTx=%d for %s\n", au_data.nChainTx, snapshot_blockhash->ToString());
+        base->m_chain_tx_count = au_data.m_chain_tx_count;
+        LogPrintf("[snapshot] set m_chain_tx_count=%d for %s\n", au_data.m_chain_tx_count, snapshot_blockhash->ToString());
     } else {
         // If this isn't called with a snapshot blockhash, make sure the cached snapshot height
         // is null. This is relevant during snapshot completion, when the blockman may be loaded
@@ -449,15 +449,15 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
                 if (m_snapshot_height && pindex->nHeight == *m_snapshot_height &&
                         pindex->GetBlockHash() == *snapshot_blockhash) {
                     // Should have been set above; don't disturb it with code below.
-                    Assert(pindex->nChainTx > 0);
-                } else if (pindex->pprev->nChainTx > 0) {
-                    pindex->nChainTx = pindex->pprev->nChainTx + pindex->nTx;
+                    Assert(pindex->m_chain_tx_count > 0);
+                } else if (pindex->pprev->m_chain_tx_count > 0) {
+                    pindex->m_chain_tx_count = pindex->pprev->m_chain_tx_count + pindex->nTx;
                 } else {
-                    pindex->nChainTx = 0;
+                    pindex->m_chain_tx_count = 0;
                     m_blocks_unlinked.insert(std::make_pair(pindex->pprev, pindex));
                 }
             } else {
-                pindex->nChainTx = pindex->nTx;
+                pindex->m_chain_tx_count = pindex->nTx;
             }
         }
         if (!(pindex->nStatus & BLOCK_FAILED_MASK) && pindex->pprev && (pindex->pprev->nStatus & BLOCK_FAILED_MASK)) {
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 9772b26aea585..02f9ecef341f7 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1725,16 +1725,16 @@ static RPCHelpMan getchaintxstats()
 
     UniValue ret(UniValue::VOBJ);
     ret.pushKV("time", (int64_t)pindex->nTime);
-    if (pindex->nChainTx) {
-        ret.pushKV("txcount", pindex->nChainTx);
+    if (pindex->m_chain_tx_count) {
+        ret.pushKV("txcount", pindex->m_chain_tx_count);
     }
     ret.pushKV("window_final_block_hash", pindex->GetBlockHash().GetHex());
     ret.pushKV("window_final_block_height", pindex->nHeight);
     ret.pushKV("window_block_count", blockcount);
     if (blockcount > 0) {
         ret.pushKV("window_interval", nTimeDiff);
-        if (pindex->nChainTx != 0 && past_block.nChainTx != 0) {
-            const auto window_tx_count = pindex->nChainTx - past_block.nChainTx;
+        if (pindex->m_chain_tx_count != 0 && past_block.m_chain_tx_count != 0) {
+            const auto window_tx_count = pindex->m_chain_tx_count - past_block.m_chain_tx_count;
             ret.pushKV("window_tx_count", window_tx_count);
             if (nTimeDiff > 0) {
                 ret.pushKV("txrate", double(window_tx_count) / nTimeDiff);
@@ -2799,7 +2799,7 @@ UniValue CreateUTXOSnapshot(
     result.pushKV("base_height", tip->nHeight);
     result.pushKV("path", path.utf8string());
     result.pushKV("txoutset_hash", maybe_stats->hashSerialized.ToString());
-    result.pushKV("nchaintx", tip->nChainTx);
+    result.pushKV("nchaintx", tip->m_chain_tx_count);
     return result;
 }
 
diff --git a/src/validation.cpp b/src/validation.cpp
index d6b3924cda0e3..9126d8e928e30 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2956,7 +2956,7 @@ static void UpdateTipLog(
     LogPrintf("%s%s: new best=%s height=%d version=0x%08x log2_work=%f tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n",
         prefix, func_name,
         tip->GetBlockHash().ToString(), tip->nHeight, tip->nVersion,
-        log(tip->nChainWork.getdouble()) / log(2.0), (unsigned long)tip->nChainTx,
+        log(tip->nChainWork.getdouble()) / log(2.0), tip->m_chain_tx_count,
         FormatISO8601DateTime(tip->GetBlockTime()),
         GuessVerificationProgress(params.TxData(), tip),
         coins_tip.DynamicMemoryUsage() * (1.0 / (1 << 20)),
@@ -3846,17 +3846,17 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd
 {
     AssertLockHeld(cs_main);
     pindexNew->nTx = block.vtx.size();
-    // Typically nChainTx will be 0 at this point, but it can be nonzero if this
+    // Typically m_chain_tx_count will be 0 at this point, but it can be nonzero if this
     // is a pruned block which is being downloaded again, or if this is an
-    // assumeutxo snapshot block which has a hardcoded nChainTx value from the
+    // assumeutxo snapshot block which has a hardcoded m_chain_tx_count value from the
     // snapshot metadata. If the pindex is not the snapshot block and the
-    // nChainTx value is not zero, assert that value is actually correct.
-    auto prev_tx_sum = [](CBlockIndex& block) { return block.nTx + (block.pprev ? block.pprev->nChainTx : 0); };
-    if (!Assume(pindexNew->nChainTx == 0 || pindexNew->nChainTx == prev_tx_sum(*pindexNew) ||
+    // m_chain_tx_count value is not zero, assert that value is actually correct.
+    auto prev_tx_sum = [](CBlockIndex& block) { return block.nTx + (block.pprev ? block.pprev->m_chain_tx_count : 0); };
+    if (!Assume(pindexNew->m_chain_tx_count == 0 || pindexNew->m_chain_tx_count == prev_tx_sum(*pindexNew) ||
                 pindexNew == GetSnapshotBaseBlock())) {
-        LogWarning("Internal bug detected: block %d has unexpected nChainTx %i that should be %i (%s %s). Please report this issue here: %s\n",
-            pindexNew->nHeight, pindexNew->nChainTx, prev_tx_sum(*pindexNew), PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT);
-        pindexNew->nChainTx = 0;
+        LogWarning("Internal bug detected: block %d has unexpected m_chain_tx_count %i that should be %i (%s %s). Please report this issue here: %s\n",
+            pindexNew->nHeight, pindexNew->m_chain_tx_count, prev_tx_sum(*pindexNew), PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT);
+        pindexNew->m_chain_tx_count = 0;
     }
     pindexNew->nFile = pos.nFile;
     pindexNew->nDataPos = pos.nPos;
@@ -3877,15 +3877,15 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd
         while (!queue.empty()) {
             CBlockIndex *pindex = queue.front();
             queue.pop_front();
-            // Before setting nChainTx, assert that it is 0 or already set to
+            // Before setting m_chain_tx_count, assert that it is 0 or already set to
             // the correct value. This assert will fail after receiving the
             // assumeutxo snapshot block if assumeutxo snapshot metadata has an
-            // incorrect hardcoded AssumeutxoData::nChainTx value.
-            if (!Assume(pindex->nChainTx == 0 || pindex->nChainTx == prev_tx_sum(*pindex))) {
-                LogWarning("Internal bug detected: block %d has unexpected nChainTx %i that should be %i (%s %s). Please report this issue here: %s\n",
-                   pindex->nHeight, pindex->nChainTx, prev_tx_sum(*pindex), PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT);
+            // incorrect hardcoded AssumeutxoData::m_chain_tx_count value.
+            if (!Assume(pindex->m_chain_tx_count == 0 || pindex->m_chain_tx_count == prev_tx_sum(*pindex))) {
+                LogWarning("Internal bug detected: block %d has unexpected m_chain_tx_count %i that should be %i (%s %s). Please report this issue here: %s\n",
+                   pindex->nHeight, pindex->m_chain_tx_count, prev_tx_sum(*pindex), PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT);
             }
-            pindex->nChainTx = prev_tx_sum(*pindex);
+            pindex->m_chain_tx_count = prev_tx_sum(*pindex);
             pindex->nSequenceId = nBlockSequenceId++;
             for (Chainstate *c : GetAll()) {
                 c->TryAddBlockIndexCandidate(pindex);
@@ -5333,17 +5333,17 @@ void ChainstateManager::CheckBlockIndex()
             // Checks for not-invalid blocks.
             assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents.
         }
-        // Make sure nChainTx sum is correctly computed.
+        // Make sure m_chain_tx_count sum is correctly computed.
         if (!pindex->pprev) {
-            // If no previous block, nTx and nChainTx must be the same.
-            assert(pindex->nChainTx == pindex->nTx);
-        } else if (pindex->pprev->nChainTx > 0 && pindex->nTx > 0) {
-            // If previous nChainTx is set and number of transactions in block is known, sum must be set.
-            assert(pindex->nChainTx == pindex->nTx + pindex->pprev->nChainTx);
+            // If no previous block, nTx and m_chain_tx_count must be the same.
+            assert(pindex->m_chain_tx_count == pindex->nTx);
+        } else if (pindex->pprev->m_chain_tx_count > 0 && pindex->nTx > 0) {
+            // If previous m_chain_tx_count is set and number of transactions in block is known, sum must be set.
+            assert(pindex->m_chain_tx_count == pindex->nTx + pindex->pprev->m_chain_tx_count);
         } else {
-            // Otherwise nChainTx should only be set if this is a snapshot
+            // Otherwise m_chain_tx_count should only be set if this is a snapshot
             // block, and must be set if it is.
-            assert((pindex->nChainTx != 0) == (pindex == snap_base));
+            assert((pindex->m_chain_tx_count != 0) == (pindex == snap_base));
         }
 
         // Chainstate-specific checks on setBlockIndexCandidates
@@ -5548,13 +5548,13 @@ bool Chainstate::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size)
 }
 
 //! Guess how far we are in the verification process at the given block index
-//! require cs_main if pindex has not been validated yet (because nChainTx might be unset)
+//! require cs_main if pindex has not been validated yet (because m_chain_tx_count might be unset)
 double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pindex) {
     if (pindex == nullptr)
         return 0.0;
 
-    if (!Assume(pindex->nChainTx > 0)) {
-        LogWarning("Internal bug detected: block %d has unset nChainTx (%s %s). Please report this issue here: %s\n",
+    if (!Assume(pindex->m_chain_tx_count > 0)) {
+        LogWarning("Internal bug detected: block %d has unset m_chain_tx_count (%s %s). Please report this issue here: %s\n",
                    pindex->nHeight, PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT);
         return 0.0;
     }
@@ -5563,13 +5563,13 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin
 
     double fTxTotal;
 
-    if (pindex->nChainTx <= data.nTxCount) {
-        fTxTotal = data.nTxCount + (nNow - data.nTime) * data.dTxRate;
+    if (pindex->m_chain_tx_count <= data.tx_count) {
+        fTxTotal = data.tx_count + (nNow - data.nTime) * data.dTxRate;
     } else {
-        fTxTotal = pindex->nChainTx + (nNow - pindex->GetBlockTime()) * data.dTxRate;
+        fTxTotal = pindex->m_chain_tx_count + (nNow - pindex->GetBlockTime()) * data.dTxRate;
     }
 
-    return std::min<double>(pindex->nChainTx / fTxTotal, 1.0);
+    return std::min<double>(pindex->m_chain_tx_count / fTxTotal, 1.0);
 }
 
 std::optional<uint256> ChainstateManager::SnapshotBlockhash() const
@@ -6026,7 +6026,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
 
     assert(index);
     assert(index == snapshot_start_block);
-    index->nChainTx = au_data.nChainTx;
+    index->m_chain_tx_count = au_data.m_chain_tx_count;
     snapshot_chainstate.setBlockIndexCandidates.insert(snapshot_start_block);
 
     LogPrintf("[snapshot] validated snapshot (%.2f MB)\n",


Test patch:
diff --git a/src/test/blockchain_tests.cpp b/src/test/blockchain_tests.cpp
index bc509d73acc2f..4ecc15041cc5c 100644
--- a/src/test/blockchain_tests.cpp
+++ b/src/test/blockchain_tests.cpp
@@ -110,4 +110,11 @@ BOOST_FIXTURE_TEST_CASE(get_prune_height, TestChain100Setup)
     CheckGetPruneHeight(blockman, chain, 100);
 }
 
+BOOST_AUTO_TEST_CASE(num_chain_tx_max)
+{
+    CBlockIndex block_index{};
+    block_index.m_chain_tx_count = std::numeric_limits<uint64_t>::max();
+    BOOST_CHECK_EQUAL(block_index.m_chain_tx_count, std::numeric_limits<uint64_t>::max());
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp
index 60d9d39b73f09..d82f16676537a 100644
--- a/src/test/fuzz/utxo_snapshot.cpp
+++ b/src/test/fuzz/utxo_snapshot.cpp
@@ -113,9 +113,9 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
             if (index->nHeight == chainman.GetSnapshotBaseHeight()) {
                 auto params{chainman.GetParams().AssumeutxoForHeight(index->nHeight)};
                 Assert(params.has_value());
-                Assert(params.value().nChainTx == index->nChainTx);
+                Assert(params.value().m_chain_tx_count == index->m_chain_tx_count);
             } else {
-                Assert(index->nChainTx == 0);
+                Assert(index->m_chain_tx_count == 0);
             }
         }
         Assert(g_chain->size() == coinscache.GetCacheSize());
diff --git a/src/test/pmt_tests.cpp b/src/test/pmt_tests.cpp
index edecf70c6f6cc..5b7ccc8e7a105 100644
--- a/src/test/pmt_tests.cpp
+++ b/src/test/pmt_tests.cpp
@@ -29,10 +29,10 @@ BOOST_FIXTURE_TEST_SUITE(pmt_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(pmt_test1)
 {
-    static const unsigned int nTxCounts[] = {1, 4, 7, 17, 56, 100, 127, 256, 312, 513, 1000, 4095};
+    static const unsigned int tx_counts[] = {1, 4, 7, 17, 56, 100, 127, 256, 312, 513, 1000, 4095};
 
     for (int i = 0; i < 12; i++) {
-        unsigned int nTx = nTxCounts[i];
+        unsigned int nTx = tx_counts[i];
 
         // build a block with some dummy transactions
         CBlock block;
diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h
index a4636365ca3e3..e604b44c16a95 100644
--- a/src/test/util/chainstate.h
+++ b/src/test/util/chainstate.h
@@ -99,7 +99,7 @@ CreateAndActivateUTXOSnapshot(
                 assert(pindex->IsValid(BlockStatus::BLOCK_VALID_TREE));
                 pindex->nStatus = BlockStatus::BLOCK_VALID_TREE;
                 pindex->nTx = 0;
-                pindex->nChainTx = 0;
+                pindex->m_chain_tx_count = 0;
                 pindex->nSequenceId = 0;
                 pindex = pindex->pprev;
             }
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index f93e3cdfb1b77..cf819f8751dd8 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -285,7 +285,7 @@ struct SnapshotTestSetup : TestChain100Setup {
         const auto& au_data = ::Params().AssumeutxoForHeight(snapshot_height);
         const CBlockIndex* tip = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip());
 
-        BOOST_CHECK_EQUAL(tip->nChainTx, au_data->nChainTx);
+        BOOST_CHECK_EQUAL(tip->m_chain_tx_count, au_data->m_chain_tx_count);
 
         // To be checked against later when we try loading a subsequent snapshot.
         uint256 loaded_snapshot_blockhash{*chainman.SnapshotBlockhash()};
@@ -465,7 +465,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
         if (i < last_assumed_valid_idx && i >= assumed_valid_start_idx) {
             index->nStatus = BlockStatus::BLOCK_VALID_TREE;
             index->nTx = 0;
-            index->nChainTx = 0;
+            index->m_chain_tx_count = 0;
         }
 
         ++num_indexes;
diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp
index 93a884be6d3d7..546d2f7b4d55b 100644
--- a/src/test/validation_tests.cpp
+++ b/src/test/validation_tests.cpp
@@ -143,11 +143,11 @@ BOOST_AUTO_TEST_CASE(test_assumeutxo)
 
     const auto out110 = *params->AssumeutxoForHeight(110);
     BOOST_CHECK_EQUAL(out110.hash_serialized.ToString(), "6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1");
-    BOOST_CHECK_EQUAL(out110.nChainTx, 111U);
+    BOOST_CHECK_EQUAL(out110.m_chain_tx_count, 111U);
 
     const auto out110_2 = *params->AssumeutxoForBlockhash(uint256S("0x696e92821f65549c7ee134edceeeeaaa4105647a3c4fd9f298c0aec0ab50425c"));
     BOOST_CHECK_EQUAL(out110_2.hash_serialized.ToString(), "6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1");
-    BOOST_CHECK_EQUAL(out110_2.nChainTx, 111U);
+    BOOST_CHECK_EQUAL(out110_2.m_chain_tx_count, 111U);
 }
 
 BOOST_AUTO_TEST_CASE(block_malleation)