add validation module and snap sync config#365
Conversation
tanishqjasoria
force-pushed
the
sync/snap-config
branch
from
71732ca to
ae5c8d8
Compare
tanishqjasoria
requested review from
damian-orzechowski,
hudem1 and
wurdum
as code owners
tanishqjasoria
requested review from
AnkushinDaniil and
svlachakis
as code owners
There was a problem hiding this comment.
Pull request overview
This PR adds validation modules for Arbitrum-specific transaction types and configures snap sync settings for the Arbitrum Sepolia network. The changes enable proper validation of custom Arbitrum transaction types (internal, retryable, deposit, unsigned, and contract transactions) and set up snap sync parameters to improve synchronization performance.
Key Changes:
- Added transaction validators for 6 Arbitrum-specific transaction types that currently accept all transactions
- Added
ArbitrumHeaderValidatorthat relaxes EIP-1559 validation and allows equal timestamps between blocks - Configured snap sync parameters (
HeaderStateDistance,StateMaxDistanceFromHead,SnapServingMaxDepth) across Arbitrum Sepolia configs - Added genesis configuration support via hex-encoded config string and initial L1 base fee
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Nethermind.Arbitrum/Execution/Transactions/ArbitrumTxValidator.cs | Added 6 transaction validator classes for Arbitrum-specific transaction types, all returning success without validation |
| src/Nethermind.Arbitrum/Execution/ArbitrumHeaderValidator.cs | Added header validator that bypasses EIP-1559 validation and allows non-decreasing timestamps |
| src/Nethermind.Arbitrum/ArbitrumPlugin.cs | Registered all transaction validators for custom Arbitrum transaction types and header validator |
| src/Nethermind.Arbitrum/Config/IArbitrumConfig.cs | Added GenesisConfig and InitialL1BaseFee config properties, updated BlockProcessingTimeout description to milliseconds |
| src/Nethermind.Arbitrum/Config/ArbitrumConfig.cs | Implemented new config properties with default values |
| src/Nethermind.Arbitrum/ArbitrumRpcModuleFactory.cs | Added genesis initialization via DigestInitMessage when GenesisConfig is provided |
| src/Nethermind.Arbitrum/Properties/configs/arbitrum-sepolia.json | Configured snap sync parameters, updated pruning boundary, added genesis config hex and initial L1 base fee, reduced FastSyncCatchUpHeightDelta |
| src/Nethermind.Arbitrum/Properties/configs/arbitrum-sepolia-archive.json | Added snap sync configuration matching the non-archive config |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
@tanishqjasoria I've opened a new pull request, #439, to work on those changes. Once the pull request is ready, I'll request review from you. |
| public sealed class ArbitrumSubmitRetryableTxValidator: ITxValidator | ||
| { | ||
| public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) | ||
| { | ||
| return ValidationResult.Success; | ||
| } | ||
| } | ||
|
|
||
| public sealed class ArbitrumRetryTxValidator: ITxValidator | ||
| { | ||
| public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) | ||
| { | ||
| return ValidationResult.Success; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
we might add some conditions here, so this acts as more of a placeholder for now.
|
|
||
| namespace Nethermind.Arbitrum.Execution; | ||
|
|
||
| public class ArbitrumHeaderValidator(IBlockTree? blockTree, ISealValidator? sealValidator, ISpecProvider? specProvider, ILogManager? logManager) : HeaderValidator(blockTree, sealValidator, specProvider, logManager) |
There was a problem hiding this comment.
You could consider decorator, probably both are fine
There was a problem hiding this comment.
but then i would have to implement the IHeaderValidator.Validate function, here i am just overiding two functions
| "Blocks": { | ||
| "PreWarmStateOnBlockProcessing": false, | ||
| "BuildBlocksOnMainState": true | ||
| "BuildBlocksOnMainState": false |
There was a problem hiding this comment.
What's the reason for disabling this? Is there a conflict here with serving snap data?
There was a problem hiding this comment.
yes, this was causing some issues - i think we discussed this as well before - something to do with generting wrong snapshot for syncing
| { | ||
| ILogger logger = logManager.GetClassLogger<ArbitrumRpcModule>(); | ||
| if (logger.IsInfo) | ||
| logger.Info($"Block hash verification enabled: verify every {verifyBlockHashConfig.VerifyEveryNBlocks} blocks, url={verifyBlockHashConfig.ArbNodeRpcUrl}"); |
There was a problem hiding this comment.
Requires merge or rebasing as this change has already been merged
There was a problem hiding this comment.
which change exactly? this is just moving this part under the else statement.
| logManager, cachedL1PriceData, processingQueue, arbitrumConfig, verifyBlockHashConfig, jsonSerializer, blocksConfig, processExitSource); | ||
| } | ||
|
|
||
| if (arbitrumConfig.GenesisConfig is not null) |
Minimum allowed line rate is |
requires: NethermindEth/nethermind#9884