Skip to content

Commit a24c432

Browse files
ceyonurmichaelkaplan13
ceyonur
and
michaelkaplan13
authored
sync: coreth PR #1292: ACP-226: min block delay verify (#1808)
Signed-off-by: Ceyhun Onur <ceyhunonur54@gmail.com> Signed-off-by: Ceyhun Onur <ceyhun.onur@avalabs.org> Co-authored-by: Michael Kaplan <55204436+michaelkaplan13@users.noreply.github.com>
1 parent 661f4b2 commit a24c432

File tree

5 files changed

+203
-82
lines changed

5 files changed

+203
-82
lines changed

‎RELEASES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
- Add `timeMilliseconds` (Unix uint64) timestamp to block header for Granite upgrade.
1010
- Add `minDelayExcess` (uint64) to block header for Granite upgrade.
1111
- Add minimum block building delays to conform the block builder to ACP-226 requirements.
12+
- Add minimum delay verification.
1213

1314
## [v0.7.9](https://github.com/ava-labs/subnet-evm/releases/tag/v0.7.9)
1415

‎miner/worker.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,13 @@ func (w *worker) commitNewWork(predicateContext *precompileconfig.PredicateConte
139139
w.mu.RLock()
140140
defer w.mu.RUnlock()
141141
var (
142-
parent = w.chain.CurrentBlock()
143-
tstart = w.clock.Time()
144-
chainExtra = params.GetExtra(w.chainConfig)
142+
parent = w.chain.CurrentBlock()
143+
chainExtra = params.GetExtra(w.chainConfig)
144+
tstart = customheader.GetNextTimestamp(parent, w.clock.Time())
145+
timestamp = uint64(tstart.Unix())
146+
timestampMS = uint64(tstart.UnixMilli())
145147
)
146148

147-
timestamp, timestampMS := customheader.GetNextTimestamp(parent, tstart)
148-
149149
header := &types.Header{
150150
ParentHash: parent.Hash(),
151151
Number: new(big.Int).Add(parent.Number, common.Big1),

‎plugin/evm/customheader/time.go‎

Lines changed: 46 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -24,30 +24,23 @@ var (
2424
ErrTimeMillisecondsRequired = errors.New("TimeMilliseconds is required after Granite activation")
2525
ErrTimeMillisecondsMismatched = errors.New("TimeMilliseconds does not match header.Time")
2626
ErrTimeMillisecondsBeforeGranite = errors.New("TimeMilliseconds should be nil before Granite activation")
27+
ErrMinDelayNotMet = errors.New("minimum block delay not met")
28+
ErrGraniteClockBehindParent = errors.New("current timestamp is not allowed to be behind than parent timestamp in Granite")
2729
)
2830

29-
// GetNextTimestamp calculates the timestamp (in seconds and milliseconds) for the next child block based on the parent's timestamp and the current time.
30-
// First return value is the timestamp in seconds, second return value is the timestamp in milliseconds.
31-
func GetNextTimestamp(parent *types.Header, now time.Time) (uint64, uint64) {
32-
var (
33-
timestamp = uint64(now.Unix())
34-
timestampMS = uint64(now.UnixMilli())
35-
)
36-
// Note: in order to support asynchronous block production, blocks are allowed to have
37-
// the same timestamp as their parent. This allows more than one block to be produced
38-
// per second.
31+
// GetNextTimestamp calculates the time for the next header based on the parent's timestamp and the current time.
32+
// This can return the parent time if now is before the parent time and TimeMilliseconds is not set (pre-Granite).
33+
func GetNextTimestamp(parent *types.Header, now time.Time) time.Time {
3934
parentExtra := customtypes.GetHeaderExtra(parent)
40-
if parent.Time >= timestamp ||
41-
(parentExtra.TimeMilliseconds != nil && *parentExtra.TimeMilliseconds >= timestampMS) {
42-
timestamp = parent.Time
43-
// If the parent has a TimeMilliseconds, use it. Otherwise, use the parent time * 1000.
44-
if parentExtra.TimeMilliseconds != nil {
45-
timestampMS = *parentExtra.TimeMilliseconds
46-
} else {
47-
timestampMS = parent.Time * 1000 // TODO: establish minimum time
48-
}
35+
// In Granite, there is a minimum delay enforced, so we cannot adjust the time with the parent's timestamp.
36+
// Instead we should have waited enough time before calling this function and before the block building.
37+
// We return the current time instead regardless and defer the verification to VerifyTime.
38+
if parent.Time < uint64(now.Unix()) || parentExtra.TimeMilliseconds != nil {
39+
return now
4940
}
50-
return timestamp, timestampMS
41+
42+
// In pre-Granite, blocks are allowed to have the same timestamp as their parent.
43+
return time.Unix(int64(parent.Time), 0)
5144
}
5245

5346
// 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) {
5851
// - Time matches TimeMilliseconds/1000 after Granite activation
5952
// - Time/TimeMilliseconds is not too far in the future
6053
// - Time/TimeMilliseconds is non-decreasing
61-
// - (TODO) Minimum block delay is enforced
54+
// - Minimum block delay is enforced
6255
func VerifyTime(extraConfig *extras.ChainConfig, parent *types.Header, header *types.Header, now time.Time) error {
6356
var (
6457
headerExtra = customtypes.GetHeaderExtra(header)
6558
parentExtra = customtypes.GetHeaderExtra(parent)
6659
)
6760

61+
// These two variables are backward-compatible with Time (seconds) fields.
62+
headerTimeMS := customtypes.HeaderTimeMilliseconds(header)
63+
parentTimeMS := customtypes.HeaderTimeMilliseconds(parent)
64+
6865
// Verify the header's timestamp is not earlier than parent's
69-
// it does include equality(==), so multiple blocks per second is ok
70-
if header.Time < parent.Time {
71-
return fmt.Errorf("%w: %d < parent %d", errBlockTooOld, header.Time, parent.Time)
66+
// This includes equality(==), so multiple blocks per milliseconds is ok
67+
// pre-Granite.
68+
if headerTimeMS < parentTimeMS {
69+
return fmt.Errorf("%w: %d < parent %d", errBlockTooOld, headerTimeMS, parentTimeMS)
7270
}
7371

74-
// Do all checks that apply only before Granite
75-
if !extraConfig.IsGranite(header.Time) {
76-
// Make sure the block isn't too far in the future
77-
if maxBlockTime := uint64(now.Add(MaxFutureBlockTime).Unix()); header.Time > maxBlockTime {
78-
return fmt.Errorf("%w: %d > allowed %d", ErrBlockTooFarInFuture, header.Time, maxBlockTime)
79-
}
72+
// Verify if the header's timestamp is not too far in the future
73+
if maxBlockTimeMS := uint64(now.Add(MaxFutureBlockTime).UnixMilli()); headerTimeMS > maxBlockTimeMS {
74+
return fmt.Errorf("%w: %d > allowed %d",
75+
ErrBlockTooFarInFuture,
76+
headerTimeMS,
77+
maxBlockTimeMS,
78+
)
79+
}
8080

81+
if !extraConfig.IsGranite(header.Time) {
8182
// This field should not be set yet.
8283
if headerExtra.TimeMilliseconds != nil {
8384
return ErrTimeMillisecondsBeforeGranite
@@ -99,22 +100,23 @@ func VerifyTime(extraConfig *extras.ChainConfig, parent *types.Header, header *t
99100
)
100101
}
101102

102-
// Verify TimeMilliseconds is not earlier than parent's TimeMilliseconds
103-
// TODO: Ensure minimum block delay is enforced
104-
if parentExtra.TimeMilliseconds != nil && *headerExtra.TimeMilliseconds < *parentExtra.TimeMilliseconds {
105-
return fmt.Errorf("%w: %d < parent %d",
106-
errBlockTooOld,
107-
*headerExtra.TimeMilliseconds,
108-
*parentExtra.TimeMilliseconds,
109-
)
103+
// Verify minimum block delay is enforced
104+
// Parent might not have a min delay excess if this is the first Granite block
105+
// in this case we cannot verify the min delay,
106+
// Otherwise parent should have been verified in VerifyMinDelayExcess
107+
if parentExtra.MinDelayExcess == nil {
108+
return nil
110109
}
111110

112-
// Verify TimeMilliseconds is not too far in the future
113-
if maxBlockTimeMillis := uint64(now.Add(MaxFutureBlockTime).UnixMilli()); *headerExtra.TimeMilliseconds > maxBlockTimeMillis {
114-
return fmt.Errorf("%w: %d > allowed %d",
115-
ErrBlockTooFarInFuture,
116-
*headerExtra.TimeMilliseconds,
117-
maxBlockTimeMillis,
111+
// This should not be underflow as we have verified that the parent's
112+
// TimeMilliseconds is earlier than the header's TimeMilliseconds above.
113+
actualDelayMS := headerTimeMS - parentTimeMS
114+
minRequiredDelayMS := parentExtra.MinDelayExcess.Delay()
115+
if actualDelayMS < minRequiredDelayMS {
116+
return fmt.Errorf("%w: actual delay %dms < required %dms",
117+
ErrMinDelayNotMet,
118+
actualDelayMS,
119+
minRequiredDelayMS,
118120
)
119121
}
120122

‎plugin/evm/customheader/time_test.go‎

Lines changed: 149 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"testing"
88
"time"
99

10+
"github.com/ava-labs/avalanchego/vms/evm/acp226"
1011
"github.com/ava-labs/libevm/core/types"
1112
"github.com/stretchr/testify/require"
1213

@@ -15,17 +16,6 @@ import (
1516
"github.com/ava-labs/subnet-evm/utils"
1617
)
1718

18-
func generateHeader(timeSeconds uint64, timeMilliseconds *uint64) *types.Header {
19-
return customtypes.WithHeaderExtra(
20-
&types.Header{
21-
Time: timeSeconds,
22-
},
23-
&customtypes.HeaderExtra{
24-
TimeMilliseconds: timeMilliseconds,
25-
},
26-
)
27-
}
28-
2919
func TestVerifyTime(t *testing.T) {
3020
var (
3121
now = time.Unix(1714339200, 123_456_789)
@@ -138,6 +128,110 @@ func TestVerifyTime(t *testing.T) {
138128
parentHeader: generateHeader(timeSeconds, nil),
139129
extraConfig: extras.TestGraniteChainConfig,
140130
},
131+
// Min delay verification tests
132+
{
133+
name: "pre_granite_no_min_delay_verification",
134+
header: generateHeader(timeSeconds, nil),
135+
parentHeader: generateHeader(timeSeconds, nil),
136+
extraConfig: extras.TestFortunaChainConfig,
137+
},
138+
{
139+
name: "granite_first_block_no_parent_min_delay_excess",
140+
header: generateHeaderWithMinDelayExcessAndTime(
141+
timeSeconds,
142+
utils.NewUint64(timeMillis),
143+
utils.NewUint64(acp226.InitialDelayExcess),
144+
),
145+
parentHeader: generateHeader(timeSeconds-1, nil), // Pre-Granite parent
146+
extraConfig: extras.TestGraniteChainConfig,
147+
},
148+
{
149+
name: "granite_initial_delay_met",
150+
header: generateHeaderWithMinDelayExcessAndTime(
151+
timeSeconds,
152+
utils.NewUint64(timeMillis),
153+
utils.NewUint64(acp226.InitialDelayExcess),
154+
),
155+
parentHeader: generateHeaderWithMinDelayExcessAndTime(
156+
timeSeconds-1,
157+
utils.NewUint64(timeMillis-2000), // 2000 ms is the exact initial delay
158+
utils.NewUint64(acp226.InitialDelayExcess),
159+
),
160+
extraConfig: extras.TestGraniteChainConfig,
161+
},
162+
{
163+
name: "granite_initial_delay_not_met",
164+
header: generateHeaderWithMinDelayExcessAndTime(
165+
timeSeconds,
166+
utils.NewUint64(timeMillis),
167+
utils.NewUint64(acp226.InitialDelayExcess),
168+
),
169+
parentHeader: generateHeaderWithMinDelayExcessAndTime(
170+
timeSeconds-1,
171+
utils.NewUint64(timeMillis-1999), // 1 ms less than required
172+
utils.NewUint64(acp226.InitialDelayExcess),
173+
),
174+
extraConfig: extras.TestGraniteChainConfig,
175+
expectedErr: ErrMinDelayNotMet,
176+
},
177+
{
178+
name: "granite_future_timestamp_within_limits",
179+
header: generateHeaderWithMinDelayExcessAndTime(
180+
timeSeconds+5, // 5 seconds in future
181+
utils.NewUint64(timeMillis+5000),
182+
utils.NewUint64(acp226.InitialDelayExcess),
183+
),
184+
parentHeader: generateHeaderWithMinDelayExcessAndTime(
185+
timeSeconds-1,
186+
utils.NewUint64(timeMillis-2000),
187+
utils.NewUint64(acp226.InitialDelayExcess),
188+
),
189+
extraConfig: extras.TestGraniteChainConfig,
190+
},
191+
{
192+
name: "granite_future_timestamp_abuse",
193+
header: generateHeaderWithMinDelayExcessAndTime(
194+
timeSeconds+15, // 15 seconds in future, exceeds MaxFutureBlockTime
195+
utils.NewUint64(timeMillis+15000),
196+
utils.NewUint64(acp226.InitialDelayExcess),
197+
),
198+
parentHeader: generateHeaderWithMinDelayExcessAndTime(
199+
timeSeconds-1,
200+
utils.NewUint64(timeMillis-2000),
201+
utils.NewUint64(acp226.InitialDelayExcess),
202+
),
203+
extraConfig: extras.TestGraniteChainConfig,
204+
expectedErr: ErrBlockTooFarInFuture,
205+
},
206+
{
207+
name: "granite_zero_delay_excess",
208+
header: generateHeaderWithMinDelayExcessAndTime(
209+
timeSeconds,
210+
utils.NewUint64(timeMillis),
211+
utils.NewUint64(0),
212+
),
213+
parentHeader: generateHeaderWithMinDelayExcessAndTime(
214+
timeSeconds,
215+
utils.NewUint64(timeMillis-1), // 1ms delay, meets zero requirement
216+
utils.NewUint64(0), // Parent has zero delay excess
217+
),
218+
extraConfig: extras.TestGraniteChainConfig,
219+
},
220+
{
221+
name: "granite_zero_delay_excess_but_zero_delay",
222+
header: generateHeaderWithMinDelayExcessAndTime(
223+
timeSeconds,
224+
utils.NewUint64(timeMillis),
225+
utils.NewUint64(0),
226+
),
227+
parentHeader: generateHeaderWithMinDelayExcessAndTime(
228+
timeSeconds,
229+
utils.NewUint64(timeMillis), // Same timestamp, zero delay
230+
utils.NewUint64(0), // Parent has zero delay excess
231+
),
232+
extraConfig: extras.TestGraniteChainConfig,
233+
expectedErr: ErrMinDelayNotMet,
234+
},
141235
}
142236

143237
for _, test := range tests {
@@ -188,14 +282,7 @@ func TestGetNextTimestamp(t *testing.T) {
188282
expectedMillis: nowSeconds * 1000, // parent.Time * 1000
189283
},
190284
{
191-
name: "current_time_equals_parent_time_with_milliseconds",
192-
parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis)),
193-
now: now,
194-
expectedSec: nowSeconds,
195-
expectedMillis: nowMillis, // parent's TimeMilliseconds
196-
},
197-
{
198-
name: "current_time_before_parent_time",
285+
name: "current_time_before_parent_time_no_milliseconds",
199286
parent: generateHeader(nowSeconds+10, nil),
200287
now: now,
201288
expectedSec: nowSeconds + 10,
@@ -205,23 +292,60 @@ func TestGetNextTimestamp(t *testing.T) {
205292
name: "current_time_before_parent_time_with_milliseconds",
206293
parent: generateHeader(nowSeconds+10, utils.NewUint64(nowMillis)),
207294
now: now,
208-
expectedSec: nowSeconds + 10,
209-
expectedMillis: nowMillis, // parent's TimeMilliseconds
295+
expectedSec: nowSeconds,
296+
expectedMillis: nowMillis,
210297
},
211298
{
212299
name: "current_time_milliseconds_before_parent_time_milliseconds",
213300
parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis+10)),
214301
now: now,
215302
expectedSec: nowSeconds,
216-
expectedMillis: nowMillis + 10, // parent's TimeMilliseconds
303+
expectedMillis: nowMillis,
304+
},
305+
{
306+
name: "current_time_equals_parent_time_with_milliseconds_granite",
307+
parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis)),
308+
now: now,
309+
expectedSec: nowSeconds,
310+
expectedMillis: nowMillis,
311+
},
312+
{
313+
name: "current_timesec_equals_parent_time_with_less_milliseconds",
314+
parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis-10)),
315+
now: now,
316+
expectedSec: nowSeconds,
317+
expectedMillis: nowMillis,
217318
},
218319
}
219320

220321
for _, test := range tests {
221322
t.Run(test.name, func(t *testing.T) {
222-
sec, millis := GetNextTimestamp(test.parent, test.now)
223-
require.Equal(t, test.expectedSec, sec)
224-
require.Equal(t, test.expectedMillis, millis)
323+
time := GetNextTimestamp(test.parent, test.now)
324+
require.Equal(t, test.expectedSec, uint64(time.Unix()))
325+
require.Equal(t, test.expectedMillis, uint64(time.UnixMilli()))
225326
})
226327
}
227328
}
329+
330+
func generateHeader(timeSeconds uint64, timeMilliseconds *uint64) *types.Header {
331+
return customtypes.WithHeaderExtra(
332+
&types.Header{
333+
Time: timeSeconds,
334+
},
335+
&customtypes.HeaderExtra{
336+
TimeMilliseconds: timeMilliseconds,
337+
},
338+
)
339+
}
340+
341+
func generateHeaderWithMinDelayExcessAndTime(timeSeconds uint64, timeMilliseconds *uint64, minDelayExcess *uint64) *types.Header {
342+
return customtypes.WithHeaderExtra(
343+
&types.Header{
344+
Time: timeSeconds,
345+
},
346+
&customtypes.HeaderExtra{
347+
TimeMilliseconds: timeMilliseconds,
348+
MinDelayExcess: (*acp226.DelayExcess)(minDelayExcess),
349+
},
350+
)
351+
}

0 commit comments

Comments
 (0)