diff --git a/miner/worker.go b/miner/worker.go index 783c1cedd6..946f9f5c63 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -149,7 +149,10 @@ func (w *worker) commitNewWork(predicateContext *precompileconfig.PredicateConte chainExtra = params.GetExtra(w.chainConfig) ) - timestamp, timestampMS := customheader.GetNextTimestamp(parent, tstart) + timestamp, timestampMS, err := customheader.GetNextTimestamp(chainExtra, parent, tstart) + if err != nil { + return nil, fmt.Errorf("failed to get next timestamp: %w", err) + } header := &types.Header{ ParentHash: parent.Hash(), diff --git a/plugin/evm/customheader/min_delay_excess.go b/plugin/evm/customheader/min_delay_excess.go index 0498fdc32c..a4cd4975d0 100644 --- a/plugin/evm/customheader/min_delay_excess.go +++ b/plugin/evm/customheader/min_delay_excess.go @@ -77,7 +77,7 @@ func VerifyMinDelayExcess( } default: // Prior to Granite there was no expected min delay excess. - // TODO (ceyonur): this can be removed after Granite is activated. + // TODO Cleanup (ceyonur): this can be removed after Granite is activated. if customtypes.GetHeaderExtra(header).MinDelayExcess != nil { return fmt.Errorf("%w: %s", errRemoteMinDelayExcessSet, header.Hash()) } diff --git a/plugin/evm/customheader/time.go b/plugin/evm/customheader/time.go index 75075d1ee4..c92fdd3ff7 100644 --- a/plugin/evm/customheader/time.go +++ b/plugin/evm/customheader/time.go @@ -24,30 +24,34 @@ 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. +// GetNextTimestamp calculates the timestamp (in seconds and milliseconds) for the header 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) { +func GetNextTimestamp(config *extras.ChainConfig, parent *types.Header, now time.Time) (uint64, uint64, error) { 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. - 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 + if parent.Time < timestamp { + return timestamp, timestampMS, nil + } + // In Granite, there is a minimum delay enforced, and if the timestamp is the same as the parent, + // the block will be rejected. + // The block builder should have already waited enough time to meet the minimum delay. + // This is to early-exit from the block building. + if config.IsGranite(timestamp) { + if uint64(customtypes.BlockTime(parent).UnixMilli()) >= timestampMS { + return 0, 0, ErrGraniteClockBehindParent } + return timestamp, timestampMS, nil } - return timestamp, timestampMS + // In pre-Granite, blocks are allowed to have the same timestamp as their parent + // Actually we don't need to return modified timestampMS, because it will be not be set if this is not + // Granite, but setting here for consistency. + return parent.Time, parent.Time * 1000, nil } // VerifyTime verifies that the header's Time and TimeMilliseconds fields are @@ -100,7 +104,6 @@ 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, @@ -109,7 +112,14 @@ func VerifyTime(extraConfig *extras.ChainConfig, parent *types.Header, header *t ) } + // Verify minimum block delay is enforced + if err := verifyMinDelay(parent, header); err != nil { + return err + } + // Verify TimeMilliseconds is not too far in the future + // Q: This still is a problem especially if someone issues a block in the future + // then the next builder will wait until potentially MaxFutureBlockTime + delay if maxBlockTimeMillis := uint64(now.Add(MaxFutureBlockTime).UnixMilli()); *headerExtra.TimeMilliseconds > maxBlockTimeMillis { return fmt.Errorf("%w: %d > allowed %d", ErrBlockTooFarInFuture, @@ -120,3 +130,38 @@ func VerifyTime(extraConfig *extras.ChainConfig, parent *types.Header, header *t return nil } + +// verifyMinDelay verifies that the minimum block delay is enforced based on the min delay excess. +func verifyMinDelay(parent *types.Header, header *types.Header) error { + headerExtra := customtypes.GetHeaderExtra(header) + parentExtra := customtypes.GetHeaderExtra(parent) + + // if parent has no TimeMilliseconds, no min delay is required + if parentExtra.TimeMilliseconds == nil { + return nil + } + + // Calculate the actual time difference in milliseconds + actualDelayMillis := *headerExtra.TimeMilliseconds - *parentExtra.TimeMilliseconds + + // 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 + minDelayExcess := customtypes.GetHeaderExtra(parent).MinDelayExcess + if minDelayExcess == nil { + return nil + } + + minRequiredDelayMillis := minDelayExcess.Delay() + + // Check if the actual delay meets the minimum requirement + if actualDelayMillis < minRequiredDelayMillis { + return fmt.Errorf("%w: actual delay %dms < required %dms", + ErrMinDelayNotMet, + actualDelayMillis, + minRequiredDelayMillis, + ) + } + + return nil +} diff --git a/plugin/evm/customheader/time_test.go b/plugin/evm/customheader/time_test.go index 375110e082..284a2a8365 100644 --- a/plugin/evm/customheader/time_test.go +++ b/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/coreth/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,139 @@ 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-1, 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_min_delay_met", + header: generateHeaderWithMinDelayExcessAndTime( + timeSeconds, + utils.NewUint64(timeMillis), + utils.NewUint64(acp226.InitialDelayExcess), + ), + parentHeader: generateHeaderWithMinDelayExcessAndTime( + timeSeconds-1, + utils.NewUint64(timeMillis-uint64(acp226.InitialDelayExcess)), // Exact minimum delay + utils.NewUint64(acp226.InitialDelayExcess), + ), + extraConfig: extras.TestGraniteChainConfig, + }, + { + name: "granite_min_delay_not_met", + header: generateHeaderWithMinDelayExcessAndTime( + timeSeconds, + utils.NewUint64(timeMillis), + utils.NewUint64(acp226.InitialDelayExcess), + ), + parentHeader: generateHeaderWithMinDelayExcessAndTime( + timeSeconds-1, + utils.NewUint64(timeMillis-100), // Only 100ms delay, 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-uint64(acp226.InitialDelayExcess)), + 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-uint64(acp226.InitialDelayExcess)), + utils.NewUint64(acp226.InitialDelayExcess), + ), + extraConfig: extras.TestGraniteChainConfig, + expectedErr: ErrBlockTooFarInFuture, + }, + { + name: "granite_min_delay_excess_updated", + header: generateHeaderWithMinDelayExcessAndTime( + timeSeconds, + utils.NewUint64(timeMillis), + utils.NewUint64(acp226.InitialDelayExcess+acp226.MaxDelayExcessDiff), + ), + parentHeader: generateHeaderWithMinDelayExcessAndTime( + timeSeconds-1, + utils.NewUint64(timeMillis-uint64(acp226.InitialDelayExcess+acp226.MaxDelayExcessDiff)), // Meets increased requirement + utils.NewUint64(acp226.InitialDelayExcess), + ), + extraConfig: extras.TestGraniteChainConfig, + }, + { + name: "granite_min_delay_excess_updated_but_delay_insufficient", + header: generateHeaderWithMinDelayExcessAndTime( + timeSeconds, + utils.NewUint64(timeMillis), + utils.NewUint64(acp226.InitialDelayExcess+acp226.MaxDelayExcessDiff), + ), + parentHeader: generateHeaderWithMinDelayExcessAndTime( + timeSeconds-1, + utils.NewUint64(timeMillis-1000), // 1000ms delay, insufficient for increased requirement + utils.NewUint64(acp226.InitialDelayExcess), + ), + extraConfig: extras.TestGraniteChainConfig, + expectedErr: ErrMinDelayNotMet, + }, + { + name: "granite_zero_delay_excess", + header: generateHeaderWithMinDelayExcessAndTime( + timeSeconds, + utils.NewUint64(timeMillis), + utils.NewUint64(0), + ), + parentHeader: generateHeaderWithMinDelayExcessAndTime( + timeSeconds-1, + 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 { @@ -162,9 +285,11 @@ func TestGetNextTimestamp(t *testing.T) { tests := []struct { name string parent *types.Header + extraConfig *extras.ChainConfig now time.Time expectedSec uint64 expectedMillis uint64 + expectedErr error }{ { name: "current_time_after_parent_time_no_milliseconds", @@ -181,47 +306,102 @@ func TestGetNextTimestamp(t *testing.T) { expectedMillis: nowMillis, }, { - name: "current_time_equals_parent_time_no_milliseconds", + name: "current_time_equals_parent_time_no_milliseconds_pre_granite", parent: generateHeader(nowSeconds, nil), + extraConfig: extras.TestFortunaChainConfig, now: now, expectedSec: nowSeconds, expectedMillis: nowSeconds * 1000, // parent.Time * 1000 }, { - name: "current_time_equals_parent_time_with_milliseconds", + name: "current_time_equals_parent_time_with_milliseconds_pre_granite", parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis)), + extraConfig: extras.TestFortunaChainConfig, now: now, expectedSec: nowSeconds, - expectedMillis: nowMillis, // parent's TimeMilliseconds + expectedMillis: nowSeconds * 1000, // parent.Time * 1000 }, { - name: "current_time_before_parent_time", + name: "current_time_before_parent_time_pre_granite", parent: generateHeader(nowSeconds+10, nil), + extraConfig: extras.TestFortunaChainConfig, now: now, expectedSec: nowSeconds + 10, expectedMillis: (nowSeconds + 10) * 1000, // parent.Time * 1000 }, { - name: "current_time_before_parent_time_with_milliseconds", + name: "current_time_before_parent_time_with_milliseconds_pre_granite", parent: generateHeader(nowSeconds+10, utils.NewUint64(nowMillis)), + extraConfig: extras.TestFortunaChainConfig, now: now, expectedSec: nowSeconds + 10, - expectedMillis: nowMillis, // parent's TimeMilliseconds + expectedMillis: (nowSeconds + 10) * 1000, // parent.Time * 1000 }, { - name: "current_time_milliseconds_before_parent_time_milliseconds", + name: "current_time_milliseconds_before_parent_time_milliseconds_pre_granite", parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis+10)), + extraConfig: extras.TestFortunaChainConfig, + now: now, + expectedSec: nowSeconds, + expectedMillis: nowSeconds * 1000, // parent.Time * 1000 + }, + { + name: "current_time_before_parent_time_granite", + parent: generateHeader(nowSeconds+10, utils.NewUint64(nowMillis)), + extraConfig: extras.TestGraniteChainConfig, + now: now, + expectedErr: ErrGraniteClockBehindParent, + }, + { + name: "current_time_equals_parent_time_with_milliseconds_granite", + parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis)), + extraConfig: extras.TestGraniteChainConfig, + now: now, + expectedErr: ErrGraniteClockBehindParent, + }, + { + name: "current_timesec_equals_parent_time_with_different_milliseconds_granite", + parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis-1000)), + extraConfig: extras.TestGraniteChainConfig, now: now, + expectedErr: nil, expectedSec: nowSeconds, - expectedMillis: nowMillis + 10, // parent's TimeMilliseconds + expectedMillis: nowMillis, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - sec, millis := GetNextTimestamp(test.parent, test.now) + if test.extraConfig == nil { + test.extraConfig = extras.TestChainConfig + } + sec, millis, err := GetNextTimestamp(test.extraConfig, test.parent, test.now) + require.ErrorIs(t, err, test.expectedErr) require.Equal(t, test.expectedSec, sec) require.Equal(t, test.expectedMillis, millis) }) } } + +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), + }, + ) +}