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

Utilize Read Only Head State #11932

Merged
merged 8 commits into from
Jan 31, 2023
Merged

Utilize Read Only Head State #11932

merged 8 commits into from
Jan 31, 2023

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Jan 29, 2023

What type of PR is this?

Feature Addition

What does this PR do? Why is it needed?

This PR introduces a method to access a read only version of the head state without performing any copies. This is very
useful in gossip when trying to fetch the head state to validate objects from gossip. This method has been introduced as
a way to more efficiently verify BLS change messages.

Which issues(s) does this PR fix?

N.A

Other notes for review

@nisdas nisdas requested a review from a team as a code owner January 29, 2023 12:44
// HeadStateReadOnly returns the read only head state of the chain.
// If the head is nil from service struct,
// it will attempt to get the head state from DB.
func (s *Service) HeadStateReadOnly(ctx context.Context) (state.ReadOnlyBeaconState, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a trivial unit test to make sure that all paths are covered here and protect from our future selfs?

Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per discussion with @nisdas, we should block this to discuss another radical approach that is #11694

@terencechain terencechain merged commit 3f67818 into develop Jan 31, 2023
@delete-merged-branch delete-merged-branch bot deleted the readOnlyHeadState branch January 31, 2023 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High High priority item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants