From 1e120b3e9630662dec5ce50eb27862b25cd9d2d1 Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 09:38:34 +0100 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 From d7fad6720fff34f5dfba732e475950d9975735d3 Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 12:46:58 +0100 Subject: [PATCH 07/10] simplified heights and timestamps logging --- .../pkg/core/state_processor_payload.go | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/mod/state-transition/pkg/core/state_processor_payload.go b/mod/state-transition/pkg/core/state_processor_payload.go index a88ab67b32..ab3634f41d 100644 --- a/mod/state-transition/pkg/core/state_processor_payload.go +++ b/mod/state-transition/pkg/core/state_processor_payload.go @@ -22,6 +22,7 @@ package core import ( "context" + "fmt" "github.com/berachain/beacon-kit/mod/config/pkg/spec" engineprimitives "github.com/berachain/beacon-kit/mod/engine-primitives/pkg/engine-primitives" @@ -47,12 +48,19 @@ func (sp *StateProcessor[ g, gCtx = errgroup.WithContext(context.Background()) ) + sp.logger.Info("processExecutionPayload", + fmt.Sprintf("consensus height %d", ctx.GetConsensusBlockHeight()), + fmt.Sprintf("payload height %d", payload.GetNumber()), + fmt.Sprintf("payload timestamp %d", payload.GetTimestamp()), + fmt.Sprintf("bound timestamp %d", payload.GetTimestamp()), + fmt.Sprintf("skip verification %t", ctx.GetSkipPayloadVerification()), + ) + // Skip payload verification if the context is configured as such. if !ctx.GetSkipPayloadVerification() { g.Go(func() error { return sp.validateExecutionPayload( gCtx, st, blk, - ctx.GetConsensusBlockHeight(), ctx.GetNextPayloadTimestamp(), ctx.GetOptimisticEngine(), ) @@ -89,13 +97,11 @@ func (sp *StateProcessor[ ctx context.Context, st BeaconStateT, blk BeaconBlockT, - consensusBlockHeight math.U64, nextPayloadTimestamp math.U64, optimisticEngine bool, ) error { if err := sp.validateStatelessPayload( blk, - consensusBlockHeight, nextPayloadTimestamp, ); err != nil { return err @@ -109,19 +115,11 @@ 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, - ) - // 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 { From 25c56332c259cead6930b39f3a4db280aef10c7a Mon Sep 17 00:00:00 2001 From: aBear Date: Tue, 12 Nov 2024 15:26:35 +0100 Subject: [PATCH 08/10] some more log fixing --- .../pkg/core/state_processor_payload.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/mod/state-transition/pkg/core/state_processor_payload.go b/mod/state-transition/pkg/core/state_processor_payload.go index ab3634f41d..269168efa6 100644 --- a/mod/state-transition/pkg/core/state_processor_payload.go +++ b/mod/state-transition/pkg/core/state_processor_payload.go @@ -22,7 +22,6 @@ package core import ( "context" - "fmt" "github.com/berachain/beacon-kit/mod/config/pkg/spec" engineprimitives "github.com/berachain/beacon-kit/mod/engine-primitives/pkg/engine-primitives" @@ -49,11 +48,11 @@ func (sp *StateProcessor[ ) sp.logger.Info("processExecutionPayload", - fmt.Sprintf("consensus height %d", ctx.GetConsensusBlockHeight()), - fmt.Sprintf("payload height %d", payload.GetNumber()), - fmt.Sprintf("payload timestamp %d", payload.GetTimestamp()), - fmt.Sprintf("bound timestamp %d", payload.GetTimestamp()), - fmt.Sprintf("skip verification %t", ctx.GetSkipPayloadVerification()), + "consensus height", ctx.GetConsensusBlockHeight().Unwrap(), + "payload height", payload.GetNumber().Unwrap(), + "payload timestamp", payload.GetTimestamp().Unwrap(), + "bound timestamp", ctx.GetNextPayloadTimestamp().Unwrap(), + "skip payload verification", ctx.GetSkipPayloadVerification(), ) // Skip payload verification if the context is configured as such. From 6a216373bc8eba5151000b4927e2701b2ebbf8b6 Mon Sep 17 00:00:00 2001 From: aBear Date: Wed, 13 Nov 2024 10:27:39 +0100 Subject: [PATCH 09/10] fixed consensus block height initialization --- mod/beacon/blockchain/process.go | 1 + mod/beacon/blockchain/receive.go | 1 + mod/beacon/validator/block_builder.go | 5 +++++ mod/beacon/validator/types.go | 5 +++++ mod/consensus/pkg/types/common.go | 4 +--- 5 files changed, 13 insertions(+), 3 deletions(-) diff --git a/mod/beacon/blockchain/process.go b/mod/beacon/blockchain/process.go index 043bba4971..40b30978dd 100644 --- a/mod/beacon/blockchain/process.go +++ b/mod/beacon/blockchain/process.go @@ -130,6 +130,7 @@ func (s *Service[ ProposerAddress: blk.GetProposerAddress(), NextPayloadTimestamp: blk.GetNextPayloadTimestamp(), + ConsensusBlockHeight: blk.GetConsensusBlockHeight(), }, st, blk.GetBeaconBlock(), diff --git a/mod/beacon/blockchain/receive.go b/mod/beacon/blockchain/receive.go index 5b2fde8f18..e4bb46c1be 100644 --- a/mod/beacon/blockchain/receive.go +++ b/mod/beacon/blockchain/receive.go @@ -130,6 +130,7 @@ func (s *Service[ SkipValidateRandao: false, ProposerAddress: blk.GetProposerAddress(), NextPayloadTimestamp: blk.GetNextPayloadTimestamp(), + ConsensusBlockHeight: blk.GetConsensusBlockHeight(), }, st, blk.GetBeaconBlock(), ) diff --git a/mod/beacon/validator/block_builder.go b/mod/beacon/validator/block_builder.go index 64b9737a94..2ba27c1d50 100644 --- a/mod/beacon/validator/block_builder.go +++ b/mod/beacon/validator/block_builder.go @@ -116,6 +116,7 @@ func (s *Service[ ctx, slotData.GetProposerAddress(), slotData.GetNextPayloadTimestamp(), + slotData.GetConsensusBlockHeight(), st, blk, ) @@ -339,6 +340,7 @@ func (s *Service[ ctx context.Context, proposerAddress []byte, nextPayloadTimestamp math.U64, + consensusBlockHeight math.U64, st BeaconStateT, blk BeaconBlockT, ) error { @@ -346,6 +348,7 @@ func (s *Service[ ctx, proposerAddress, nextPayloadTimestamp, + consensusBlockHeight, st, blk, ) @@ -368,6 +371,7 @@ func (s *Service[ ctx context.Context, proposerAddress []byte, nextPayloadTimestamp math.U64, + consensusBlockHeight math.U64, st BeaconStateT, blk BeaconBlockT, ) (common.Root, error) { @@ -385,6 +389,7 @@ func (s *Service[ SkipValidateRandao: true, ProposerAddress: proposerAddress, NextPayloadTimestamp: nextPayloadTimestamp, + ConsensusBlockHeight: consensusBlockHeight, }, st, blk, ); err != nil { diff --git a/mod/beacon/validator/types.go b/mod/beacon/validator/types.go index cf99073884..10a05713ed 100644 --- a/mod/beacon/validator/types.go +++ b/mod/beacon/validator/types.go @@ -197,6 +197,11 @@ type SlotData[AttestationDataT, SlashingInfoT 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 } // StateProcessor defines the interface for processing the state. diff --git a/mod/consensus/pkg/types/common.go b/mod/consensus/pkg/types/common.go index 9daa8c23da..5e7ab6f9c7 100644 --- a/mod/consensus/pkg/types/common.go +++ b/mod/consensus/pkg/types/common.go @@ -47,9 +47,7 @@ func (c *commonConsensusData) GetNextPayloadTimestamp() math.U64 { } // GetConsensusBlockHeight returns the height of consensus block, -// -// which may be different from execution payload height -// +// 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 From 7480048d6c574da50d3924919c27d094b8845bc3 Mon Sep 17 00:00:00 2001 From: aBear Date: Wed, 13 Nov 2024 10:55:39 +0100 Subject: [PATCH 10/10] setting block timestamp in a backward compatible manner for Bartio --- mod/beacon/blockchain/execution_engine.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/mod/beacon/blockchain/execution_engine.go b/mod/beacon/blockchain/execution_engine.go index 6cd29fea9d..a9f04348d8 100644 --- a/mod/beacon/blockchain/execution_engine.go +++ b/mod/beacon/blockchain/execution_engine.go @@ -22,7 +22,9 @@ package blockchain import ( "context" + "time" + "github.com/berachain/beacon-kit/mod/config/pkg/spec" engineprimitives "github.com/berachain/beacon-kit/mod/engine-primitives/pkg/engine-primitives" ) @@ -74,12 +76,24 @@ func (s *Service[ return } + nextPayloadTime := blk.GetNextPayloadTimestamp().Unwrap() + + // We set timestamp check on Bartio for backward compatibility reasons + // TODO: drop this we drop other Bartio special cases. + if s.chainSpec.DepositEth1ChainID() == spec.BartioChainID { + nextPayloadTime = max( + //#nosec:G701 + uint64(time.Now().Unix()+1), + uint64((lph.GetTimestamp() + 1)), + ) + } + prevBlockRoot := beaconBlk.HashTreeRoot() if _, err := s.localBuilder.RequestPayloadAsync( ctx, stCopy, beaconBlk.GetSlot()+1, - blk.GetNextPayloadTimestamp().Unwrap(), + nextPayloadTime, prevBlockRoot, lph.GetBlockHash(), lph.GetParentHash(),