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

Find an approach to consolidate the existing validator Map with the new finalized validator index cache #14436

Closed
james-prysm opened this issue Sep 10, 2024 · 2 comments · Fixed by #14497
Labels
Cleanup Code health! Electra electra hardfork Tracking Gotta Catch 'Em All

Comments

@james-prysm
Copy link
Contributor

💎 Issue

Background

Changes introduced by EIP6110 breaks the existing validator index invariant and consensus layer clients will have to deal with a fact that a validator index becomes fork dependent, i.e. a validator with the same pubkey can have different indexes in different block tree branches.

To solve this, we introduced a new cache that saves the finalized public key to validator index with the usage of it's search laid above the existing validator map.

related PRS:#13943 ,#14146 ,#14173

Description

This puts us in the predicament of maintaining 2 different caches for validators, because we want:

  1. To look up based on the point of reference state’s point of view ( the existing validator map that is not reorg safe AKA valMapHandler)
  2. To look up based on the canonical chain (new finalized validator index cache)

Ideally we somehow merge these 2 caches or have an elegant way of maintaining outside of the beacon state because of the following statement:

Detailed analysis shows that process_deposit function is the only place requiring a fork dependent (pubkey, index) cache.

@james-prysm james-prysm added Tracking Gotta Catch 'Em All Cleanup Code health! Electra electra hardfork labels Sep 10, 2024
@james-prysm
Copy link
Contributor Author

may no longer be needed with ethereum/consensus-specs#3818

Finalizes deposit request position in the queue before applying it to the state. The outcome is that no new validator is created before the corresponding deposit is finalized hence no fork aware (pubkey, index) cache is required.

@james-prysm
Copy link
Contributor Author

removing it outright and falling back on the map is potentially not very efficient

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code health! Electra electra hardfork Tracking Gotta Catch 'Em All
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant