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

skip eth1data voting after electra #14835

Merged
merged 15 commits into from
Jan 31, 2025
Merged
3 changes: 3 additions & 0 deletions beacon-chain/core/helpers/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ go_library(
"beacon_committee.go",
"block.go",
"genesis.go",
"legacy.go",
"metrics.go",
"randao.go",
"rewards_penalties.go",
Expand Down Expand Up @@ -52,6 +53,7 @@ go_test(
"attestation_test.go",
"beacon_committee_test.go",
"block_test.go",
"legacy_test.go",
"private_access_fuzz_noop_test.go", # keep
"private_access_test.go",
"randao_test.go",
Expand Down Expand Up @@ -86,5 +88,6 @@ go_test(
"//time:go_default_library",
"//time/slots:go_default_library",
"@com_github_prysmaticlabs_go_bitfield//:go_default_library",
"@com_github_stretchr_testify//require:go_default_library",
],
)
32 changes: 32 additions & 0 deletions beacon-chain/core/helpers/legacy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package helpers

import (
"github.com/prysmaticlabs/prysm/v5/beacon-chain/state"
"github.com/prysmaticlabs/prysm/v5/math"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v5/runtime/version"
)

// IsLegacyDepositProcessPeriod determines if the current state should use the legacy deposit process.
func IsLegacyDepositProcessPeriod(beaconState state.BeaconState, canonicalEth1Data *ethpb.Eth1Data) bool {
// Before the Electra upgrade, always use the legacy deposit process.
if beaconState.Version() < version.Electra {
return true
}

// Should not happen, but if the canonicalEth1Data is nil
// default to legacy
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this fine?

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 impossible, I don't think this is the right approach

if canonicalEth1Data == nil {
return true
}

// Handle the transition period between the legacy and the new deposit process.
requestsStartIndex, err := beaconState.DepositRequestsStartIndex()
if err != nil {
// If we can't get the deposit requests start index,
// we should default to the legacy deposit process.
return true
}
eth1DepositIndexLimit := math.Min(canonicalEth1Data.DepositCount, requestsStartIndex)
Copy link
Member

Choose a reason for hiding this comment

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

return beaconState.Eth1DepositIndex() < eth1DepositIndexLimit
}
111 changes: 111 additions & 0 deletions beacon-chain/core/helpers/legacy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package helpers_test

import (
"math"
"testing"

"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/state"
state_native "github.com/prysmaticlabs/prysm/v5/beacon-chain/state/state-native"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/stretchr/testify/require"
)

func TestIsLegacyDepositProcessPeriod(t *testing.T) {
tests := []struct {
name string
state state.BeaconState
canonicalEth1Data *ethpb.Eth1Data
want bool
}{
{
name: "pre-electra",
state: func() state.BeaconState {
st, err := state_native.InitializeFromProtoDeneb(&ethpb.BeaconStateDeneb{
Eth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
Eth1DepositIndex: 1,
})
require.NoError(t, err)
return st
}(),
canonicalEth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
want: true,
},
{
name: "nil canonicalEth1Data defaults to legacy",
state: func() state.BeaconState {
st, err := state_native.InitializeFromProtoDeneb(&ethpb.BeaconStateDeneb{
Eth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
Eth1DepositIndex: 1,
})
require.NoError(t, err)
return st
}(),
canonicalEth1Data: nil,
want: true,
},
{
name: "post-electra, pending deposits from pre-electra",
state: func() state.BeaconState {
st, err := state_native.InitializeFromProtoElectra(&ethpb.BeaconStateElectra{
Eth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
DepositRequestsStartIndex: math.MaxUint64,
Eth1DepositIndex: 1,
})
require.NoError(t, err)
return st
}(),
canonicalEth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
want: true,
},
{
name: "post-electra, no pending deposits from pre-alpaca",
state: func() state.BeaconState {
st, err := state_native.InitializeFromProtoElectra(&ethpb.BeaconStateElectra{
Eth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
DepositRequestsStartIndex: 1,
Eth1DepositIndex: 5,
})
require.NoError(t, err)
return st
}(),
canonicalEth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := helpers.IsLegacyDepositProcessPeriod(tt.state, tt.canonicalEth1Data); got != tt.want {
t.Errorf("isLegacyDepositProcessPeriod() = %v, want %v", got, tt.want)
}
})
}
}
21 changes: 2 additions & 19 deletions beacon-chain/rpc/prysm/v1alpha1/validator/proposer_deposits.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/cache"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/state"
"github.com/prysmaticlabs/prysm/v5/config/params"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
Expand Down Expand Up @@ -98,7 +99,7 @@ func (vs *Server) deposits(
// as the legacy deposit process is deprecated. (EIP-6110)
// NOTE: During the transition period, the legacy deposit process
// may still be active and managed. This function handles that scenario.
if !isLegacyDepositProcessPeriod(beaconState, canonicalEth1Data) {
if !helpers.IsLegacyDepositProcessPeriod(beaconState, canonicalEth1Data) {
return []*ethpb.Deposit{}, nil
}

Expand Down Expand Up @@ -285,21 +286,3 @@ func shouldRebuildTrie(totalDepCount, unFinalizedDeps uint64) bool {
unFinalizedCompute := unFinalizedDeps * params.BeaconConfig().DepositContractTreeDepth
return unFinalizedCompute > totalDepCount
}

// isLegacyDepositProcessPeriod determines if the current state should use the legacy deposit process.
func isLegacyDepositProcessPeriod(beaconState state.BeaconState, canonicalEth1Data *ethpb.Eth1Data) bool {
// Before the Electra upgrade, always use the legacy deposit process.
if beaconState.Version() < version.Electra {
return true
}

// Handle the transition period between the legacy and the new deposit process.
requestsStartIndex, err := beaconState.DepositRequestsStartIndex()
if err != nil {
// If we can't get the deposit requests start index,
// we should default to the legacy deposit process.
return true
}
eth1DepositIndexLimit := math.Min(canonicalEth1Data.DepositCount, requestsStartIndex)
return beaconState.Eth1DepositIndex() < eth1DepositIndexLimit
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@ package validator

import (
"context"
"math"
"math/big"
"testing"

mock "github.com/prysmaticlabs/prysm/v5/beacon-chain/blockchain/testing"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/cache/depositsnapshot"
mockExecution "github.com/prysmaticlabs/prysm/v5/beacon-chain/execution/testing"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/state"
state_native "github.com/prysmaticlabs/prysm/v5/beacon-chain/state/state-native"
"github.com/prysmaticlabs/prysm/v5/config/params"
"github.com/prysmaticlabs/prysm/v5/container/trie"
Expand Down Expand Up @@ -214,85 +212,3 @@ func TestProposer_PendingDeposits_Electra(t *testing.T) {
assert.Equal(t, 0, len(deposits), "Received unexpected number of pending deposits")

}

func TestIsLegacyDepositProcessPeriod(t *testing.T) {
tests := []struct {
name string
state state.BeaconState
canonicalEth1Data *ethpb.Eth1Data
want bool
}{
{
name: "pre-electra",
state: func() state.BeaconState {
st, err := state_native.InitializeFromProtoDeneb(&ethpb.BeaconStateDeneb{
Eth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
Eth1DepositIndex: 1,
})
require.NoError(t, err)
return st
}(),
canonicalEth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
want: true,
},
{
name: "post-electra, pending deposits from pre-electra",
state: func() state.BeaconState {
st, err := state_native.InitializeFromProtoElectra(&ethpb.BeaconStateElectra{
Eth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
DepositRequestsStartIndex: math.MaxUint64,
Eth1DepositIndex: 1,
})
require.NoError(t, err)
return st
}(),
canonicalEth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
want: true,
},
{
name: "post-electra, no pending deposits from pre-alpaca",
state: func() state.BeaconState {
st, err := state_native.InitializeFromProtoElectra(&ethpb.BeaconStateElectra{
Eth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
DepositRequestsStartIndex: 1,
Eth1DepositIndex: 5,
})
require.NoError(t, err)
return st
}(),
canonicalEth1Data: &ethpb.Eth1Data{
BlockHash: []byte("0x0"),
DepositRoot: make([]byte, 32),
DepositCount: 5,
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isLegacyDepositProcessPeriod(tt.state, tt.canonicalEth1Data); got != tt.want {
t.Errorf("isLegacyDepositProcessPeriod() = %v, want %v", got, tt.want)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/pkg/errors"
fastssz "github.com/prysmaticlabs/fastssz"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/blocks"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/state"
"github.com/prysmaticlabs/prysm/v5/config/features"
"github.com/prysmaticlabs/prysm/v5/config/params"
Expand All @@ -30,10 +31,17 @@ import (
// - Otherwise:
// - Determine the vote with the highest count. Prefer the vote with the highest eth1 block height in the event of a tie.
// - This vote's block is the eth1 block to use for the block proposal.
//
// After Electra and eth1 deposit transition period voting will no longer be needed
func (vs *Server) eth1DataMajorityVote(ctx context.Context, beaconState state.BeaconState) (*ethpb.Eth1Data, error) {
ctx, cancel := context.WithTimeout(ctx, eth1dataTimeout)
defer cancel()

// post eth1 deposits, the Eth 1 data will then be frozen
if !helpers.IsLegacyDepositProcessPeriod(beaconState, vs.HeadFetcher.HeadETH1Data()) {
return vs.HeadFetcher.HeadETH1Data(), nil
}

Copy link
Contributor

Choose a reason for hiding this comment

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

First, I'm not quite sure that eth1_data will be frozen after the transition period, as it's up to each client team how to handle Eth1 polling after Electra.
Second, if we decided to freeze eth1_data, isn't it fine to simply fetch from beaconState, like beaconState.Eth1Data()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thats's a good point should just do it from beacon state. there is an update to the spec here
ethereum/consensus-specs#4106

Copy link
Contributor

Choose a reason for hiding this comment

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

DepositRequestHaveStarted seems much better to read, and more aligned with the latest spec. I will use this for my work also!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i should rename to Requests oops

slot := beaconState.Slot()
votingPeriodStartTime := vs.slotStartTime(slot)

Expand Down
38 changes: 38 additions & 0 deletions beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2698,6 +2698,44 @@ func TestProposer_Eth1Data_MajorityVote(t *testing.T) {
expectedHash := []byte("eth1data")
assert.DeepEqual(t, expectedHash, hash)
})

t.Run("post electra the head eth1data should be returned", func(t *testing.T) {
p := mockExecution.New().
InsertBlock(50, earliestValidTime, []byte("earliest")).
InsertBlock(100, latestValidTime, []byte("latest"))
p.Eth1Data = &ethpb.Eth1Data{
BlockHash: []byte("eth1data"),
}

depositCache, err := depositsnapshot.New()
require.NoError(t, err)

beaconState, err := state_native.InitializeFromProtoElectra(&ethpb.BeaconStateElectra{
Slot: slot,
Eth1DataVotes: []*ethpb.Eth1Data{
{BlockHash: []byte("electra"), DepositCount: 1},
},
})
require.NoError(t, err)

ps := &Server{
ChainStartFetcher: p,
Eth1InfoFetcher: p,
Eth1BlockFetcher: p,
BlockFetcher: p,
DepositFetcher: depositCache,
HeadFetcher: &mock.ChainService{ETH1Data: &ethpb.Eth1Data{BlockHash: []byte("legacy"), DepositCount: 0}},
}

ctx := context.Background()
majorityVoteEth1Data, err := ps.eth1DataMajorityVote(ctx, beaconState)
require.NoError(t, err)

hash := majorityVoteEth1Data.BlockHash

expectedHash := []byte("legacy")
assert.DeepEqual(t, expectedHash, hash)
})
}

func TestProposer_FilterAttestation(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions changelog/james-prysm_electra-eth1voting.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Added

- check to stop eth1 voting after electra and eth1 deposits stop
Loading
Loading