fix(beacon, blockchain): Fix payload timestamp build and verification (#2160)

Author: abi87

mod/beacon/blockchain/execution_engine.go

@@ -24,6 +24,7 @@ import (
 	"context"
 	"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"
 )
@@ -76,7 +77,11 @@ func (s *Service[
 		return
 	}
 
-	nextPayloadTime := blk.GetNextPayloadTimestamp().Unwrap()
+	nextPayloadTime := payloadtime.Next(
+		blk.GetConsensusTime(),
+		lph.GetTimestamp(),
+		true, // buildOptimistically
+	).Unwrap()
 
 	// We set timestamp check on Bartio for backward compatibility reasons
 	// TODO: drop this we drop other Bartio special cases.

mod/beacon/blockchain/process.go

@@ -128,8 +128,8 @@ func (s *Service[
 			// actually irrelevant at this point.
 			SkipPayloadVerification: false,
 
-			ProposerAddress:      blk.GetProposerAddress(),
-			NextPayloadTimestamp: blk.GetNextPayloadTimestamp(),
+			ProposerAddress: blk.GetProposerAddress(),
+			ConsensusTime:   blk.GetConsensusTime(),
 		},
 		st,
 		blk.GetBeaconBlock(),

mod/beacon/blockchain/receive.go

@@ -24,6 +24,7 @@ import (
 	"context"
 	"time"
 
+	payloadtime "github.com/berachain/beacon-kit/mod/beacon/payload-time"
 	engineerrors "github.com/berachain/beacon-kit/mod/engine-primitives/pkg/errors"
 	"github.com/berachain/beacon-kit/mod/errors"
 	"github.com/berachain/beacon-kit/mod/primitives/pkg/transition"
@@ -32,14 +33,14 @@ import (
 // VerifyIncomingBlock verifies the state root of an incoming block
 // and logs the process.
 func (s *Service[
-	_, ConsensusBlockT, BeaconBlockT, _, _, _, _, _, _, _, _,
+	_, ConsensusBlockT, BeaconBlockT, _, _, _, _, _, ExecutionPayloadHeaderT, _, _,
 ]) VerifyIncomingBlock(
 	ctx context.Context,
 	blk ConsensusBlockT,
 ) error {
 	var (
-		beaconBlk            = blk.GetBeaconBlock()
-		nextPayloadTimestamp = blk.GetNextPayloadTimestamp()
+		beaconBlk     = blk.GetBeaconBlock()
+		consensusTime = blk.GetConsensusTime()
 	)
 
 	// Grab a copy of the state to verify the incoming block.
@@ -81,10 +82,20 @@ func (s *Service[
 		)
 
 		if s.shouldBuildOptimisticPayloads() {
+			var lph ExecutionPayloadHeaderT
+			lph, err = preState.GetLatestExecutionPayloadHeader()
+			if err != nil {
+				return err
+			}
+
 			go s.handleRebuildPayloadForRejectedBlock(
 				ctx,
 				preState,
-				nextPayloadTimestamp,
+				payloadtime.Next(
+					consensusTime,
+					lph.GetTimestamp(),
+					true, // buildOptimistically
+				),
 			)
 		}
 
@@ -98,11 +109,20 @@ func (s *Service[
 	)
 
 	if s.shouldBuildOptimisticPayloads() {
+		lph, err := postState.GetLatestExecutionPayloadHeader()
+		if err != nil {
+			return err
+		}
+
 		go s.handleOptimisticPayloadBuild(
 			ctx,
 			postState,
 			beaconBlk,
-			nextPayloadTimestamp,
+			payloadtime.Next(
+				consensusTime,
+				lph.GetTimestamp(),
+				true, // buildOptimistically
+			),
 		)
 	}
 
@@ -129,7 +149,7 @@ func (s *Service[
 			SkipValidateResult:      false,
 			SkipValidateRandao:      false,
 			ProposerAddress:         blk.GetProposerAddress(),
-			NextPayloadTimestamp:    blk.GetNextPayloadTimestamp(),
+			ConsensusTime:           blk.GetConsensusTime(),
 		},
 		st, blk.GetBeaconBlock(),
 	)

mod/beacon/blockchain/types.go

@@ -49,10 +49,9 @@ type ConsensusBlock[BeaconBlockT any] interface {
 	// selected by consensus to propose the block
 	GetProposerAddress() []byte
 
-	// 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
+	// GetConsensusTime returns the timestamp of current consensus request.
+	// It is used to build next payload and to validate currentpayload.
+	GetConsensusTime() math.U64
 }
 
 // BeaconBlock represents a beacon block interface.

mod/beacon/go.mod

@@ -11,6 +11,7 @@ require (
 	github.com/berachain/beacon-kit/mod/errors v0.0.0-20240806211103-d1105603bfc0
 	github.com/berachain/beacon-kit/mod/log v0.0.0-20240809202957-3e3f169ad720
 	github.com/berachain/beacon-kit/mod/primitives v0.0.0-20240911165923-82f71ec86570
+	github.com/stretchr/testify v1.9.0
 	golang.org/x/sync v0.8.0
 )
 
@@ -84,7 +85,6 @@ require (
 	github.com/rogpeppe/go-internal v1.12.0 // indirect
 	github.com/sasha-s/go-deadlock v0.3.5 // indirect
 	github.com/shirou/gopsutil v3.21.11+incompatible // indirect
-	github.com/stretchr/testify v1.9.0 // indirect
 	github.com/supranational/blst v0.3.13 // indirect
 	github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect
 	github.com/tklauser/go-sysconf v0.3.14 // indirect

mod/beacon/payload-time/time.go

@@ -0,0 +1,72 @@
+// 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 payloadtime
+
+import (
+	"errors"
+	"fmt"
+
+	"github.com/berachain/beacon-kit/mod/primitives/pkg/math"
+)
+
+// ErrTooFarInTheFuture is returned when the payload timestamp
+// in a block exceeds the time bound.
+var ErrTooFarInTheFuture = errors.New("timestamp too far in the future")
+
+func Verify(
+	consensusTime,
+	parentPayloadTimestamp,
+	payloadTimestamp math.U64,
+) error {
+	bound := max(
+		consensusTime+1,
+		parentPayloadTimestamp+1,
+	)
+	if payloadTimestamp > bound {
+		return fmt.Errorf(
+			"%w: timestamp bound: %d, got: %d",
+			ErrTooFarInTheFuture,
+			bound, payloadTimestamp,
+		)
+	}
+	return nil
+}
+
+func Next(
+	consensusTime,
+	parentPayloadTimestamp math.U64,
+	buildOptimistically bool,
+) math.U64 {
+	delta := math.U64(0)
+	if buildOptimistically {
+		// we're building a payload to be included into next block.
+		// We estimate it to be included next second. If this estimate
+		// turns out wrong (cause consensus block are finalized faster or
+		// slower than consensusTime+1 sec), we're still fine as long as
+		// Verify pass which should always to since:
+		// Next.consensusTime <= Verify.consensusTime
+		delta = 1
+	}
+	return max(
+		consensusTime+delta,
+		parentPayloadTimestamp+1,
+	)
+}

mod/beacon/payload-time/time_test.go

@@ -0,0 +1,110 @@
+// 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 payloadtime_test
+
+import (
+	"testing"
+	"time"
+
+	payloadtime "github.com/berachain/beacon-kit/mod/beacon/payload-time"
+	"github.com/berachain/beacon-kit/mod/primitives/pkg/math"
+	"github.com/stretchr/testify/require"
+)
+
+// TestNextTimestampVerifies checks that next payload timestamp
+// built via payloadtime.Next always pass payloadtime.Verify.
+func TestNextTimestampVerifies(t *testing.T) {
+	tests := []struct {
+		name        string
+		times       func() (time.Time, time.Time)
+		expectedErr error
+	}{
+		{
+			name: "Payload timestamp < consensus timestamp",
+			times: func() (time.Time, time.Time) {
+				consensusTime := time.Now().Truncate(time.Second)
+				parentPayloadTimestamp := consensusTime.Add(-10 * time.Second)
+				return consensusTime, parentPayloadTimestamp
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "Payload timestamp == consensus timestamp",
+			times: func() (time.Time, time.Time) {
+				consensusTime := time.Now().Truncate(time.Second)
+				parentPayloadTimestamp := consensusTime
+				return consensusTime, parentPayloadTimestamp
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "Payload timestamp > consensus timestamp",
+			times: func() (time.Time, time.Time) {
+				consensusTime := time.Now().Truncate(time.Second)
+				parentPayloadTimestamp := consensusTime.Add(10 * time.Second)
+				return consensusTime, parentPayloadTimestamp
+			},
+			expectedErr: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			consensusTime, parentPayloadTimestamp := tt.times()
+
+			// Optimistic build case
+			nextPayload := payloadtime.Next(
+				math.U64(consensusTime.Unix()),
+				math.U64(parentPayloadTimestamp.Unix()),
+				true, // buildOptimistically
+			)
+
+			gotErr := payloadtime.Verify(
+				math.U64(consensusTime.Unix()),
+				math.U64(parentPayloadTimestamp.Unix()),
+				nextPayload,
+			)
+			if tt.expectedErr == nil {
+				require.NoError(t, gotErr)
+			} else {
+				require.ErrorIs(t, tt.expectedErr, gotErr)
+			}
+
+			// Just in time build case
+			nextPayload = payloadtime.Next(
+				math.U64(consensusTime.Unix()),
+				math.U64(parentPayloadTimestamp.Unix()),
+				false, // buildOptimistically
+			)
+
+			gotErr = payloadtime.Verify(
+				math.U64(consensusTime.Unix()),
+				math.U64(parentPayloadTimestamp.Unix()),
+				nextPayload,
+			)
+			if tt.expectedErr == nil {
+				require.NoError(t, gotErr)
+			} else {
+				require.ErrorIs(t, tt.expectedErr, gotErr)
+			}
+		})
+	}
+}