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

feature: Add support for block validation (flashbots_validateBuilderSubmissionV3) #7335

Open
wants to merge 56 commits into
base: master
Choose a base branch
from
Open

feature: Add support for block validation (flashbots_validateBuilderSubmissionV3) #7335

wants to merge 56 commits into from

Conversation

rjnrohit
Copy link
Contributor

@rjnrohit rjnrohit commented Aug 16, 2024 •
edited
Loading

Closes #7462

This PR aims to implement flashbots endpoint (flashbots_validateBuilderSubmissionV3) ref: https://github.com/flashbots/builder/blob/df9c765067d57ab4b2d0ad39dbb156cbe4965778/eth/block-validation/api.go#L225-L246

  1. It validates the sent payload's block, blobs and verifies if the ParentHash, BlockHash, GasLimit, and GasUsed in the params.Message match the sent payload and produce an error otherwise.
  2. It tries to imitate block processing step and also process withdrawals
  3. It tries to validate proposer payment using Txns & receipts generated in the block processing step

Changes

It introduces a new JsonRpc plugin called Nethermind.Flashbots which implements a new endpoint called flashbots_validateBuilderSubmissionV3

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

I'll write the tests once I'm done with manual testing

@rjnrohit rjnrohit force-pushed the feature/flashbots_endpoints branch from cbd73ae to 9eb6f70 Compare August 16, 2024 07:51
@rjnrohit rjnrohit marked this pull request as ready for review September 5, 2024 12:36
@rjnrohit rjnrohit requested a review from rubo as a code owner September 5, 2024 12:36
LukaszRozmej
LukaszRozmej requested changes Sep 9, 2024
View reviewed changes
src/Nethermind/Nethermind.Consensus/Processing/ReadOnlyTxProcessingEnv.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.BlockValidation/BlockValidation.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.BlockValidation/IFlashbotsRpcModule.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Consensus/IGasLimitCalculator.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.BlockValidation/Handlers/ValidateBuilderSubmissionHandler.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.BlockValidation/Handlers/ValidateBuilderSubmissionHandler.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.BlockValidation/Handlers/ValidateBuilderSubmissionHandler.cs Outdated
Comment on lines 126 to 130
if (!_blockTree.IsBetterThanHead(block.Header))
{
error = $"Block {block.Header.Hash} is not better than head";
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

We can validate only on future blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it seems from this snippet: https://github.com/flashbots/builder/blob/df9c765067d57ab4b2d0ad39dbb156cbe4965778/core/blockchain.go#L2464

definition at: https://github.com/flashbots/builder/blob/df9c765067d57ab4b2d0ad39dbb156cbe4965778/core/forkchoice.go#L77

src/Nethermind/Nethermind.BlockValidation/Handlers/ValidateBuilderSubmissionHandler.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.BlockValidation/Handlers/ValidateBuilderSubmissionHandler.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.BlockValidation/Handlers/ValidateBuilderSubmissionHandler.cs Outdated
Comment on lines 322 to 326
if (block is not null && !ValidateBlobsBundle(block.Transactions, blobsBundle, out string? blobsError))
{
if (_logger.IsWarn) _logger.Warn($"Invalid blobs bundle. Result of {payloadStr}. Error: {blobsError}");
return BlockValidationResult.Invalid(blobsError ?? "Blobs bundle validation failed");
}
Copy link
Member

Choose a reason for hiding this comment

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

A but weird that we do validations after processing a block, shouldn't we do it first? (It would be less costly)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/flashbots/builder/blob/df9c765067d57ab4b2d0ad39dbb156cbe4965778/eth/block-validation/api.go#L240

I followed the same order as they did, but yeah I agree doing this way would be costly

Copy link
Member

Choose a reason for hiding this comment

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

W should consider changing it later, maybe @Ruteri knows why this is the order of checks?

rjnrohit and others added 14 commits September 12, 2024 15:24
@rjnrohit
address PR comments
ffc25f0
@LukaszRozmej
Hookup BlockValidation plugin in Nethermind.Runner
1bebf60
@core-repository-dispatch-app @LukaszRozmej @rjnrohit
Update Fast Sync configuration in Nethermind repository (#7401)
570dce0 
Co-authored-by: LukaszRozmej <LukaszRozmej@users.noreply.github.com>
@LukaszRozmej @MarekM25
@smartprogrammer93 @rjnrohit
Feature/geth like system tx (#7252)
bec7a5d 
Co-authored-by: MarekM25 <marekm2504@gmail.com>
Co-authored-by: Ahmad Bitar <smartprogrammer@windowslive.com>
@Arindam2407 @LukaszRozmej
@marcindsobczak @rjnrohit
Heuristic tx censorship detection (#7259)
8b8cd01 
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Marcin Sobczak <marcindsobczak@gmail.com>
@emlautarom1 @LukaszRozmej
@jmederosalvarado @rjnrohit
Refactor TxValidator (#7386)
69e36dd 
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Jorge Mederos <46798594+jmederosalvarado@users.noreply.github.com>
@MarekM25 @rjnrohit
Moved BlockProductionTimeout to config, increased BlockProductionTime…
17533eb 
…out (#7378)
@benaadams @rjnrohit
Reduce memory use during Archive Sync (#7407)
c1f9455
@MarekM25 @rjnrohit
Update Supported Networks in Readme (#7412)
5aebe9b
@benaadams @rjnrohit
Change Windows native Allocator (#7418)
37b9fb4
@benaadams @rjnrohit
Fast address equals (#7417)
4a17264
@benaadams @rjnrohit
Reduce backlog threshold for GC (#7415)
413a5b3
@kamilchodola @rjnrohit
Bump unstable to 1.29.0 (#7416)
bb3753d
@rjnrohit
enable flashBots module
4373e5a
@rjnrohit rjnrohit requested a review from MarekM25 as a code owner September 12, 2024 16:36
@rjnrohit rjnrohit requested a review from a team as a code owner October 10, 2024 07:22
@rjnrohit rjnrohit force-pushed the feature/flashbots_endpoints branch from afc3ecd to 4eeeb9f Compare October 10, 2024 09:45
@jmederosalvarado jmederosalvarado self-requested a review October 10, 2024 17:30
@jmederosalvarado jmederosalvarado removed their request for review November 25, 2024 14:50
@LukaszRozmej LukaszRozmej mentioned this pull request Dec 23, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LukaszRozmej LukaszRozmej LukaszRozmej requested changes

@rubo rubo Awaiting requested review from rubo rubo is a code owner

@MarekM25 MarekM25 Awaiting requested review from MarekM25

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement flashbots endpoint (flashbots_validateBuilderSubmissionV3)
7 participants
@rjnrohit @LukaszRozmej @Arindam2407 @emlautarom1 @MarekM25 @benaadams @kamilchodola