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

Improve cache efficiency of attestation checks #2677

Closed
arnetheduck opened this issue Jun 25, 2021 · 1 comment
Closed

Improve cache efficiency of attestation checks #2677

arnetheduck opened this issue Jun 25, 2021 · 1 comment
Milestone

Comments

@arnetheduck
Copy link
Member

Attestation checks currently use an EpochRef to validate the committee, based on the target vote:

epochRef = pool.dag.getEpochRef(target, attestation.data.target.epoch)

During periods of instability, it is often the case that the target vote is off, specially close to epoch transitions - in particular, clients might miss the last few blocks in the epoch and vote for a target with skipped slots - when we use the target directly, it means we must replay the state with the skipped slots in order to recreate the shuffling needed to validate the attestation in the context it was created.

However, the shuffling is already fixed one epoch prior - it would be possible to instead rely on (target.blck.atSlot(target.epoch-2).getLastSlotInEpoch()) - this is sometimes called the dependent root - and extract a lookahead shuffling from that state instead.

Relying on this older epoch will filter out a lot of the noise because we'll be looking at older data - any off-by-one or off-by-two-blocks votes will no longer cause replays.

The lookahead (epoch + 1) shuffling is normally not computed - EpochRef stores the shuffling for EpochRef.epoch - notably, we can only precompute the shuffling this way - we can't fill in the other EpochRef fields.

This means we must either extend EpochRef to also hold the shuffling for the next epoch or split out the shuffling into a separate cache.

An instance of this kind of cache inefficiency can be seen in #2675 where a series of poorly formed attestations led to a period of heightened memory usage that could have been avoided with the above scheme.

@arnetheduck
Copy link
Member Author

One way I see this working is that we have a seed -> shuffling cache and store the seed in EpochRef - alternatively, we store the shuffling as a ref in EpochRef.

With increased load on the REST API for duty lookahead, this cache increasingly becomes important

arnetheduck added a commit that referenced this issue Aug 18, 2022
In order to avoid full replays when validating attestations hailing from
untaken forks, it's better to keep shufflings separate from `EpochRef`
and perform a lookahead on the shuffling when processing the block that
determines them.

This also helps performance in the case where REST clients are trying to
perform lookahead on attestation duties and decreases memory usage by
sharing shufflings between EpochRef instances of the same dependent
root.
@zah zah closed this as completed in 0d9fd54 Aug 22, 2022
@zah zah added this to the v22.8.0 milestone Aug 23, 2022
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

2 participants