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
Checkpoint balances cache #4290
Conversation
Performance Report✔️ no performance regression detected Full benchmark results
|
const checkpointSlot = computeStartSlotAtEpoch(checkpoint.epoch); | ||
const head = this.forkChoice.getHead(); | ||
// Find a state in the same branch of checkpoint at same epoch. Balances should exactly the same | ||
for (const descendantBlock of this.forkChoice.forwardIterateDescendants(checkpoint.rootHex)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we consume forwardIterateDescendants
only once to improve performance a bit? either extract to an array, or that function could return an array itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's very unlikely to hit this code path I would not do that for simplicity. Metrics will clearly show if we have to run the iterator twice too often, only then optimize
const blockDelaySec = (Math.floor(Date.now() / 1000) - postState.genesisTime) % chain.config.SECONDS_PER_SLOT; | ||
const blockRoot = toHexString(chain.config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message)); | ||
// Should compute checkpoint balances before forkchoice.onBlock | ||
chain.checkpointBalancesCache.processState(blockRoot, postState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling fc_store.onVerifiedBlock() in a similar pattern as lighthouse sounds better than here.
Motivation
Right now whenever we provide a justified checkpoint to forkchoice, we provide balances too but forkchoice may switch justified checkpoint so we want to provide a way to cache balances from checkpoints
Description
justifiedBalancesGetter
function: