refactor: introduce CbTx version enum class, adjust version names (#5725

)

## Issue being fixed or feature implemented
- The name `CB_V19_VERSION` is confusing because CbTx v2 was introduced
in v14, not v19
https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-0.14.0.md#dip0004---coinbase-payload-v2
- There are magic numbers instead of constants in some places
- `CheckCbTx` should check whatever the upper limit is, not
`CB_V20_VERSION` specifically

## What was done?
Turn CbTx versions into enum using self-describing names

## How Has This Been Tested?
Run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

src/evo/cbtx.cpp

@@ -33,7 +33,7 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-payload");
     }
 
-    if (cbTx.nVersion == 0 || cbTx.nVersion > CCbTx::CB_V20_VERSION) {
+    if (cbTx.nVersion == CCbTx::Version::INVALID || cbTx.nVersion >= CCbTx::Version::UNKNOWN) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
     }
 
@@ -43,12 +43,12 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati
         }
 
         bool fDIP0008Active = pindexPrev->nHeight >= Params().GetConsensus().DIP0008Height;
-        if (fDIP0008Active && cbTx.nVersion < CCbTx::CB_V19_VERSION) {
+        if (fDIP0008Active && cbTx.nVersion < CCbTx::Version::MERKLE_ROOT_QUORUMS) {
             return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
         }
 
         bool isV20 = llmq::utils::IsV20Active(pindexPrev);
-        if ((isV20 && cbTx.nVersion < CCbTx::CB_V20_VERSION) || (!isV20 && cbTx.nVersion >= CCbTx::CB_V20_VERSION)) {
+        if ((isV20 && cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) || (!isV20 && cbTx.nVersion >= CCbTx::Version::CLSIG_AND_BALANCE)) {
             return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
         }
     }
@@ -91,7 +91,7 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, const
         int64_t nTime3 = GetTimeMicros(); nTimeMerkleMNL += nTime3 - nTime2;
         LogPrint(BCLog::BENCHMARK, "          - CalcCbTxMerkleRootMNList: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), nTimeMerkleMNL * 0.000001);
 
-        if (cbTx.nVersion >= 2) {
+        if (cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_QUORUMS) {
             if (!CalcCbTxMerkleRootQuorums(block, pindex->pprev, quorum_block_processor, calculatedMerkleRoot, state)) {
                 // pass the state returned by the function above
                 return false;
@@ -332,7 +332,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons
         return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload");
     }
 
-    if (cbTx.nVersion < CCbTx::CB_V20_VERSION) {
+    if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
         return true;
     }
 
@@ -433,7 +433,7 @@ bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, co
 std::string CCbTx::ToString() const
 {
     return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, creditPoolBalance=%d.%08d)",
-        nVersion, nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString(), bestCLHeightDiff, bestCLSignature.ToString(),
+        static_cast<uint16_t>(nVersion), nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString(), bestCLHeightDiff, bestCLSignature.ToString(),
         creditPoolBalance / COIN, creditPoolBalance % COIN);
 }
 
@@ -467,7 +467,7 @@ std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(co
 
     CCbTx& cbtx = opt_cbtx.value();
 
-    if (cbtx.nVersion < CCbTx::CB_V20_VERSION) {
+    if (cbtx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
         return std::nullopt;
     }
 

src/evo/cbtx.h

@@ -29,11 +29,16 @@ UniValue ValueFromAmount(const CAmount& amount);
 class CCbTx
 {
 public:
-    static constexpr auto SPECIALTX_TYPE = TRANSACTION_COINBASE;
-    static constexpr uint16_t CB_V19_VERSION = 2;
-    static constexpr uint16_t CB_V20_VERSION = 3;
+    enum class Version : uint16_t {
+        INVALID = 0,
+        MERKLE_ROOT_MNLIST = 1,
+        MERKLE_ROOT_QUORUMS = 2,
+        CLSIG_AND_BALANCE = 3,
+        UNKNOWN,
+    };
 
-    uint16_t nVersion{CB_V19_VERSION};
+    static constexpr auto SPECIALTX_TYPE = TRANSACTION_COINBASE;
+    Version nVersion{Version::MERKLE_ROOT_QUORUMS};
     int32_t nHeight{0};
     uint256 merkleRootMNList;
     uint256 merkleRootQuorums;
@@ -45,9 +50,9 @@ class CCbTx
     {
         READWRITE(obj.nVersion, obj.nHeight, obj.merkleRootMNList);
 
-        if (obj.nVersion >= CB_V19_VERSION) {
+        if (obj.nVersion >= Version::MERKLE_ROOT_QUORUMS) {
             READWRITE(obj.merkleRootQuorums);
-            if (obj.nVersion >= CB_V20_VERSION) {
+            if (obj.nVersion >= Version::CLSIG_AND_BALANCE) {
                 READWRITE(COMPACTSIZE(obj.bestCLHeightDiff));
                 READWRITE(obj.bestCLSignature);
                 READWRITE(obj.creditPoolBalance);
@@ -65,9 +70,9 @@ class CCbTx
         obj.pushKV("version", (int)nVersion);
         obj.pushKV("height", nHeight);
         obj.pushKV("merkleRootMNList", merkleRootMNList.ToString());
-        if (nVersion >= CB_V19_VERSION) {
+        if (nVersion >= Version::MERKLE_ROOT_QUORUMS) {
             obj.pushKV("merkleRootQuorums", merkleRootQuorums.ToString());
-            if (nVersion >= CB_V20_VERSION) {
+            if (nVersion >= Version::CLSIG_AND_BALANCE) {
                 obj.pushKV("bestCLHeightDiff", static_cast<int>(bestCLHeightDiff));
                 obj.pushKV("bestCLSignature", bestCLSignature.ToString());
                 obj.pushKV("creditPoolBalance", ValueFromAmount(creditPoolBalance));
@@ -76,6 +81,7 @@ class CCbTx
         return obj;
     }
 };
+template<> struct is_serializable_enum<CCbTx::Version> : std::true_type {};
 
 bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state);
 

src/evo/simplifiedmns.cpp

@@ -269,7 +269,7 @@ UniValue CSimplifiedMNListDiff::ToJson(bool extended) const
     CCbTx cbTxPayload;
     if (GetTxPayload(*cbTx, cbTxPayload)) {
         obj.pushKV("merkleRootMNList", cbTxPayload.merkleRootMNList.ToString());
-        if (cbTxPayload.nVersion >= 2) {
+        if (cbTxPayload.nVersion >= CCbTx::Version::MERKLE_ROOT_QUORUMS) {
             obj.pushKV("merkleRootQuorums", cbTxPayload.merkleRootQuorums.ToString());
         }
     }

src/miner.cpp

@@ -202,11 +202,11 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
         CCbTx cbTx;
 
         if (fV20Active_context) {
-            cbTx.nVersion = CCbTx::CB_V20_VERSION;
+            cbTx.nVersion = CCbTx::Version::CLSIG_AND_BALANCE;
         } else if (fDIP0008Active_context) {
-            cbTx.nVersion = CCbTx::CB_V19_VERSION;
+            cbTx.nVersion = CCbTx::Version::MERKLE_ROOT_QUORUMS;
         } else {
-            cbTx.nVersion = 1;
+            cbTx.nVersion = CCbTx::Version::MERKLE_ROOT_MNLIST;
         }
 
         cbTx.nHeight = nHeight;