From 1335b7d02c86c21d4fa18b977b1cf7eb0cb7c1ef Mon Sep 17 00:00:00 2001 From: aBear Date: Fri, 25 Oct 2024 21:55:40 +0200 Subject: [PATCH] nits --- mod/beacon/blockchain/types.go | 3 +++ mod/beacon/validator/types.go | 5 +++-- .../pkg/cometbft/service/middleware/middleware.go | 6 +++++- mod/consensus/pkg/types/consensus_block.go | 6 +++--- mod/consensus/pkg/types/slot_data.go | 6 +++--- mod/node-core/pkg/components/interfaces.go | 3 +++ mod/primitives/pkg/transition/context.go | 8 +++++--- mod/state-transition/pkg/core/types.go | 5 +++-- 8 files changed, 28 insertions(+), 14 deletions(-) diff --git a/mod/beacon/blockchain/types.go b/mod/beacon/blockchain/types.go index 7ad1da0101..8a38d7a743 100644 --- a/mod/beacon/blockchain/types.go +++ b/mod/beacon/blockchain/types.go @@ -45,6 +45,9 @@ type AvailabilityStore[BeaconBlockBodyT any] interface { type ConsensusBlock[BeaconBlockT any] interface { GetBeaconBlock() BeaconBlockT + // GetNextPayloadTimestamp returns the timestamp proposed by + // consensus for the next payload to be proposed. It is also + // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 } diff --git a/mod/beacon/validator/types.go b/mod/beacon/validator/types.go index 13c4aca02d..c327753707 100644 --- a/mod/beacon/validator/types.go +++ b/mod/beacon/validator/types.go @@ -190,8 +190,9 @@ type SlotData[AttestationDataT, SlashingInfoT any] interface { GetAttestationData() []AttestationDataT // GetSlashingInfo returns the slashing info of the incoming slot. GetSlashingInfo() []SlashingInfoT - // GetNextPayloadTimestamp returns the consensus proposed timestamp - // for the next execution payload + // GetNextPayloadTimestamp returns the timestamp proposed by + // consensus for the next payload to be proposed. It is also + // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 } diff --git a/mod/consensus/pkg/cometbft/service/middleware/middleware.go b/mod/consensus/pkg/cometbft/service/middleware/middleware.go index 01b9abe48c..e4e3dcd02e 100644 --- a/mod/consensus/pkg/cometbft/service/middleware/middleware.go +++ b/mod/consensus/pkg/cometbft/service/middleware/middleware.go @@ -42,7 +42,7 @@ type ABCIMiddleware[ // chainSpec is the chain specification. chainSpec common.ChainSpec // minimum delay among blocks, useful to set a strictly increasing - // execution payload timestamp + // execution payload timestamp. minPayloadDelay time.Duration // dispatcher is the central dispatcher to dispatcher types.EventDispatcher @@ -80,6 +80,10 @@ func NewABCIMiddleware[ ) *ABCIMiddleware[ BeaconBlockT, BlobSidecarsT, GenesisT, SlotDataT, ] { + // We may build execution payload optimistically, i.e. build execution + // payload for next block while current block is being verified and not yet + // finalized. Hence we need a minPayloadDelay that guarantees that: + // curr minPayloadDelay := min( cmtCfg.Consensus.TimeoutPropose, cmtCfg.Consensus.TimeoutPrevote, diff --git a/mod/consensus/pkg/types/consensus_block.go b/mod/consensus/pkg/types/consensus_block.go index 84dadd70d4..7f16c0cfea 100644 --- a/mod/consensus/pkg/types/consensus_block.go +++ b/mod/consensus/pkg/types/consensus_block.go @@ -29,9 +29,6 @@ import ( type ConsensusBlock[BeaconBlockT any] struct { blk BeaconBlockT - // nextPayloadTimestamp is the timestamp proposed by consensus - // for the next payload to be proposed. - // TODO: consider validating that it is strictly bounded and increasing nextPayloadTimestamp math.U64 } @@ -51,6 +48,9 @@ func (b *ConsensusBlock[BeaconBlockT]) GetBeaconBlock() BeaconBlockT { return b.blk } +// GetNextPayloadTimestamp returns the timestamp proposed by consensus +// for the next payload to be proposed. It is also used to bound +// current payload upon validation. func (b *ConsensusBlock[_]) GetNextPayloadTimestamp() math.U64 { return b.nextPayloadTimestamp } diff --git a/mod/consensus/pkg/types/slot_data.go b/mod/consensus/pkg/types/slot_data.go index b2af7c1e05..c6acd77323 100644 --- a/mod/consensus/pkg/types/slot_data.go +++ b/mod/consensus/pkg/types/slot_data.go @@ -34,9 +34,9 @@ type SlotData[AttestationDataT, SlashingInfoT any] struct { attestationData []AttestationDataT // slashingInfo is the slashing info of the incoming slot. slashingInfo []SlashingInfoT - // nextPayloadTimestamp is the timestamp proposed by consensus - // for the next payload to be proposed. - // TODO: consider validating that it is strictly bounded and increasing + // nextPayloadTimestamp is the timestamp proposed by + // consensus for the next payload to be proposed. It is also + // used to bound current payload upon validation nextPayloadTimestamp math.U64 } diff --git a/mod/node-core/pkg/components/interfaces.go b/mod/node-core/pkg/components/interfaces.go index 3971446bc9..819ed2fc39 100644 --- a/mod/node-core/pkg/components/interfaces.go +++ b/mod/node-core/pkg/components/interfaces.go @@ -83,6 +83,9 @@ type ( ConsensusBlock[BeaconBlockT any] interface { GetBeaconBlock() BeaconBlockT + // GetNextPayloadTimestamp returns the timestamp proposed by + // consensus for the next payload to be proposed. It is also + // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 } diff --git a/mod/primitives/pkg/transition/context.go b/mod/primitives/pkg/transition/context.go index 0c0ca1ed4f..771f9189ae 100644 --- a/mod/primitives/pkg/transition/context.go +++ b/mod/primitives/pkg/transition/context.go @@ -42,8 +42,9 @@ type Context struct { // SkipValidateResult indicates whether to validate the result of // the state transition. SkipValidateResult bool - // NextPayloadTimestamp is consensus proposed timestamp for the - // next payload to be built + // NextPayloadTimestamp is the timestamp proposed by + // consensus for the next payload to be proposed. It is also + // used to bound current payload upon validation NextPayloadTimestamp math.U64 } @@ -73,7 +74,8 @@ func (c *Context) GetSkipValidateResult() bool { } // GetNextPayloadTimestamp returns the timestamp proposed by consensus -// for the next payload to be built. +// for the next payload to be proposed. It is also used to bound +// current payload upon validation. func (c *Context) GetNextPayloadTimestamp() math.U64 { return c.NextPayloadTimestamp } diff --git a/mod/state-transition/pkg/core/types.go b/mod/state-transition/pkg/core/types.go index ace8cbd29e..a186f4dc0e 100644 --- a/mod/state-transition/pkg/core/types.go +++ b/mod/state-transition/pkg/core/types.go @@ -118,8 +118,9 @@ type Context interface { // GetSkipValidateResult returns whether to validate the result of the state // transition. GetSkipValidateResult() bool - // GetNextPayloadTimestamp returns the consensus proposed timestamp - // for the next payload to be built. + // GetNextPayloadTimestamp returns the timestamp proposed by + // consensus for the next payload to be proposed. It is also + // used to bound current payload upon validation GetNextPayloadTimestamp() math.U64 }