Skip to content

Commit

Permalink
fix(state-transition): Correctly enforce deposits length to avoid pan…
Browse files Browse the repository at this point in the history
…ics (berachain#2369)

Co-authored-by: aBear <[email protected]>
  • Loading branch information
calbera and abi87 authored Jan 18, 2025
1 parent ce54f45 commit 7b02411
Show file tree
Hide file tree
Showing 3 changed files with 252 additions and 15 deletions.
16 changes: 8 additions & 8 deletions state-transition/core/state_processor_staking.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,14 @@ import (
"github.com/berachain/beacon-kit/state-transition/core/state"
)

// processOperations processes the operations and ensures they match the
// local state.
// processOperations processes the operations and ensures they match the local state.
func (sp *StateProcessor[_]) processOperations(
ctx context.Context, st *state.StateDB, blk *ctypes.BeaconBlock,
) error {
// Verify that outstanding deposits are processed
// up to the maximum number of deposits

// Unlike Eth 2.0 specs we don't check that
// len(body.deposits) == min(MAX_DEPOSITS,
// state.eth1_data.deposit_count - state.eth1_deposit_index)
// Verify that outstanding deposits are processed up to the maximum number of deposits.
//
// Unlike Eth 2.0 specs we don't check the following:
// `len(body.deposits) == min(MAX_DEPOSITS, state.eth1_data.deposit_count - state.eth1_deposit_index)`.
// Instead we directly compare block deposits with store ones.
deposits := blk.GetBody().GetDeposits()
if uint64(len(deposits)) > sp.cs.MaxDepositsPerBlock() {
Expand All @@ -52,16 +49,19 @@ func (sp *StateProcessor[_]) processOperations(
sp.cs.MaxDepositsPerBlock(), len(deposits),
)
}

if err := sp.validateNonGenesisDeposits(
ctx, st, deposits, blk.GetBody().GetEth1Data().DepositRoot,
); err != nil {
return err
}

for _, dep := range deposits {
if err := sp.processDeposit(st, dep); err != nil {
return err
}
}

return st.SetEth1Data(blk.GetBody().Eth1Data)
}

Expand Down
16 changes: 12 additions & 4 deletions state-transition/core/validation_deposits.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,21 @@ func (sp *StateProcessor[_]) validateNonGenesisDeposits(
return err
}

var localDeposits ctypes.Deposits
localDeposits, err = sp.ds.GetDepositsByIndex(ctx, 0, depositIndex+uint64(len(blkDeposits)))
expectedLocalDepositsLen := depositIndex + uint64(len(blkDeposits))
localDeposits, err := sp.ds.GetDepositsByIndex(ctx, 0, expectedLocalDepositsLen)
if err != nil {
return err
}

// First check that the block's deposits 1) have contiguous indices and 2) match the local
// First verify that the local store returned all the expected deposits
if uint64(len(localDeposits)) != expectedLocalDepositsLen {
return errors.Wrapf(ErrDepositsLengthMismatch,
"local deposit count: %d, expected deposit count: %d",
len(localDeposits), expectedLocalDepositsLen,
)
}

// Then check that the block's deposits 1) have contiguous indices and 2) match the local
// view of the block's deposits.
for i, blkDeposit := range blkDeposits {
blkDepositIndex := blkDeposit.GetIndex().Unwrap()
Expand All @@ -103,7 +111,7 @@ func (sp *StateProcessor[_]) validateNonGenesisDeposits(
}
}

// Then check that the historical deposits root matches locally what's on the beacon block.
// Finally check that the historical deposits root matches locally what's on the beacon block.
if !localDeposits.HashTreeRoot().Equals(blkDepositRoot) {
return ErrDepositsRootMismatch
}
Expand Down
235 changes: 232 additions & 3 deletions state-transition/core/validation_deposits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ package core_test
import (
"testing"

"github.com/berachain/beacon-kit/chain"
"github.com/berachain/beacon-kit/config/spec"
"github.com/berachain/beacon-kit/consensus-types/types"
engineprimitives "github.com/berachain/beacon-kit/engine-primitives/engine-primitives"
"github.com/berachain/beacon-kit/node-core/components"
Expand All @@ -39,9 +41,7 @@ func TestInvalidDeposits(t *testing.T) {
var (
minBalance = math.Gwei(cs.EjectionBalance() + cs.EffectiveBalanceIncrement())
maxBalance = math.Gwei(cs.MaxEffectiveBalance())
credentials0 = types.NewCredentialsFromExecutionAddress(
common.ExecutionAddress{},
)
credentials0 = types.NewCredentialsFromExecutionAddress(common.ExecutionAddress{})
)

// Setup initial state with one validator
Expand Down Expand Up @@ -107,3 +107,232 @@ func TestInvalidDeposits(t *testing.T) {
require.Error(t, err)
require.ErrorContains(t, err, "deposit mismatched")
}

func TestInvalidDepositsCount(t *testing.T) {
cs := setupChain(t, components.BoonetChainSpecType)
sp, st, ds, ctx := setupState(t, cs)

var (
maxBalance = math.Gwei(cs.MaxEffectiveBalance())
credentials0 = types.NewCredentialsFromExecutionAddress(common.ExecutionAddress{})
)

// Setup initial state with one validator
var (
genDeposits = types.Deposits{
{
Pubkey: [48]byte{0x00},
Credentials: credentials0,
Amount: maxBalance,
Index: 0,
},
}
genPayloadHeader = new(types.ExecutionPayloadHeader).Empty()
genVersion = version.FromUint32[common.Version](version.Deneb)
)
require.NoError(t, ds.EnqueueDeposits(ctx, genDeposits))
_, err := sp.InitializePreminedBeaconStateFromEth1(
st, genDeposits, genPayloadHeader, genVersion,
)
require.NoError(t, err)

// Create the correct deposits.
correctDeposits := types.Deposits{
{
Pubkey: [48]byte{0x01},
Credentials: credentials0,
Amount: maxBalance,
Index: 1,
},
{
Pubkey: [48]byte{0x02},
Credentials: credentials0,
Amount: maxBalance,
Index: 2,
},
}

// Create test block with the correct deposits.
depRoot := append(genDeposits, correctDeposits...).HashTreeRoot()
blk := buildNextBlock(
t,
st,
&types.BeaconBlockBody{
ExecutionPayload: &types.ExecutionPayload{
Timestamp: 10,
ExtraData: []byte("testing"),
Transactions: [][]byte{},
Withdrawals: []*engineprimitives.Withdrawal{
st.EVMInflationWithdrawal(),
},
BaseFeePerGas: math.NewU256(0),
},
Eth1Data: types.NewEth1Data(depRoot),
Deposits: correctDeposits,
},
)

// Add JUST 1 correct deposit to local store. This node SHOULD fail to verify.
require.NoError(t, ds.EnqueueDeposits(ctx, types.Deposits{correctDeposits[0]}))

// Run transition.
_, err = sp.Transition(ctx, st, blk)
require.Error(t, err)
require.ErrorContains(t, err, "deposits lengths mismatched")
}

func TestLocalDepositsExceedBlockDeposits(t *testing.T) {
csData := spec.BaseSpec()
csData.DepositEth1ChainID = spec.BoonetEth1ChainID
csData.MaxDepositsPerBlock = 1 // Set only 1 deposit allowed per block.
cs, err := chain.NewSpec(csData)
require.NoError(t, err)
sp, st, ds, ctx := setupState(t, cs)

var (
maxBalance = math.Gwei(cs.MaxEffectiveBalance())
credentials0 = types.NewCredentialsFromExecutionAddress(common.ExecutionAddress{})
)

// Setup initial state with one validator
var (
genDeposits = types.Deposits{
{
Pubkey: [48]byte{0x00},
Credentials: credentials0,
Amount: maxBalance,
Index: 0,
},
}
genPayloadHeader = new(types.ExecutionPayloadHeader).Empty()
genVersion = version.FromUint32[common.Version](version.Deneb)
)
require.NoError(t, ds.EnqueueDeposits(ctx, genDeposits))
_, err = sp.InitializePreminedBeaconStateFromEth1(
st, genDeposits, genPayloadHeader, genVersion,
)
require.NoError(t, err)

// Create the block deposits.
blockDeposits := types.Deposits{
{
Pubkey: [48]byte{0x01},
Credentials: credentials0,
Amount: maxBalance,
Index: 1,
},
}

// Create test block with the correct deposits.
depRoot := append(genDeposits, blockDeposits...).HashTreeRoot()
blk := buildNextBlock(
t,
st,
&types.BeaconBlockBody{
ExecutionPayload: &types.ExecutionPayload{
Timestamp: 10,
ExtraData: []byte("testing"),
Transactions: [][]byte{},
Withdrawals: []*engineprimitives.Withdrawal{
st.EVMInflationWithdrawal(),
},
BaseFeePerGas: math.NewU256(0),
},
Eth1Data: types.NewEth1Data(depRoot),
Deposits: blockDeposits,
},
)

extraLocalDeposit := &types.Deposit{
Pubkey: [48]byte{0x01},
Credentials: credentials0,
Amount: maxBalance,
Index: 2,
}

// Add both deposits to local store (which includes more than what's in the block).
require.NoError(t, ds.EnqueueDeposits(ctx, append(blockDeposits, extraLocalDeposit)))

// Run transition.
_, err = sp.Transition(ctx, st, blk)
require.NoError(t, err)
}

func TestLocalDepositsExceedBlockDepositsBadRoot(t *testing.T) {
csData := spec.BaseSpec()
csData.DepositEth1ChainID = spec.BoonetEth1ChainID
csData.MaxDepositsPerBlock = 1 // Set only 1 deposit allowed per block.
cs, err := chain.NewSpec(csData)
require.NoError(t, err)
sp, st, ds, ctx := setupState(t, cs)

var (
maxBalance = math.Gwei(cs.MaxEffectiveBalance())
credentials0 = types.NewCredentialsFromExecutionAddress(common.ExecutionAddress{})
)

// Setup initial state with one validator
var (
genDeposits = types.Deposits{
{
Pubkey: [48]byte{0x00},
Credentials: credentials0,
Amount: maxBalance,
Index: 0,
},
}
genPayloadHeader = new(types.ExecutionPayloadHeader).Empty()
genVersion = version.FromUint32[common.Version](version.Deneb)
)
require.NoError(t, ds.EnqueueDeposits(ctx, genDeposits))
_, err = sp.InitializePreminedBeaconStateFromEth1(
st, genDeposits, genPayloadHeader, genVersion,
)
require.NoError(t, err)

// Create the block deposits.
blockDeposits := types.Deposits{
{
Pubkey: [48]byte{0x01},
Credentials: credentials0,
Amount: maxBalance,
Index: 1,
},
}

extraLocalDeposit := &types.Deposit{
Pubkey: [48]byte{0x01},
Credentials: credentials0,
Amount: maxBalance,
Index: 2,
}

// Now, the block proposer ends up adding the correct 1 deposit per block, BUT spoofs the
// deposits root to use the entire deposits list.
badDepRoot := append(genDeposits, append(blockDeposits, extraLocalDeposit)...).HashTreeRoot()
blk := buildNextBlock(
t,
st,
&types.BeaconBlockBody{
ExecutionPayload: &types.ExecutionPayload{
Timestamp: 10,
ExtraData: []byte("testing"),
Transactions: [][]byte{},
Withdrawals: []*engineprimitives.Withdrawal{
st.EVMInflationWithdrawal(),
},
BaseFeePerGas: math.NewU256(0),
},
Eth1Data: types.NewEth1Data(badDepRoot),
Deposits: blockDeposits,
},
)

// Add both deposits to local store (which includes more than what's in the block).
require.NoError(t, ds.EnqueueDeposits(ctx, append(blockDeposits, extraLocalDeposit)))

// Run transition.
_, err = sp.Transition(ctx, st, blk)
require.Error(t, err)
require.ErrorContains(t, err, "deposits root mismatch")
}

0 comments on commit 7b02411

Please sign in to comment.