Electra: BeaconState implementation #13919

prestonvanloon · 2024-04-26T02:02:19Z

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Required for Electra.

Which issues(s) does this PR fix?

The beacon state is blocking progress on most electra PRs. #13903 #13898 #13893 #13911 #13888

Other notes for review

~~We might want to split this into smaller PRs. I split up the changes into logical commits that could be their on PRs, in order.~~

Merge the following PRs before this can be made ready for review:

beacon-chain/core/helpers/validators.go

beacon-chain/state/interfaces.go

prestonvanloon

Left some commentary from self review. This PR is ready to review and is very large so it will need more than 1 approval.

prestonvanloon · 2024-05-02T20:31:48Z

beacon-chain/core/blocks/exit_test.go

-	undo := util.HackDenebMaxuint(t)
-	defer undo()


This was renamed to HackElectraMaxuint and serves the same purpose. It's no longer needed in this particular test.

beacon-chain/state/interfaces.go

prestonvanloon · 2024-05-02T20:42:10Z

beacon-chain/state/state-native/getters_withdrawal.go


-// hasETH1WithdrawalCredential returns whether the validator has an ETH1
-// Withdrawal prefix. It assumes that the caller has a lock on the state
-func hasETH1WithdrawalCredential(val *ethpb.Validator) bool {


Moved these to helpers library

prestonvanloon · 2024-05-02T20:43:27Z

beacon-chain/state/state-native/getters_withdrawal_test.go

-	require.Equal(t, false, hasETH1WithdrawalCredential(v))
-}
+func TestExpectedWithdrawals(t *testing.T) {
+	for _, stateVersion := range []int{version.Capella, version.Deneb, version.Electra} {


I refactored TestExpectedWIthdrawals_$FORK to be a more table driven style instead of a copy and paste with the version changed.

prestonvanloon · 2024-05-02T20:44:24Z

beacon-chain/state/state-native/getters_withdrawal_test.go

+		})
+	}
+
+	t.Run("electra all pending partial withdrawals", func(t *testing.T) {


This test load a ssz test from the spec tests repo as a fully hydrated example rather than hand crafting test data.

prestonvanloon · 2024-05-02T20:45:51Z

beacon-chain/state/state-native/setters_churn_test.go

+	// This case is copied from spectest mainnet/electra/operations/voluntary_exit/pyspec_tests/exit_existing_churn_and_churn_limit_balance/pre.ssz_snappy
+	// and that case was retrieved from the consensus-spec-tests repository at tag v1.5.0-alpha.1.
+	// The spec tests shows that the exit epoch is 262 for validator 0 performing a voluntary exit.
+	serializedBytes, err := util.BazelFileBytes("testdata/electra_exit_0.ssz_snappy")


Another spec test example test data. Helped me find an off by one bug 😅

prestonvanloon · 2024-05-02T20:46:52Z

beacon-chain/state/state-native/setters_payload_header.go

@@ -21,6 +23,9 @@ func (b *BeaconState) SetLatestExecutionPayloadHeader(val interfaces.ExecutionDa

 	switch header := val.Proto().(type) {
 	case *enginev1.ExecutionPayload:
+		if b.version != version.Bellatrix {


I found it odd and bad that we could have a beacon state that accepted the wrong type of execution payload, so I added that verification for each case and the appropriate test.

prestonvanloon · 2024-05-02T20:50:21Z

beacon-chain/state/stateutil/slice_root.go

+)
+
+// SliceRoot computes the root of a slice of hashable objects.
+func SliceRoot[T ssz.Hashable](slice []T, limit uint64) ([32]byte, error) {


This is the same logic exactly from beacon-chain/state/stateutil/historical_summaries_root.go, but parameterized / generic version

prestonvanloon · 2024-05-02T20:50:45Z

beacon-chain/state/stateutil/trie_helpers.go

 func Merkleize(leaves [][]byte) [][][]byte {
 	hashFunc := hash.CustomSHA256Hasher()
 	layers := make([][][]byte, ssz.Depth(uint64(len(leaves)))+1)
-	for len(leaves) != 32 {
+	for len(leaves)%32 != 0 {


This was a bug where we would allocate infinite memory if the beacon state had more than 32 fields. Wow!

bug of the week

testing/util/electra.go

Update the electra withdrawal example with a ssz state containing pending partial withdrawals

…s.go

…ndency

…lsSweep/

…n ActiveBalanceAtIndex

prestonvanloon · 2024-05-03T19:40:30Z

All of @rkapka feedback is addressed. This PR is ready for another round of reviews

prestonvanloon · 2024-05-03T20:31:00Z

Rebased onto develop and broken by #13948. Trying to figure how how to fix most effectively.

prestonvanloon · 2024-05-04T12:29:54Z

Build is fixed as of 4ff0357

rkapka

Fantastic work!

beacon-chain/state/state-native/getters_consolidation.go

kasey · 2024-05-06T15:54:24Z

beacon-chain/state/state-native/getters_withdrawal_test.go

-			s.validators[i] = val
-		}
-		expected, err := s.ExpectedWithdrawals()
+		serializedSSZ, err := snappy.Decode(nil /* dst */, serializedBytes)


This could be a nice general test helper (get example state for fork).

If someone finds that useful, I would be happy to help move this to some shared method. I thought this was a rather specific example that I crawled the spectests repo to find.

Didn't mean that as feedback to move it btw, fine where it is.

kasey · 2024-05-06T17:57:25Z

beacon-chain/state/stateutil/slice_root.go

+)
+
+// SliceRoot computes the root of a slice of hashable objects.
+func SliceRoot[T ssz.Hashable](slice []T, limit uint64) ([32]byte, error) {


@rkapka

* Electra: Beacon State * Electra: Beacon state fixes from PR 13919 * Add missing tests - part 1 * Split eip_7251_root.go into different files and reuse/share code with historical state summaries root. It's identical! * Add missing tests - part 2 * deposit receipts start index getters and setters (#13947) * adding in getters and setters for deposit receipts start index * adding tests * gaz * Add missing tests - part 3 of 3 Update the electra withdrawal example with a ssz state containing pending partial withdrawals * add tests for beacon-chain/state/state-native/getters_balance_deposits.go * Add electra field to testing/util/block.go execution payload * godoc commentary on public methods * Fix failing test * Add balances index out of bounds check and relevant tests. * Revert switch case electra * Instead of copying spectest data into testdata, use the spectest dependency * Deepsource fixes * Address @rkapka PR feedback * s/MaxPendingPartialsPerWithdrawalSweep/MaxPendingPartialsPerWithdrawalsSweep/ * Use multivalue slice compatible accessors for validator and balance in ActiveBalanceAtIndex * More @rkapka feedback. What a great reviewer! * More tests for branching logic in ExitEpochAndUpdateChurn * fix build --------- Co-authored-by: james-prysm <90280386+james-prysm@users.noreply.github.com>

prestonvanloon mentioned this pull request Apr 26, 2024

beacon-chain/cache: Convert tests to cache_test blackbox testing #13920

Merged

prestonvanloon force-pushed the electra-beacon-state branch from 67ba5d7 to f667110 Compare April 26, 2024 03:40

This was referenced Apr 26, 2024

Electra: beacon-chain/core/helpers #13921

Merged

Electra: Minor proto changes, cloner additions #13923

Merged

Electra: HTR util for DepositReceipt and ExecutionLayerWithdrawalRequest #13924

Merged

Electra: Consensus types additions #13925

Closed

prestonvanloon force-pushed the electra-beacon-state branch 3 times, most recently from 4626076 to a41da61 Compare April 26, 2024 17:03

kasey reviewed Apr 26, 2024

View reviewed changes

beacon-chain/core/helpers/validators.go Outdated Show resolved Hide resolved

prestonvanloon force-pushed the electra-beacon-state branch 2 times, most recently from 063f077 to e980f86 Compare April 26, 2024 22:12

james-prysm mentioned this pull request Apr 29, 2024

Electra: upgrade #13933

Merged

1 task

prestonvanloon force-pushed the electra-beacon-state branch 5 times, most recently from 00b695f to 6fd2121 Compare April 30, 2024 01:47

james-prysm reviewed May 1, 2024

View reviewed changes

beacon-chain/state/interfaces.go Outdated Show resolved Hide resolved

prestonvanloon force-pushed the electra-beacon-state branch 4 times, most recently from b597662 to 477c9cb Compare May 2, 2024 20:29

prestonvanloon marked this pull request as ready for review May 2, 2024 20:29

prestonvanloon requested a review from a team as a code owner May 2, 2024 20:29

prestonvanloon requested review from saolyn, potuz and james-prysm May 2, 2024 20:29

prestonvanloon force-pushed the electra-beacon-state branch from 3f4df33 to 4950d24 Compare May 2, 2024 20:47

prestonvanloon commented May 2, 2024

View reviewed changes

prestonvanloon added 14 commits May 3, 2024 14:38

Add missing tests - part 3 of 3

8c8d9d3

Update the electra withdrawal example with a ssz state containing pending partial withdrawals

add tests for beacon-chain/state/state-native/getters_balance_deposit…

be3a999

…s.go

Add electra field to testing/util/block.go execution payload

45a914f

godoc commentary on public methods

f542639

Fix failing test

1339d2d

Add balances index out of bounds check and relevant tests.

49d5b88

Revert switch case electra

3b961ec

Instead of copying spectest data into testdata, use the spectest depe…

6b4e4a9

…ndency

Deepsource fixes

c6d6bfc

Address @rkapka PR feedback

41a6263

s/MaxPendingPartialsPerWithdrawalSweep/MaxPendingPartialsPerWithdrawa…

b8a29ef

…lsSweep/

Use multivalue slice compatible accessors for validator and balance i…

2bfac80

…n ActiveBalanceAtIndex

More @rkapka feedback. What a great reviewer!

0258b67

More tests for branching logic in ExitEpochAndUpdateChurn

67dd466

prestonvanloon force-pushed the electra-beacon-state branch from 00d1f54 to 67dd466 Compare May 3, 2024 19:39

fix build

4ff0357

rkapka approved these changes May 6, 2024

View reviewed changes

james-prysm approved these changes May 6, 2024

View reviewed changes

kasey reviewed May 6, 2024

View reviewed changes

beacon-chain/state/state-native/getters_consolidation.go Show resolved Hide resolved

kasey reviewed May 6, 2024

View reviewed changes

prestonvanloon mentioned this pull request May 6, 2024

Update state readme #13890

Merged

kasey approved these changes May 6, 2024

View reviewed changes

prestonvanloon added this pull request to the merge queue May 6, 2024

Merged via the queue into develop with commit 9b2934f May 6, 2024
16 of 17 checks passed

prestonvanloon deleted the electra-beacon-state branch May 6, 2024 18:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Electra: BeaconState implementation #13919

Electra: BeaconState implementation #13919

prestonvanloon commented Apr 26, 2024 •

edited

Loading

prestonvanloon left a comment

prestonvanloon May 2, 2024

prestonvanloon May 2, 2024

prestonvanloon May 2, 2024

prestonvanloon May 2, 2024

prestonvanloon May 2, 2024

prestonvanloon May 2, 2024

prestonvanloon May 2, 2024

kasey May 6, 2024

prestonvanloon May 2, 2024

james-prysm May 2, 2024

prestonvanloon commented May 3, 2024

prestonvanloon commented May 3, 2024

prestonvanloon commented May 4, 2024

rkapka left a comment

kasey May 6, 2024

prestonvanloon May 6, 2024

kasey May 6, 2024

kasey May 6, 2024

Electra: BeaconState implementation #13919

Electra: BeaconState implementation #13919

Conversation

prestonvanloon commented Apr 26, 2024 • edited Loading

prestonvanloon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestonvanloon commented May 3, 2024

prestonvanloon commented May 3, 2024

prestonvanloon commented May 4, 2024

rkapka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestonvanloon commented Apr 26, 2024 •

edited

Loading