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.

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

Fix OP Stack header validation / Remove snapshot from snap-synced chains #7588

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion src/Nethermind/Chains/op-mainnet.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"rip7212TransitionTimestamp": "0x668eb001",
"opGraniteTransitionTimestamp": "0x66e1be81",

"terminalTotalDifficulty": "0"
"terminalTotalDifficulty": "210470125"
Copy link
Contributor

Choose a reason for hiding this comment

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

From where are we getting this value?

},
"genesis": {
"seal": {
Expand All @@ -93,6 +93,11 @@
"stateRoot": "0xeddb4c1786789419153a27c4c80ff44a2226b6eda04f7e22ce5bae892ea568eb"
},
"nodes": [
"enode://87a32fd13bd596b2ffca97020e31aef4ddcc1bbd4b95bb633d16c1329f654f34049ed240a36b449fda5e5225d70fe40bc667f53c304b71f8e68fc9d448690b51@3.231.138.188:30301",
"enode://ca21ea8f176adb2e229ce2d700830c844af0ea941a1d8152a9513b966fe525e809c3a6c73a2c18a12b74ed6ec4380edf91662778fe0b79f6a591236e49e176f9@184.72.129.189:30301",
"enode://acf4507a211ba7c1e52cdf4eef62cdc3c32e7c9c47998954f7ba024026f9a6b2150cd3f0b734d9c78e507ab70d59ba61dfe5c45e1078c7ad0775fb251d7735a2@3.220.145.177:30301",
"enode://8a5a5006159bf079d06a04e5eceab2a1ce6e0f721875b2a9c96905336219dbe14203d38f70f3754686a6324f786c2f9852d8c0dd3adac2d080f4db35efc678c5@3.231.11.52:30301",
"enode://cdadbe835308ad3557f9a1de8db411da1a260a98f8421d62da90e71da66e55e98aaa8e90aa7ce01b408a54e4bd2253d701218081ded3dbe5efbbc7b41d7cef79@54.198.153.150:30301",
Comment on lines +96 to +100
Copy link
Contributor

Choose a reason for hiding this comment

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

From where are we getting these nodes?

"enode://ca2774c3c401325850b2477fd7d0f27911efbf79b1e8b335066516e2bd8c4c9e0ba9696a94b1cb030a88eac582305ff55e905e64fb77fe0edcd70a4e5296d3ec@34.65.175.185:30305",
"enode://dd751a9ef8912be1bfa7a5e34e2c3785cc5253110bd929f385e07ba7ac19929fb0e0c5d93f77827291f4da02b2232240fbc47ea7ce04c46e333e452f8656b667@34.65.107.0:30305",
"enode://c5d289b56a77b6a2342ca29956dfd07aadf45364dde8ab20d1dc4efd4d1bc6b4655d902501daea308f4d8950737a4e93a4dfedd17b49cd5760ffd127837ca965@34.65.202.239:30305",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private bool ValidateGasUsed(BlockHeader header, ref string? error)
return true;
}

private bool ValidateParent(BlockHeader header, BlockHeader? parent, ref string? error)
protected bool ValidateParent(BlockHeader header, BlockHeader? parent, ref string? error)
{
if (parent is null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

namespace Nethermind.Merge.Plugin
{
public sealed class MergeHeaderValidator : HeaderValidator
public class MergeHeaderValidator : HeaderValidator
{
// https://eips.ethereum.org/EIPS/eip-3675#constants
private const int MaxExtraDataBytes = 32;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,9 @@ bool HasMoreToSync(out BlockHeader[]? headers, out int headersToRequest)
}

// can move this to block tree now?
if (!_blockValidator.ValidateSuggestedBlock(currentBlock, out _))
if (!_blockValidator.ValidateSuggestedBlock(currentBlock, out string? errorMessage))
{
string message = InvalidBlockHelper.GetMessage(currentBlock, "invalid block sent by peer") +
string message = InvalidBlockHelper.GetMessage(currentBlock, $"invalid block sent by peer. {errorMessage}") +
$" PeerInfo {bestPeer}";
if (_logger.IsWarn) _logger.Warn(message);
throw new EthSyncException(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,14 @@ protected override ITransactionProcessor CreateTransactionProcessor(CodeInfoRepo
protected override IHeaderValidator CreateHeaderValidator()
{
if (api.InvalidChainTracker is null) throw new StepDependencyException(nameof(api.InvalidChainTracker));
if (api.BlockTree is null) throw new StepDependencyException(nameof(api.BlockTree));
if (api.SealValidator is null) throw new StepDependencyException(nameof(api.SealValidator));
if (api.SpecProvider is null) throw new StepDependencyException(nameof(api.SpecProvider));
if (api.SpecHelper is null) throw new StepDependencyException(nameof(api.SpecHelper));
if (api.LogManager is null) throw new StepDependencyException(nameof(api.LogManager));

OptimismHeaderValidator opHeaderValidator = new(
api.PoSSwitcher,
api.BlockTree,
api.SealValidator,
api.SpecProvider,
Expand Down
28 changes: 25 additions & 3 deletions src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs
Original file line number Diff line number Diff line change
@@ -1,21 +1,43 @@
// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using Nethermind.Blockchain;
using Nethermind.Consensus;
using Nethermind.Consensus.Messages;
using Nethermind.Consensus.Validators;
using Nethermind.Core;
using Nethermind.Core.Specs;
using Nethermind.Int256;
using Nethermind.Logging;
using Nethermind.Merge.Plugin;

namespace Nethermind.Optimism;

public class OptimismHeaderValidator(
public class PreBedrockHeaderValidator(
IBlockTree? blockTree,
ISealValidator? sealValidator,
ISpecProvider? specProvider,
ILogManager? logManager)
: HeaderValidator(blockTree, sealValidator, specProvider, logManager)
ILogManager? logManager) : HeaderValidator(blockTree, sealValidator, specProvider, logManager)
{
public override bool Validate(BlockHeader header, BlockHeader? parent, bool isUncle, [NotNullWhen(false)] out string? error)
{
error = null;
return ValidateParent(header, parent, ref error);
Copy link
Contributor

Choose a reason for hiding this comment

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

In the PR description you mention:

Fix header validation logic for pre-bedrock block to only verify parent header matches parent hash

But we're not actually doing that since ValidateParent checks that:

  • For non genesis block, we have a non-null parent
  • For genesis block, we perform a specific check on certain fields.

Is this something in progress? Otherwise, I would expect to see something like

if (header.ParentHash != parent.Hash)
{
    if (_logger.IsDebug) _logger.Debug($"Parent hash mismatch in block header ({header.Hash})");
    error = BlockErrorMessages.MismatchedParentHash;
    return false;
}

}
}

public class OptimismHeaderValidator(
IPoSSwitcher poSSwitcher,
IBlockTree blockTree,
ISealValidator sealValidator,
ISpecProvider specProvider,
ILogManager logManager)
: MergeHeaderValidator(
poSSwitcher,
new PreBedrockHeaderValidator(blockTree, sealValidator, specProvider, logManager),
blockTree, specProvider, sealValidator, logManager)
{
protected override bool ValidateGasLimitRange(BlockHeader header, BlockHeader parent, IReleaseSpec spec, ref string? error) => true;
}
8 changes: 6 additions & 2 deletions src/Nethermind/Nethermind.Optimism/OptimismPlugin.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
using Nethermind.Specs.ChainSpecStyle;
using Nethermind.Serialization.Rlp;
using Nethermind.Optimism.Rpc;
using Nethermind.Db;

namespace Nethermind.Optimism;

Expand Down Expand Up @@ -99,7 +100,9 @@ public Task Init(INethermindApi api)
ArgumentNullException.ThrowIfNull(_api.BlockTree);
ArgumentNullException.ThrowIfNull(_api.EthereumEcdsa);

_api.PoSSwitcher = AlwaysPoS.Instance;
ArgumentNullException.ThrowIfNull(_api.SpecProvider);

_api.PoSSwitcher = new OptimismPoSSwitcher(_api.SpecProvider, _api.ChainSpec.Optimism.BedrockBlockNumber);

_blockCacheService = new BlockCacheService();
_api.EthereumEcdsa = new OptimismEthereumEcdsa(_api.EthereumEcdsa);
Expand Down Expand Up @@ -133,6 +136,7 @@ public Task InitSynchronization()
ArgumentNullException.ThrowIfNull(_api.SyncPeerPool);
ArgumentNullException.ThrowIfNull(_api.NodeStatsManager);
ArgumentNullException.ThrowIfNull(_api.BlockchainProcessor);
ArgumentNullException.ThrowIfNull(_api.BetterPeerStrategy);

ArgumentNullException.ThrowIfNull(_blockCacheService);
ArgumentNullException.ThrowIfNull(_invalidChainTracker);
Expand All @@ -144,7 +148,7 @@ public Task InitSynchronization()

_beaconPivot = new BeaconPivot(_syncConfig, _api.DbProvider.MetadataDb, _api.BlockTree, _api.PoSSwitcher, _api.LogManager);
_beaconSync = new BeaconSync(_beaconPivot, _api.BlockTree, _syncConfig, _blockCacheService, _api.PoSSwitcher, _api.LogManager);
_api.BetterPeerStrategy = new MergeBetterPeerStrategy(null!, _api.PoSSwitcher, _beaconPivot, _api.LogManager);
_api.BetterPeerStrategy = new MergeBetterPeerStrategy(_api.BetterPeerStrategy, _api.PoSSwitcher, _beaconPivot, _api.LogManager);
_api.Pivot = _beaconPivot;

MergeBlockDownloaderFactory blockDownloaderFactory = new MergeBlockDownloaderFactory(
Expand Down
38 changes: 38 additions & 0 deletions src/Nethermind/Nethermind.Optimism/OptimismPoSSwitcher.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System;
using Nethermind.Consensus;
using Nethermind.Core;
using Nethermind.Core.Crypto;
using Nethermind.Core.Specs;
using Nethermind.Int256;
using Nethermind.Optimism;

public class OptimismPoSSwitcher(ISpecProvider specProvider, long bedrockBlockNumber) : IPoSSwitcher
{
public UInt256? TerminalTotalDifficulty => specProvider.TerminalTotalDifficulty;

public UInt256? FinalTotalDifficulty => TerminalTotalDifficulty;

public bool TransitionFinished => true;

public Hash256? ConfiguredTerminalBlockHash => null;

public long? ConfiguredTerminalBlockNumber => null;

public event EventHandler TerminalBlockReached { add { } remove { } }

public void ForkchoiceUpdated(BlockHeader newHeadHash, Hash256 finalizedHash) { }

public (bool IsTerminal, bool IsPostMerge) GetBlockConsensusInfo(BlockHeader header)
{
return (header.Number == bedrockBlockNumber - 1, header.IsPostMerge = header.Number >= bedrockBlockNumber);
}

public bool HasEverReachedTerminalBlock() => true;

public bool IsPostMerge(BlockHeader header) => GetBlockConsensusInfo(header).IsPostMerge;

public bool TryUpdateTerminalBlock(BlockHeader header)
{
throw new NotImplementedException("Should never be called in OP Stack chains");
}
}
10 changes: 2 additions & 8 deletions src/Nethermind/Nethermind.Runner/configs/op-mainnet.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"FastSyncCatchUpHeightDelta": "10000000000",
"PivotNumber": 126270000,
"PivotHash": "0xd3404c9404e47cbbcd688c1620119495cad1dab37631b556cbbb6d4ce0cea54f",
"PivotTotalDifficulty": "0",
"PivotTotalDifficulty": "210470125",
"MaxAttemptsToUpdatePivot": 0
},
"Discovery": {
Expand All @@ -39,13 +39,7 @@
"Merge": {
"Enabled": true
},
"Snapshot": {
"Enabled": true,
"DownloadUrl": "http://optimism-snapshot.nethermind.io/op-mainnet-genesis-v2.zip",
"SnapshotFileName": "op-mainnet-genesis-v2.zip",
"Checksum": "0x9b08eeb974c46f257c8bca812b119849590d92975ac86e4215d0d5647c26b147"
},
"Optimism": {
"SequencerUrl": "https://mainnet-sequencer.optimism.io"
}
}
}
Loading