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

[Cancun]{Spec} Goerli config and fix for ChainSpecBasedSpecProvider #6409

Merged
merged 10 commits into from
Dec 28, 2023
Merged

[Cancun]{Spec} Goerli config and fix for ChainSpecBasedSpecProvider #6409

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
26efbad
init
smartprogrammer93 Dec 22, 2023
dc80827
format
smartprogrammer93 Dec 22, 2023
e2105c1
format
smartprogrammer93 Dec 22, 2023
9fb3350
Empty-Commit
smartprogrammer93 Dec 22, 2023
20a7951
make tests granular
LukaszRozmej Dec 22, 2023
6d67c69
Remove timestamp consecutive block transition. Keep first timestamp t…
LukaszRozmej Dec 22, 2023
aab7910
change approach to test fix
MarekM25 Dec 22, 2023
3fb6a30
fixes
smartprogrammer93 Dec 23, 2023
5e2f532
fix future fork info
smartprogrammer93 Dec 23, 2023
ae088c3
Fully separate block forks from timtestamp ones (#6419)
flcl42 Dec 26, 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
24 changes: 15 additions & 9 deletions src/Nethermind/Chains/goerli.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@
"eip3855TransitionTimestamp": "0x6410F460",
"eip3860TransitionTimestamp": "0x6410F460",
"eip4895TransitionTimestamp": "0x6410F460",
"eip1153TransitionTimestamp": "0x65A77460",
"eip4788TransitionTimestamp": "0x65A77460",
"eip4844TransitionTimestamp": "0x65A77460",
"eip5656TransitionTimestamp": "0x65A77460",
"eip6780TransitionTimestamp": "0x65A77460",
"eip4788ContractAddress": "0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4788 address is hardcoded, so we can remove this

"terminalTotalDifficulty": "A4A470",
"gasLimitBoundDivisor": "0x400",
"maxCodeSize": "0x6000",
Expand All @@ -66,15 +72,15 @@
"timestamp": "0x5c51a607"
},
"nodes": [
"enode://011f758e6552d105183b1761c5e2dea0111bc20fd5f6422bc7f91e0fabbec9a6595caf6239b37feb773dddd3f87240d99d859431891e4a642cf2a0a9e6cbb98a@51.141.78.53:30303",
"enode://176b9417f511d05b6b2cf3e34b756cf0a7096b3094572a8f6ef4cdcb9d1f9d00683bf0f83347eebdf3b81c3521c2332086d9592802230bf528eaf606a1d9677b@13.93.54.137:30303",
"enode://46add44b9f13965f7b9875ac6b85f016f341012d84f975377573800a863526f4da19ae2c620ec73d11591fa9510e992ecc03ad0751f53cc02f7c7ed6d55c7291@94.237.54.114:30313",
"enode://c1f8b7c2ac4453271fa07d8e9ecf9a2e8285aa0bd0c07df0131f47153306b0736fd3db8924e7a9bf0bed6b1d8d4f87362a71b033dc7c64547728d953e43e59b2@52.64.155.147:30303",
"enode://f4a9c6ee28586009fb5a96c8af13a58ed6d8315a9eee4772212c1d4d9cebe5a8b8a78ea4434f318726317d04a3f531a1ef0420cf9752605a562cfe858c46e263@213.186.16.82:30303",
"enode://a61215641fb8714a373c80edbfa0ea8878243193f57c96eeb44d0bc019ef295abd4e044fd619bfc4c59731a73fb79afe84e9ab6da0c743ceb479cbb6d263fa91@3.11.147.67:30303",
"enode://b5948a2d3e9d486c4d75bf32713221c2bd6cf86463302339299bd227dc2e276cd5a1c7ca4f43a0e9122fe9af884efed563bd2a1fd28661f3b5f5ad7bf1de5949@18.218.250.66:30303",
"enode://d2b720352e8216c9efc470091aa91ddafc53e222b32780f505c817ceef69e01d5b0b0797b69db254c586f493872352f5a022b4d8479a00fc92ec55f9ad46a27e@88.99.70.182:30303",
"enode://d4f764a48ec2a8ecf883735776fdefe0a3949eb0ca476bd7bc8d0954a9defe8fea15ae5da7d40b5d2d59ce9524a99daedadf6da6283fca492cc80b53689fb3b3@46.4.99.122:32109"
"enode://011f758e6552d105183b1761c5e2dea0111bc20fd5f6422bc7f91e0fabbec9a6595caf6239b37feb773dddd3f87240d99d859431891e4a642cf2a0a9e6cbb98a@51.141.78.53:30303",
"enode://176b9417f511d05b6b2cf3e34b756cf0a7096b3094572a8f6ef4cdcb9d1f9d00683bf0f83347eebdf3b81c3521c2332086d9592802230bf528eaf606a1d9677b@13.93.54.137:30303",
"enode://46add44b9f13965f7b9875ac6b85f016f341012d84f975377573800a863526f4da19ae2c620ec73d11591fa9510e992ecc03ad0751f53cc02f7c7ed6d55c7291@94.237.54.114:30313",
"enode://c1f8b7c2ac4453271fa07d8e9ecf9a2e8285aa0bd0c07df0131f47153306b0736fd3db8924e7a9bf0bed6b1d8d4f87362a71b033dc7c64547728d953e43e59b2@52.64.155.147:30303",
"enode://f4a9c6ee28586009fb5a96c8af13a58ed6d8315a9eee4772212c1d4d9cebe5a8b8a78ea4434f318726317d04a3f531a1ef0420cf9752605a562cfe858c46e263@213.186.16.82:30303",
"enode://a61215641fb8714a373c80edbfa0ea8878243193f57c96eeb44d0bc019ef295abd4e044fd619bfc4c59731a73fb79afe84e9ab6da0c743ceb479cbb6d263fa91@3.11.147.67:30303",
"enode://b5948a2d3e9d486c4d75bf32713221c2bd6cf86463302339299bd227dc2e276cd5a1c7ca4f43a0e9122fe9af884efed563bd2a1fd28661f3b5f5ad7bf1de5949@18.218.250.66:30303",
"enode://d2b720352e8216c9efc470091aa91ddafc53e222b32780f505c817ceef69e01d5b0b0797b69db254c586f493872352f5a022b4d8479a00fc92ec55f9ad46a27e@88.99.70.182:30303",
"enode://d4f764a48ec2a8ecf883735776fdefe0a3949eb0ca476bd7bc8d0954a9defe8fea15ae5da7d40b5d2d59ce9524a99daedadf6da6283fca492cc80b53689fb3b3@46.4.99.122:32109"
],
"accounts": {
"0x0000000000000000000000000000000000000000": {
Expand Down
5 changes: 3 additions & 2 deletions src/Nethermind/Nethermind.Network.Test/ForkInfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ public void Fork_id_and_hash_as_expected_with_merge_fork_id(long head, ulong hea
[TestCase(4_600_000L, 0ul, "0x757a1c47", 5_062_605ul, "Future Berlin block")]
[TestCase(5_062_605L, 0ul, "0xB8C6299D", 1678832736ul, "First London block")]
[TestCase(6_000_000, 0ul, "0xB8C6299D", 1678832736ul, "Future London block")]
[TestCase(6_000_001, 1678832736ul, "0xf9843abf", 0ul, "First Shanghai timestamp")]
[TestCase(6_000_001, 2678832736ul, "0xf9843abf", 0ul, "Future Shanghai timestamp")]
[TestCase(6_000_001, 1678832736ul, "0xf9843abf", 1705473120ul, "First Shanghai timestamp")]
[TestCase(6_000_001, 1705473119ul, "0xf9843abf", 1705473120ul, "Future Shanghai timestamp")]
[TestCase(6_000_001, 1705473120ul, "0x70cc14e2", 0ul, "First Cancun timestamp")]
public void Fork_id_and_hash_as_expected_on_goerli(long head, ulong headTimestamp, string forkHashHex, ulong next, string description)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cosmetic: add future Cancun

{
Test(head, headTimestamp, KnownHashes.GoerliGenesis, forkHashHex, next, description, GoerliSpecProvider.Instance, "goerli.json");
Expand Down
4 changes: 3 additions & 1 deletion 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 @@ -194,7 +194,9 @@ public void Goerli_loads_properly()
(ForkActivation)(GoerliSpecProvider.LondonBlockNumber - 1),
(ForkActivation)GoerliSpecProvider.LondonBlockNumber,
new ForkActivation(GoerliSpecProvider.LondonBlockNumber + 1, GoerliSpecProvider.ShanghaiTimestamp),
new ForkActivation(GoerliSpecProvider.LondonBlockNumber + 1, GoerliSpecProvider.ShanghaiTimestamp + 100000000) // far in future
new ForkActivation(GoerliSpecProvider.LondonBlockNumber + 1, GoerliSpecProvider.CancunTimestamp -1),
new ForkActivation(GoerliSpecProvider.LondonBlockNumber + 2, GoerliSpecProvider.CancunTimestamp),
new ForkActivation(GoerliSpecProvider.LondonBlockNumber + 2, GoerliSpecProvider.CancunTimestamp + 100000000) // far in future
};

CompareSpecProviders(goerli, provider, forkActivationsToTest);
Expand Down
1 change: 1 addition & 0 deletions src/Nethermind/Nethermind.Specs.Test/ChainSpecStyle/ChainSpecLoaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public void Can_load_goerli()
chainSpec.LondonBlockNumber.Should().Be(GoerliSpecProvider.LondonBlockNumber);
chainSpec.ShanghaiTimestamp.Should().Be(GoerliSpecProvider.ShanghaiTimestamp);
chainSpec.ShanghaiTimestamp.Should().Be(GoerliSpecProvider.Instance.TimestampFork);
chainSpec.CancunTimestamp.Should().Be(GoerliSpecProvider.CancunTimestamp);
}

[Test]
Expand Down
4 changes: 2 additions & 2 deletions src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecBasedSpecProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private static (ForkActivation, ReleaseSpec Spec)[] CreateTransitions(

foreach (ulong releaseStartTimestamp in transitionTimestamps)
{
long activationBlockNumber = biggestBlockTransition;
long activationBlockNumber = ++biggestBlockTransition;
ForkActivation forkActivation = (activationBlockNumber, releaseStartTimestamp);
ReleaseSpec releaseSpec = CreateReleaseSpec(chainSpec, activationBlockNumber, releaseStartTimestamp);
transitions[index++] = (forkActivation, releaseSpec);
Expand All @@ -149,7 +149,7 @@ private static ForkActivation[] CreateTransitionActivations(SortedSet<long> tran

foreach (ulong timestamp in transitionTimestamps)
{
transitionActivations[index++] = new ForkActivation(biggestBlockTransition, timestamp);
transitionActivations[index++] = new ForkActivation(++biggestBlockTransition, timestamp);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because our GetSpec method usies binary search. And sometimes when the blocknumber only fork and timestamp gork has the same blocknumber it takes the timestamp one onstead of the blocknumber one

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a more detailed explanation with examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you remove the ++ and run chainspecbasedspecproviderTests goerli will fail. the search function will return the shanghai spec for london, just becuase london and shanghai have the same blockNumber (biggestBlockNumber)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah and it fails on... london test :)
Will provide better test separation shortly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed 2nd ++, added a comment on first ++ .

Copy link
Contributor

@MarekM25 MarekM25 Dec 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smartprogrammer93 isn't a test issue assumption issue? we can assume in tests that London and Shangai will have different block numbers and don't touch the code

}

return transitionActivations;
Expand Down
4 changes: 2 additions & 2 deletions src/Nethermind/Nethermind.Specs/Forks/16_Cancun.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ protected Cancun()
{
Name = "Cancun";
IsEip1153Enabled = true;
IsEip5656Enabled = true;
IsEip4788Enabled = true;
IsEip4844Enabled = true;
IsEip5656Enabled = true;
IsEip6780Enabled = true;
IsEip4788Enabled = true;
Eip4788ContractAddress = new Address("0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02");
}

Expand Down
7 changes: 5 additions & 2 deletions src/Nethermind/Nethermind.Specs/GoerliSpecProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class GoerliSpecProvider : ISpecProvider
public const long LondonBlockNumber = 5_062_605;
public const ulong BeaconChainGenesisTimestamp = 0x6059f460;
public const ulong ShanghaiTimestamp = 0x6410f460;
public const ulong CancunTimestamp = 0x65A77460;

private GoerliSpecProvider() { }

Expand All @@ -28,7 +29,8 @@ public IReleaseSpec GetSpec(ForkActivation forkActivation)
_ => forkActivation.Timestamp switch
{
null or < ShanghaiTimestamp => London.Instance,
_ => Shanghai.Instance
< CancunTimestamp => Shanghai.Instance,
_ => Cancun.Instance
}
};
}
Expand All @@ -53,7 +55,8 @@ public void UpdateMergeTransitionInfo(long? blockNumber, UInt256? terminalTotalD
(ForkActivation)IstanbulBlockNumber,
(ForkActivation)BerlinBlockNumber,
(ForkActivation)LondonBlockNumber,
(LondonBlockNumber, ShanghaiTimestamp)
(LondonBlockNumber + 1, ShanghaiTimestamp),
(LondonBlockNumber + 2, CancunTimestamp)
};

public static readonly GoerliSpecProvider Instance = new();
Expand Down