nishant's feedback

beacon-chain/core/helpers/legacy.go

@@ -2,27 +2,20 @@ 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
+// DepositRequestHaveStarted determines if the deposit requests have started
+func DepositRequestHaveStarted(beaconState state.BeaconState) bool {
+	if beaconState.Version() >= version.Electra {
+		requestsStartIndex, err := beaconState.DepositRequestsStartIndex()
+		if err == nil {
+			// deposit_requests_start_index will only be set once
+			// eth1data will be frozen
+			if beaconState.Eth1DepositIndex() == requestsStartIndex {
+				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
-	}
-
-	//canonicalEth1Data should never be nil
-	eth1DepositIndexLimit := math.Min(canonicalEth1Data.DepositCount, requestsStartIndex)
-	return beaconState.Eth1DepositIndex() < eth1DepositIndexLimit
+	return false
 }

beacon-chain/core/helpers/legacy_test.go

@@ -1,94 +1,33 @@
 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/prysmaticlabs/prysm/v5/testing/util"
 	"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: "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)
-			}
-		})
-	}
+// TestDepositRequestHaveStarted contains several test cases for depositRequestHaveStarted.
+func TestDepositRequestHaveStarted(t *testing.T) {
+	t.Run("Version below Electra returns false", func(t *testing.T) {
+		st, _ := util.DeterministicGenesisStateBellatrix(t, 1)
+		result := helpers.DepositRequestHaveStarted(st)
+		require.False(t, result)
+	})
+
+	t.Run("Version is Electra or higher, no error, but Eth1DepositIndex != requestsStartIndex returns false", func(t *testing.T) {
+		st, _ := util.DeterministicGenesisStateElectra(t, 1)
+		require.NoError(t, st.SetEth1DepositIndex(1))
+		result := helpers.DepositRequestHaveStarted(st)
+		require.False(t, result)
+	})
+
+	t.Run("Version is Electra or higher, no error, and Eth1DepositIndex == requestsStartIndex returns true", func(t *testing.T) {
+		st, _ := util.DeterministicGenesisStateElectra(t, 1)
+		require.NoError(t, st.SetEth1DepositIndex(33))
+		require.NoError(t, st.SetDepositRequestsStartIndex(33))
+		result := helpers.DepositRequestHaveStarted(st)
+		require.True(t, result)
+	})
 }

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_deposits.go

@@ -72,6 +72,10 @@ func (vs *Server) packDepositsAndAttestations(
 // this eth1data has enough support to be considered for deposits inclusion. If current vote has
 // enough support, then use that vote for basis of determining deposits, otherwise use current state
 // eth1data.
+// In the post-electra phase, this function will usually return an empty list,
+// 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.
 func (vs *Server) deposits(
 	ctx context.Context,
 	beaconState state.BeaconState,
@@ -88,21 +92,19 @@ func (vs *Server) deposits(
 		log.Warn("not connected to eth1 node, skip pending deposit insertion")
 		return []*ethpb.Deposit{}, nil
 	}
+
+	// skip legacy deposits if eth1 deposit index is already at the index of deposit requests start
+	if helpers.DepositRequestHaveStarted(beaconState) {
+		return []*ethpb.Deposit{}, nil
+	}
+
 	// Need to fetch if the deposits up to the state's latest eth1 data matches
 	// the number of all deposits in this RPC call. If not, then we return nil.
 	canonicalEth1Data, canonicalEth1DataHeight, err := vs.canonicalEth1Data(ctx, beaconState, currentVote)
 	if err != nil {
 		return nil, err
 	}
 
-	// In the post-electra phase, this function will usually return an empty list,
-	// 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 !helpers.IsLegacyDepositProcessPeriod(beaconState, canonicalEth1Data) {
-		return []*ethpb.Deposit{}, nil
-	}
-
 	_, genesisEth1Block := vs.Eth1InfoFetcher.GenesisExecutionChainInfo()
 	if genesisEth1Block.Cmp(canonicalEth1DataHeight) == 0 {
 		return []*ethpb.Deposit{}, nil

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_eth1data.go

@@ -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"
@@ -15,24 +16,9 @@ import (
 	"github.com/prysmaticlabs/prysm/v5/crypto/rand"
 	"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
 	ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
-	"github.com/prysmaticlabs/prysm/v5/runtime/version"
 	"github.com/prysmaticlabs/prysm/v5/time/slots"
 )
 
-func depositRequestHaveStarted(beaconState state.BeaconState) bool {
-	if beaconState.Version() >= version.Electra {
-		requestsStartIndex, err := beaconState.DepositRequestsStartIndex()
-		if err == nil {
-			// deposit_requests_start_index will only be set once
-			// eth1data will be frozen
-			if beaconState.Eth1DepositIndex() == requestsStartIndex {
-				return true
-			}
-		}
-	}
-	return false
-}
-
 // eth1DataMajorityVote determines the appropriate eth1data for a block proposal using
 // an algorithm called Voting with the Majority. The algorithm works as follows:
 //   - Determine the timestamp for the start slot for the eth1 voting period.
@@ -52,7 +38,7 @@ func (vs *Server) eth1DataMajorityVote(ctx context.Context, beaconState state.Be
 	defer cancel()
 
 	// post eth1 deposits, the Eth 1 data will then be frozen
-	if depositRequestHaveStarted(beaconState) {
+	if helpers.DepositRequestHaveStarted(beaconState) {
 		return beaconState.Eth1Data(), nil
 	}