sync: coreth PR #1292: ACP-226: min block delay verify

RELEASES.md

@@ -9,6 +9,7 @@
   - Add `timeMilliseconds` (Unix uint64) timestamp to block header for Granite upgrade.
   - Add `minDelayExcess` (uint64) to block header for Granite upgrade.
   - Add minimum block building delays to conform the block builder to ACP-226 requirements.
+  - Add minimum delay verification.
 
 ## [v0.7.9](https://github.com/ava-labs/subnet-evm/releases/tag/v0.7.9)
 

miner/worker.go

@@ -139,13 +139,13 @@ func (w *worker) commitNewWork(predicateContext *precompileconfig.PredicateConte
 	w.mu.RLock()
 	defer w.mu.RUnlock()
 	var (
-		parent     = w.chain.CurrentBlock()
-		tstart     = w.clock.Time()
-		chainExtra = params.GetExtra(w.chainConfig)
+		parent      = w.chain.CurrentBlock()
+		chainExtra  = params.GetExtra(w.chainConfig)
+		tstart      = customheader.GetNextTimestamp(parent, w.clock.Time())
+		timestamp   = uint64(tstart.Unix())
+		timestampMS = uint64(tstart.UnixMilli())
 	)
 
-	timestamp, timestampMS := customheader.GetNextTimestamp(parent, tstart)
-
 	header := &types.Header{
 		ParentHash: parent.Hash(),
 		Number:     new(big.Int).Add(parent.Number, common.Big1),

plugin/evm/customheader/time.go

@@ -24,30 +24,23 @@ var (
 	ErrTimeMillisecondsRequired      = errors.New("TimeMilliseconds is required after Granite activation")
 	ErrTimeMillisecondsMismatched    = errors.New("TimeMilliseconds does not match header.Time")
 	ErrTimeMillisecondsBeforeGranite = errors.New("TimeMilliseconds should be nil before Granite activation")
+	ErrMinDelayNotMet                = errors.New("minimum block delay not met")
+	ErrGraniteClockBehindParent      = errors.New("current timestamp is not allowed to be behind than parent timestamp in Granite")
 )
 
-// GetNextTimestamp calculates the timestamp (in seconds and milliseconds) for the next child block based on the parent's timestamp and the current time.
-// First return value is the timestamp in seconds, second return value is the timestamp in milliseconds.
-func GetNextTimestamp(parent *types.Header, now time.Time) (uint64, uint64) {
-	var (
-		timestamp   = uint64(now.Unix())
-		timestampMS = uint64(now.UnixMilli())
-	)
-	// Note: in order to support asynchronous block production, blocks are allowed to have
-	// the same timestamp as their parent. This allows more than one block to be produced
-	// per second.
+// GetNextTimestamp calculates the time for the next header based on the parent's timestamp and the current time.
+// This can return the parent time if now is before the parent time and TimeMilliseconds is not set (pre-Granite).
+func GetNextTimestamp(parent *types.Header, now time.Time) time.Time {
 	parentExtra := customtypes.GetHeaderExtra(parent)
-	if parent.Time >= timestamp ||
-		(parentExtra.TimeMilliseconds != nil && *parentExtra.TimeMilliseconds >= timestampMS) {
-		timestamp = parent.Time
-		// If the parent has a TimeMilliseconds, use it. Otherwise, use the parent time * 1000.
-		if parentExtra.TimeMilliseconds != nil {
-			timestampMS = *parentExtra.TimeMilliseconds
-		} else {
-			timestampMS = parent.Time * 1000 // TODO: establish minimum time
-		}
+	// In Granite, there is a minimum delay enforced, so we cannot adjust the time with the parent's timestamp.
+	// Instead we should have waited enough time before calling this function and before the block building.
+	// We return the current time instead regardless and defer the verification to VerifyTime.
+	if parent.Time < uint64(now.Unix()) || parentExtra.TimeMilliseconds != nil {
+		return now
 	}
-	return timestamp, timestampMS
+
+	// In pre-Granite, blocks are allowed to have the same timestamp as their parent.
+	return time.Unix(int64(parent.Time), 0)
 }
 
 // VerifyTime verifies that the header's Time and TimeMilliseconds fields are
@@ -58,26 +51,34 @@ func GetNextTimestamp(parent *types.Header, now time.Time) (uint64, uint64) {
 // - Time matches TimeMilliseconds/1000 after Granite activation
 // - Time/TimeMilliseconds is not too far in the future
 // - Time/TimeMilliseconds is non-decreasing
-// - (TODO) Minimum block delay is enforced
+// - Minimum block delay is enforced
 func VerifyTime(extraConfig *extras.ChainConfig, parent *types.Header, header *types.Header, now time.Time) error {
 	var (
 		headerExtra = customtypes.GetHeaderExtra(header)
 		parentExtra = customtypes.GetHeaderExtra(parent)
 	)
 
+	// These two variables are backward-compatible with Time (seconds) fields.
+	headerTimeMS := customtypes.HeaderTimeMilliseconds(header)
+	parentTimeMS := customtypes.HeaderTimeMilliseconds(parent)
+
 	// Verify the header's timestamp is not earlier than parent's
-	// it does include equality(==), so multiple blocks per second is ok
-	if header.Time < parent.Time {
-		return fmt.Errorf("%w: %d < parent %d", errBlockTooOld, header.Time, parent.Time)
+	// This includes equality(==), so multiple blocks per milliseconds is ok
+	// pre-Granite.
+	if headerTimeMS < parentTimeMS {
+		return fmt.Errorf("%w: %d < parent %d", errBlockTooOld, headerTimeMS, parentTimeMS)
 	}
 
-	// Do all checks that apply only before Granite
-	if !extraConfig.IsGranite(header.Time) {
-		// Make sure the block isn't too far in the future
-		if maxBlockTime := uint64(now.Add(MaxFutureBlockTime).Unix()); header.Time > maxBlockTime {
-			return fmt.Errorf("%w: %d > allowed %d", ErrBlockTooFarInFuture, header.Time, maxBlockTime)
-		}
+	// Verify if the header's timestamp is not too far in the future
+	if maxBlockTimeMS := uint64(now.Add(MaxFutureBlockTime).UnixMilli()); headerTimeMS > maxBlockTimeMS {
+		return fmt.Errorf("%w: %d > allowed %d",
+			ErrBlockTooFarInFuture,
+			headerTimeMS,
+			maxBlockTimeMS,
+		)
+	}
 
+	if !extraConfig.IsGranite(header.Time) {
 		// This field should not be set yet.
 		if headerExtra.TimeMilliseconds != nil {
 			return ErrTimeMillisecondsBeforeGranite
@@ -99,22 +100,23 @@ func VerifyTime(extraConfig *extras.ChainConfig, parent *types.Header, header *t
 		)
 	}
 
-	// Verify TimeMilliseconds is not earlier than parent's TimeMilliseconds
-	// TODO: Ensure minimum block delay is enforced
-	if parentExtra.TimeMilliseconds != nil && *headerExtra.TimeMilliseconds < *parentExtra.TimeMilliseconds {
-		return fmt.Errorf("%w: %d < parent %d",
-			errBlockTooOld,
-			*headerExtra.TimeMilliseconds,
-			*parentExtra.TimeMilliseconds,
-		)
+	// Verify minimum block delay is enforced
+	// Parent might not have a min delay excess if this is the first Granite block
+	// in this case we cannot verify the min delay,
+	// Otherwise parent should have been verified in VerifyMinDelayExcess
+	if parentExtra.MinDelayExcess == nil {
+		return nil
 	}
 
-	// Verify TimeMilliseconds is not too far in the future
-	if maxBlockTimeMillis := uint64(now.Add(MaxFutureBlockTime).UnixMilli()); *headerExtra.TimeMilliseconds > maxBlockTimeMillis {
-		return fmt.Errorf("%w: %d > allowed %d",
-			ErrBlockTooFarInFuture,
-			*headerExtra.TimeMilliseconds,
-			maxBlockTimeMillis,
+	// This should not be underflow as we have verified that the parent's
+	// TimeMilliseconds is earlier than the header's TimeMilliseconds above.
+	actualDelayMS := headerTimeMS - parentTimeMS
+	minRequiredDelayMS := parentExtra.MinDelayExcess.Delay()
+	if actualDelayMS < minRequiredDelayMS {
+		return fmt.Errorf("%w: actual delay %dms < required %dms",
+			ErrMinDelayNotMet,
+			actualDelayMS,
+			minRequiredDelayMS,
 		)
 	}
 

plugin/evm/customheader/time_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ava-labs/avalanchego/vms/evm/acp226"
 	"github.com/ava-labs/libevm/core/types"
 	"github.com/stretchr/testify/require"
 
@@ -15,17 +16,6 @@ import (
 	"github.com/ava-labs/subnet-evm/utils"
 )
 
-func generateHeader(timeSeconds uint64, timeMilliseconds *uint64) *types.Header {
-	return customtypes.WithHeaderExtra(
-		&types.Header{
-			Time: timeSeconds,
-		},
-		&customtypes.HeaderExtra{
-			TimeMilliseconds: timeMilliseconds,
-		},
-	)
-}
-
 func TestVerifyTime(t *testing.T) {
 	var (
 		now         = time.Unix(1714339200, 123_456_789)
@@ -138,6 +128,110 @@ func TestVerifyTime(t *testing.T) {
 			parentHeader: generateHeader(timeSeconds, nil),
 			extraConfig:  extras.TestGraniteChainConfig,
 		},
+		// Min delay verification tests
+		{
+			name:         "pre_granite_no_min_delay_verification",
+			header:       generateHeader(timeSeconds, nil),
+			parentHeader: generateHeader(timeSeconds, nil),
+			extraConfig:  extras.TestFortunaChainConfig,
+		},
+		{
+			name: "granite_first_block_no_parent_min_delay_excess",
+			header: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds,
+				utils.NewUint64(timeMillis),
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			parentHeader: generateHeader(timeSeconds-1, nil), // Pre-Granite parent
+			extraConfig:  extras.TestGraniteChainConfig,
+		},
+		{
+			name: "granite_initial_delay_met",
+			header: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds,
+				utils.NewUint64(timeMillis),
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			parentHeader: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds-1,
+				utils.NewUint64(timeMillis-2000), // 2000 ms is the exact initial delay
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			extraConfig: extras.TestGraniteChainConfig,
+		},
+		{
+			name: "granite_initial_delay_not_met",
+			header: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds,
+				utils.NewUint64(timeMillis),
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			parentHeader: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds-1,
+				utils.NewUint64(timeMillis-1999), // 1 ms less than required
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			extraConfig: extras.TestGraniteChainConfig,
+			expectedErr: ErrMinDelayNotMet,
+		},
+		{
+			name: "granite_future_timestamp_within_limits",
+			header: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds+5, // 5 seconds in future
+				utils.NewUint64(timeMillis+5000),
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			parentHeader: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds-1,
+				utils.NewUint64(timeMillis-2000),
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			extraConfig: extras.TestGraniteChainConfig,
+		},
+		{
+			name: "granite_future_timestamp_abuse",
+			header: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds+15, // 15 seconds in future, exceeds MaxFutureBlockTime
+				utils.NewUint64(timeMillis+15000),
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			parentHeader: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds-1,
+				utils.NewUint64(timeMillis-2000),
+				utils.NewUint64(acp226.InitialDelayExcess),
+			),
+			extraConfig: extras.TestGraniteChainConfig,
+			expectedErr: ErrBlockTooFarInFuture,
+		},
+		{
+			name: "granite_zero_delay_excess",
+			header: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds,
+				utils.NewUint64(timeMillis),
+				utils.NewUint64(0),
+			),
+			parentHeader: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds,
+				utils.NewUint64(timeMillis-1), // 1ms delay, meets zero requirement
+				utils.NewUint64(0),            // Parent has zero delay excess
+			),
+			extraConfig: extras.TestGraniteChainConfig,
+		},
+		{
+			name: "granite_zero_delay_excess_but_zero_delay",
+			header: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds,
+				utils.NewUint64(timeMillis),
+				utils.NewUint64(0),
+			),
+			parentHeader: generateHeaderWithMinDelayExcessAndTime(
+				timeSeconds,
+				utils.NewUint64(timeMillis), // Same timestamp, zero delay
+				utils.NewUint64(0),          // Parent has zero delay excess
+			),
+			extraConfig: extras.TestGraniteChainConfig,
+			expectedErr: ErrMinDelayNotMet,
+		},
 	}
 
 	for _, test := range tests {
@@ -188,14 +282,7 @@ func TestGetNextTimestamp(t *testing.T) {
 			expectedMillis: nowSeconds * 1000, // parent.Time * 1000
 		},
 		{
-			name:           "current_time_equals_parent_time_with_milliseconds",
-			parent:         generateHeader(nowSeconds, utils.NewUint64(nowMillis)),
-			now:            now,
-			expectedSec:    nowSeconds,
-			expectedMillis: nowMillis, // parent's TimeMilliseconds
-		},
-		{
-			name:           "current_time_before_parent_time",
+			name:           "current_time_before_parent_time_no_milliseconds",
 			parent:         generateHeader(nowSeconds+10, nil),
 			now:            now,
 			expectedSec:    nowSeconds + 10,
@@ -205,23 +292,60 @@ func TestGetNextTimestamp(t *testing.T) {
 			name:           "current_time_before_parent_time_with_milliseconds",
 			parent:         generateHeader(nowSeconds+10, utils.NewUint64(nowMillis)),
 			now:            now,
-			expectedSec:    nowSeconds + 10,
-			expectedMillis: nowMillis, // parent's TimeMilliseconds
+			expectedSec:    nowSeconds,
+			expectedMillis: nowMillis,
 		},
 		{
 			name:           "current_time_milliseconds_before_parent_time_milliseconds",
 			parent:         generateHeader(nowSeconds, utils.NewUint64(nowMillis+10)),
 			now:            now,
 			expectedSec:    nowSeconds,
-			expectedMillis: nowMillis + 10, // parent's TimeMilliseconds
+			expectedMillis: nowMillis,
+		},
+		{
+			name:           "current_time_equals_parent_time_with_milliseconds_granite",
+			parent:         generateHeader(nowSeconds, utils.NewUint64(nowMillis)),
+			now:            now,
+			expectedSec:    nowSeconds,
+			expectedMillis: nowMillis,
+		},
+		{
+			name:           "current_timesec_equals_parent_time_with_less_milliseconds",
+			parent:         generateHeader(nowSeconds, utils.NewUint64(nowMillis-10)),
+			now:            now,
+			expectedSec:    nowSeconds,
+			expectedMillis: nowMillis,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			sec, millis := GetNextTimestamp(test.parent, test.now)
-			require.Equal(t, test.expectedSec, sec)
-			require.Equal(t, test.expectedMillis, millis)
+			time := GetNextTimestamp(test.parent, test.now)
+			require.Equal(t, test.expectedSec, uint64(time.Unix()))
+			require.Equal(t, test.expectedMillis, uint64(time.UnixMilli()))
 		})
 	}
 }
+
+func generateHeader(timeSeconds uint64, timeMilliseconds *uint64) *types.Header {
+	return customtypes.WithHeaderExtra(
+		&types.Header{
+			Time: timeSeconds,
+		},
+		&customtypes.HeaderExtra{
+			TimeMilliseconds: timeMilliseconds,
+		},
+	)
+}
+
+func generateHeaderWithMinDelayExcessAndTime(timeSeconds uint64, timeMilliseconds *uint64, minDelayExcess *uint64) *types.Header {
+	return customtypes.WithHeaderExtra(
+		&types.Header{
+			Time: timeSeconds,
+		},
+		&customtypes.HeaderExtra{
+			TimeMilliseconds: timeMilliseconds,
+			MinDelayExcess:   (*acp226.DelayExcess)(minDelayExcess),
+		},
+	)
+}