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

Add electra DB #13975

Merged
merged 4 commits into from
May 9, 2024
Merged

Add electra DB #13975

merged 4 commits into from
May 9, 2024

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented May 9, 2024

This PR adds electra changes to db. Mainly two things and a note
1.) Add electra block support
2.) Add electra state support
3.) Refactor saveStatesEfficientInternal into multiple helpers. The functionality remains the same. It needs to be done to reduce cognitive complexity or CI will be red

@terencechain terencechain added the Ready For Review A pull request ready for code review label May 9, 2024
@terencechain terencechain requested a review from a team as a code owner May 9, 2024 15:24
@terencechain terencechain force-pushed the electra-db branch 2 times, most recently from a5711b4 to 87a64f9 Compare May 9, 2024 16:03
james-prysm
james-prysm previously approved these changes May 9, 2024
Copy link
Contributor

@james-prysm james-prysm left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -51,3 +51,17 @@ func hasDenebBlindKey(enc []byte) bool {
}
return bytes.Equal(enc[:len(denebBlindKey)], denebBlindKey)
}

func hasElectraKey(enc []byte) bool {
if len(electraKey) >= len(enc) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't really need to do this, it would be fine to just call bytes.Equal.

Copy link
Member

Choose a reason for hiding this comment

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

@kasey it is required or a runtime error for slice bounds out of range will happen when len(enc) is less than len(electraKey)

Copy link
Member

@prestonvanloon prestonvanloon May 9, 2024

Choose a reason for hiding this comment

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

You are thinking of bytes.Prefix, i think. It would be safe to call bytes.HasPrefix without this check

https://pkg.go.dev/bytes?utm_source=godoc#HasPrefix

func hasElectraKey(enc []byte) bool {
  return bytes.hasPrefix(enc, electraKey)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, this is basically replicating bytes.HasPrefix.

kasey
kasey previously approved these changes May 9, 2024
@terencechain terencechain dismissed stale reviews from kasey and james-prysm via ff65896 May 9, 2024 20:35
@terencechain terencechain added this pull request to the merge queue May 9, 2024
Merged via the queue into develop with commit 102128c May 9, 2024
16 of 17 checks passed
@terencechain terencechain deleted the electra-db branch May 9, 2024 21:55
ErnestK pushed a commit to ErnestK/prysm that referenced this pull request May 19, 2024
* Add electra DB

* Fix typo

* Revert deep ssz change

---------

Co-authored-by: james-prysm <90280386+james-prysm@users.noreply.github.com>
nisdas pushed a commit that referenced this pull request Jul 4, 2024
* Add electra DB

* Fix typo

* Revert deep ssz change

---------

Co-authored-by: james-prysm <90280386+james-prysm@users.noreply.github.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants