Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(state-transition): flipped transaction context attributes logic #2451

Merged
merged 20 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions beacon/blockchain/finalize_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (s *Service) executeStateTransition(
defer s.metrics.measureStateTransitionDuration(startTime)
valUpdates, err := s.stateProcessor.Transition(
&transition.Context{
Context: ctx,
ConsensusCtx: ctx,

MeterGas: true,

Expand All @@ -177,13 +177,13 @@ func (s *Service) executeStateTransition(
// of validators in their process proposal call and thus
// the "verification aspect" of this NewPayload call is
// actually irrelevant at this point.
SkipPayloadVerification: false,
VerifyPayload: true,
rezbera marked this conversation as resolved.
Show resolved Hide resolved

// We skip randao validation in FinalizeBlock since either
// 1. we validated it during ProcessProposal at the head of the chain OR
// 2. we are bootstrapping and implicitly trust that the randao was validated by
// the super majority during ProcessProposal of the given block height.
SkipValidateRandao: true,
ValidateRandao: false,

ProposerAddress: blk.GetProposerAddress(),
ConsensusTime: blk.GetConsensusTime(),
Expand Down
16 changes: 8 additions & 8 deletions beacon/blockchain/process_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,14 +326,14 @@ func (s *Service) verifyStateRoot(
// We run with a non-optimistic engine here to ensure
// that the proposer does not try to push through a bad block.
&transition.Context{
Context: ctx,
MeterGas: false,
OptimisticEngine: false,
SkipPayloadVerification: false,
SkipValidateResult: false,
SkipValidateRandao: false,
ProposerAddress: proposerAddress,
ConsensusTime: consensusTime,
ConsensusCtx: ctx,
MeterGas: false,
OptimisticEngine: false,
VerifyPayload: true,
ValidateResult: true,
ValidateRandao: true,
ProposerAddress: proposerAddress,
ConsensusTime: consensusTime,
},
st, blk,
)
Expand Down
16 changes: 8 additions & 8 deletions beacon/validator/block_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,14 +374,14 @@ func (s *Service) computeStateRoot(
// engine enabled here would affect the proposer when
// the payload in their block has come from a remote builder.
&transition.Context{
Context: ctx,
MeterGas: false,
OptimisticEngine: true,
SkipPayloadVerification: true,
SkipValidateResult: true,
SkipValidateRandao: true,
ProposerAddress: proposerAddress,
ConsensusTime: consensusTime,
ConsensusCtx: ctx,
MeterGas: false,
OptimisticEngine: true,
VerifyPayload: false,
ValidateResult: false,
ValidateRandao: false,
ProposerAddress: proposerAddress,
ConsensusTime: consensusTime,
},
st, blk,
); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/beacond/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type (
SidecarFactory = dablob.SidecarFactory

// StateProcessor is the type alias for the state processor interface.
StateProcessor = core.StateProcessor[*Context]
StateProcessor = core.StateProcessor

// StorageBackend is the type alias for the storage backend interface.
StorageBackend = storage.Backend
Expand Down
33 changes: 0 additions & 33 deletions node-core/components/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,39 +204,6 @@ type (
) (*v1.ProcessProposalResponse, error)
}

// // Context defines an interface for managing state transition context.
// Context[T any] interface {
// context.Context
// // Wrap returns a new context with the given context.
// Wrap(context.Context) T
// // OptimisticEngine sets the optimistic engine flag to true.
// OptimisticEngine() T
// // SkipPayloadVerification sets the skip payload verification flag to
// // true.
// SkipPayloadVerification() T
// // SkipValidateRandao sets the skip validate randao flag to true.
// SkipValidateRandao() T
// // SkipValidateResult sets the skip validate result flag to true.
// SkipValidateResult() T
// // GetOptimisticEngine returns whether to optimistically assume the
// // execution client has the correct state when certain errors are
// // returned
// // by the execution engine.
// GetOptimisticEngine() bool
// // GetSkipPayloadVerification returns whether to skip verifying the
// // payload
// // if
// // it already exists on the execution client.
// GetSkipPayloadVerification() bool
// // GetSkipValidateRandao returns whether to skip validating the RANDAO
// // reveal.
// GetSkipValidateRandao() bool
// // GetSkipValidateResult returns whether to validate the result of the
// // state
// // transition.
// GetSkipValidateResult() bool
// }

// Deposit is the interface for a deposit.
Deposit[
T any,
Expand Down
2 changes: 1 addition & 1 deletion node-core/components/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func ProvideStateProcessor[
LoggerT log.AdvancedLogger[LoggerT],
](
in StateProcessorInput[LoggerT],
) *core.StateProcessor[*Context] {
) *core.StateProcessor {
return core.NewStateProcessor[*Context](
in.Logger.With("service", "state-processor"),
in.ChainSpec,
Expand Down
69 changes: 14 additions & 55 deletions primitives/transition/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,71 +28,30 @@ import (

// Context is the context for the state transition.
type Context struct {
context.Context

// ConsensusCtx is the context passed by CometBFT callbacks
// We pass it down to be able to cancel processing (although
// currently CometBFT context is set to TODO)
ConsensusCtx context.Context
rezbera marked this conversation as resolved.
Show resolved Hide resolved
// MeterGas controls whether gas data related to the execution
// layer payload should be meter or not. We currently meter only
// finalized blocks.
MeterGas bool
// OptimisticEngine indicates whether to optimistically assume
// the execution client has the correct state certain errors
// are returned by the execution engine.
OptimisticEngine bool
// SkipPayloadVerification indicates whether to skip calling NewPayload
// on the execution client. This can be done when the node is not
// VerifyPayload indicates whether to call NewPayload on the
// execution client. This can be done when the node is not
// syncing, and the payload is already known to the execution client.
SkipPayloadVerification bool
// SkipValidateRandao indicates whether to skip validating the Randao mix.
SkipValidateRandao bool
// SkipValidateResult indicates whether to validate the result of
VerifyPayload bool
shotes marked this conversation as resolved.
Show resolved Hide resolved
// ValidateRandao indicates whether to validate the Randao mix.
ValidateRandao bool
// ValidateResult indicates whether to validate the result of
// the state transition.
SkipValidateResult bool
ValidateResult bool
// Address of current block proposer
ProposerAddress []byte
// ConsensusTime returns the timestamp of current consensus request.
// It is used to build next payload and to validate currentpayload.
ConsensusTime math.U64
}

func (c *Context) GetMeterGas() bool {
return c.MeterGas
}

// GetOptimisticEngine returns whether to optimistically assume the execution
// client has the correct state when certain errors are returned by the
// execution engine.
func (c *Context) GetOptimisticEngine() bool {
return c.OptimisticEngine
}

// GetSkipPayloadVerification returns whether to skip calling NewPayload on the
// execution client. This can be done when the node is not syncing, and the
// payload is already known to the execution client.
func (c *Context) GetSkipPayloadVerification() bool {
return c.SkipPayloadVerification
}

// GetSkipValidateRandao returns whether to skip validating the Randao mix.
func (c *Context) GetSkipValidateRandao() bool {
return c.SkipValidateRandao
}

// GetSkipValidateResult returns whether to validate the result of the state
// transition.
func (c *Context) GetSkipValidateResult() bool {
return c.SkipValidateResult
}

// GetProposerAddress returns the address of the validator
// selected by consensus to propose the block.
func (c *Context) GetProposerAddress() []byte {
return c.ProposerAddress
}

// GetConsensusTime returns the timestamp of current consensus request.
// It is used to build next payload and to validate currentpayload.
func (c *Context) GetConsensusTime() math.U64 {
return c.ConsensusTime
}

// Unwrap returns the underlying standard context.
func (c *Context) Unwrap() context.Context {
return c.Context
}
34 changes: 17 additions & 17 deletions state-transition/core/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (

// StateProcessor is a basic Processor, which takes care of the
// main state transition for the beacon chain.
type StateProcessor[ContextT Context] struct {
type StateProcessor struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're fixing this I love the removal of generics. But I would prefer using the transition.Context as an interface so that the fields of the struct are not mutable in the stateProcessor. Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a slight preference for the current proposal (exported attributes).
If we keep the getter, I think we should unexport the attributes and so introduce a constructor too.
Happy to go with the majority here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @berachain/core

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for un-exporting the struct fields initialized via NewXXX method and having simple read only getters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added encapsulation, LMK how do you like this

// logger is used for logging information and errors.
logger log.Logger
// cs is the chain specification for the beacon chain.
Expand All @@ -58,7 +58,7 @@ type StateProcessor[ContextT Context] struct {

// NewStateProcessor creates a new state processor.
func NewStateProcessor[
ContextT Context,
ContextT *transition.Context,
](
logger log.Logger,
cs chain.Spec,
Expand All @@ -67,8 +67,8 @@ func NewStateProcessor[
signer crypto.BLSSigner,
fGetAddressFromPubKey func(crypto.BLSPubkey) ([]byte, error),
telemetrySink TelemetrySink,
) *StateProcessor[ContextT] {
return &StateProcessor[ContextT]{
) *StateProcessor {
return &StateProcessor{
logger: logger,
cs: cs,
executionEngine: executionEngine,
Expand All @@ -80,8 +80,8 @@ func NewStateProcessor[
}

// Transition is the main function for processing a state transition.
func (sp *StateProcessor[ContextT]) Transition(
ctx ContextT,
func (sp *StateProcessor) Transition(
ctx *transition.Context,
st *state.StateDB,
blk *ctypes.BeaconBlock,
) (transition.ValidatorUpdates, error) {
Expand All @@ -106,7 +106,7 @@ func (sp *StateProcessor[ContextT]) Transition(
// NOTE: if process slots is called across multiple epochs (the given slot is more than 1 multiple
// ahead of the current state slot), then validator updates will be returned in the order they are
// processed, which may effectually override each other.
func (sp *StateProcessor[_]) ProcessSlots(
func (sp *StateProcessor) ProcessSlots(
st *state.StateDB, slot math.Slot,
) (transition.ValidatorUpdates, error) {
var res transition.ValidatorUpdates
Expand Down Expand Up @@ -143,7 +143,7 @@ func (sp *StateProcessor[_]) ProcessSlots(
}

// processSlot is run when a slot is missed.
func (sp *StateProcessor[_]) processSlot(st *state.StateDB) error {
func (sp *StateProcessor) processSlot(st *state.StateDB) error {
stateSlot, err := st.GetSlot()
if err != nil {
return err
Expand Down Expand Up @@ -181,8 +181,8 @@ func (sp *StateProcessor[_]) processSlot(st *state.StateDB) error {

// ProcessBlock processes the block, it optionally verifies the
// state root.
func (sp *StateProcessor[ContextT]) ProcessBlock(
ctx ContextT, st *state.StateDB, blk *ctypes.BeaconBlock,
func (sp *StateProcessor) ProcessBlock(
ctx *transition.Context, st *state.StateDB, blk *ctypes.BeaconBlock,
) error {
if err := sp.processBlockHeader(ctx, st, blk); err != nil {
return err
Expand All @@ -206,7 +206,7 @@ func (sp *StateProcessor[ContextT]) ProcessBlock(

// If we are skipping validate, we can skip calculating the state
// root to save compute.
if ctx.GetSkipValidateResult() {
if !ctx.ValidateResult {
return nil
}

Expand All @@ -225,7 +225,7 @@ func (sp *StateProcessor[ContextT]) ProcessBlock(

// processEpoch processes the epoch and ensures it matches the local state. Currently
// beacon-kit does not enforce rewards, penalties, and slashing for validators.
func (sp *StateProcessor[_]) processEpoch(st *state.StateDB) (transition.ValidatorUpdates, error) {
func (sp *StateProcessor) processEpoch(st *state.StateDB) (transition.ValidatorUpdates, error) {
slot, err := st.GetSlot()
if err != nil {
return nil, err
Expand Down Expand Up @@ -271,8 +271,8 @@ func (sp *StateProcessor[_]) processEpoch(st *state.StateDB) (transition.Validat
}

// processBlockHeader processes the header and ensures it matches the local state.
func (sp *StateProcessor[ContextT]) processBlockHeader(
ctx ContextT, st *state.StateDB, blk *ctypes.BeaconBlock,
func (sp *StateProcessor) processBlockHeader(
ctx *transition.Context, st *state.StateDB, blk *ctypes.BeaconBlock,
) error {
// Ensure the block slot matches the state slot.
slot, err := st.GetSlot()
Expand Down Expand Up @@ -304,10 +304,10 @@ func (sp *StateProcessor[ContextT]) processBlockHeader(
if err != nil {
return err
}
if !bytes.Equal(stateProposerAddress, ctx.GetProposerAddress()) {
if !bytes.Equal(stateProposerAddress, ctx.ProposerAddress) {
return errors.Wrapf(
ErrProposerMismatch, "store key: %s, consensus key: %s",
stateProposerAddress, ctx.GetProposerAddress(),
stateProposerAddress, ctx.ProposerAddress,
)
}

Expand Down Expand Up @@ -344,7 +344,7 @@ func (sp *StateProcessor[ContextT]) processBlockHeader(

// processEffectiveBalanceUpdates as defined in the Ethereum 2.0 specification.
// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#effective-balances-updates
func (sp *StateProcessor[_]) processEffectiveBalanceUpdates(st *state.StateDB) error {
func (sp *StateProcessor) processEffectiveBalanceUpdates(st *state.StateDB) error {
// Update effective balances with hysteresis
validators, err := st.GetValidators()
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions state-transition/core/state_processor_genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
// InitializePreminedBeaconStateFromEth1 initializes the beacon state.
//
//nolint:gocognit,funlen // todo fix.
func (sp *StateProcessor[_]) InitializePreminedBeaconStateFromEth1(
func (sp *StateProcessor) InitializePreminedBeaconStateFromEth1(
st *statedb.StateDB,
deposits ctypes.Deposits,
execPayloadHeader *ctypes.ExecutionPayloadHeader,
Expand Down Expand Up @@ -149,7 +149,7 @@ func (sp *StateProcessor[_]) InitializePreminedBeaconStateFromEth1(
return validatorSetsDiffs(nil, activeVals), nil
}

func (sp *StateProcessor[_]) processGenesisActivation(st *statedb.StateDB) error {
func (sp *StateProcessor) processGenesisActivation(st *statedb.StateDB) error {
vals, err := st.GetValidators()
if err != nil {
return fmt.Errorf("genesis activation, failed listing validators: %w", err)
Expand Down
Loading
Loading