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

Pruning pending deposits in deposit cache for post-Electra(EIP-6110) #14698

Open
syjn99 opened this issue Dec 8, 2024 · 0 comments
Open

Pruning pending deposits in deposit cache for post-Electra(EIP-6110) #14698

syjn99 opened this issue Dec 8, 2024 · 0 comments

Comments

@syjn99
Copy link
Contributor

syjn99 commented Dec 8, 2024

💎 Issue

Background

As EIP-6110 is included in the upcoming upgrade(Electra), deposits are directly bridged from EL to CL via execution requests(EIP-7685). Thus, the implementation of EIP-4881 only matters when providing a deposit snapshot via RPC(Let me know if there's other things to consider).

When a block producer builds a block, packDepositsAndAttestations will always return empty deposit list from local when EIP-6110 is fully enabled. Returning early can help for reducing the total building time in this case, so I already worked on this issue at PR #14697.

Description

However, I found we need to tackle with the pending deposits which are managed by DepositCache. (NOTE: This pending deposit is not equal to the new field introduced in Electra) As eth1_deposit_index will be anchored to the certain value, insertFinalizedDeposits will never finalize the included deposits after EIP-6110 applied.

func (s *Service) insertFinalizedDeposits(ctx context.Context, fRoot [32]byte) {
ctx, span := trace.StartSpan(ctx, "blockChain.insertFinalizedDeposits")
defer span.End()
startTime := time.Now()
// Update deposit cache.
finalizedState, err := s.cfg.StateGen.StateByRoot(ctx, fRoot)
if err != nil {
log.WithError(err).Error("could not fetch finalized state")
return
}
// We update the cache up to the last deposit index in the finalized block's state.
// We can be confident that these deposits will be included in some block
// because the Eth1 follow distance makes such long-range reorgs extremely unlikely.
eth1DepositIndex, err := mathutil.Int(finalizedState.Eth1DepositIndex())
if err != nil {
log.WithError(err).Error("could not cast eth1 deposit index")
return
}
// The deposit index in the state is always the index of the next deposit
// to be included(rather than the last one to be processed). This was most likely
// done as the state cannot represent signed integers.
finalizedEth1DepIdx := eth1DepositIndex - 1
if err = s.cfg.DepositCache.InsertFinalizedDeposits(ctx, int64(finalizedEth1DepIdx), common.Hash(finalizedState.Eth1Data().BlockHash),
0 /* Setting a zero value as we have no access to block height */); err != nil {
log.WithError(err).Error("could not insert finalized deposits")
return
}
// Deposit proofs are only used during state transition and can be safely removed to save space.
if err = s.cfg.DepositCache.PruneProofs(ctx, int64(finalizedEth1DepIdx)); err != nil {
log.WithError(err).Error("could not prune deposit proofs")
}
// Prune deposits which have already been finalized, the below method prunes all pending deposits (non-inclusive) up
// to the provided eth1 deposit index.
s.cfg.DepositCache.PrunePendingDeposits(ctx, int64(eth1DepositIndex)) // lint:ignore uintcast -- Deposit index should not exceed int64 in your lifetime.
log.WithField("duration", time.Since(startTime).String()).Debugf("Finalized deposit insertion completed at index %d", finalizedEth1DepIdx)
}

As a result, pending deposits are never pruned by this logic, leading the heap memory allocation will never be freed. I had investigated this issue for a couple of hours, but it seems there needs a way to notify the "finalized" deposit index to the deposit cache to prune the pending deposits.

func processDepositRequest(beaconState state.BeaconState, request *enginev1.DepositRequest) (state.BeaconState, error) {
requestsStartIndex, err := beaconState.DepositRequestsStartIndex()
if err != nil {
return nil, errors.Wrap(err, "could not get deposit requests start index")
}
if requestsStartIndex == params.BeaconConfig().UnsetDepositRequestsStartIndex {
if request == nil {
return nil, errors.New("nil deposit request")
}
if err := beaconState.SetDepositRequestsStartIndex(request.Index); err != nil {
return nil, errors.Wrap(err, "could not set deposit requests start index")
}
}
if err := beaconState.AppendPendingDeposit(&ethpb.PendingDeposit{
PublicKey: bytesutil.SafeCopyBytes(request.Pubkey),
WithdrawalCredentials: bytesutil.SafeCopyBytes(request.WithdrawalCredentials),
Amount: request.Amount,
Signature: bytesutil.SafeCopyBytes(request.Signature),
Slot: beaconState.Slot(),
}); err != nil {
return nil, errors.Wrap(err, "could not append deposit request")
}
return beaconState, nil
}

My first idea is following. Feedback is welcomed.

  1. Add a feed that notifies the last deposit index(and slot) that is included in the state.
  2. Execution service subscribes to this feed, and keeps updating the last deposit index(and slot).
  3. If the finalized checkpoint is updated, other pruning logic runs.
@syjn99 syjn99 changed the title Pruning pending deposits in deposit snapshot for post-Electra(EIP-6110) Pruning pending deposits in deposit cache for post-Electra(EIP-6110) Dec 8, 2024
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

No branches or pull requests

1 participant