Skip to content

Commit

Permalink
fix(state-transitions): verify deposits against contract (#2115)
Browse files Browse the repository at this point in the history
Signed-off-by: Cal Bera <calbera@berachain.com>
Co-authored-by: Cal Bera <calbera@berachain.com>
  • Loading branch information
abi87 and calbera authored Nov 27, 2024
1 parent 86b41a4 commit f241e64
Show file tree
Hide file tree
Showing 22 changed files with 432 additions and 136 deletions.
6 changes: 3 additions & 3 deletions beacond/cmd/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func DefaultComponents() []any {
*DepositContract, *DepositStore, *ExecutionPayload,
*ExecutionPayloadHeader, *Logger,
],
components.ProvideDepositStore[*Deposit],
components.ProvideDepositStore[*Deposit, *Logger],
components.ProvideDispatcher[
*ConsensusBlock, *BeaconBlock,
*ConsensusSidecars, *BlobSidecars,
Expand Down Expand Up @@ -121,8 +121,8 @@ func DefaultComponents() []any {
],
components.ProvideStateProcessor[
*Logger, *BeaconBlock, *BeaconBlockBody, *BeaconBlockHeader,
*BeaconState, *BeaconStateMarshallable, *Deposit, *ExecutionPayload,
*ExecutionPayloadHeader, *KVStore,
*BeaconState, *BeaconStateMarshallable, *Deposit, *DepositStore,
*ExecutionPayload, *ExecutionPayloadHeader, *KVStore,
],
components.ProvideKVStore[*BeaconBlockHeader, *ExecutionPayloadHeader],
components.ProvideStorageBackend[
Expand Down
2 changes: 1 addition & 1 deletion build/scripts/codegen.mk
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ generate: ## generate all the code
(cd $$module && \
GETH_PKG_INCLUDE=$(GETH_PKG_INCLUDE) go generate ./...) || exit 1; \
done
@go run github.com/vektra/mockery/v2@latest
@go run github.com/vektra/mockery/v2@v2.49.0

generate-check:
@$(MAKE) forge-build
Expand Down
10 changes: 9 additions & 1 deletion mod/beacon/validator/block_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"time"

payloadtime "github.com/berachain/beacon-kit/mod/beacon/payload-time"
"github.com/berachain/beacon-kit/mod/config/pkg/spec"
engineprimitives "github.com/berachain/beacon-kit/mod/engine-primitives/pkg/engine-primitives"
"github.com/berachain/beacon-kit/mod/primitives/pkg/bytes"
"github.com/berachain/beacon-kit/mod/primitives/pkg/common"
Expand Down Expand Up @@ -288,12 +289,19 @@ func (s *Service[
// Set the KZG commitments on the block body.
body.SetBlobKzgCommitments(blobsBundle.GetCommitments())

// Dequeue deposits from the state.
depositIndex, err := st.GetEth1DepositIndex()
if err != nil {
return ErrNilDepositIndexStart
}

// Dequeue deposits from the state.
// Bartio and Boonet pre Fork2 have deposit broken and undervalidated
// Any other network should build deposits the right way
if !(s.chainSpec.DepositEth1ChainID() == spec.BartioChainID ||
(s.chainSpec.DepositEth1ChainID() == spec.BoonetEth1ChainID &&
blk.GetSlot() < math.U64(spec.BoonetFork2Height))) {
depositIndex++
}
deposits, err := s.sb.DepositStore().GetDepositsByIndex(
depositIndex,
s.chainSpec.MaxDepositsPerBlock(),
Expand Down
8 changes: 8 additions & 0 deletions mod/config/pkg/spec/special_cases.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

package spec

import "math"

// Special cased Bartio for some ad-hoc handling due to the way
// some bugs were handled on Bartio. To be removed.
const (
Expand All @@ -28,3 +30,9 @@ const (
//nolint:lll // temporary.
BArtioValRoot = "0x9147586693b6e8faa837715c0f3071c2000045b54233901c2e7871b15872bc43"
)

const ( // Planned hard-fork upgrades on boonet.
BoonetFork1Height uint64 = 69420

BoonetFork2Height uint64 = math.MaxUint64
)
8 changes: 8 additions & 0 deletions mod/consensus-types/pkg/types/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,14 @@ 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 &&
d.Credentials == rhs.Credentials &&
d.Amount == rhs.Amount &&
d.Signature == rhs.Signature &&
d.Index == rhs.Index
}

// GetAmount returns the deposit amount in gwei.
func (d *Deposit) GetAmount() math.Gwei {
Expand Down
9 changes: 9 additions & 0 deletions mod/execution/pkg/deposit/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ func (s *Service[
BeaconBlockT, _, _, _, _,
]) depositFetcher(ctx context.Context, event async.Event[BeaconBlockT]) {
blockNum := event.Data().GetBody().GetExecutionPayload().GetNumber()
if blockNum < s.eth1FollowDistance {
s.logger.Info(
"depositFetcher, nothing to fetch",
"block num", blockNum,
"eth1FollowDistance", s.eth1FollowDistance,
)
return
}

s.fetchAndStoreDeposits(ctx, blockNum-s.eth1FollowDistance)
}

Expand Down
13 changes: 10 additions & 3 deletions mod/node-core/pkg/components/deposit_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ import (
)

// DepositStoreInput is the input for the dep inject framework.
type DepositStoreInput struct {
type DepositStoreInput[
LoggerT log.AdvancedLogger[LoggerT],
] struct {
depinject.In
Logger LoggerT
AppOpts config.AppOptions
}

Expand All @@ -48,8 +51,9 @@ func ProvideDepositStore[
DepositT Deposit[
DepositT, *ForkData, WithdrawalCredentials,
],
LoggerT log.AdvancedLogger[LoggerT],
](
in DepositStoreInput,
in DepositStoreInput[LoggerT],
) (*depositstore.KVStore[DepositT], error) {
name := "deposits"
dir := cast.ToString(in.AppOpts.Get(flags.FlagHome)) + "/data"
Expand All @@ -58,7 +62,10 @@ func ProvideDepositStore[
return nil, err
}

return depositstore.NewStore[DepositT](storage.NewKVStoreProvider(kvp)), nil
return depositstore.NewStore[DepositT](
storage.NewKVStoreProvider(kvp),
in.Logger.With("service", "deposit-store"),
), nil
}

// DepositPrunerInput is the input for the deposit pruner.
Expand Down
2 changes: 2 additions & 0 deletions mod/node-core/pkg/components/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ type (
crypto.BLSSignature,
uint64,
) T
// Equals returns true if the Deposit is equal to the other.
Equals(T) bool
// GetIndex returns the index of the deposit.
GetIndex() math.U64
// GetAmount returns the amount of the deposit.
Expand Down
8 changes: 6 additions & 2 deletions mod/node-core/pkg/components/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type StateProcessorInput[
ExecutionPayloadT, ExecutionPayloadHeaderT, WithdrawalsT,
],
ExecutionPayloadHeaderT ExecutionPayloadHeader[ExecutionPayloadHeaderT],
DepositT Deposit[DepositT, *ForkData, WithdrawalCredentials],
WithdrawalT Withdrawal[WithdrawalT],
WithdrawalsT Withdrawals[WithdrawalT],
] struct {
Expand All @@ -50,7 +51,8 @@ type StateProcessorInput[
PayloadID,
WithdrawalsT,
]
Signer crypto.BLSSigner
DepositStore DepositStore[DepositT]
Signer crypto.BLSSigner
}

// ProvideStateProcessor provides the state processor to the depinject
Expand All @@ -70,6 +72,7 @@ func ProvideStateProcessor[
],
BeaconStateMarshallableT any,
DepositT Deposit[DepositT, *ForkData, WithdrawalCredentials],
DepositStoreT DepositStore[DepositT],
ExecutionPayloadT ExecutionPayload[
ExecutionPayloadT, ExecutionPayloadHeaderT, WithdrawalsT,
],
Expand All @@ -84,7 +87,7 @@ func ProvideStateProcessor[
in StateProcessorInput[
LoggerT,
ExecutionPayloadT, ExecutionPayloadHeaderT,
WithdrawalT, WithdrawalsT,
DepositT, WithdrawalT, WithdrawalsT,
],
) *core.StateProcessor[
BeaconBlockT, BeaconBlockBodyT, BeaconBlockHeaderT,
Expand Down Expand Up @@ -114,6 +117,7 @@ func ProvideStateProcessor[
in.Logger.With("service", "state-processor"),
in.ChainSpec,
in.ExecutionEngine,
in.DepositStore,
in.Signer,
crypto.GetAddressFromPubKey,
)
Expand Down
160 changes: 160 additions & 0 deletions mod/state-transition/pkg/core/deposits_validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
// SPDX-License-Identifier: BUSL-1.1
//
// Copyright (C) 2024, Berachain Foundation. All rights reserved.
// Use of this software is governed by the Business Source License included
// in the LICENSE file of this repository and at www.mariadb.com/bsl11.
//
// ANY USE OF THE LICENSED WORK IN VIOLATION OF THIS LICENSE WILL AUTOMATICALLY
// TERMINATE YOUR RIGHTS UNDER THIS LICENSE FOR THE CURRENT AND ALL OTHER
// VERSIONS OF THE LICENSED WORK.
//
// THIS LICENSE DOES NOT GRANT YOU ANY RIGHT IN ANY TRADEMARK OR LOGO OF
// LICENSOR OR ITS AFFILIATES (PROVIDED THAT YOU MAY USE A TRADEMARK OR LOGO OF
// LICENSOR AS EXPRESSLY REQUIRED BY THIS LICENSE).
//
// TO THE EXTENT PERMITTED BY APPLICABLE LAW, THE LICENSED WORK IS PROVIDED ON
// AN “AS IS” BASIS. LICENSOR HEREBY DISCLAIMS ALL WARRANTIES AND CONDITIONS,
// EXPRESS OR IMPLIED, INCLUDING (WITHOUT LIMITATION) WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE, NON-INFRINGEMENT, AND
// TITLE.

package core

import (
"fmt"

"github.com/berachain/beacon-kit/mod/config/pkg/spec"
"github.com/berachain/beacon-kit/mod/errors"
"github.com/berachain/beacon-kit/mod/primitives/pkg/math"
)

func (sp *StateProcessor[
_, _, _, BeaconStateT, _, DepositT,
_, _, _, _, _, _, _, _, _, _, _,
]) validateGenesisDeposits(
st BeaconStateT,
deposits []DepositT,
) error {
switch {
case sp.cs.DepositEth1ChainID() == spec.BartioChainID:
// Bartio does not properly validate deposits index
// We skip checks for backward compatibility
return nil

case sp.cs.DepositEth1ChainID() == spec.BoonetEth1ChainID:
// Boonet inherited the bug from Bartio and it may have added some
// validators before we activate the fork. So we skip validation
// before fork activation
return nil

default:
if _, err := st.GetEth1DepositIndex(); err == nil {
// there should not be Eth1DepositIndex stored before
// genesis first deposit
return errors.Wrap(
ErrDepositMismatch,
"Eth1DepositIndex should be unset at genesis",
)
}
if len(deposits) == 0 {
// there should be at least a validator in genesis
return errors.Wrap(
ErrDepositsLengthMismatch,
"at least one validator should be in genesis",
)
}
for i, deposit := range deposits {
// deposit indices should be contiguous
if deposit.GetIndex() != math.U64(i) {
return errors.Wrapf(
ErrDepositIndexOutOfOrder,
"genesis deposit index: %d, expected index: %d",
deposit.GetIndex().Unwrap(), i,
)
}
}
return nil
}
}

func (sp *StateProcessor[
_, _, _, BeaconStateT, _, DepositT,
_, _, _, _, _, _, _, _, _, _, _,
]) validateNonGenesisDeposits(
st BeaconStateT,
deposits []DepositT,
) error {
slot, err := st.GetSlot()
if err != nil {
return fmt.Errorf(
"failed loading slot while processing deposits: %w", err,
)
}
switch {
case sp.cs.DepositEth1ChainID() == spec.BartioChainID:
// Bartio does not properly validate deposits index
// We skip checks for backward compatibility
return nil

case sp.cs.DepositEth1ChainID() == spec.BoonetEth1ChainID &&
slot < math.U64(spec.BoonetFork2Height):
// Boonet inherited the bug from Bartio and it may have added some
// validators before we activate the fork. So we skip validation
// before fork activation
return nil

default:
// Verify that outstanding deposits match those listed by contract
var depositIndex uint64
depositIndex, err = st.GetEth1DepositIndex()
if err != nil {
return err
}
expectedStartIdx := depositIndex + 1

var localDeposits []DepositT
localDeposits, err = sp.ds.GetDepositsByIndex(
expectedStartIdx,
sp.cs.MaxDepositsPerBlock(),
)
if err != nil {
return err
}

sp.logger.Info(
"processOperations",
"Expected deposit start index", expectedStartIdx,
"Expected deposits length", len(localDeposits),
)

if len(localDeposits) != len(deposits) {
return errors.Wrapf(
ErrDepositsLengthMismatch,
"local: %d, payload: %d", len(localDeposits), len(deposits),
)
}

for i, sd := range localDeposits {
// Deposit indices should be contiguous
//#nosec:G701 // i never negative
expectedIdx := expectedStartIdx + uint64(i)
if sd.GetIndex().Unwrap() != expectedIdx {
return errors.Wrapf(
ErrDepositIndexOutOfOrder,
"local deposit index: %d, expected index: %d",
sd.GetIndex().Unwrap(), expectedIdx,
)
}

if !sd.Equals(deposits[i]) {
return errors.Wrapf(
ErrDepositMismatch,
"local deposit: %+v, payload deposit: %+v",
sd, deposits[i],
)
}
}

return nil
}
}
14 changes: 14 additions & 0 deletions mod/state-transition/pkg/core/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,22 @@ var (
// match the expected value.
ErrSlotMismatch = errors.New("slot mismatch")

// ErrProposerMismatch is returned when proposer referenced in block does
// not match with proposer reported by consensus.
ErrProposerMismatch = errors.New("proposer key mismatch")

// ErrDepositsLengthMismatch is returned when length of deposits
// listed in block is different from deposits from store.
ErrDepositsLengthMismatch = errors.New("deposits lengths mismatched")

// ErrDepositMismatch is returned when a specific deposit listed in
// block is different from the corrispondent one from store.
ErrDepositMismatch = errors.New("deposit mismatched")

// ErrDepositIndexOutOfOrder is returned when deposits are not in
// contiguous order.
ErrDepositIndexOutOfOrder = errors.New("deposit index out of order")

// ErrParentRootMismatch is returned when the parent root in an execution
// payload does not match the expected value.
ErrParentRootMismatch = errors.New("parent root mismatch")
Expand Down
Loading

0 comments on commit f241e64

Please sign in to comment.