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 historical summaries to state and processing #11842

Merged
merged 24 commits into from
Jan 11, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9cb48be
Add historical summaries to state and processing
terencechain Jan 3, 2023
144218c
Process historical roots test
terencechain Jan 6, 2023
055e225
Passing spec tests
terencechain Jan 6, 2023
91b15c1
Fix shas and tests
terencechain Jan 9, 2023
5898b39
Merge branch 'develop' into historical-summaries
terencechain Jan 9, 2023
d875647
Merge branch 'develop' into historical-summaries
terencechain Jan 9, 2023
3f6f4b6
Fix mainnet spectest sha
terencechain Jan 9, 2023
8b53c65
Fix tests
terencechain Jan 10, 2023
79ba8dd
Merge branch 'develop' into historical-summaries
terencechain Jan 10, 2023
60d106c
Merge branch 'develop' into historical-summaries
terencechain Jan 10, 2023
ffd165e
Update beacon-chain/core/epoch/epoch_processing.go
terencechain Jan 11, 2023
7886997
Update beacon-chain/core/epoch/epoch_processing_test.go
terencechain Jan 11, 2023
261229e
Update proto/prysm/v1alpha1/beacon_state.proto
terencechain Jan 11, 2023
549993f
Update beacon-chain/state/state-native/hasher.go
terencechain Jan 11, 2023
f9a040a
Radek's feedback
terencechain Jan 11, 2023
77b00ab
Merge branch 'develop' into historical-summaries
rkapka Jan 11, 2023
782d500
Getters error
terencechain Jan 11, 2023
c0e35d3
Merge branch 'historical-summaries' of github.com:prysmaticlabs/prysm…
terencechain Jan 11, 2023
957e4ec
Merge branch 'develop' of github.com:prysmaticlabs/prysm into histori…
terencechain Jan 11, 2023
0cc7843
Dont return
terencechain Jan 11, 2023
c4eee98
Fix else
terencechain Jan 11, 2023
9fe0774
Fix tests
terencechain Jan 11, 2023
93c8ccb
Fix test
terencechain Jan 11, 2023
93bc55d
Rm white space
terencechain Jan 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ filegroup(
url = "https://github.com/eth-clients/slashing-protection-interchange-tests/archive/b8413ca42dc92308019d0d4db52c87e9e125c4e9.tar.gz",
)

consensus_spec_version = "v1.3.0-alpha.2"
consensus_spec_version = "v1.3.0-rc.0"

bls_test_version = "v0.1.1"

Expand All @@ -204,7 +204,7 @@ filegroup(
visibility = ["//visibility:public"],
)
""",
sha256 = "a92c41058dc17ced811cc85570cd6f8af761aedfcbd2dd7dd4fb64ac961d76f9",
sha256 = "44c39a68242a4b731865040ea3009ea1a695ee3377d2891cecf8e6a721f0102b",
url = "https://github.com/ethereum/consensus-spec-tests/releases/download/%s/general.tar.gz" % consensus_spec_version,
)

Expand All @@ -220,7 +220,7 @@ filegroup(
visibility = ["//visibility:public"],
)
""",
sha256 = "49a7944da92429ac8f41347f19837762247cdbf00e628c285d1b826e58e4096d",
sha256 = "c9aaf6f7c1ca1264ec9a45872d5058f3377ebdd9dbcb987796f7302d86f98934",
url = "https://github.com/ethereum/consensus-spec-tests/releases/download/%s/minimal.tar.gz" % consensus_spec_version,
)

Expand Down Expand Up @@ -251,7 +251,7 @@ filegroup(
visibility = ["//visibility:public"],
)
""",
sha256 = "a72b7457c403f6b76567d4d7bec19d01bedf7d5ef1d6f2c3a9e09ee86d401a14",
sha256 = "531eff00498acf5965d8890d83e3561a64f7b23a75d7f312915291482da2bf52",
strip_prefix = "consensus-specs-" + consensus_spec_version[1:],
url = "https://github.com/ethereum/consensus-specs/archive/refs/tags/%s.tar.gz" % consensus_spec_version,
)
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/altair/transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func ProcessEpoch(ctx context.Context, state state.BeaconState) (state.BeaconSta
if err != nil {
return nil, err
}
state, err = e.ProcessHistoricalRootsUpdate(state)
state, err = e.ProcessHistoricalDataUpdate(state)
if err != nil {
return nil, err
}
Expand Down
5 changes: 5 additions & 0 deletions beacon-chain/core/epoch/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@ go_library(
"//beacon-chain/core/time:go_default_library",
"//beacon-chain/core/validators:go_default_library",
"//beacon-chain/state:go_default_library",
"//beacon-chain/state/stateutil:go_default_library",
"//config/fieldparams:go_default_library",
"//config/params:go_default_library",
"//consensus-types/primitives:go_default_library",
"//math:go_default_library",
"//proto/prysm/v1alpha1:go_default_library",
"//proto/prysm/v1alpha1/attestation:go_default_library",
"//runtime/version:go_default_library",
"@com_github_pkg_errors//:go_default_library",
],
)
Expand All @@ -33,8 +36,10 @@ go_test(
deps = [
"//beacon-chain/core/helpers:go_default_library",
"//beacon-chain/core/time:go_default_library",
"//beacon-chain/core/transition:go_default_library",
"//beacon-chain/state:go_default_library",
"//beacon-chain/state/state-native:go_default_library",
"//beacon-chain/state/stateutil:go_default_library",
"//config/fieldparams:go_default_library",
"//config/params:go_default_library",
"//consensus-types/primitives:go_default_library",
Expand Down
53 changes: 31 additions & 22 deletions beacon-chain/core/epoch/epoch_processing.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ import (
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/time"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/validators"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/state"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/state/stateutil"
fieldparams "github.com/prysmaticlabs/prysm/v3/config/fieldparams"
"github.com/prysmaticlabs/prysm/v3/config/params"
types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v3/math"
ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1/attestation"
"github.com/prysmaticlabs/prysm/v3/runtime/version"
)

// sortableIndices implements the Sort interface to sort newly activated validator indices
Expand Down Expand Up @@ -349,33 +352,39 @@ func ProcessRandaoMixesReset(state state.BeaconState) (state.BeaconState, error)
return state, nil
}

// ProcessHistoricalRootsUpdate processes the updates to historical root accumulator during epoch processing.
//
// Spec pseudocode definition:
//
// def process_historical_roots_update(state: BeaconState) -> None:
// # Set historical root accumulator
// next_epoch = Epoch(get_current_epoch(state) + 1)
// if next_epoch % (SLOTS_PER_HISTORICAL_ROOT // SLOTS_PER_EPOCH) == 0:
// historical_batch = HistoricalBatch(block_roots=state.block_roots, state_roots=state.state_roots)
// state.historical_roots.append(hash_tree_root(historical_batch))
func ProcessHistoricalRootsUpdate(state state.BeaconState) (state.BeaconState, error) {
// ProcessHistoricalDataUpdate processes the updates to historical data during epoch processing.
// For Capella state, per spec, historical summaries is updated instead of historical roots.
terencechain marked this conversation as resolved.
Show resolved Hide resolved
func ProcessHistoricalDataUpdate(state state.BeaconState) (state.BeaconState, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I decided to embed two different processing logic for historical data so that callers don't have to worry about what fork uses what.
Another approach is to implement two different functions ProcessHistoricalRootsUpdate and ProcessHistoricalSummariesUpdate with some duplicated conditions and let the callers decide which to call

Copy link
Contributor

@kasey kasey Jan 11, 2023

Choose a reason for hiding this comment

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

+1 to encapsulating that here, the way you're doing it

currentEpoch := time.CurrentEpoch(state)
nextEpoch := currentEpoch + 1

// Set historical root accumulator.
epochsPerHistoricalRoot := params.BeaconConfig().SlotsPerHistoricalRoot.DivSlot(params.BeaconConfig().SlotsPerEpoch)
if nextEpoch.Mod(uint64(epochsPerHistoricalRoot)) == 0 {
historicalBatch := &ethpb.HistoricalBatch{
BlockRoots: state.BlockRoots(),
StateRoots: state.StateRoots(),
}
batchRoot, err := historicalBatch.HashTreeRoot()
if err != nil {
return nil, errors.Wrap(err, "could not hash historical batch")
}
if err := state.AppendHistoricalRoots(batchRoot); err != nil {
return nil, err
if state.Version() >= version.Capella {
br, err := stateutil.ArraysRoot(state.BlockRoots(), fieldparams.BlockRootsLength)
if err != nil {
return nil, err
}
sr, err := stateutil.ArraysRoot(state.StateRoots(), fieldparams.StateRootsLength)
if err != nil {
return nil, err
}
if err := state.AppendHistoricalSummariesUpdate(&ethpb.HistoricalSummary{BlockSummaryRoot: br[:], StateSummaryRoot: sr[:]}); err != nil {
return nil, err
}
} else {
historicalBatch := &ethpb.HistoricalBatch{
BlockRoots: state.BlockRoots(),
StateRoots: state.StateRoots(),
}
batchRoot, err := historicalBatch.HashTreeRoot()
if err != nil {
return nil, errors.Wrap(err, "could not hash historical batch")
}
if err := state.AppendHistoricalRoots(batchRoot); err != nil {
return nil, err
}
}
}

Expand Down Expand Up @@ -426,7 +435,7 @@ func ProcessFinalUpdates(state state.BeaconState) (state.BeaconState, error) {
}

// Set historical root accumulator.
state, err = ProcessHistoricalRootsUpdate(state)
state, err = ProcessHistoricalDataUpdate(state)
if err != nil {
return nil, err
}
Expand Down
70 changes: 70 additions & 0 deletions beacon-chain/core/epoch/epoch_processing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import (
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/epoch"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/helpers"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/time"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/transition"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/state"
state_native "github.com/prysmaticlabs/prysm/v3/beacon-chain/state/state-native"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/state/stateutil"
fieldparams "github.com/prysmaticlabs/prysm/v3/config/fieldparams"
"github.com/prysmaticlabs/prysm/v3/config/params"
types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives"
Expand Down Expand Up @@ -455,3 +457,71 @@ func TestProcessSlashings_BadValue(t *testing.T) {
_, err = epoch.ProcessSlashings(s, params.BeaconConfig().ProportionalSlashingMultiplier)
require.ErrorContains(t, "addition overflows", err)
}

func TestProcessHistoricalRootsUpdate(t *testing.T) {
terencechain marked this conversation as resolved.
Show resolved Hide resolved
tests := []struct {
name string
st func() state.BeaconState
verifier func(state.BeaconState)
}{
{
name: "no change",
st: func() state.BeaconState {
st, _ := util.DeterministicGenesisState(t, 1)
return st
},
verifier: func(st state.BeaconState) {
require.Equal(t, 0, len(st.HistoricalRoots()))
},
},
{
name: "before capella can process and get historical root",
st: func() state.BeaconState {
st, _ := util.DeterministicGenesisState(t, 1)
st, err := transition.ProcessSlots(context.Background(), st, params.BeaconConfig().SlotsPerHistoricalRoot-1)
require.NoError(t, err)
return st
},
verifier: func(st state.BeaconState) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Verify historical summaries were not updates?

require.Equal(t, 1, len(st.HistoricalRoots()))

b := &ethpb.HistoricalBatch{
BlockRoots: st.BlockRoots(),
StateRoots: st.StateRoots(),
}
r, err := b.HashTreeRoot()
require.NoError(t, err)
require.DeepEqual(t, r[:], st.HistoricalRoots()[0])
},
},
{
name: "after capella can process and get historical summary",
st: func() state.BeaconState {
st, _ := util.DeterministicGenesisStateCapella(t, 1)
st, err := transition.ProcessSlots(context.Background(), st, params.BeaconConfig().SlotsPerHistoricalRoot-1)
require.NoError(t, err)
return st
},
verifier: func(st state.BeaconState) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Verify historical roots were not updated?

require.Equal(t, 1, len(st.HistoricalSummaries()))

br, err := stateutil.ArraysRoot(st.BlockRoots(), fieldparams.BlockRootsLength)
require.NoError(t, err)
sr, err := stateutil.ArraysRoot(st.StateRoots(), fieldparams.StateRootsLength)
require.NoError(t, err)
b := &ethpb.HistoricalSummary{
BlockSummaryRoot: br[:],
StateSummaryRoot: sr[:],
}
require.DeepEqual(t, b, st.HistoricalSummaries()[0])
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := epoch.ProcessHistoricalDataUpdate(tt.st())
require.NoError(t, err)
tt.verifier(got)
})
}
}
2 changes: 2 additions & 0 deletions beacon-chain/state/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type ReadOnlyBeaconState interface {
Fork() *ethpb.Fork
LatestBlockHeader() *ethpb.BeaconBlockHeader
HistoricalRoots() [][]byte
HistoricalSummaries() []*ethpb.HistoricalSummary
Slashings() []uint64
FieldReferencesCount() map[string]uint64
MarshalSSZ() ([]byte, error)
Expand Down Expand Up @@ -85,6 +86,7 @@ type WriteOnlyBeaconState interface {
SetSlashings(val []uint64) error
UpdateSlashingsAtIndex(idx, val uint64) error
AppendHistoricalRoots(root [32]byte) error
AppendHistoricalSummariesUpdate(*ethpb.HistoricalSummary) error
Copy link
Contributor

Choose a reason for hiding this comment

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

This Update part sounds weird. How about AppendHistoricalSummaries?

SetLatestExecutionPayloadHeader(payload interfaces.ExecutionData) error
SetNextWithdrawalIndex(i uint64) error
SetNextWithdrawalValidatorIndex(i types.ValidatorIndex) error
Expand Down
1 change: 1 addition & 0 deletions beacon-chain/state/state-native/beacon_state_mainnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type BeaconState struct {
blockRoots *customtypes.BlockRoots
stateRoots *customtypes.StateRoots
historicalRoots customtypes.HistoricalRoots
historicalSummaries []*ethpb.HistoricalSummary
eth1Data *ethpb.Eth1Data
eth1DataVotes []*ethpb.Eth1Data
eth1DepositIndex uint64
Expand Down
1 change: 1 addition & 0 deletions beacon-chain/state/state-native/beacon_state_minimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type BeaconState struct {
blockRoots *customtypes.BlockRoots
stateRoots *customtypes.StateRoots
historicalRoots customtypes.HistoricalRoots
historicalSummaries []*ethpb.HistoricalSummary
eth1Data *ethpb.Eth1Data
eth1DataVotes []*ethpb.Eth1Data
eth1DepositIndex uint64
Expand Down
18 changes: 18 additions & 0 deletions beacon-chain/state/state-native/getters_misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,21 @@ func (b *BeaconState) balancesLength() int {

return len(b.balances)
}

// HistoricalSummaries of the beacon state.
func (b *BeaconState) HistoricalSummaries() []*ethpb.HistoricalSummary {
if b.historicalSummaries == nil {
Copy link
Member Author

@terencechain terencechain Jan 9, 2023

Choose a reason for hiding this comment

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

I skipped the versioning checks here and HistoricalRoots() because the list would just be emptied and error out upon return. I don't think it's a big deal, and I didn't feel like changing the return argument to include an error. Happy to change it otherwise

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't most of the other getters return an error when a property is accessed on a version of the state which does not have that property?

return nil
}

b.lock.RLock()
defer b.lock.RUnlock()

return b.historicalSummariesVal()
}

// historicalSummariesVal of the beacon state.
// This assumes that a lock is already held on BeaconState.
func (b *BeaconState) historicalSummariesVal() []*ethpb.HistoricalSummary {
return ethpb.CopyHistoricalSummaries(b.historicalSummaries)
}
2 changes: 2 additions & 0 deletions beacon-chain/state/state-native/getters_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ func (b *BeaconState) ToProtoUnsafe() interface{} {
LatestExecutionPayloadHeader: b.latestExecutionPayloadHeaderCapella,
NextWithdrawalIndex: b.nextWithdrawalIndex,
NextWithdrawalValidatorIndex: b.nextWithdrawalValidatorIndex,
HistoricalSummaries: b.historicalSummaries,
}
default:
return nil
Expand Down Expand Up @@ -252,6 +253,7 @@ func (b *BeaconState) ToProto() interface{} {
LatestExecutionPayloadHeader: b.latestExecutionPayloadHeaderCapellaVal(),
NextWithdrawalIndex: b.nextWithdrawalIndex,
NextWithdrawalValidatorIndex: b.nextWithdrawalValidatorIndex,
HistoricalSummaries: b.historicalSummariesVal(),
}
default:
return nil
Expand Down
46 changes: 46 additions & 0 deletions beacon-chain/state/state-native/hasher.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package state_native

import (
"bytes"
"context"
"encoding/binary"
"fmt"

"github.com/pkg/errors"
nativetypes "github.com/prysmaticlabs/prysm/v3/beacon-chain/state/state-native/types"
Expand All @@ -12,6 +14,7 @@ import (
"github.com/prysmaticlabs/prysm/v3/crypto/hash"
"github.com/prysmaticlabs/prysm/v3/encoding/bytesutil"
"github.com/prysmaticlabs/prysm/v3/encoding/ssz"
ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v3/runtime/version"
"go.opencensus.io/trace"
)
Expand Down Expand Up @@ -255,7 +258,50 @@ func ComputeFieldRootsWithHasher(ctx context.Context, state *BeaconState) ([][]b
nextWithdrawalValidatorIndexRoot := make([]byte, 32)
binary.LittleEndian.PutUint64(nextWithdrawalValidatorIndexRoot, uint64(state.nextWithdrawalValidatorIndex))
fieldRoots[nativetypes.NextWithdrawalValidatorIndex.RealPosition()] = nextWithdrawalValidatorIndexRoot

// Historical summary root.
historicalSummaryRoot, err := historicalSummaryRoot(state.historicalSummaries)
if err != nil {
return nil, errors.Wrap(err, "could not compute historical summary merkleization")
}
fieldRoots[nativetypes.HistoricalSummaries.RealPosition()] = historicalSummaryRoot[:]
}

return fieldRoots, nil
}

func historicalSummaryRoot(summaries []*ethpb.HistoricalSummary) ([32]byte, error) {
Copy link
Member

Choose a reason for hiding this comment

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

This should be in the stateutil package, since there is where we keep all the state hashing functions at

max := uint64(fieldparams.HistoricalRootsLength)
if uint64(len(summaries)) > max {
return [32]byte{}, fmt.Errorf("historical summary exceeds max length %d", max)
}

hasher := hash.CustomSHA256Hasher()
roots := make([][32]byte, len(summaries))
for i := 0; i < len(summaries); i++ {
r, err := summaries[i].HashTreeRoot()
if err != nil {
return [32]byte{}, errors.Wrap(err, "could not merkleize historical summary")
}
roots[i] = r
}

summariesRoot, err := ssz.BitwiseMerkleize(
hasher,
roots,
uint64(len(roots)),
fieldparams.HistoricalRootsLength,
)
if err != nil {
return [32]byte{}, errors.Wrap(err, "could not compute epoch attestations merkleization")
terencechain marked this conversation as resolved.
Show resolved Hide resolved
}
summariesLenBuf := new(bytes.Buffer)
if err := binary.Write(summariesLenBuf, binary.LittleEndian, uint64(len(summaries))); err != nil {
return [32]byte{}, errors.Wrap(err, "could not marshal historical summary length")
}
// We need to mix in the length of the slice.
summariesLenRoot := make([]byte, 32)
copy(summariesLenRoot, summariesLenBuf.Bytes())
res := ssz.MixInLength(summariesRoot, summariesLenRoot)
return res, nil
}
Loading