From c07479b99afa1b77c8871b64f6bb68b78450375d Mon Sep 17 00:00:00 2001 From: Potuz Date: Wed, 7 May 2025 13:06:30 -0300 Subject: [PATCH] remove error return from HistoricalRoots (#15255) * remove error return from HistoricalRoots * Radek's review --- api/server/structs/conversions_state.go | 35 ++++--------------- beacon-chain/core/altair/upgrade.go | 6 +--- beacon-chain/core/altair/upgrade_test.go | 6 ++-- beacon-chain/core/capella/upgrade.go | 6 +--- beacon-chain/core/deneb/upgrade.go | 6 +--- beacon-chain/core/deneb/upgrade_test.go | 6 ++-- beacon-chain/core/electra/upgrade.go | 6 +--- beacon-chain/core/electra/upgrade_test.go | 6 ++-- .../core/epoch/epoch_processing_test.go | 12 +++---- beacon-chain/core/execution/upgrade.go | 6 +--- beacon-chain/core/execution/upgrade_test.go | 6 ++-- beacon-chain/core/fulu/upgrade.go | 6 +--- beacon-chain/core/fulu/upgrade_test.go | 6 ++-- beacon-chain/state/interfaces.go | 2 +- .../state/state-native/getters_misc.go | 6 ++-- changelog/potuz_historical_roots.md | 2 ++ tools/exploredb/main.go | 10 ++---- 17 files changed, 36 insertions(+), 97 deletions(-) create mode 100644 changelog/potuz_historical_roots.md diff --git a/api/server/structs/conversions_state.go b/api/server/structs/conversions_state.go index c8de720271..69c9668699 100644 --- a/api/server/structs/conversions_state.go +++ b/api/server/structs/conversions_state.go @@ -26,10 +26,7 @@ func BeaconStateFromConsensus(st beaconState.BeaconState) (*BeaconState, error) for i, r := range srcSr { sr[i] = hexutil.Encode(r) } - srcHr, err := st.HistoricalRoots() - if err != nil { - return nil, err - } + srcHr := st.HistoricalRoots() hr := make([]string, len(srcHr)) for i, r := range srcHr { hr[i] = hexutil.Encode(r) @@ -116,10 +113,7 @@ func BeaconStateAltairFromConsensus(st beaconState.BeaconState) (*BeaconStateAlt for i, r := range srcSr { sr[i] = hexutil.Encode(r) } - srcHr, err := st.HistoricalRoots() - if err != nil { - return nil, err - } + srcHr := st.HistoricalRoots() hr := make([]string, len(srcHr)) for i, r := range srcHr { hr[i] = hexutil.Encode(r) @@ -225,10 +219,7 @@ func BeaconStateBellatrixFromConsensus(st beaconState.BeaconState) (*BeaconState for i, r := range srcSr { sr[i] = hexutil.Encode(r) } - srcHr, err := st.HistoricalRoots() - if err != nil { - return nil, err - } + srcHr := st.HistoricalRoots() hr := make([]string, len(srcHr)) for i, r := range srcHr { hr[i] = hexutil.Encode(r) @@ -347,10 +338,7 @@ func BeaconStateCapellaFromConsensus(st beaconState.BeaconState) (*BeaconStateCa for i, r := range srcSr { sr[i] = hexutil.Encode(r) } - srcHr, err := st.HistoricalRoots() - if err != nil { - return nil, err - } + srcHr := st.HistoricalRoots() hr := make([]string, len(srcHr)) for i, r := range srcHr { hr[i] = hexutil.Encode(r) @@ -488,10 +476,7 @@ func BeaconStateDenebFromConsensus(st beaconState.BeaconState) (*BeaconStateDene for i, r := range srcSr { sr[i] = hexutil.Encode(r) } - srcHr, err := st.HistoricalRoots() - if err != nil { - return nil, err - } + srcHr := st.HistoricalRoots() hr := make([]string, len(srcHr)) for i, r := range srcHr { hr[i] = hexutil.Encode(r) @@ -629,10 +614,7 @@ func BeaconStateElectraFromConsensus(st beaconState.BeaconState) (*BeaconStateEl for i, r := range srcSr { sr[i] = hexutil.Encode(r) } - srcHr, err := st.HistoricalRoots() - if err != nil { - return nil, err - } + srcHr := st.HistoricalRoots() hr := make([]string, len(srcHr)) for i, r := range srcHr { hr[i] = hexutil.Encode(r) @@ -815,10 +797,7 @@ func BeaconStateFuluFromConsensus(st beaconState.BeaconState) (*BeaconStateFulu, for i, r := range srcSr { sr[i] = hexutil.Encode(r) } - srcHr, err := st.HistoricalRoots() - if err != nil { - return nil, err - } + srcHr := st.HistoricalRoots() hr := make([]string, len(srcHr)) for i, r := range srcHr { hr[i] = hexutil.Encode(r) diff --git a/beacon-chain/core/altair/upgrade.go b/beacon-chain/core/altair/upgrade.go index 654b5cf533..f0876bd75e 100644 --- a/beacon-chain/core/altair/upgrade.go +++ b/beacon-chain/core/altair/upgrade.go @@ -67,10 +67,6 @@ func UpgradeToAltair(ctx context.Context, state state.BeaconState) (state.Beacon epoch := time.CurrentEpoch(state) numValidators := state.NumValidators() - hrs, err := state.HistoricalRoots() - if err != nil { - return nil, err - } s := ðpb.BeaconStateAltair{ GenesisTime: state.GenesisTime(), GenesisValidatorsRoot: state.GenesisValidatorsRoot(), @@ -83,7 +79,7 @@ func UpgradeToAltair(ctx context.Context, state state.BeaconState) (state.Beacon LatestBlockHeader: state.LatestBlockHeader(), BlockRoots: state.BlockRoots(), StateRoots: state.StateRoots(), - HistoricalRoots: hrs, + HistoricalRoots: state.HistoricalRoots(), Eth1Data: state.Eth1Data(), Eth1DataVotes: state.Eth1DataVotes(), Eth1DepositIndex: state.Eth1DepositIndex(), diff --git a/beacon-chain/core/altair/upgrade_test.go b/beacon-chain/core/altair/upgrade_test.go index 0064421b5e..ddc7316bb9 100644 --- a/beacon-chain/core/altair/upgrade_test.go +++ b/beacon-chain/core/altair/upgrade_test.go @@ -82,10 +82,8 @@ func TestUpgradeToAltair(t *testing.T) { require.DeepSSZEqual(t, preForkState.LatestBlockHeader(), aState.LatestBlockHeader()) require.DeepSSZEqual(t, preForkState.BlockRoots(), aState.BlockRoots()) require.DeepSSZEqual(t, preForkState.StateRoots(), aState.StateRoots()) - r1, err := preForkState.HistoricalRoots() - require.NoError(t, err) - r2, err := aState.HistoricalRoots() - require.NoError(t, err) + r1 := preForkState.HistoricalRoots() + r2 := aState.HistoricalRoots() require.DeepSSZEqual(t, r1, r2) require.DeepSSZEqual(t, preForkState.Eth1Data(), aState.Eth1Data()) require.DeepSSZEqual(t, preForkState.Eth1DataVotes(), aState.Eth1DataVotes()) diff --git a/beacon-chain/core/capella/upgrade.go b/beacon-chain/core/capella/upgrade.go index 855f809bc3..70de97acaa 100644 --- a/beacon-chain/core/capella/upgrade.go +++ b/beacon-chain/core/capella/upgrade.go @@ -42,10 +42,6 @@ func UpgradeToCapella(state state.BeaconState) (state.BeaconState, error) { return nil, err } - hrs, err := state.HistoricalRoots() - if err != nil { - return nil, err - } s := ðpb.BeaconStateCapella{ GenesisTime: state.GenesisTime(), GenesisValidatorsRoot: state.GenesisValidatorsRoot(), @@ -58,7 +54,7 @@ func UpgradeToCapella(state state.BeaconState) (state.BeaconState, error) { LatestBlockHeader: state.LatestBlockHeader(), BlockRoots: state.BlockRoots(), StateRoots: state.StateRoots(), - HistoricalRoots: hrs, + HistoricalRoots: state.HistoricalRoots(), Eth1Data: state.Eth1Data(), Eth1DataVotes: state.Eth1DataVotes(), Eth1DepositIndex: state.Eth1DepositIndex(), diff --git a/beacon-chain/core/deneb/upgrade.go b/beacon-chain/core/deneb/upgrade.go index 8a65ba5a6a..f3ded4dae5 100644 --- a/beacon-chain/core/deneb/upgrade.go +++ b/beacon-chain/core/deneb/upgrade.go @@ -57,10 +57,6 @@ func UpgradeToDeneb(state state.BeaconState) (state.BeaconState, error) { if err != nil { return nil, err } - historicalRoots, err := state.HistoricalRoots() - if err != nil { - return nil, err - } s := ðpb.BeaconStateDeneb{ GenesisTime: state.GenesisTime(), @@ -74,7 +70,7 @@ func UpgradeToDeneb(state state.BeaconState) (state.BeaconState, error) { LatestBlockHeader: state.LatestBlockHeader(), BlockRoots: state.BlockRoots(), StateRoots: state.StateRoots(), - HistoricalRoots: historicalRoots, + HistoricalRoots: state.HistoricalRoots(), Eth1Data: state.Eth1Data(), Eth1DataVotes: state.Eth1DataVotes(), Eth1DepositIndex: state.Eth1DepositIndex(), diff --git a/beacon-chain/core/deneb/upgrade_test.go b/beacon-chain/core/deneb/upgrade_test.go index b6393daeae..f8dd6ac00e 100644 --- a/beacon-chain/core/deneb/upgrade_test.go +++ b/beacon-chain/core/deneb/upgrade_test.go @@ -47,10 +47,8 @@ func TestUpgradeToDeneb(t *testing.T) { require.NoError(t, err) require.DeepSSZEqual(t, make([]uint64, numValidators), s) - hr1, err := preForkState.HistoricalRoots() - require.NoError(t, err) - hr2, err := mSt.HistoricalRoots() - require.NoError(t, err) + hr1 := preForkState.HistoricalRoots() + hr2 := mSt.HistoricalRoots() require.DeepEqual(t, hr1, hr2) f := mSt.Fork() diff --git a/beacon-chain/core/electra/upgrade.go b/beacon-chain/core/electra/upgrade.go index a95cd055a1..6fed3e0901 100644 --- a/beacon-chain/core/electra/upgrade.go +++ b/beacon-chain/core/electra/upgrade.go @@ -170,10 +170,6 @@ func UpgradeToElectra(beaconState state.BeaconState) (state.BeaconState, error) if err != nil { return nil, err } - historicalRoots, err := beaconState.HistoricalRoots() - if err != nil { - return nil, err - } excessBlobGas, err := payloadHeader.ExcessBlobGas() if err != nil { return nil, err @@ -223,7 +219,7 @@ func UpgradeToElectra(beaconState state.BeaconState) (state.BeaconState, error) LatestBlockHeader: beaconState.LatestBlockHeader(), BlockRoots: beaconState.BlockRoots(), StateRoots: beaconState.StateRoots(), - HistoricalRoots: historicalRoots, + HistoricalRoots: beaconState.HistoricalRoots(), Eth1Data: beaconState.Eth1Data(), Eth1DataVotes: beaconState.Eth1DataVotes(), Eth1DepositIndex: beaconState.Eth1DepositIndex(), diff --git a/beacon-chain/core/electra/upgrade_test.go b/beacon-chain/core/electra/upgrade_test.go index eaf1250e8c..fb83f0b527 100644 --- a/beacon-chain/core/electra/upgrade_test.go +++ b/beacon-chain/core/electra/upgrade_test.go @@ -77,10 +77,8 @@ func TestUpgradeToElectra(t *testing.T) { require.NoError(t, err) require.DeepSSZEqual(t, make([]uint64, numValidators), s) - hr1, err := preForkState.HistoricalRoots() - require.NoError(t, err) - hr2, err := mSt.HistoricalRoots() - require.NoError(t, err) + hr1 := preForkState.HistoricalRoots() + hr2 := mSt.HistoricalRoots() require.DeepEqual(t, hr1, hr2) f := mSt.Fork() diff --git a/beacon-chain/core/epoch/epoch_processing_test.go b/beacon-chain/core/epoch/epoch_processing_test.go index 7e4bb505fc..ef7085002c 100644 --- a/beacon-chain/core/epoch/epoch_processing_test.go +++ b/beacon-chain/core/epoch/epoch_processing_test.go @@ -148,8 +148,7 @@ func TestProcessFinalUpdates_CanProcess(t *testing.T) { assert.DeepNotEqual(t, params.BeaconConfig().ZeroHash[:], mix, "latest RANDAO still zero hashes") // Verify historical root accumulator was appended. - roots, err := newS.HistoricalRoots() - require.NoError(t, err) + roots := newS.HistoricalRoots() assert.Equal(t, 1, len(roots), "Unexpected slashed balance") currAtt, err := newS.CurrentEpochAttestations() require.NoError(t, err) @@ -379,8 +378,7 @@ func TestProcessHistoricalDataUpdate(t *testing.T) { return st }, verifier: func(st state.BeaconState) { - roots, err := st.HistoricalRoots() - require.NoError(t, err) + roots := st.HistoricalRoots() require.Equal(t, 0, len(roots)) }, }, @@ -393,8 +391,7 @@ func TestProcessHistoricalDataUpdate(t *testing.T) { return st }, verifier: func(st state.BeaconState) { - roots, err := st.HistoricalRoots() - require.NoError(t, err) + roots := st.HistoricalRoots() require.Equal(t, 1, len(roots)) b := ðpb.HistoricalBatch{ @@ -431,8 +428,7 @@ func TestProcessHistoricalDataUpdate(t *testing.T) { StateSummaryRoot: sr[:], } require.DeepEqual(t, b, summaries[0]) - hrs, err := st.HistoricalRoots() - require.NoError(t, err) + hrs := st.HistoricalRoots() require.DeepEqual(t, hrs, [][]byte{}) }, }, diff --git a/beacon-chain/core/execution/upgrade.go b/beacon-chain/core/execution/upgrade.go index 861014647b..b12870802e 100644 --- a/beacon-chain/core/execution/upgrade.go +++ b/beacon-chain/core/execution/upgrade.go @@ -35,10 +35,6 @@ func UpgradeToBellatrix(state state.BeaconState) (state.BeaconState, error) { return nil, err } - hrs, err := state.HistoricalRoots() - if err != nil { - return nil, err - } s := ðpb.BeaconStateBellatrix{ GenesisTime: state.GenesisTime(), GenesisValidatorsRoot: state.GenesisValidatorsRoot(), @@ -51,7 +47,7 @@ func UpgradeToBellatrix(state state.BeaconState) (state.BeaconState, error) { LatestBlockHeader: state.LatestBlockHeader(), BlockRoots: state.BlockRoots(), StateRoots: state.StateRoots(), - HistoricalRoots: hrs, + HistoricalRoots: state.HistoricalRoots(), Eth1Data: state.Eth1Data(), Eth1DataVotes: state.Eth1DataVotes(), Eth1DepositIndex: state.Eth1DepositIndex(), diff --git a/beacon-chain/core/execution/upgrade_test.go b/beacon-chain/core/execution/upgrade_test.go index 254b2381b4..ec1566e012 100644 --- a/beacon-chain/core/execution/upgrade_test.go +++ b/beacon-chain/core/execution/upgrade_test.go @@ -24,10 +24,8 @@ func TestUpgradeToBellatrix(t *testing.T) { require.DeepSSZEqual(t, preForkState.LatestBlockHeader(), mSt.LatestBlockHeader()) require.DeepSSZEqual(t, preForkState.BlockRoots(), mSt.BlockRoots()) require.DeepSSZEqual(t, preForkState.StateRoots(), mSt.StateRoots()) - r1, err := preForkState.HistoricalRoots() - require.NoError(t, err) - r2, err := mSt.HistoricalRoots() - require.NoError(t, err) + r1 := preForkState.HistoricalRoots() + r2 := mSt.HistoricalRoots() require.DeepSSZEqual(t, r1, r2) require.DeepSSZEqual(t, preForkState.Eth1Data(), mSt.Eth1Data()) require.DeepSSZEqual(t, preForkState.Eth1DataVotes(), mSt.Eth1DataVotes()) diff --git a/beacon-chain/core/fulu/upgrade.go b/beacon-chain/core/fulu/upgrade.go index 048fc7ec69..bdb13375e4 100644 --- a/beacon-chain/core/fulu/upgrade.go +++ b/beacon-chain/core/fulu/upgrade.go @@ -57,10 +57,6 @@ func UpgradeToFulu(beaconState state.BeaconState) (state.BeaconState, error) { if err != nil { return nil, err } - historicalRoots, err := beaconState.HistoricalRoots() - if err != nil { - return nil, err - } excessBlobGas, err := payloadHeader.ExcessBlobGas() if err != nil { return nil, err @@ -118,7 +114,7 @@ func UpgradeToFulu(beaconState state.BeaconState) (state.BeaconState, error) { LatestBlockHeader: beaconState.LatestBlockHeader(), BlockRoots: beaconState.BlockRoots(), StateRoots: beaconState.StateRoots(), - HistoricalRoots: historicalRoots, + HistoricalRoots: beaconState.HistoricalRoots(), Eth1Data: beaconState.Eth1Data(), Eth1DataVotes: beaconState.Eth1DataVotes(), Eth1DepositIndex: beaconState.Eth1DepositIndex(), diff --git a/beacon-chain/core/fulu/upgrade_test.go b/beacon-chain/core/fulu/upgrade_test.go index e45f077c03..a4c2e629d8 100644 --- a/beacon-chain/core/fulu/upgrade_test.go +++ b/beacon-chain/core/fulu/upgrade_test.go @@ -43,10 +43,8 @@ func TestUpgradeToFulu(t *testing.T) { require.DeepSSZEqual(t, preForkState.BlockRoots(), mSt.BlockRoots()) require.DeepSSZEqual(t, preForkState.StateRoots(), mSt.StateRoots()) - hr1, err := preForkState.HistoricalRoots() - require.NoError(t, err) - hr2, err := mSt.HistoricalRoots() - require.NoError(t, err) + hr1 := preForkState.HistoricalRoots() + hr2 := mSt.HistoricalRoots() require.DeepEqual(t, hr1, hr2) require.DeepSSZEqual(t, preForkState.Eth1Data(), mSt.Eth1Data()) diff --git a/beacon-chain/state/interfaces.go b/beacon-chain/state/interfaces.go index eac55014f1..0b9d221746 100644 --- a/beacon-chain/state/interfaces.go +++ b/beacon-chain/state/interfaces.go @@ -68,7 +68,7 @@ type ReadOnlyBeaconState interface { Slot() primitives.Slot Fork() *ethpb.Fork LatestBlockHeader() *ethpb.BeaconBlockHeader - HistoricalRoots() ([][]byte, error) + HistoricalRoots() [][]byte HistoricalSummaries() ([]*ethpb.HistoricalSummary, error) Slashings() []uint64 FieldReferencesCount() map[string]uint64 diff --git a/beacon-chain/state/state-native/getters_misc.go b/beacon-chain/state/state-native/getters_misc.go index 0c8ab476a6..5a92306477 100644 --- a/beacon-chain/state/state-native/getters_misc.go +++ b/beacon-chain/state/state-native/getters_misc.go @@ -73,15 +73,15 @@ func (b *BeaconState) forkVal() *ethpb.Fork { } // HistoricalRoots based on epochs stored in the beacon state. -func (b *BeaconState) HistoricalRoots() ([][]byte, error) { +func (b *BeaconState) HistoricalRoots() [][]byte { if b.historicalRoots == nil { - return nil, nil + return nil } b.lock.RLock() defer b.lock.RUnlock() - return b.historicalRoots.Slice(), nil + return b.historicalRoots.Slice() } // HistoricalSummaries of the beacon state. diff --git a/changelog/potuz_historical_roots.md b/changelog/potuz_historical_roots.md new file mode 100644 index 0000000000..c0ed356db4 --- /dev/null +++ b/changelog/potuz_historical_roots.md @@ -0,0 +1,2 @@ +### Ignored +- HistoricalRoots should not return an error. diff --git a/tools/exploredb/main.go b/tools/exploredb/main.go index 188be83b63..c475778e50 100644 --- a/tools/exploredb/main.go +++ b/tools/exploredb/main.go @@ -336,13 +336,9 @@ func printStates(stateC <-chan *modifiedState, doneC chan<- bool) { log.Infof("block_roots : size = %s, count = %d", humanize.Bytes(size), count) size, count = sizeAndCountOfByteList(st.StateRoots()) log.Infof("state_roots : size = %s, count = %d", humanize.Bytes(size), count) - roots, err := st.HistoricalRoots() - if err != nil { - log.WithError(err).Error("could not get historical roots") - } else { - size, count = sizeAndCountOfByteList(roots) - log.Infof("historical_roots : size = %s, count = %d", humanize.Bytes(size), count) - } + roots := st.HistoricalRoots() + size, count = sizeAndCountOfByteList(roots) + log.Infof("historical_roots : size = %s, count = %d", humanize.Bytes(size), count) log.Infof("eth1_data : sizeSSZ = %s", humanize.Bytes(uint64(st.Eth1Data().SizeSSZ()))) size, count = sizeAndCountGeneric(st.Eth1DataVotes(), nil) log.Infof("eth1_data_votes : sizeSSZ = %s, count = %d", humanize.Bytes(size), count)