Skip to content

Conversation

@calbera
Copy link
Contributor

@calbera calbera commented May 28, 2025

Super simple implementation of EIP-6110 style deposits

3 requirements for the implementation to be so simple:

  1. Given an 8192 limit of deposits per block, we need to guarantee that an EL doesn't include more than 8192 deposits in 1 block.

    • This is guaranteed with the current gas limit of 30 million.
    • 1 deposit on Berachain costs at least 45,000 gas (sample deposit)
    • 8192 deposits would cost 45,000 * 8192 = 368,640.000 gas
    • As long as gas per block stays under 368 million gas, we are good ✅
      • If we do increase gas, we can also increase the maximum amount of 8192 accordingly.
  2. Given the way deposits are taken from EL deposit contract events and then processed in CL, as is we will lose deposits from the 2 blocks right before the Electra1 fork (which starts using 6110) is activated.

    • Currently the way the system works:
      • in finalize_block N, we store any new EL deposits from EL block N-1
      • in build_block N+1, we include these new EL deposits from EL block N-1
    • With 6110:
      • in build_block N, the EL will now include the deposits events that are in EL block N itself
    • So if Electra1 activates at block N+1 (previously for this block N+1 we would have included deposits from EL block N-1). Now with new fork, we will just include for block N+1 the deposits from EL block N+1. This means EL deposits from blocks N-1 & N will be skipped 🚨
    • ⚠️ Potential workarounds include:
      • asking people to not deposit for the 5 seconds before the hard fork lol
      • adding a special case around the fork activation that retrieves the previous 2 EL blocks' deposits to specifically include them
  3. Forking on the EL to change the deposit event signature to Berachain's: 0x68af751683498a9f9be59fe8b0d52a64dd155255d85cdb29fea30b1e3f891d46 (sample deposit)

    • 1 line change in geth here
    • Change(s) in In reth here

TODOs:

  • Handle edge case around fork activation. Approach TBD
  • Unit tests for deposit requests.
  • Simulation tests for deposit requests.

@codecov
Copy link

codecov bot commented May 28, 2025

Codecov Report

Attention: Patch coverage is 41.48148% with 79 lines in your changes missing coverage. Please review.

Project coverage is 60.54%. Comparing base (87d043a) to head (fdac98b).

Files with missing lines Patch % Lines
state-transition/core/state_processor_forks.go 2.43% 40 Missing ⚠️
state-transition/core/state_processor_staking.go 61.22% 14 Missing and 5 partials ⚠️
beacon/validator/block_builder.go 45.45% 14 Missing and 4 partials ⚠️
chain/spec.go 33.33% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2794      +/-   ##
==========================================
- Coverage   60.71%   60.54%   -0.17%     
==========================================
  Files         349      349              
  Lines       16117    16183      +66     
  Branches       22       22              
==========================================
+ Hits         9785     9798      +13     
- Misses       5586     5638      +52     
- Partials      746      747       +1     
Files with missing lines Coverage Δ
beacon/blockchain/finalize_block.go 69.84% <100.00%> (-0.24%) ⬇️
config/spec/mainnet.go 100.00% <100.00%> (ø)
state-transition/core/validation_deposits.go 50.00% <ø> (ø)
chain/spec.go 73.77% <33.33%> (-1.02%) ⬇️
beacon/validator/block_builder.go 62.06% <45.45%> (-1.47%) ⬇️
state-transition/core/state_processor_staking.go 61.49% <61.22%> (-0.22%) ⬇️
state-transition/core/state_processor_forks.go 64.67% <2.43%> (-18.41%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@calbera calbera changed the title [do not merge] wip 6110 poc [do not merge] wip electra 1 fork for 6110 May 29, 2025
@calbera calbera changed the title [do not merge] wip electra 1 fork for 6110 [do not merge] wip electra1 fork for 6110 May 29, 2025
requests, err := blk.GetBody().GetExecutionRequests()
if err != nil {
// EIP-6110 Deposits.
for _, dep := range requests.Deposits {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the biggest change is here: Post this fork, we will just accept the deposits from here as we have instant finality. No deposits queue or epoch processing is necessary here; real-time instant deposits very easily done

@rezbera
Copy link
Contributor

rezbera commented May 29, 2025

Supportive of this approach. Especially since its <200 line diff and gets rid of the CL querying the EL which isn't a great pattern.

Agreed that the Block Gas Limit is a good enough protection from hitting the deposit queue. It's unlikely we'll be x10'ing the gas limit arbitrarily in the coming months as that's not a bottleneck.

As you noted, the EL change is trivial.

There is a point to be noted that we deviate from the spec by removing the deposit queue altogether, which is present in the spec. In the spec, deposit processing is done per epoch, whereas we do it per block. This allows us to avoid any BeaconState changes. IMO the processing overhead of deposits is low enough that it doesn't need to be batched into an epoch (don't have empirical data to support this tho).

Unsure on the solution for the overlap period between old system and new. Perhaps it's just a custom state transition as part of upgradeToElectra1

@calbera
Copy link
Contributor Author

calbera commented Jun 2, 2025

There is a point to be noted that we deviate from the spec by removing the deposit queue altogether, which is present in the spec. In the spec, deposit processing is done per epoch, whereas we do it per block. This allows us to avoid any BeaconState changes. IMO the processing overhead of deposits is low enough that it doesn't need to be batched into an epoch (don't have empirical data to support this tho).

It does "deviate from spec" but not any more than we already are "deviated from the spec". Currently also deposits are processed block by block and the state transition operation for each is not too expensive (signatures are not verified on repeat -- most -- deposits). This change only affects the "lag" at which deposits are processed but not the frequency of processing itself.

@calbera calbera changed the title [do not merge] wip electra1 fork for 6110 [WIP] feat(deposits): Introduce Electra1 fork for EIP-6110 style processing Jun 3, 2025
@calbera calbera requested a review from a team June 3, 2025 04:29
@calbera calbera linked an issue Jun 3, 2025 that may be closed by this pull request
@calbera calbera requested a review from rezbera June 3, 2025 05:14
@abi87
Copy link
Contributor

abi87 commented Jun 5, 2025

I don't think the risk of loosing deposits is acceptable, nor like the idea of adding code to patch it if it happens.
I like this proposal as an end state, but I believe the right way to activate it is to make deposits fail at the EL level for the blocks around fork activation (which then can be timestamp based).
This way there are no loss of founds possible (just some gas), no code to be added to CL and possibly the deposits revert may be even remove post fork activation if no deposits are done in that window.

@calbera
Copy link
Contributor Author

calbera commented Aug 31, 2025

I don't think the risk of loosing deposits is acceptable, nor like the idea of adding code to patch it if it happens. I like this proposal as an end state, but I believe the right way to activate it is to make deposits fail at the EL level for the blocks around fork activation (which then can be timestamp based). This way there are no loss of founds possible (just some gas), no code to be added to CL and possibly the deposits revert may be even remove post fork activation if no deposits are done in that window.

Making deposits at the EL level will require specific opinionated code, potentially in the EVM. This would defeat the purpose of not adding code to the CL as we'd have to manage this regardless. Will work on adding the special state transition to catch up the 2 blocks deposits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EIP-6110: Supply validator deposits on chain

4 participants