From 33f42fec477eef70bdbaade1297a18c14f1975c2 Mon Sep 17 00:00:00 2001 From: aBear Date: Mon, 11 Nov 2024 15:59:19 +0100 Subject: [PATCH 1/8] let consensus control payload verification --- mod/beacon/blockchain/process.go | 20 +++---------------- mod/beacon/blockchain/types.go | 5 +++++ .../pkg/cometbft/service/middleware/abci.go | 6 ++++++ mod/consensus/pkg/types/consensus_block.go | 8 ++++++++ mod/node-core/pkg/components/interfaces.go | 5 +++++ 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/mod/beacon/blockchain/process.go b/mod/beacon/blockchain/process.go index 043bba4971..85d1fc4004 100644 --- a/mod/beacon/blockchain/process.go +++ b/mod/beacon/blockchain/process.go @@ -113,23 +113,9 @@ func (s *Service[ // bad peer, and we would likely AppHash anyways. OptimisticEngine: true, - // When we are NOT synced to the tip, process proposal - // does NOT get called and thus we must ensure that - // NewPayload is called to get the execution - // client the payload. - // - // When we are synced to the tip, we can skip the - // NewPayload call since we already gave our execution client - // the payload in process proposal. - // - // In both cases the payload was already accepted by a majority - // of validators in their process proposal call and thus - // the "verification aspect" of this NewPayload call is - // actually irrelevant at this point. - SkipPayloadVerification: false, - - ProposerAddress: blk.GetProposerAddress(), - NextPayloadTimestamp: blk.GetNextPayloadTimestamp(), + SkipPayloadVerification: !blk.GetVerifyPayload(), + ProposerAddress: blk.GetProposerAddress(), + NextPayloadTimestamp: blk.GetNextPayloadTimestamp(), }, st, blk.GetBeaconBlock(), diff --git a/mod/beacon/blockchain/types.go b/mod/beacon/blockchain/types.go index 20906230b5..d894b2a18e 100644 --- a/mod/beacon/blockchain/types.go +++ b/mod/beacon/blockchain/types.go @@ -53,6 +53,11 @@ type ConsensusBlock[BeaconBlockT any] interface { // consensus for the next payload to be proposed. It is also // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 + + // GetVerifyPayload signals whether execution payload should be + // verified or not. We should always verify it except when finalizing + // replayed blocks. + GetVerifyPayload() bool } // BeaconBlock represents a beacon block interface. diff --git a/mod/consensus/pkg/cometbft/service/middleware/abci.go b/mod/consensus/pkg/cometbft/service/middleware/abci.go index a4c73a4450..7725e13eb6 100644 --- a/mod/consensus/pkg/cometbft/service/middleware/abci.go +++ b/mod/consensus/pkg/cometbft/service/middleware/abci.go @@ -221,6 +221,7 @@ func (h *ABCIMiddleware[ blk, req.GetProposerAddress(), req.GetTime().Add(h.minPayloadDelay), + true, // verify payload ) blkEvent := async.NewEvent(ctx, async.BeaconBlockReceived, consensusBlk) if err = h.dispatcher.Publish(blkEvent); err != nil { @@ -346,6 +347,11 @@ func (h *ABCIMiddleware[ blk, req.GetProposerAddress(), req.GetTime().Add(h.minPayloadDelay), + + // Finalize may be called while syncing. In such a case + // we can skip payload verification since block is guaranteed + // to have been accepted by a super majority of validators. + req.SyncingToHeight == req.Height, ) blkEvent := async.NewEvent( ctx, diff --git a/mod/consensus/pkg/types/consensus_block.go b/mod/consensus/pkg/types/consensus_block.go index 0f4b88e5d9..16d8d94076 100644 --- a/mod/consensus/pkg/types/consensus_block.go +++ b/mod/consensus/pkg/types/consensus_block.go @@ -31,6 +31,8 @@ type ConsensusBlock[BeaconBlockT any] struct { // some consensus data useful to build and verify the block *commonConsensusData + + verifyPayload bool } // New creates a new ConsensusBlock instance. @@ -38,6 +40,7 @@ func (b *ConsensusBlock[BeaconBlockT]) New( beaconBlock BeaconBlockT, proposerAddress []byte, nextPayloadTimestamp time.Time, + verifyPayload bool, ) *ConsensusBlock[BeaconBlockT] { b = &ConsensusBlock[BeaconBlockT]{ blk: beaconBlock, @@ -45,6 +48,7 @@ func (b *ConsensusBlock[BeaconBlockT]) New( proposerAddress: proposerAddress, nextPayloadTimestamp: math.U64(nextPayloadTimestamp.Unix()), }, + verifyPayload: verifyPayload, } return b } @@ -52,3 +56,7 @@ func (b *ConsensusBlock[BeaconBlockT]) New( func (b *ConsensusBlock[BeaconBlockT]) GetBeaconBlock() BeaconBlockT { return b.blk } + +func (b *ConsensusBlock[BeaconBlockT]) GetVerifyPayload() bool { + return b.verifyPayload +} diff --git a/mod/node-core/pkg/components/interfaces.go b/mod/node-core/pkg/components/interfaces.go index 6ec8f1156e..aebd937d04 100644 --- a/mod/node-core/pkg/components/interfaces.go +++ b/mod/node-core/pkg/components/interfaces.go @@ -91,6 +91,11 @@ type ( // consensus for the next payload to be proposed. It is also // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 + + // GetVerifyPayload signals whether execution payload should be + // verified or not. We should always verify it except when finalizing + // replayed blocks. + GetVerifyPayload() bool } // BeaconBlock represents a generic interface for a beacon block. From aed28ab7b15a11176c5ab4e26057212d3f5fd334 Mon Sep 17 00:00:00 2001 From: aBear Date: Mon, 11 Nov 2024 17:20:24 +0100 Subject: [PATCH 2/8] switched off payload verify upon block finalization --- mod/beacon/blockchain/process.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod/beacon/blockchain/process.go b/mod/beacon/blockchain/process.go index 85d1fc4004..7cf0cadb3b 100644 --- a/mod/beacon/blockchain/process.go +++ b/mod/beacon/blockchain/process.go @@ -113,7 +113,7 @@ func (s *Service[ // bad peer, and we would likely AppHash anyways. OptimisticEngine: true, - SkipPayloadVerification: !blk.GetVerifyPayload(), + SkipPayloadVerification: true, // TODO: tmp debuggin ProposerAddress: blk.GetProposerAddress(), NextPayloadTimestamp: blk.GetNextPayloadTimestamp(), }, From 1e120b3e9630662dec5ce50eb27862b25cd9d2d1 Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 09:38:34 +0100 Subject: [PATCH 3/8] skipped execution payload timestamp validator on Bartio --- mod/config/pkg/spec/special_cases.go | 30 +++++++++++++++++++ mod/consensus-types/pkg/types/payload.go | 4 +-- .../pkg/core/state_processor_genesis.go | 11 ++----- .../pkg/core/state_processor_payload.go | 15 ++++++---- .../pkg/core/state_processor_staking.go | 3 +- 5 files changed, 46 insertions(+), 17 deletions(-) create mode 100644 mod/config/pkg/spec/special_cases.go diff --git a/mod/config/pkg/spec/special_cases.go b/mod/config/pkg/spec/special_cases.go new file mode 100644 index 0000000000..5407715bc2 --- /dev/null +++ b/mod/config/pkg/spec/special_cases.go @@ -0,0 +1,30 @@ +// 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 spec + +// Special cased Bartio for some ad-hoc handling due to the way +// some bugs were handled on Bartio. To be removed. +const ( + BartioChainID uint64 = 80084 + + //nolint:lll // temporary. + BArtioValRoot = "0x9147586693b6e8faa837715c0f3071c2000045b54233901c2e7871b15872bc43" +) diff --git a/mod/consensus-types/pkg/types/payload.go b/mod/consensus-types/pkg/types/payload.go index 04cc476bec..eb0d472a1c 100644 --- a/mod/consensus-types/pkg/types/payload.go +++ b/mod/consensus-types/pkg/types/payload.go @@ -21,6 +21,7 @@ package types import ( + "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/errors" "github.com/berachain/beacon-kit/mod/primitives/pkg/bytes" @@ -569,8 +570,7 @@ func (p *ExecutionPayload) ToHeader( // TODO: This is live on bArtio with a bug and needs to be hardforked // off of. This is a temporary solution to avoid breaking changes. - //nolint:mnd // don't want the circular dep. - if eth1ChainID == 80084 { + if eth1ChainID == spec.BartioChainID { txsRoot = engineprimitives.BartioTransactions( p.GetTransactions(), ).HashTreeRoot() diff --git a/mod/state-transition/pkg/core/state_processor_genesis.go b/mod/state-transition/pkg/core/state_processor_genesis.go index 9e7502b0df..7633ddfa5a 100644 --- a/mod/state-transition/pkg/core/state_processor_genesis.go +++ b/mod/state-transition/pkg/core/state_processor_genesis.go @@ -21,6 +21,7 @@ package core import ( + "github.com/berachain/beacon-kit/mod/config/pkg/spec" "github.com/berachain/beacon-kit/mod/primitives/pkg/common" "github.com/berachain/beacon-kit/mod/primitives/pkg/constants" "github.com/berachain/beacon-kit/mod/primitives/pkg/encoding/hex" @@ -29,12 +30,6 @@ import ( "github.com/berachain/beacon-kit/mod/primitives/pkg/version" ) -//nolint:lll // temporary. -const ( - bArtioValRoot = "0x9147586693b6e8faa837715c0f3071c2000045b54233901c2e7871b15872bc43" - bArtioChainID = 80084 -) - // InitializePreminedBeaconStateFromEth1 initializes the beacon state. // //nolint:gocognit,funlen // todo fix. @@ -119,9 +114,9 @@ func (sp *StateProcessor[ } // Handle special case bartio genesis. - if sp.cs.DepositEth1ChainID() == bArtioChainID { + if sp.cs.DepositEth1ChainID() == spec.BartioChainID { if err = st.SetGenesisValidatorsRoot( - common.Root(hex.MustToBytes(bArtioValRoot))); err != nil { + common.Root(hex.MustToBytes(spec.BArtioValRoot))); err != nil { return nil, err } } else if err = st. diff --git a/mod/state-transition/pkg/core/state_processor_payload.go b/mod/state-transition/pkg/core/state_processor_payload.go index a1cfd02234..98486de6ed 100644 --- a/mod/state-transition/pkg/core/state_processor_payload.go +++ b/mod/state-transition/pkg/core/state_processor_payload.go @@ -23,6 +23,7 @@ package core import ( "context" + "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/errors" "github.com/berachain/beacon-kit/mod/primitives/pkg/math" @@ -107,12 +108,14 @@ func (sp *StateProcessor[ body := blk.GetBody() payload := body.GetExecutionPayload() - if pt := payload.GetTimestamp(); pt >= nextPayloadTimestamp { - return errors.Wrapf( - ErrTooFarInTheFuture, - "payload timestamp, max: %d, got: %d", - nextPayloadTimestamp, pt, - ) + if sp.cs.DepositEth1ChainID() == spec.BartioChainID { + if pt := payload.GetTimestamp(); pt >= nextPayloadTimestamp { + return errors.Wrapf( + ErrTooFarInTheFuture, + "payload timestamp, max: %d, got: %d", + nextPayloadTimestamp, pt, + ) + } } // Verify the number of withdrawals. diff --git a/mod/state-transition/pkg/core/state_processor_staking.go b/mod/state-transition/pkg/core/state_processor_staking.go index e6387c41bb..c161d32864 100644 --- a/mod/state-transition/pkg/core/state_processor_staking.go +++ b/mod/state-transition/pkg/core/state_processor_staking.go @@ -23,6 +23,7 @@ package core import ( "fmt" + "github.com/berachain/beacon-kit/mod/config/pkg/spec" "github.com/berachain/beacon-kit/mod/consensus-types/pkg/types" "github.com/berachain/beacon-kit/mod/errors" "github.com/berachain/beacon-kit/mod/primitives/pkg/common" @@ -205,7 +206,7 @@ func (sp *StateProcessor[ ) // TODO: This is a bug that lives on bArtio. Delete this eventually. - if sp.cs.DepositEth1ChainID() == bArtioChainID { + if sp.cs.DepositEth1ChainID() == spec.BartioChainID { // Note in AddValidatorBartio we implicitly increase // the balance from state st. This is unlike AddValidator. return st.AddValidatorBartio(val) From 5d2cd2cb167bf91866a475bb564a05adb8c57c5c Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 09:53:06 +0100 Subject: [PATCH 4/8] add logging to state transitions --- beacond/cmd/defaults.go | 2 +- mod/node-core/pkg/components/state_processor.go | 9 ++++++++- mod/state-transition/pkg/core/helpers_test.go | 2 ++ mod/state-transition/pkg/core/state_processor.go | 5 +++++ mod/state-transition/pkg/core/state_processor_payload.go | 6 ++++++ 5 files changed, 22 insertions(+), 2 deletions(-) diff --git a/beacond/cmd/defaults.go b/beacond/cmd/defaults.go index 7d88537e24..79e542e036 100644 --- a/beacond/cmd/defaults.go +++ b/beacond/cmd/defaults.go @@ -120,7 +120,7 @@ func DefaultComponents() []any { *BeaconBlock, *BeaconBlockBody, *BeaconBlockHeader, ], components.ProvideStateProcessor[ - *BeaconBlock, *BeaconBlockBody, *BeaconBlockHeader, + *Logger, *BeaconBlock, *BeaconBlockBody, *BeaconBlockHeader, *BeaconState, *BeaconStateMarshallable, *Deposit, *ExecutionPayload, *ExecutionPayloadHeader, *KVStore, ], diff --git a/mod/node-core/pkg/components/state_processor.go b/mod/node-core/pkg/components/state_processor.go index 5a0e156ec4..0a66ce90a9 100644 --- a/mod/node-core/pkg/components/state_processor.go +++ b/mod/node-core/pkg/components/state_processor.go @@ -24,6 +24,7 @@ import ( "cosmossdk.io/depinject" engineprimitives "github.com/berachain/beacon-kit/mod/engine-primitives/pkg/engine-primitives" "github.com/berachain/beacon-kit/mod/execution/pkg/engine" + "github.com/berachain/beacon-kit/mod/log" "github.com/berachain/beacon-kit/mod/primitives/pkg/common" "github.com/berachain/beacon-kit/mod/primitives/pkg/crypto" "github.com/berachain/beacon-kit/mod/state-transition/pkg/core" @@ -32,6 +33,7 @@ import ( // StateProcessorInput is the input for the state processor for the depinject // framework. type StateProcessorInput[ + LoggerT any, ExecutionPayloadT ExecutionPayload[ ExecutionPayloadT, ExecutionPayloadHeaderT, WithdrawalsT, ], @@ -40,6 +42,7 @@ type StateProcessorInput[ WithdrawalsT Withdrawals[WithdrawalT], ] struct { depinject.In + Logger LoggerT ChainSpec common.ChainSpec ExecutionEngine *engine.Engine[ ExecutionPayloadT, @@ -53,6 +56,7 @@ type StateProcessorInput[ // ProvideStateProcessor provides the state processor to the depinject // framework. func ProvideStateProcessor[ + LoggerT log.AdvancedLogger[LoggerT], BeaconBlockT BeaconBlock[BeaconBlockT, BeaconBlockBodyT, BeaconBlockHeaderT], BeaconBlockBodyT BeaconBlockBody[ BeaconBlockBodyT, *AttestationData, DepositT, @@ -78,7 +82,9 @@ func ProvideStateProcessor[ WithdrawalT Withdrawal[WithdrawalT], ]( in StateProcessorInput[ - ExecutionPayloadT, ExecutionPayloadHeaderT, WithdrawalT, WithdrawalsT, + LoggerT, + ExecutionPayloadT, ExecutionPayloadHeaderT, + WithdrawalT, WithdrawalsT, ], ) *core.StateProcessor[ BeaconBlockT, BeaconBlockBodyT, BeaconBlockHeaderT, @@ -105,6 +111,7 @@ func ProvideStateProcessor[ WithdrawalsT, WithdrawalCredentials, ]( + in.Logger.With("service", "state-processor"), in.ChainSpec, in.ExecutionEngine, in.Signer, diff --git a/mod/state-transition/pkg/core/helpers_test.go b/mod/state-transition/pkg/core/helpers_test.go index 2b84fcbd56..801c696d31 100644 --- a/mod/state-transition/pkg/core/helpers_test.go +++ b/mod/state-transition/pkg/core/helpers_test.go @@ -32,6 +32,7 @@ import ( storetypes "cosmossdk.io/store/types" "github.com/berachain/beacon-kit/mod/consensus-types/pkg/types" engineprimitives "github.com/berachain/beacon-kit/mod/engine-primitives/pkg/engine-primitives" + "github.com/berachain/beacon-kit/mod/log/pkg/noop" "github.com/berachain/beacon-kit/mod/node-core/pkg/components" "github.com/berachain/beacon-kit/mod/primitives/pkg/common" "github.com/berachain/beacon-kit/mod/primitives/pkg/crypto" @@ -130,6 +131,7 @@ func createStateProcessor( engineprimitives.Withdrawals, types.WithdrawalCredentials, ]( + noop.NewLogger[any](), cs, execEngine, signer, diff --git a/mod/state-transition/pkg/core/state_processor.go b/mod/state-transition/pkg/core/state_processor.go index 34e92d6be7..4a4a050509 100644 --- a/mod/state-transition/pkg/core/state_processor.go +++ b/mod/state-transition/pkg/core/state_processor.go @@ -24,6 +24,7 @@ import ( "bytes" "github.com/berachain/beacon-kit/mod/errors" + "github.com/berachain/beacon-kit/mod/log" "github.com/berachain/beacon-kit/mod/primitives/pkg/common" "github.com/berachain/beacon-kit/mod/primitives/pkg/constants" "github.com/berachain/beacon-kit/mod/primitives/pkg/crypto" @@ -77,6 +78,8 @@ type StateProcessor[ }, WithdrawalCredentialsT ~[32]byte, ] struct { + // logger is used for logging information and errors. + logger log.Logger // cs is the chain specification for the beacon chain. cs common.ChainSpec // signer is the BLS signer used for cryptographic operations. @@ -140,6 +143,7 @@ func NewStateProcessor[ }, WithdrawalCredentialsT ~[32]byte, ]( + logger log.Logger, cs common.ChainSpec, executionEngine ExecutionEngine[ ExecutionPayloadT, ExecutionPayloadHeaderT, WithdrawalsT, @@ -158,6 +162,7 @@ func NewStateProcessor[ ExecutionPayloadHeaderT, ForkT, ForkDataT, KVStoreT, ValidatorT, ValidatorsT, WithdrawalT, WithdrawalsT, WithdrawalCredentialsT, ]{ + logger: logger, cs: cs, executionEngine: executionEngine, signer: signer, diff --git a/mod/state-transition/pkg/core/state_processor_payload.go b/mod/state-transition/pkg/core/state_processor_payload.go index 98486de6ed..8d79b52eba 100644 --- a/mod/state-transition/pkg/core/state_processor_payload.go +++ b/mod/state-transition/pkg/core/state_processor_payload.go @@ -108,6 +108,12 @@ func (sp *StateProcessor[ body := blk.GetBody() payload := body.GetExecutionPayload() + sp.logger.Info("validateStatelessPayload", + "payload height", payload.GetNumber(), + "payload timestamp", payload.GetTimestamp(), + "bound timestamp", nextPayloadTimestamp, + ) + if sp.cs.DepositEth1ChainID() == spec.BartioChainID { if pt := payload.GetTimestamp(); pt >= nextPayloadTimestamp { return errors.Wrapf( From 675abd46e9e107e09619384c6dbd6d8523ef68b6 Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 10:25:09 +0100 Subject: [PATCH 5/8] logged consensus height along with payload height --- mod/beacon/blockchain/types.go | 5 +++++ mod/consensus/pkg/types/common.go | 14 ++++++++++++++ mod/node-core/pkg/components/interfaces.go | 5 +++++ mod/primitives/pkg/transition/context.go | 12 ++++++++++++ .../pkg/core/state_processor_payload.go | 10 +++++++++- mod/state-transition/pkg/core/types.go | 4 ++++ 6 files changed, 49 insertions(+), 1 deletion(-) diff --git a/mod/beacon/blockchain/types.go b/mod/beacon/blockchain/types.go index 20906230b5..02d8857cee 100644 --- a/mod/beacon/blockchain/types.go +++ b/mod/beacon/blockchain/types.go @@ -53,6 +53,11 @@ type ConsensusBlock[BeaconBlockT any] interface { // consensus for the next payload to be proposed. It is also // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 + + // GetConsensusBlockHeight returns the height of consensus block, + // which may be different from execution payload height + // in some networks. Currently only used for logging + GetConsensusBlockHeight() math.U64 } // BeaconBlock represents a beacon block interface. diff --git a/mod/consensus/pkg/types/common.go b/mod/consensus/pkg/types/common.go index 9f56280c10..81e2d025bd 100644 --- a/mod/consensus/pkg/types/common.go +++ b/mod/consensus/pkg/types/common.go @@ -23,9 +23,14 @@ package types import "github.com/berachain/beacon-kit/mod/primitives/pkg/math" type commonConsensusData struct { + // use to verify block builder proposerAddress []byte + // used to build next block and validated current payload timestamp nextPayloadTimestamp math.U64 + + // useful for logging + consensusBlkHeight math.U64 } // GetProposerAddress returns the address of the validator @@ -40,3 +45,12 @@ func (c *commonConsensusData) GetProposerAddress() []byte { func (c *commonConsensusData) GetNextPayloadTimestamp() math.U64 { return c.nextPayloadTimestamp } + +// GetConsensusBlockHeight returns the height of consensus block, +// +// which may be different from execution payload height +// +// in some networks. Currently only used for logging. +func (c *commonConsensusData) GetConsensusBlockHeight() math.U64 { + return c.consensusBlkHeight +} diff --git a/mod/node-core/pkg/components/interfaces.go b/mod/node-core/pkg/components/interfaces.go index 6ec8f1156e..e9d9d78200 100644 --- a/mod/node-core/pkg/components/interfaces.go +++ b/mod/node-core/pkg/components/interfaces.go @@ -91,6 +91,11 @@ type ( // consensus for the next payload to be proposed. It is also // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 + + // GetConsensusBlockHeight returns the height of consensus block, + // which may be different from execution payload height + // in some networks. Currently only used for logging + GetConsensusBlockHeight() math.U64 } // BeaconBlock represents a generic interface for a beacon block. diff --git a/mod/primitives/pkg/transition/context.go b/mod/primitives/pkg/transition/context.go index 0eeb29ba29..0881f0b000 100644 --- a/mod/primitives/pkg/transition/context.go +++ b/mod/primitives/pkg/transition/context.go @@ -48,6 +48,11 @@ type Context struct { // consensus for the next payload to be proposed. It is also // used to bound current payload upon validation NextPayloadTimestamp math.U64 + + // ConsensusBlockHeight is the height of consensus block, + // which may be different from execution payload height + // in some networks. Currently only used for logging + ConsensusBlockHeight math.U64 } // GetOptimisticEngine returns whether to optimistically assume the execution @@ -88,6 +93,13 @@ func (c *Context) GetNextPayloadTimestamp() math.U64 { return c.NextPayloadTimestamp } +// GetConsensusBlockHeight returns the height of consensus block, +// which may be different from execution payload height +// in some networks. Currently only used for logging. +func (c *Context) GetConsensusBlockHeight() math.U64 { + return c.ConsensusBlockHeight +} + // Unwrap returns the underlying standard context. func (c *Context) Unwrap() context.Context { return c.Context diff --git a/mod/state-transition/pkg/core/state_processor_payload.go b/mod/state-transition/pkg/core/state_processor_payload.go index 8d79b52eba..162a77d54d 100644 --- a/mod/state-transition/pkg/core/state_processor_payload.go +++ b/mod/state-transition/pkg/core/state_processor_payload.go @@ -52,6 +52,7 @@ func (sp *StateProcessor[ g.Go(func() error { return sp.validateExecutionPayload( gCtx, st, blk, + ctx.GetConsensusBlockHeight(), ctx.GetNextPayloadTimestamp(), ctx.GetOptimisticEngine(), ) @@ -88,10 +89,15 @@ func (sp *StateProcessor[ ctx context.Context, st BeaconStateT, blk BeaconBlockT, + consensusBlockHeight math.U64, nextPayloadTimestamp math.U64, optimisticEngine bool, ) error { - if err := sp.validateStatelessPayload(blk, nextPayloadTimestamp); err != nil { + if err := sp.validateStatelessPayload( + blk, + consensusBlockHeight, + nextPayloadTimestamp, + ); err != nil { return err } return sp.validateStatefulPayload(ctx, st, blk, optimisticEngine) @@ -103,12 +109,14 @@ func (sp *StateProcessor[ _, _, _, _, _, _, _, _, _, _, _, _, _, ]) validateStatelessPayload( blk BeaconBlockT, + consensusBlockHeight math.U64, nextPayloadTimestamp math.U64, ) error { body := blk.GetBody() payload := body.GetExecutionPayload() sp.logger.Info("validateStatelessPayload", + "consensus height", consensusBlockHeight, "payload height", payload.GetNumber(), "payload timestamp", payload.GetTimestamp(), "bound timestamp", nextPayloadTimestamp, diff --git a/mod/state-transition/pkg/core/types.go b/mod/state-transition/pkg/core/types.go index 0d5f00ee34..22cf213b9f 100644 --- a/mod/state-transition/pkg/core/types.go +++ b/mod/state-transition/pkg/core/types.go @@ -125,6 +125,10 @@ type Context interface { // consensus for the next payload to be proposed. It is also // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 + // GetConsensusBlockHeight returns the height of consensus block, + // which may be different from execution payload height + // in some networks. Currently only used for logging + GetConsensusBlockHeight() math.U64 } // Deposit is the interface for a deposit. From d32eae0685f25c9da5b843aeb38efc0cd7e36da6 Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 10:28:44 +0100 Subject: [PATCH 6/8] wrong test sign --- mod/state-transition/pkg/core/state_processor_payload.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mod/state-transition/pkg/core/state_processor_payload.go b/mod/state-transition/pkg/core/state_processor_payload.go index 162a77d54d..4f64639149 100644 --- a/mod/state-transition/pkg/core/state_processor_payload.go +++ b/mod/state-transition/pkg/core/state_processor_payload.go @@ -122,7 +122,8 @@ func (sp *StateProcessor[ "bound timestamp", nextPayloadTimestamp, ) - if sp.cs.DepositEth1ChainID() == spec.BartioChainID { + // We skip timestamp check on Bartio for backward compability reasons + if sp.cs.DepositEth1ChainID() != spec.BartioChainID { if pt := payload.GetTimestamp(); pt >= nextPayloadTimestamp { return errors.Wrapf( ErrTooFarInTheFuture, From 73b7780a437d78734792c629be09fdf7910b9d5e Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 10:36:31 +0100 Subject: [PATCH 7/8] nits --- mod/node-core/pkg/components/state_processor.go | 2 +- mod/state-transition/pkg/core/state_processor_payload.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mod/node-core/pkg/components/state_processor.go b/mod/node-core/pkg/components/state_processor.go index 0a66ce90a9..bef50d3a71 100644 --- a/mod/node-core/pkg/components/state_processor.go +++ b/mod/node-core/pkg/components/state_processor.go @@ -33,7 +33,7 @@ import ( // StateProcessorInput is the input for the state processor for the depinject // framework. type StateProcessorInput[ - LoggerT any, + LoggerT log.AdvancedLogger[LoggerT], ExecutionPayloadT ExecutionPayload[ ExecutionPayloadT, ExecutionPayloadHeaderT, WithdrawalsT, ], diff --git a/mod/state-transition/pkg/core/state_processor_payload.go b/mod/state-transition/pkg/core/state_processor_payload.go index 4f64639149..a88ab67b32 100644 --- a/mod/state-transition/pkg/core/state_processor_payload.go +++ b/mod/state-transition/pkg/core/state_processor_payload.go @@ -122,7 +122,8 @@ func (sp *StateProcessor[ "bound timestamp", nextPayloadTimestamp, ) - // We skip timestamp check on Bartio for backward compability reasons + // We skip timestamp check on Bartio for backward compatibility reasons + // TODO: enforce the check when we drop other Bartio special cases. if sp.cs.DepositEth1ChainID() != spec.BartioChainID { if pt := payload.GetTimestamp(); pt >= nextPayloadTimestamp { return errors.Wrapf( From 94e55ee3e49411c1c1e6034af00f3583225b1882 Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 11:03:55 +0100 Subject: [PATCH 8/8] nit typo --- mod/consensus/pkg/types/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod/consensus/pkg/types/common.go b/mod/consensus/pkg/types/common.go index 81e2d025bd..9daa8c23da 100644 --- a/mod/consensus/pkg/types/common.go +++ b/mod/consensus/pkg/types/common.go @@ -26,7 +26,7 @@ type commonConsensusData struct { // use to verify block builder proposerAddress []byte - // used to build next block and validated current payload timestamp + // used to build next block and validate current payload timestamp nextPayloadTimestamp math.U64 // useful for logging