x.cpp

diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp
index 76d50b8..704be83 100644
--- a/src/gtest/test_checktransaction.cpp
+++ b/src/gtest/test_checktransaction.cpp
@@ -513,14 +513,14 @@ TEST(checktransaction_tests, OverwinterExpiryHeight) {
     }
 
     {
-        mtx.nExpiryHeight = TX_EXPIRY_HEIGHT_THRESHOLD;
+        mtx.nExpiryHeight = TX_EXPIRY_HEIGHT_THRESHOLD - 1;
         CTransaction tx(mtx);
         MockCValidationState state;
         EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state));
     }
 
     {
-        mtx.nExpiryHeight = TX_EXPIRY_HEIGHT_THRESHOLD + 1;
+        mtx.nExpiryHeight = TX_EXPIRY_HEIGHT_THRESHOLD;
         CTransaction tx(mtx);
         MockCValidationState state;
         EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-expiry-height-too-high", false)).Times(1);
@@ -562,30 +562,40 @@ class UNSAFE_CTransaction : public CTransaction {
 };
 
 
-// Test bad Overwinter version numbers in CheckTransactionWithoutProofVerification
-TEST(checktransaction_tests, OverwinterVersionNumber) {
+// Test bad Overwinter version number in CheckTransactionWithoutProofVerification
+TEST(checktransaction_tests, OverwinterVersionNumberLow) {
     CMutableTransaction mtx = GetValidTransaction();
     mtx.vjoinsplit.resize(0);
     mtx.fOverwintered = true;
-    mtx.nVersion = 3;
+    mtx.nVersion = OVERWINTER_MIN_TX_VERSION - 1;
     mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
     mtx.nExpiryHeight = 0;
 
-    {
-        mtx.nVersion = CTransaction::OVERWINTER_MIN_CURRENT_VERSION - 1;
-        UNSAFE_CTransaction tx(mtx);
-        MockCValidationState state;
-        EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-overwinter-version-too-low", false)).Times(1);
-        CheckTransactionWithoutProofVerification(tx, state);
-    }
+    UNSAFE_CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-overwinter-version-too-low", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
 
-    {
-        mtx.nVersion = CTransaction::OVERWINTER_MAX_CURRENT_VERSION + 1;
-        UNSAFE_CTransaction tx(mtx);
-        MockCValidationState state;
-        EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-overwinter-version-too-high", false)).Times(1);
-        CheckTransactionWithoutProofVerification(tx, state);
-    }
+// Test bad Overwinter version number in ContextualCheckTransaction
+TEST(checktransaction_tests, OverwinterVersionNumberHigh) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
+
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0);
+    mtx.fOverwintered = true;
+    mtx.nVersion = OVERWINTER_MAX_TX_VERSION + 1;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+
+    UNSAFE_CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-overwinter-version-too-high", false)).Times(1);
+    ContextualCheckTransaction(tx, state, 1, 100);
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
 }
 
 
diff --git a/src/main.cpp b/src/main.cpp
index 9dc46f3..a17cebe 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -873,6 +873,12 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state,
                             REJECT_INVALID, "tx-overwinter-flag-not-set");
         }
 
+        // Reject transactions with invalid version
+        if (tx.fOverwintered && tx.nVersion > OVERWINTER_MAX_TX_VERSION ) {
+            return state.DoS(100, error("CheckTransaction(): overwinter version too high"),
+                REJECT_INVALID, "bad-tx-overwinter-version-too-high");
+        }
+
         // Reject transactions intended for Sprout
         if (!tx.fOverwintered) {
             return state.DoS(dosLevel, error("ContextualCheckTransaction: overwinter is active"),
@@ -926,9 +932,9 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
      *    This is unnecessary for Overwinter transactions since the parser now
      *    interprets the sign bit as fOverwintered, so tx.nVersion is always >=0,
      *    and when Overwinter is not active ContextualCheckTransaction rejects
-     *    transactions with fOverwintered set.  When Overwinter has activated and
-     *    fOverwintered is set, this function CheckTransactionWithoutProofVerification
-     *    checks to make sure that tx.nVersion avoids the following ranges:
+     *    transactions with fOverwintered set.  When fOverwintered is set,
+     *    this function and ContextualCheckTransaction will together check to
+     *    ensure tx.nVersion avoids the following ranges:
      *        0 <= tx.nVersion < OVERWINTER_MIN_TX_VERSION
      *        OVERWINTER_MAX_TX_VERSION < tx.nVersion <= INT32_MAX
      */
@@ -941,15 +947,11 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
             return state.DoS(100, error("CheckTransaction(): overwinter version too low"),
                 REJECT_INVALID, "bad-tx-overwinter-version-too-low");
         }
-        if (tx.nVersion > OVERWINTER_MAX_TX_VERSION ) {
-            return state.DoS(100, error("CheckTransaction(): overwinter version too high for client"),
-                REJECT_INVALID, "bad-tx-overwinter-version-too-high");
-        }
         if (tx.nVersionGroupId != OVERWINTER_VERSION_GROUP_ID) {
             return state.DoS(100, error("CheckTransaction(): unknown tx version group id"),
                     REJECT_INVALID, "bad-tx-version-group-id");
         }
-        if (tx.nExpiryHeight > TX_EXPIRY_HEIGHT_THRESHOLD) {
+        if (tx.nExpiryHeight >= TX_EXPIRY_HEIGHT_THRESHOLD) {
             return state.DoS(100, error("CheckTransaction(): expiry height is too high"),
                             REJECT_INVALID, "bad-tx-expiry-height-too-high");
         }
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h
index 9c57ac1..28347d6 100644
--- a/src/primitives/transaction.h
+++ b/src/primitives/transaction.h
@@ -309,7 +309,7 @@ static constexpr uint32_t TX_EXPIRY_HEIGHT_THRESHOLD = 500000000;
 
 // Overwinter version group id
 static constexpr uint32_t OVERWINTER_VERSION_GROUP_ID = 0x03C48270;
-static_assert(OVERWINTER_VERSION_GROUP_ID != 0, "version group id must be non-zero as specified in ZIP");
+static_assert(OVERWINTER_VERSION_GROUP_ID != 0, "version group id must be non-zero as specified in ZIP 202");
 
 struct CMutableTransaction;