Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Properly activate timestamp fork #6386

Merged
merged 2 commits into from
Dec 18, 2023
Merged

Properly activate timestamp fork #6386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d945afe
Activate timestamp fork even if block matches block only fork
flcl42 Dec 18, 2023
a9cc841
Remove duplication
flcl42 Dec 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 46 additions & 7 deletions src/Nethermind/Nethermind.Specs.Test/ChainSpecStyle/ChainSpecBasedSpecProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: LGPL-3.0-only

using System;
using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand Down Expand Up @@ -98,15 +99,16 @@ public void Logs_warning_when_timestampActivation_happens_before_blockActivation
expectedSpec.IsEip3855Enabled = isEip3855Enabled;
expectedSpec.Eip1559TransitionBlock = 0;
expectedSpec.DifficultyBombDelay = 0;
TestSpecProvider testProvider = TestSpecProvider.Instance;
testProvider.SpecToReturn = expectedSpec;
testProvider.TerminalTotalDifficulty = 0;
testProvider.GenesisSpec = expectedSpec;
List<ForkActivation> forkActivationsToTest = new()
{
(blockNumber, timestamp),
};
CompareSpecProviders(testProvider, provider, forkActivationsToTest);

foreach (ForkActivation activation in forkActivationsToTest)
{
provider.GetSpec(activation);
}

if (receivesWarning)
{
logger.Received(1).Warn(Arg.Is("Chainspec file is misconfigured! Timestamp transition is configured to happen before the last block transition."));
Expand Down Expand Up @@ -290,8 +292,8 @@ public void Gnosis_loads_properly()
(ForkActivation)(GnosisSpecProvider.BerlinBlockNumber),
(ForkActivation)(GnosisSpecProvider.LondonBlockNumber -1),
(ForkActivation)(GnosisSpecProvider.LondonBlockNumber),
(1, GnosisSpecProvider.ShanghaiTimestamp - 1),
(1, GnosisSpecProvider.ShanghaiTimestamp),
(GnosisSpecProvider.LondonBlockNumber, GnosisSpecProvider.ShanghaiTimestamp - 1),
(GnosisSpecProvider.LondonBlockNumber, GnosisSpecProvider.ShanghaiTimestamp),
(999_999_999, 999_999_999) // far in the future
};

Expand Down Expand Up @@ -831,6 +833,43 @@ void TestTransitions(ForkActivation activation, Action<ReleaseSpec> changes)
TestTransitions((40001L, 1000000024), r => { r.IsEip1153Enabled = r.IsEip2537Enabled = true; });
}

[TestCaseSource(nameof(BlockNumbersAndTimestampsNearForkActivations))]
public void Forks_should_be_selected_properly_for_exact_matches(ForkActivation forkActivation, bool isEip3651Enabled, bool isEip3198Enabled, bool isEip3855Enabled)
{
ISpecProvider provider = new CustomSpecProvider(
(new ForkActivation(0), new ReleaseSpec() { IsEip3651Enabled = true }),
(new ForkActivation(2, 10), new ReleaseSpec() { IsEip3651Enabled = true, IsEip3198Enabled = true, }),
(new ForkActivation(2, 20), new ReleaseSpec() { IsEip3651Enabled = true, IsEip3198Enabled = true, IsEip3855Enabled = true })
);

IReleaseSpec spec = provider.GetSpec(forkActivation);
Assert.Multiple(() =>
{
Assert.That(spec.IsEip3651Enabled, Is.EqualTo(isEip3651Enabled));
Assert.That(spec.IsEip3198Enabled, Is.EqualTo(isEip3198Enabled));
Assert.That(spec.IsEip3855Enabled, Is.EqualTo(isEip3855Enabled));
});
}

public static IEnumerable BlockNumbersAndTimestampsNearForkActivations
{
get
{
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);
yield return new TestCaseData(new ForkActivation(2, 11), true, true, false);
yield return new TestCaseData(new ForkActivation(2, 19), true, true, false);
yield return new TestCaseData(new ForkActivation(2, 20), true, true, true);
yield return new TestCaseData(new ForkActivation(2, 21), true, true, true);
yield return new TestCaseData(new ForkActivation(3, 10), true, true, false);
yield return new TestCaseData(new ForkActivation(3, 11), true, true, false);
yield return new TestCaseData(new ForkActivation(3, 19), true, true, false);
yield return new TestCaseData(new ForkActivation(3, 20), true, true, true);
yield return new TestCaseData(new ForkActivation(3, 21), true, true, true);
}
}

private static IEnumerable<ulong> GetTransitionTimestamps(ChainParameters parameters) => parameters.GetType()
.Properties()
.Where(p => p.Name.EndsWith("TransitionTimestamp", StringComparison.Ordinal))
Expand Down
11 changes: 10 additions & 1 deletion src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecBasedSpecProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace Nethermind.Specs.ChainSpecStyle
public class ChainSpecBasedSpecProvider : ISpecProvider
{
private (ForkActivation Activation, ReleaseSpec Spec)[] _transitions;
private (ForkActivation Activation, ReleaseSpec Spec)[] _timestampOnlyTransitions;
private ForkActivation? _firstTimestampActivation;

private readonly ChainSpec _chainSpec;
Expand Down Expand Up @@ -98,6 +99,7 @@ static void Add(SortedSet<T> transitions, T value, T? minValueExclusive)
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)
{
Expand Down Expand Up @@ -268,6 +270,8 @@ public void UpdateMergeTransitionInfo(long? blockNumber, UInt256? terminalTotalD

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)
{
Expand All @@ -276,9 +280,14 @@ public IReleaseSpec GetSpec(ForkActivation activation)
{
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)
flcl42 marked this conversation as resolved.
Show resolved Hide resolved
{
consideredTransitions = _timestampOnlyTransitions;
}
}

return _transitions.TryGetSearchedItem(activation,
return consideredTransitions.TryGetSearchedItem(activation,
CompareTransitionOnActivation,
out (ForkActivation Activation, ReleaseSpec Spec) transition)
? transition.Spec
Expand Down