perf(state-transition): add caches for indexed pending deposits by pubkey and verified counts#9247
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces a naive pending validator cache with a more robust indexing system in EpochCache to optimize deposit processing. It introduces pendingDepositPubkeyIndex and pendingValidatorVerifiedCount to track deposits and their signature validity. Feedback focuses on performance optimizations, including removing redundant type casting and hex conversions in processDepositRequest, caching hex representations of public keys to avoid repeated conversions during index initialization, and exploring more efficient cloning strategies for the new cache maps.
| const stateGloas = state as CachedBeaconStateGloas; | ||
| const pubkeyHex = toPubkeyHex(pubkey); |
There was a problem hiding this comment.
The variables stateGloas and pubkeyHex are already defined and computed in the previous if (fork >= ForkSeq.gloas) block (lines 89-90). You should move their definitions to the function scope or merge the logic to avoid redundant type casting and hex conversion, which can be expensive when processing many deposit requests.
| const stateGloas = state as CachedBeaconStateGloas; | |
| const pubkeyHex = toPubkeyHex(pubkey); | |
| const stateGloas = state as CachedBeaconStateGloas; | |
| const pubkeyHex = toPubkeyHex(pubkey); | |
| const pendingDepositPubkeyIndex = ensurePendingDepositPubkeyIndex(stateGloas); | |
| const newPendingDepositIndex = state.pendingDeposits.length - 1; | |
| const bucket = pendingDepositPubkeyIndex.get(pubkeyHex); | |
| if (bucket) { | |
| bucket.push(newPendingDepositIndex); | |
| } else { | |
| pendingDepositPubkeyIndex.set(pubkeyHex, [newPendingDepositIndex]); | |
| } | |
| const verifiedPendingValidatorCount = getPendingValidatorVerifiedCount(stateGloas); | |
| const cachedCount = verifiedPendingValidatorCount.get(pubkeyHex); | |
| if ( | |
| cachedCount !== undefined && | |
| isValidDepositSignature(state.config, pubkey, withdrawalCredentials, amount, signature) | |
| ) { | |
| verifiedPendingValidatorCount.set(pubkeyHex, cachedCount + 1); | |
| } |
| for (let i = 0; i < state.pendingDeposits.length; i++) { | ||
| const pubkeyHex = toPubkeyHex(state.pendingDeposits.getReadonly(i).pubkey); |
There was a problem hiding this comment.
Iterating over all pending deposits and converting every pubkey to hex can be a performance bottleneck if the queue is large. While this is only done when the cache is initialized, consider if the hex representation can be cached or if PendingDeposit can store it to avoid repeated toPubkeyHex calls.
| pendingDepositPubkeyIndex: | ||
| this.pendingDepositPubkeyIndex !== null | ||
| ? new Map( | ||
| Array.from(this.pendingDepositPubkeyIndex, ([pubkeyHex, indexes]) => [pubkeyHex, [...indexes]] as const) | ||
| ) | ||
| : null, |
There was a problem hiding this comment.
Deep cloning the pendingDepositPubkeyIndex map and all its internal arrays on every state clone (which happens per block) might be expensive if there are many pending deposits. While necessary for state isolation since the arrays are mutated in-place, consider if a more efficient copy-on-write structure or a shallow copy with lazy array cloning could be used if performance becomes an issue.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c9b7b4e65
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pendingDepositPubkeyIndex: | ||
| this.pendingDepositPubkeyIndex !== null | ||
| ? new Map( | ||
| Array.from(this.pendingDepositPubkeyIndex, ([pubkeyHex, indexes]) => [pubkeyHex, [...indexes]] as const) | ||
| ) |
There was a problem hiding this comment.
EpochCache.clone() now deep-copies pendingDepositPubkeyIndex for every state.clone(), but state.clone() is on the hot path for each block/slot transition (via getCachedBeaconState().clone() calling epochCtx.clone()). Once this cache is initialized, clone cost becomes O(number of pending deposits) even when no deposit requests are being processed, which can significantly slow production block processing with large pending queues. This cache should be rebuilt lazily in the envelope path (or copied copy-on-write), not eagerly copied in every generic clone.
Useful? React with 👍 / 👎.
1 participant
Motivation
Gloas deposit request processing needs to determine whether a pubkey already has a valid pending validator deposit. The previous approach requires scanning
state.pendingDepositsand BLS-verifying every pending deposit signature to build a global set of pending validator pubkeys . The pr reeduces the overhead by caching pubkey-indexed pending deposit lookups and verified counts on the epoch cache.Description
This PR replaces the eager pending-validator pubkey lookup in Gloas deposit request processing with two caches stored on state.epochCtx:
With these caches, lodestar can jump directly to the pending deposits for one pubkey and verify only those entries instead of scanning and verifying the full queue. The caches are updated when new pending deposits are appended.
Addresses #9181
AI Assistance Disclosure
Claude was used for this optimisation