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

[Feature] - Store Only Blinded Beacon Blocks Post-Merge #11010

Merged
merged 51 commits into from
Jul 13, 2022

Conversation

rauljordan
Copy link
Contributor

@rauljordan rauljordan commented Jul 8, 2022

What type of PR is this?

Feature

What does this PR do? Why is it needed?

In a design document here, we outline how important it is that Prysm only stores blinded beacon blocks post-merge in the database.

if we do not have this feature, we foresee the storage requirements of Prysm growing significantly over time. Today, the avg Ethereum block size is 82Kb. Assuming we get a block every 10 seconds, this means 492Kb per minute, or 29.52 Mb per hour. Given there are 8760 hours per year, this could amount to 8760*29.52Mb = 258Gb per year that needs to be stored in Geth and in Prysm. It is unreasonable to store this same data twice, which is why this feature is important for us to ship.

https://etherscan.io/chart/blocksize see here for avg blocksize data at the time of writing, on July 5th, 2022.

This PR adds a feature flag --enable-only-blinded-beacon-blocks which stores Bellatrix beacon blocks as blinded format in our database. This will change our behavior with p2p and API requests to reconstruct full Bellatrix blocks from blinded format.

Which issues(s) does this PR fix?

Fixes #10589

@rauljordan rauljordan marked this pull request as ready for review July 8, 2022 16:34
@rauljordan rauljordan requested a review from a team as a code owner July 8, 2022 16:34
@rauljordan rauljordan added the Priority: High High priority item label Jul 8, 2022
@@ -537,6 +537,27 @@ func (s *Service) InsertSlashingsToForkChoiceStore(ctx context.Context, slashing
}
}

func getBlockPayloadHash(blk interfaces.BeaconBlock) ([32]byte, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function needed edits to be future-proof. That is, it should be able to deal with blocks that have an execution payload or a payload header (blinded blocks)

@rauljordan rauljordan added the Blocked Blocked by research or external factors label Jul 8, 2022
@rauljordan
Copy link
Contributor Author

Some optimistic sync scenarios break here

beacon-chain/blockchain/execution_engine.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/process_block.go Outdated Show resolved Hide resolved
@@ -68,7 +68,7 @@ type EngineCaller interface {
ExchangeTransitionConfiguration(
ctx context.Context, cfg *pb.TransitionConfiguration,
) error
ExecutionBlockByHash(ctx context.Context, hash common.Hash) (*pb.ExecutionBlock, error)
ExecutionBlockByHash(ctx context.Context, hash common.Hash, withTxs bool) (*pb.ExecutionBlock, error)
Copy link
Member

Choose a reason for hiding this comment

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

I know we don't like bool flag as argument but I think it's fine here

config/features/config.go Outdated Show resolved Hide resolved
@rauljordan rauljordan removed the Blocked Blocked by research or external factors label Jul 13, 2022
@@ -123,6 +121,10 @@ var (
Name: "enable-gossip-batch-aggregation",
Usage: "Enables new methods to further aggregate our gossip batches before verifying them.",
}
EnableOnlyBlindedBeaconBlocks = &cli.BoolFlag{
Copy link
Member

Choose a reason for hiding this comment

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

Can this not be exported?

Suggested change
EnableOnlyBlindedBeaconBlocks = &cli.BoolFlag{
enableOnlyBlindedBeaconBlocks = &cli.BoolFlag{

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@terencechain it's used in a DB method that tells the user via a log to remove the flag if they want to resync full blocks. Without exporting it, i would have to hardcode the flag name as a string, which can change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High High priority item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate execution payloads in the DB
3 participants