Skip to content

Commit

Permalink
fix(state-transition): enforce valid eth1 withdrawal credentials (#2231)
Browse files Browse the repository at this point in the history
Co-authored-by: aBear <[email protected]>
  • Loading branch information
calbera and abi87 authored Dec 10, 2024
1 parent b00e245 commit e70bafa
Show file tree
Hide file tree
Showing 10 changed files with 258 additions and 72 deletions.
7 changes: 7 additions & 0 deletions consensus-types/types/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ func (d *Deposit) GetTree() (*fastssz.Node, error) {
/* -------------------------------------------------------------------------- */
/* Getters and Setters */
/* -------------------------------------------------------------------------- */

// Equals returns true if the Deposit is equal to the other.
func (d *Deposit) Equals(rhs *Deposit) bool {
return d.Pubkey == rhs.Pubkey &&
Expand Down Expand Up @@ -219,3 +220,9 @@ func (d *Deposit) GetSignature() crypto.BLSSignature {
func (d *Deposit) GetWithdrawalCredentials() WithdrawalCredentials {
return d.Credentials
}

// HasEth1WithdrawalCredentials returns true if the deposit has eth1 withdrawal
// credentials.
func (d *Deposit) HasEth1WithdrawalCredentials() bool {
return d.Credentials[0] == EthSecp256k1CredentialPrefix
}
11 changes: 8 additions & 3 deletions consensus-types/types/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,15 @@ func (v Validator) IsEligibleForActivation(finalizedEpoch math.Epoch) bool {
v.ActivationEpoch == math.Epoch(constants.FarFutureEpoch)
}

// IsEligibleForActivationQueue is defined slightly differently from Ethereum 2.0 Spec
// IsEligibleForActivationQueue is defined slightly differently from Ethereum
// 2.0 Spec
// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#is_eligible_for_activation_queue
//
//nolint:lll
func (v Validator) IsEligibleForActivationQueue(threshold math.Gwei) bool {
return v.ActivationEligibilityEpoch == math.Epoch(constants.FarFutureEpoch) &&
return v.ActivationEligibilityEpoch == math.Epoch(
constants.FarFutureEpoch,
) &&
v.EffectiveBalance >= threshold
}

Expand Down Expand Up @@ -290,7 +293,9 @@ func (v Validator) IsFullyWithdrawable(
// https://github.com/ethereum/consensus-specs/blob/dev/specs/capella/beacon-chain.md#is_partially_withdrawable_validator
//
//nolint:lll
func (v Validator) IsPartiallyWithdrawable(balance, maxEffectiveBalance math.Gwei) bool {
func (v Validator) IsPartiallyWithdrawable(
balance, maxEffectiveBalance math.Gwei,
) bool {
hasExcessBalance := balance > maxEffectiveBalance
return v.HasEth1WithdrawalCredentials() &&
v.HasMaxEffectiveBalance(maxEffectiveBalance) && hasExcessBalance
Expand Down
3 changes: 3 additions & 0 deletions node-core/components/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,9 @@ type (
GetPubkey() crypto.BLSPubkey
// GetWithdrawalCredentials returns the withdrawal credentials.
GetWithdrawalCredentials() WithdrawalCredentialsT
// HasEth1WithdrawalCredentials returns true if the deposit has eth1
// withdrawal credentials.
HasEth1WithdrawalCredentials() bool
// VerifySignature verifies the deposit and creates a validator.
VerifySignature(
forkData ForkDataT,
Expand Down
29 changes: 21 additions & 8 deletions state-transition/core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (s *StateDB[
// NOTE: This function is modified from the spec to allow a fixed withdrawal
// (as the first withdrawal) used for EVM inflation.
//
//nolint:lll,funlen // TODO: Simplify when dropping special cases.
//nolint:lll,funlen,gocognit // TODO: Simplify when dropping special cases.
func (s *StateDB[
_, _, _, _, _, _, ValidatorT, _, WithdrawalT, _,
]) ExpectedWithdrawals() ([]WithdrawalT, error) {
Expand Down Expand Up @@ -268,16 +268,16 @@ func (s *StateDB[
return nil, err
}

withdrawalAddress, err = validator.
GetWithdrawalCredentials().ToExecutionAddress()
if err != nil {
return nil, err
}

// Set the amount of the withdrawal depending on the balance of the
// validator.
//nolint:gocritic // ok.
//nolint:gocritic,nestif // ok.
if validator.IsFullyWithdrawable(balance, epoch) {
withdrawalAddress, err = validator.
GetWithdrawalCredentials().ToExecutionAddress()
if err != nil {
return nil, err
}

withdrawals = append(withdrawals, withdrawal.New(
math.U64(withdrawalIndex),
validatorIndex,
Expand All @@ -290,6 +290,12 @@ func (s *StateDB[
} else if validator.IsPartiallyWithdrawable(
balance, math.Gwei(s.cs.MaxEffectiveBalance()),
) {
withdrawalAddress, err = validator.
GetWithdrawalCredentials().ToExecutionAddress()
if err != nil {
return nil, err
}

withdrawals = append(withdrawals, withdrawal.New(
math.U64(withdrawalIndex),
validatorIndex,
Expand All @@ -302,6 +308,13 @@ func (s *StateDB[
} else if s.cs.DepositEth1ChainID() == spec.BartioChainID {
// Backward compatibility with Bartio
// TODO: Drop this when we drop other Bartio special cases.

withdrawalAddress, err = validator.
GetWithdrawalCredentials().ToExecutionAddress()
if err != nil {
return nil, err
}

withdrawal = withdrawal.New(
math.U64(withdrawalIndex),
validatorIndex,
Expand Down
10 changes: 7 additions & 3 deletions state-transition/core/state_processor_genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ func (sp *StateProcessor[
return sp.validatorSetsDiffs(nil, activeVals), nil
}

//nolint:lll // let it be.
func (sp *StateProcessor[
_, _, _, BeaconStateT, _, _, _, _, _, _, _, _, ValidatorT, _, _, _, _,
]) processGenesisActivation(
Expand All @@ -173,9 +172,14 @@ func (sp *StateProcessor[
default:
vals, err := st.GetValidators()
if err != nil {
return fmt.Errorf("genesis activation, failed listing validators: %w", err)
return fmt.Errorf(
"genesis activation, failed listing validators: %w",
err,
)
}
minEffectiveBalance := math.Gwei(sp.cs.EjectionBalance() + sp.cs.EffectiveBalanceIncrement())
minEffectiveBalance := math.Gwei(
sp.cs.EjectionBalance() + sp.cs.EffectiveBalanceIncrement(),
)

var idx math.ValidatorIndex
for _, val := range vals {
Expand Down
80 changes: 64 additions & 16 deletions state-transition/core/state_processor_genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,42 +49,66 @@ func TestInitialize(t *testing.T) {
{
Pubkey: [48]byte{0x01},
Amount: maxBalance,
Index: uint64(0),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x01},
),
Index: uint64(0),
},
{
Pubkey: [48]byte{0x02},
Amount: minBalance + increment,
Index: uint64(1),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x02},
),
Index: uint64(1),
},
{
Pubkey: [48]byte{0x03},
Amount: minBalance,
Index: uint64(2),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x03},
),
Index: uint64(2),
},
{
Pubkey: [48]byte{0x04},
Amount: 2 * maxBalance,
Index: uint64(3),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x04},
),
Index: uint64(3),
},
{
Pubkey: [48]byte{0x05},
Amount: minBalance - increment,
Index: uint64(4),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x05},
),
Index: uint64(4),
},
{
Pubkey: [48]byte{0x06},
Amount: minBalance + increment*3/2,
Index: uint64(5),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x06},
),
Index: uint64(5),
},
{
Pubkey: [48]byte{0x07},
Amount: maxBalance + increment/10,
Index: uint64(6),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x07},
),
Index: uint64(6),
},
{
Pubkey: [48]byte{0x08},
Amount: minBalance + increment*99/100,
Index: uint64(7),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x08},
),
Index: uint64(7),
},
}
goodDeposits = []*types.Deposit{
Expand Down Expand Up @@ -179,42 +203,66 @@ func TestInitializeBartio(t *testing.T) {
{
Pubkey: [48]byte{0x01},
Amount: maxBalance,
Index: uint64(0),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x01},
),
Index: uint64(0),
},
{
Pubkey: [48]byte{0x02},
Amount: minBalance + increment,
Index: uint64(1),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x02},
),
Index: uint64(1),
},
{
Pubkey: [48]byte{0x03},
Amount: minBalance,
Index: uint64(2),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x03},
),
Index: uint64(2),
},
{
Pubkey: [48]byte{0x04},
Amount: 2 * maxBalance,
Index: uint64(3),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x04},
),
Index: uint64(3),
},
{
Pubkey: [48]byte{0x05},
Amount: minBalance - increment,
Index: uint64(4),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x05},
),
Index: uint64(4),
},
{
Pubkey: [48]byte{0x06},
Amount: minBalance + increment*3/2,
Index: uint64(5),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x06},
),
Index: uint64(5),
},
{
Pubkey: [48]byte{0x07},
Amount: maxBalance + increment/10,
Index: uint64(6),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x07},
),
Index: uint64(6),
},
{
Pubkey: [48]byte{0x08},
Amount: minBalance + increment*99/100,
Index: uint64(7),
Credentials: types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{0x08},
),
Index: uint64(7),
},
}
goodDeposits = []*types.Deposit{
Expand Down
11 changes: 10 additions & 1 deletion state-transition/core/state_processor_staking.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,16 @@ func (sp *StateProcessor[
// Get the current epoch.
epoch := sp.cs.SlotToEpoch(slot)

// Verify that the deposit has the ETH1 withdrawal credentials.
if !dep.HasEth1WithdrawalCredentials() {
// Ignore deposits with non-ETH1 withdrawal credentials.
sp.logger.Info(
"ignoring deposit with non-ETH1 withdrawal credentials",
"deposit_index", dep.GetIndex(),
)
return nil
}

// Verify that the message was signed correctly.
var d ForkDataT
if err = dep.VerifySignature(
Expand All @@ -174,7 +184,6 @@ func (sp *StateProcessor[
"deposit_index", dep.GetIndex(),
"error", err,
)

return nil
}

Expand Down
Loading

0 comments on commit e70bafa

Please sign in to comment.