Fully separate block forks from timtestamp ones

src/Nethermind/Nethermind.Specs.Test/ChainSpecStyle/ChainSpecBasedSpecProviderTests.cs

@@ -842,6 +842,9 @@ public static IEnumerable BlockNumbersAndTimestampsNearForkActivations
     {
         get
         {
+            yield return new TestCaseData(new ForkActivation(1), true, false, false);
+            yield return new TestCaseData(new ForkActivation(2), true, false, false);
+            yield return new TestCaseData(new ForkActivation(3), true, false, false);
             yield return new TestCaseData(new ForkActivation(1, 9), true, false, false);
             yield return new TestCaseData(new ForkActivation(2, 9), true, false, false);
             yield return new TestCaseData(new ForkActivation(2, 10), true, true, false);

src/Nethermind/Nethermind.Specs.Test/CustomSpecProvider.cs

@@ -1,86 +1,59 @@
 // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited
 // SPDX-License-Identifier: LGPL-3.0-only
 
-using System;
 using System.Linq;
 using Nethermind.Core;
-using Nethermind.Core.Collections;
 using Nethermind.Core.Specs;
 using Nethermind.Int256;
+using Nethermind.Specs.ChainSpecStyle;
 using Nethermind.Specs.Forks;
 
-namespace Nethermind.Specs.Test
-{
-    public class CustomSpecProvider : ISpecProvider
-    {
-        private ForkActivation? _theMergeBlock = null;
-        private readonly (ForkActivation Activation, IReleaseSpec Spec)[] _transitions;
-
-        public ulong NetworkId { get; }
-        public ulong ChainId { get; }
+namespace Nethermind.Specs.Test;
 
-        public ForkActivation[] TransitionActivations { get; }
-
-        public CustomSpecProvider(params (ForkActivation Activation, IReleaseSpec Spec)[] transitions) : this(TestBlockchainIds.NetworkId, TestBlockchainIds.ChainId, transitions)
-        {
-        }
-
-        public CustomSpecProvider(ulong networkId, ulong chainId, params (ForkActivation Activation, IReleaseSpec Spec)[] transitions)
-        {
-            NetworkId = networkId;
-            ChainId = chainId;
+public class CustomSpecProvider : SpecProviderBase, ISpecProvider
+{
+    private ForkActivation? _theMergeBlock = null;
 
-            if (transitions.Length == 0)
-            {
-                throw new ArgumentException($"There must be at least one release specified when instantiating {nameof(CustomSpecProvider)}", $"{nameof(transitions)}");
-            }
+    public ulong NetworkId { get; }
+    public ulong ChainId { get; }
 
-            _transitions = transitions.OrderBy(r => r.Activation).ToArray();
-            TransitionActivations = _transitions.Select(t => t.Activation).ToArray();
+    public CustomSpecProvider(params (ForkActivation Activation, IReleaseSpec Spec)[] transitions) : this(TestBlockchainIds.NetworkId, TestBlockchainIds.ChainId, transitions)
+    {
+    }
 
-            if (transitions[0].Activation.BlockNumber != 0L)
-            {
-                throw new ArgumentException($"First release specified when instantiating {nameof(CustomSpecProvider)} should be at genesis block (0)", $"{nameof(transitions)}");
-            }
-        }
+    public CustomSpecProvider(ulong networkId, ulong chainId, params (ForkActivation Activation, IReleaseSpec Spec)[] transitions)
+    {
+        NetworkId = networkId;
+        ChainId = chainId;
 
-        public void UpdateMergeTransitionInfo(long? blockNumber, UInt256? terminalTotalDifficulty = null)
-        {
-            if (blockNumber is not null)
-                _theMergeBlock = (ForkActivation)blockNumber;
-            if (terminalTotalDifficulty is not null)
-                TerminalTotalDifficulty = terminalTotalDifficulty;
-        }
+        (ForkActivation Activation, IReleaseSpec Spec)[] orderedTransitions = transitions.OrderBy(r => r.Activation).ToArray();
 
-        public ForkActivation? MergeBlockNumber => _theMergeBlock;
+        LoadTransitions(orderedTransitions);
 
-        public ulong TimestampFork { get; set; } = ISpecProvider.TimestampForkNever;
-        public UInt256? TerminalTotalDifficulty { get; set; }
+        TransitionActivations = orderedTransitions.Select(t => t.Activation).ToArray();
+    }
 
-#pragma warning disable CS8602
-#pragma warning disable CS8603
-        public IReleaseSpec GenesisSpec => _transitions.Length == 0 ? null : _transitions[0].Spec;
-#pragma warning restore CS8603
-#pragma warning restore CS8602
+    public void UpdateMergeTransitionInfo(long? blockNumber, UInt256? terminalTotalDifficulty = null)
+    {
+        if (blockNumber is not null)
+            _theMergeBlock = (ForkActivation)blockNumber;
+        if (terminalTotalDifficulty is not null)
+            TerminalTotalDifficulty = terminalTotalDifficulty;
+    }
 
-        public IReleaseSpec GetSpec(ForkActivation forkActivation) =>
-            _transitions.TryGetSearchedItem(forkActivation,
-                CompareTransitionOnBlock,
-                out (ForkActivation Activation, IReleaseSpec Spec) transition)
-                ? transition.Spec
-                : GenesisSpec;
+    public ForkActivation? MergeBlockNumber => _theMergeBlock;
 
-        private static int CompareTransitionOnBlock(ForkActivation forkActivation, (ForkActivation Activation, IReleaseSpec Spec) transition) =>
-            forkActivation.CompareTo(transition.Activation);
+    public ulong TimestampFork { get; set; } = ISpecProvider.TimestampForkNever;
+    public UInt256? TerminalTotalDifficulty { get; set; }
 
-        public long? DaoBlockNumber
+    public long? DaoBlockNumber
+    {
+        get
         {
-            get
-            {
-                (ForkActivation forkActivation, IReleaseSpec daoRelease) = _transitions.SingleOrDefault(t => t.Spec == Dao.Instance);
-                return daoRelease is not null ? forkActivation.BlockNumber : null;
-            }
+            (ForkActivation forkActivation, IReleaseSpec? daoRelease) = _blockTransitions.SingleOrDefault(t => t.Spec == Dao.Instance);
+            return daoRelease is not null ? forkActivation.BlockNumber : null;
         }
-
     }
+
 }
+

src/Nethermind/Nethermind.Specs.Test/CustomSpecProviderTests.cs

@@ -21,10 +21,6 @@ public void When_no_transitions_specified_throws_argument_exception()
         public void When_first_release_is_not_at_block_zero_then_throws_argument_exception()
         {
             Assert.Throws<ArgumentException>(() => _ = new CustomSpecProvider(((ForkActivation)1, Byzantium.Instance)), "ordered");
-
-            Assert.Throws<ArgumentException>(() => _ = new CustomSpecProvider(
-                ((ForkActivation)1, Byzantium.Instance),
-                ((ForkActivation)0, Frontier.Instance)), "not ordered");
         }
 
         [Test]

src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecBasedSpecProvider.cs

@@ -6,26 +6,20 @@
 using System.Linq;
 using System.Numerics;
 using System.Reflection;
-using Nethermind.Core.Collections;
 using Nethermind.Core.Specs;
 using Nethermind.Int256;
 using Nethermind.Logging;
 
 namespace Nethermind.Specs.ChainSpecStyle
 {
-    public class ChainSpecBasedSpecProvider : ISpecProvider
+    public class ChainSpecBasedSpecProvider : SpecProviderBase, ISpecProvider
     {
-        private (ForkActivation Activation, ReleaseSpec Spec)[] _transitions;
-        private (ForkActivation Activation, ReleaseSpec Spec)[] _timestampOnlyTransitions;
-        private ForkActivation? _firstTimestampActivation;
-
         private readonly ChainSpec _chainSpec;
-        private readonly ILogger _logger;
 
         public ChainSpecBasedSpecProvider(ChainSpec chainSpec, ILogManager logManager = null)
+            : base(logManager?.GetClassLogger<ChainSpecBasedSpecProvider>() ?? LimboTraceLogger.Instance)
         {
             _chainSpec = chainSpec ?? throw new ArgumentNullException(nameof(chainSpec));
-            _logger = logManager?.GetClassLogger<ChainSpecBasedSpecProvider>() ?? LimboTraceLogger.Instance;
             BuildTransitions();
         }
 
@@ -96,10 +90,12 @@ static void Add(SortedSet<T> transitions, T value, T? minValueExclusive)
                 transitionBlockNumbers.Add(bombDelay.Key);
             }
 
+
+            (ForkActivation Activation, IReleaseSpec Spec)[] allTransitions = CreateTransitions(_chainSpec, transitionBlockNumbers, transitionTimestamps);
+
+            LoadTransitions(allTransitions);
+
             TransitionActivations = CreateTransitionActivations(transitionBlockNumbers, transitionTimestamps);
-            _transitions = CreateTransitions(_chainSpec, transitionBlockNumbers, transitionTimestamps);
-            _firstTimestampActivation = TransitionActivations.FirstOrDefault(t => t.Timestamp is not null);
-            _timestampOnlyTransitions = _transitions.SkipWhile(t => t.Activation.Timestamp is null).ToArray();
 
             if (_chainSpec.Parameters.TerminalPoWBlockNumber is not null)
             {
@@ -109,13 +105,13 @@ static void Add(SortedSet<T> transitions, T value, T? minValueExclusive)
             TerminalTotalDifficulty = _chainSpec.Parameters.TerminalTotalDifficulty;
         }
 
-        private static (ForkActivation, ReleaseSpec Spec)[] CreateTransitions(
+        private static (ForkActivation, IReleaseSpec Spec)[] CreateTransitions(
             ChainSpec chainSpec,
             SortedSet<long> transitionBlockNumbers,
             SortedSet<ulong> transitionTimestamps)
         {
-            (ForkActivation Activation, ReleaseSpec Spec)[] transitions = new (ForkActivation, ReleaseSpec Spec)[transitionBlockNumbers.Count + transitionTimestamps.Count];
-            long biggestBlockTransition = transitionBlockNumbers.Max + 1; // We assume first timestamp based transition will happen after last blockNumber based transition (in block numbers)
+            (ForkActivation Activation, IReleaseSpec Spec)[] transitions = new (ForkActivation, IReleaseSpec Spec)[transitionBlockNumbers.Count + transitionTimestamps.Count];
+            long biggestBlockTransition = transitionBlockNumbers.Max;
 
             int index = 0;
             foreach (long releaseStartBlock in transitionBlockNumbers)
@@ -137,7 +133,7 @@ private static (ForkActivation, ReleaseSpec Spec)[] CreateTransitions(
 
         private static ForkActivation[] CreateTransitionActivations(SortedSet<long> transitionBlockNumbers, SortedSet<ulong> transitionTimestamps)
         {
-            long biggestBlockTransition = transitionBlockNumbers.Max + 1;
+            long biggestBlockTransition = transitionBlockNumbers.Max;
 
             ForkActivation[] transitionActivations = new ForkActivation[transitionBlockNumbers.Count - 1 + transitionTimestamps.Count];
 
@@ -266,41 +262,9 @@ public void UpdateMergeTransitionInfo(long? blockNumber, UInt256? terminalTotalD
 
         public UInt256? TerminalTotalDifficulty { get; private set; }
 
-        public IReleaseSpec GenesisSpec => _transitions.Length == 0 ? null : _transitions[0].Spec;
-
-        public IReleaseSpec GetSpec(ForkActivation activation)
-        {
-            (ForkActivation Activation, ReleaseSpec Spec)[] consideredTransitions = _transitions;
-
-            // TODO: Is this actually needed? Can this be tricked with invalid activation check if someone would fake timestamp from the future?
-            if (_firstTimestampActivation is not null && activation.Timestamp is not null)
-            {
-                if (_firstTimestampActivation.Value.Timestamp < activation.Timestamp
-                    && _firstTimestampActivation.Value.BlockNumber > activation.BlockNumber)
-                {
-                    if (_logger.IsWarn) _logger.Warn($"Chainspec file is misconfigured! Timestamp transition is configured to happen before the last block transition.");
-                }
-
-                if (_firstTimestampActivation.Value.Timestamp <= activation.Timestamp)
-                {
-                    consideredTransitions = _timestampOnlyTransitions;
-                }
-            }
-
-            return consideredTransitions.TryGetSearchedItem(activation,
-                CompareTransitionOnActivation,
-                out (ForkActivation Activation, ReleaseSpec Spec) transition)
-                ? transition.Spec
-                : GenesisSpec;
-        }
-
-        private static int CompareTransitionOnActivation(ForkActivation activation, (ForkActivation Activation, ReleaseSpec Spec) transition) =>
-            activation.CompareTo(transition.Activation);
-
         public long? DaoBlockNumber => _chainSpec.DaoForkBlockNumber;
 
         public ulong NetworkId => _chainSpec.NetworkId;
         public ulong ChainId => _chainSpec.ChainId;
-        public ForkActivation[] TransitionActivations { get; private set; }
     }
 }

src/Nethermind/Nethermind.Specs/ChainSpecStyle/SpecProviderBase.cs

@@ -0,0 +1,71 @@
+// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited
+// SPDX-License-Identifier: LGPL-3.0-only
+
+using Nethermind.Core.Collections;
+using Nethermind.Core.Specs;
+using Nethermind.Logging;
+using System;
+using System.Linq;
+
+namespace Nethermind.Specs.ChainSpecStyle;
+
+public abstract class SpecProviderBase
+{
+    protected (ForkActivation Activation, IReleaseSpec Spec)[] _blockTransitions;
+    private (ForkActivation Activation, IReleaseSpec Spec)[] _timestampTransitions;
+    private ForkActivation? _firstTimestampActivation;
+    protected readonly ILogger _logger;
+
+    public SpecProviderBase(ILogger logger = null)
+    {
+        _logger = logger;
+    }
+
+    protected void LoadTransitions((ForkActivation Activation, IReleaseSpec Spec)[] transitions)
+    {
+        if (transitions.Length == 0)
+        {
+            throw new ArgumentException($"There must be at least one release specified when instantiating {GetType()}", $"{nameof(transitions)}");
+        }
+
+        if (transitions.First().Activation.BlockNumber != 0L)
+        {
+            throw new ArgumentException($"First release specified when instantiating {GetType()} should be at genesis block (0)", $"{nameof(transitions)}");
+        }
+
+        _blockTransitions = transitions.TakeWhile(t => t.Activation.Timestamp is null).ToArray();
+        _timestampTransitions = transitions.SkipWhile(t => t.Activation.Timestamp is null).ToArray();
+        _firstTimestampActivation = _timestampTransitions.Length != 0 ? _timestampTransitions.First().Activation : null;
+        GenesisSpec = transitions.First().Spec;
+    }
+
+    public ForkActivation[] TransitionActivations { get; protected set; }
+
+    public IReleaseSpec GenesisSpec { get; private set; }
+
+    public IReleaseSpec GetSpec(ForkActivation activation)
+    {
+        static int CompareTransitionOnActivation(ForkActivation activation, (ForkActivation Activation, IReleaseSpec Spec) transition) =>
+           activation.CompareTo(transition.Activation);
+
+        (ForkActivation Activation, IReleaseSpec Spec)[] consideredTransitions = _blockTransitions;
+
+        if (_firstTimestampActivation is not null && activation.Timestamp is not null)
+        {
+            if (_firstTimestampActivation.Value.Timestamp < activation.Timestamp
+                && _firstTimestampActivation.Value.BlockNumber > activation.BlockNumber)
+            {
+                if (_logger is not null && _logger.IsWarn) _logger.Warn($"Chainspec file is misconfigured! Timestamp transition is configured to happen before the last block transition.");
+            }
+
+            if (_firstTimestampActivation.Value.Timestamp <= activation.Timestamp)
+                consideredTransitions = _timestampTransitions;
+        }
+
+        return consideredTransitions.TryGetSearchedItem(activation,
+            CompareTransitionOnActivation,
+            out (ForkActivation Activation, IReleaseSpec Spec) transition)
+            ? transition.Spec
+            : GenesisSpec;
+    }
+}