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

move credential helpers to ReadOnlyValidator methods #14808

Merged
merged 5 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 9 additions & 3 deletions beacon-chain/core/electra/consolidations.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/pkg/errors"
"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"
"github.com/prysmaticlabs/prysm/v5/config/params"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
Expand Down Expand Up @@ -233,13 +234,18 @@ func ProcessConsolidationRequests(ctx context.Context, st state.BeaconState, req
return fmt.Errorf("failed to fetch source validator: %w", err) // This should never happen.
}

roSrcV, err := state_native.NewValidator(srcV)
if err != nil {
return err
}

tgtV, err := st.ValidatorAtIndexReadOnly(tgtIdx)
if err != nil {
return fmt.Errorf("failed to fetch target validator: %w", err) // This should never happen.
}

// Verify source withdrawal credentials
if !helpers.HasExecutionWithdrawalCredentials(srcV) {
if !roSrcV.HasExecutionWithdrawalCredentials() {
continue
}
// Confirm source_validator.withdrawal_credentials[12:] == consolidation_request.source_address
Expand All @@ -248,7 +254,7 @@ func ProcessConsolidationRequests(ctx context.Context, st state.BeaconState, req
}

// Target validator must have their withdrawal credentials set appropriately.
if !helpers.HasCompoundingWithdrawalCredential(tgtV) {
if !tgtV.HasCompoundingWithdrawalCredentials() {
continue
}

Expand Down Expand Up @@ -364,7 +370,7 @@ func IsValidSwitchToCompoundingRequest(st state.BeaconState, req *enginev1.Conso
return false
}

if !helpers.HasETH1WithdrawalCredential(srcV) {
if !srcV.HasETH1WithdrawalCredentials() {
return false
}

Expand Down
5 changes: 2 additions & 3 deletions beacon-chain/core/electra/effective_balance_updates.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package electra
import (
"fmt"

"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/state"
"github.com/prysmaticlabs/prysm/v5/config/params"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
Expand Down Expand Up @@ -40,7 +39,7 @@ func ProcessEffectiveBalanceUpdates(st state.BeaconState) error {

// Update effective balances with hysteresis.
validatorFunc := func(idx int, val state.ReadOnlyValidator) (newVal *ethpb.Validator, err error) {
if val == nil {
if val.IsNil() {
return nil, fmt.Errorf("validator %d is nil in state", idx)
}
if idx >= len(bals) {
Expand All @@ -49,7 +48,7 @@ func ProcessEffectiveBalanceUpdates(st state.BeaconState) error {
balance := bals[idx]

effectiveBalanceLimit := params.BeaconConfig().MinActivationBalance
if helpers.HasCompoundingWithdrawalCredential(val) {
if val.HasCompoundingWithdrawalCredentials() {
effectiveBalanceLimit = params.BeaconConfig().MaxEffectiveBalanceElectra
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestProcessEffectiveBalnceUpdates(t *testing.T) {
Validators: []*eth.Validator{
{
EffectiveBalance: params.BeaconConfig().MinActivationBalance / 2,
WithdrawalCredentials: nil,
WithdrawalCredentials: make([]byte, 32),
},
},
Balances: []uint64{
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/electra/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func UpgradeToElectra(beaconState state.BeaconState) (state.BeaconState, error)
if val.ActivationEpoch() == params.BeaconConfig().FarFutureEpoch {
preActivationIndices = append(preActivationIndices, primitives.ValidatorIndex(index))
}
if helpers.HasCompoundingWithdrawalCredential(val) {
if val.HasCompoundingWithdrawalCredentials() {
compoundWithdrawalIndices = append(compoundWithdrawalIndices, primitives.ValidatorIndex(index))
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/core/electra/withdrawals.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func ProcessWithdrawalRequests(ctx context.Context, st state.BeaconState, wrs []
return nil, err
}
// Verify withdrawal credentials
hasCorrectCredential := helpers.HasExecutionWithdrawalCredentials(validator)
hasCorrectCredential := validator.HasExecutionWithdrawalCredentials()
wc := validator.GetWithdrawalCredentials()
isCorrectSourceAddress := bytes.Equal(wc[12:], wr.SourceAddress)
if !hasCorrectCredential || !isCorrectSourceAddress {
Expand Down Expand Up @@ -165,7 +165,7 @@ func ProcessWithdrawalRequests(ctx context.Context, st state.BeaconState, wrs []
hasExcessBalance := vBal > params.BeaconConfig().MinActivationBalance+pendingBalanceToWithdraw

// Only allow partial withdrawals with compounding withdrawal credentials
if helpers.HasCompoundingWithdrawalCredential(validator) && hasSufficientEffectiveBalance && hasExcessBalance {
if validator.HasCompoundingWithdrawalCredentials() && hasSufficientEffectiveBalance && hasExcessBalance {
// Spec definition:
// to_withdraw = min(
// state.balances[index] - MIN_ACTIVATION_BALANCE - pending_balance_to_withdraw,
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/fulu/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func UpgradeToFulu(beaconState state.BeaconState) (state.BeaconState, error) {
if val.ActivationEpoch() == params.BeaconConfig().FarFutureEpoch {
preActivationIndices = append(preActivationIndices, primitives.ValidatorIndex(index))
}
if helpers.HasCompoundingWithdrawalCredential(val) {
if val.HasCompoundingWithdrawalCredentials() {
compoundWithdrawalIndices = append(compoundWithdrawalIndices, primitives.ValidatorIndex(index))
}
return nil
Expand Down
1 change: 0 additions & 1 deletion beacon-chain/core/helpers/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ go_library(
"//beacon-chain/state:go_default_library",
"//config/fieldparams:go_default_library",
"//config/params:go_default_library",
"//consensus-types/interfaces:go_default_library",
"//consensus-types/primitives:go_default_library",
"//container/slice:go_default_library",
"//container/trie:go_default_library",
Expand Down
68 changes: 5 additions & 63 deletions beacon-chain/core/helpers/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/prysmaticlabs/prysm/v5/beacon-chain/state"
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/config/params"
"github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v5/crypto/hash"
"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
Expand Down Expand Up @@ -515,63 +514,6 @@ func LastActivatedValidatorIndex(ctx context.Context, st state.ReadOnlyBeaconSta
return lastActivatedvalidatorIndex, nil
}

// hasETH1WithdrawalCredential returns whether the validator has an ETH1
// Withdrawal prefix. It assumes that the caller has a lock on the state
func HasETH1WithdrawalCredential(val interfaces.WithWithdrawalCredentials) bool {
if val == nil {
return false
}
return isETH1WithdrawalCredential(val.GetWithdrawalCredentials())
}

func isETH1WithdrawalCredential(creds []byte) bool {
return bytes.HasPrefix(creds, []byte{params.BeaconConfig().ETH1AddressWithdrawalPrefixByte})
}

// HasCompoundingWithdrawalCredential checks if the validator has a compounding withdrawal credential.
// New in Electra EIP-7251: https://eips.ethereum.org/EIPS/eip-7251
//
// Spec definition:
//
// def has_compounding_withdrawal_credential(validator: Validator) -> bool:
// """
// Check if ``validator`` has an 0x02 prefixed "compounding" withdrawal credential.
// """
// return is_compounding_withdrawal_credential(validator.withdrawal_credentials)
func HasCompoundingWithdrawalCredential(v interfaces.WithWithdrawalCredentials) bool {
if v == nil {
return false
}
return IsCompoundingWithdrawalCredential(v.GetWithdrawalCredentials())
}

// IsCompoundingWithdrawalCredential checks if the credentials are a compounding withdrawal credential.
//
// Spec definition:
//
// def is_compounding_withdrawal_credential(withdrawal_credentials: Bytes32) -> bool:
// return withdrawal_credentials[:1] == COMPOUNDING_WITHDRAWAL_PREFIX
func IsCompoundingWithdrawalCredential(creds []byte) bool {
return bytes.HasPrefix(creds, []byte{params.BeaconConfig().CompoundingWithdrawalPrefixByte})
}

// HasExecutionWithdrawalCredentials checks if the validator has an execution withdrawal credential or compounding credential.
// New in Electra EIP-7251: https://eips.ethereum.org/EIPS/eip-7251
//
// Spec definition:
//
// def has_execution_withdrawal_credential(validator: Validator) -> bool:
// """
// Check if ``validator`` has a 0x01 or 0x02 prefixed withdrawal credential.
// """
// return has_compounding_withdrawal_credential(validator) or has_eth1_withdrawal_credential(validator)
func HasExecutionWithdrawalCredentials(v interfaces.WithWithdrawalCredentials) bool {
if v == nil {
return false
}
return HasCompoundingWithdrawalCredential(v) || HasETH1WithdrawalCredential(v)
}

// IsSameWithdrawalCredentials returns true if both validators have the same withdrawal credentials.
//
// return a.withdrawal_credentials[12:] == b.withdrawal_credentials[12:]
Expand Down Expand Up @@ -606,10 +548,10 @@ func IsFullyWithdrawableValidator(val state.ReadOnlyValidator, balance uint64, e

// Electra / EIP-7251 logic
if fork >= version.Electra {
return HasExecutionWithdrawalCredentials(val) && val.WithdrawableEpoch() <= epoch
return val.HasExecutionWithdrawalCredentials() && val.WithdrawableEpoch() <= epoch
}

return HasETH1WithdrawalCredential(val) && val.WithdrawableEpoch() <= epoch
return val.HasETH1WithdrawalCredentials() && val.WithdrawableEpoch() <= epoch
}

// IsPartiallyWithdrawableValidator returns whether the validator is able to perform a
Expand Down Expand Up @@ -650,7 +592,7 @@ func isPartiallyWithdrawableValidatorElectra(val state.ReadOnlyValidator, balanc
hasMaxBalance := val.EffectiveBalance() == maxEB
hasExcessBalance := balance > maxEB

return HasExecutionWithdrawalCredentials(val) &&
return val.HasExecutionWithdrawalCredentials() &&
hasMaxBalance &&
hasExcessBalance
}
Expand All @@ -670,7 +612,7 @@ func isPartiallyWithdrawableValidatorElectra(val state.ReadOnlyValidator, balanc
func isPartiallyWithdrawableValidatorCapella(val state.ReadOnlyValidator, balance uint64, epoch primitives.Epoch) bool {
hasMaxBalance := val.EffectiveBalance() == params.BeaconConfig().MaxEffectiveBalance
hasExcessBalance := balance > params.BeaconConfig().MaxEffectiveBalance
return HasETH1WithdrawalCredential(val) && hasExcessBalance && hasMaxBalance
return val.HasETH1WithdrawalCredentials() && hasExcessBalance && hasMaxBalance
}

// ValidatorMaxEffectiveBalance returns the maximum effective balance for a validator.
Expand All @@ -686,7 +628,7 @@ func isPartiallyWithdrawableValidatorCapella(val state.ReadOnlyValidator, balanc
// else:
// return MIN_ACTIVATION_BALANCE
func ValidatorMaxEffectiveBalance(val state.ReadOnlyValidator) uint64 {
if HasCompoundingWithdrawalCredential(val) {
if val.HasCompoundingWithdrawalCredentials() {
return params.BeaconConfig().MaxEffectiveBalanceElectra
}
return params.BeaconConfig().MinActivationBalance
Expand Down
20 changes: 12 additions & 8 deletions beacon-chain/core/helpers/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -910,13 +910,15 @@ func TestProposerIndexFromCheckpoint(t *testing.T) {
func TestHasETH1WithdrawalCredentials(t *testing.T) {
creds := []byte{0xFA, 0xCC}
v := &ethpb.Validator{WithdrawalCredentials: creds}
require.Equal(t, false, helpers.HasETH1WithdrawalCredential(v))
roV, err := state_native.NewValidator(v)
require.NoError(t, err)
require.Equal(t, false, roV.HasETH1WithdrawalCredentials())
creds = []byte{params.BeaconConfig().ETH1AddressWithdrawalPrefixByte, 0xCC}
v = &ethpb.Validator{WithdrawalCredentials: creds}
require.Equal(t, true, helpers.HasETH1WithdrawalCredential(v))
roV, err = state_native.NewValidator(v)
require.NoError(t, err)
require.Equal(t, true, roV.HasETH1WithdrawalCredentials())
// No Withdrawal cred
v = &ethpb.Validator{}
require.Equal(t, false, helpers.HasETH1WithdrawalCredential(v))
}

func TestHasCompoundingWithdrawalCredential(t *testing.T) {
Expand All @@ -931,11 +933,12 @@ func TestHasCompoundingWithdrawalCredential(t *testing.T) {
{"Does not have compounding withdrawal credential",
&ethpb.Validator{WithdrawalCredentials: bytesutil.PadTo([]byte{0x00}, 32)},
false},
{"Handles nil case", nil, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, helpers.HasCompoundingWithdrawalCredential(tt.validator))
roV, err := state_native.NewValidator(tt.validator)
require.NoError(t, err)
assert.Equal(t, tt.want, roV.HasCompoundingWithdrawalCredentials())
})
}
}
Expand All @@ -955,11 +958,12 @@ func TestHasExecutionWithdrawalCredentials(t *testing.T) {
{"Does not have compounding withdrawal credential or eth1 withdrawal credential",
&ethpb.Validator{WithdrawalCredentials: bytesutil.PadTo([]byte{0x00}, 32)},
false},
{"Handles nil case", nil, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, helpers.HasExecutionWithdrawalCredentials(tt.validator))
roV, err := state_native.NewValidator(tt.validator)
require.NoError(t, err)
assert.Equal(t, tt.want, roV.HasExecutionWithdrawalCredentials())
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/rpc/eth/events/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ func TestStreamEvents_OperationsEvents(t *testing.T) {
defer testSync.cleanup()

st := tc.getState()
v := &eth.Validator{ExitEpoch: math.MaxUint64, EffectiveBalance: params.BeaconConfig().MinActivationBalance}
v := &eth.Validator{ExitEpoch: math.MaxUint64, EffectiveBalance: params.BeaconConfig().MinActivationBalance, WithdrawalCredentials: make([]byte, 32)}
require.NoError(t, st.SetValidators([]*eth.Validator{v}))
currentSlot := primitives.Slot(0)
// to avoid slot processing
Expand Down
3 changes: 3 additions & 0 deletions beacon-chain/state/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ type ReadOnlyValidator interface {
Copy() *ethpb.Validator
Slashed() bool
IsNil() bool
HasETH1WithdrawalCredentials() bool
HasCompoundingWithdrawalCredentials() bool
HasExecutionWithdrawalCredentials() bool
}

// ReadOnlyValidators defines a struct which only has read access to validators methods.
Expand Down
22 changes: 22 additions & 0 deletions beacon-chain/state/state-native/readonly_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/state"
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/config/params"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
)
Expand Down Expand Up @@ -88,6 +89,27 @@ func (v readOnlyValidator) IsNil() bool {
return v.validator == nil
}

// HasETH1WithdrawalCredentials returns true if the validator has an ETH1 withdrawal credentials.
func (v readOnlyValidator) HasETH1WithdrawalCredentials() bool {
if v.IsNil() {
return false
}
return v.validator.WithdrawalCredentials[0] == params.BeaconConfig().ETH1AddressWithdrawalPrefixByte
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will panic if WithdrawalCredentials is nil, this could be changed in v.IsNil but anyway if we reach code with a nil validator entry we are already booked and there's no reason to continue running

}

// HasCompoundingWithdrawalCredentials returns true if the validator has a compounding withdrawal credentials.
func (v readOnlyValidator) HasCompoundingWithdrawalCredentials() bool {
if v.IsNil() {
return false
}
return v.validator.WithdrawalCredentials[0] == params.BeaconConfig().CompoundingWithdrawalPrefixByte
}

// HasExecutionWithdrawalCredentials returns true if the validator has an execution withdrawal credentials.
func (v readOnlyValidator) HasExecutionWithdrawalCredentials() bool {
return v.HasETH1WithdrawalCredentials() || v.HasCompoundingWithdrawalCredentials()
}

// Copy returns a new validator from the read only validator
func (v readOnlyValidator) Copy() *ethpb.Validator {
pubKey := v.PublicKey()
Expand Down
2 changes: 2 additions & 0 deletions changelog/potuz_credentials_as_methods.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
### Changed
- Remove helpers to check for execution/compounding withdrawal credentials and expose them as methods.
Loading