From ef12dfb0a337d6c6195636c38699a64e2bb7e8fa Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Sep 2025 00:21:56 +0300 Subject: [PATCH 01/19] add is retry logic --- plugin/evm/block_builder.go | 30 ++++++++++++++---------------- plugin/evm/vm.go | 2 +- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index d8dd0c5d5f..58acd8382e 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -37,10 +37,11 @@ type blockBuilder struct { pendingSignal *lock.Cond buildBlockLock sync.Mutex - // lastBuildTime is the time when the last block was built. + // lastBuiltHeight is the height of the last block was built. // This is used to ensure that we don't build blocks too frequently, // but at least after a minimum delay of minBlockBuildingRetryDelay. - lastBuildTime time.Time + lastBuiltHeight uint64 + isRetry bool } // NewBlockBuilder creates a new block builder. extraMempool is an optional mempool (can be nil) that @@ -58,10 +59,11 @@ func (vm *VM) NewBlockBuilder(extraMempool extension.BuilderMempool) *blockBuild } // handleGenerateBlock is called from the VM immediately after BuildBlock. -func (b *blockBuilder) handleGenerateBlock() { +func (b *blockBuilder) handleGenerateBlock(currentHeight uint64) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() - b.lastBuildTime = time.Now() + b.isRetry = b.lastBuiltHeight == currentHeight + b.lastBuiltHeight = currentHeight } // needToBuild returns true if there are outstanding transactions to be issued @@ -116,38 +118,34 @@ func (b *blockBuilder) awaitSubmittedTxs() { // waitForEvent waits until a block needs to be built. // It returns only after at least [minBlockBuildingRetryDelay] passed from the last time a block was built. func (b *blockBuilder) waitForEvent(ctx context.Context) (commonEng.Message, error) { - lastBuildTime, err := b.waitForNeedToBuild(ctx) + isRetry, err := b.waitForNeedToBuild(ctx) if err != nil { return 0, err } - timeSinceLastBuildTime := time.Since(lastBuildTime) - if b.lastBuildTime.IsZero() || timeSinceLastBuildTime >= MinBlockBuildingRetryDelay { - b.ctx.Log.Debug("Last time we built a block was long enough ago, no need to wait", - zap.Duration("timeSinceLastBuildTime", timeSinceLastBuildTime), - ) + if !isRetry { + b.ctx.Log.Debug("This is the first time we attempt to build a block at this height, no need to wait") return commonEng.PendingTxs, nil } - timeUntilNextBuild := MinBlockBuildingRetryDelay - timeSinceLastBuildTime b.ctx.Log.Debug("Last time we built a block was too recent, waiting", - zap.Duration("timeUntilNextBuild", timeUntilNextBuild), + zap.Duration("timeUntilNextBuild", MinBlockBuildingRetryDelay), ) select { case <-ctx.Done(): return 0, ctx.Err() - case <-time.After(timeUntilNextBuild): + case <-time.After(MinBlockBuildingRetryDelay): return commonEng.PendingTxs, nil } } // waitForNeedToBuild waits until needToBuild returns true. // It returns the last time a block was built. -func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (time.Time, error) { +func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (bool, error) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() for !b.needToBuild() { if err := b.pendingSignal.Wait(ctx); err != nil { - return time.Time{}, err + return false, err } } - return b.lastBuildTime, nil + return b.isRetry, nil } diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 1cffb3bbe7..eea558cf57 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -910,7 +910,7 @@ func (vm *VM) buildBlockWithContext(_ context.Context, proposerVMBlockCtx *block } block, err := vm.miner.GenerateBlock(predicateCtx) - vm.builder.handleGenerateBlock() + vm.builder.handleGenerateBlock(vm.blockChain.CurrentHeader().Number.Uint64()) if err != nil { return nil, fmt.Errorf("%w: %w", vmerrors.ErrGenerateBlockFailed, err) } From cf66f59ce32432b0999d83b9a199cdd78b45d321 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Sep 2025 16:59:18 +0300 Subject: [PATCH 02/19] update waitForEvent by using parent hash + last build time --- plugin/evm/block_builder.go | 43 ++++++++++++++++++++++++------------- plugin/evm/vm.go | 2 +- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index 58acd8382e..a4020c09ed 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -10,6 +10,7 @@ import ( "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/avalanchego/utils/lock" + "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/log" "github.com/holiman/uint256" "go.uber.org/zap" @@ -37,11 +38,11 @@ type blockBuilder struct { pendingSignal *lock.Cond buildBlockLock sync.Mutex - // lastBuiltHeight is the height of the last block was built. - // This is used to ensure that we don't build blocks too frequently, + // lastBuiltParentHash is the parent hash of the last block was built. + // This and lastBuildTime are used to ensure that we don't build blocks too frequently, // but at least after a minimum delay of minBlockBuildingRetryDelay. - lastBuiltHeight uint64 - isRetry bool + lastBuiltParentHash common.Hash + lastBuildTime time.Time } // NewBlockBuilder creates a new block builder. extraMempool is an optional mempool (can be nil) that @@ -59,11 +60,16 @@ func (vm *VM) NewBlockBuilder(extraMempool extension.BuilderMempool) *blockBuild } // handleGenerateBlock is called from the VM immediately after BuildBlock. -func (b *blockBuilder) handleGenerateBlock(currentHeight uint64) { +func (b *blockBuilder) handleGenerateBlock(currentParentHash common.Hash) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() - b.isRetry = b.lastBuiltHeight == currentHeight - b.lastBuiltHeight = currentHeight + // if this is a retry, set the last build time to the current time + if b.lastBuiltParentHash == currentParentHash { + b.lastBuildTime = time.Now() + } else { // else reset the timer if this is the first time we build this block + b.lastBuildTime = time.Time{} + } + b.lastBuiltParentHash = currentParentHash } // needToBuild returns true if there are outstanding transactions to be issued @@ -118,34 +124,41 @@ func (b *blockBuilder) awaitSubmittedTxs() { // waitForEvent waits until a block needs to be built. // It returns only after at least [minBlockBuildingRetryDelay] passed from the last time a block was built. func (b *blockBuilder) waitForEvent(ctx context.Context) (commonEng.Message, error) { - isRetry, err := b.waitForNeedToBuild(ctx) + lastBuildTime, err := b.waitForNeedToBuild(ctx) if err != nil { return 0, err } - if !isRetry { - b.ctx.Log.Debug("This is the first time we attempt to build a block at this height, no need to wait") + timeSinceLastBuildTime := time.Since(lastBuildTime) + // if this is the first time we try to build a block + // or if the time since the last build is greater than the minimum retry delay + // then we can build a block immediately. + if lastBuildTime.IsZero() || timeSinceLastBuildTime >= MinBlockBuildingRetryDelay { + b.ctx.Log.Debug("Last time we built a block was long enough ago, no need to wait", + zap.Duration("timeSinceLastBuildTime", timeSinceLastBuildTime), + ) return commonEng.PendingTxs, nil } + timeUntilNextBuild := MinBlockBuildingRetryDelay - time.Since(lastBuildTime) b.ctx.Log.Debug("Last time we built a block was too recent, waiting", - zap.Duration("timeUntilNextBuild", MinBlockBuildingRetryDelay), + zap.Duration("timeUntilNextBuild", timeUntilNextBuild), ) select { case <-ctx.Done(): return 0, ctx.Err() - case <-time.After(MinBlockBuildingRetryDelay): + case <-time.After(timeUntilNextBuild): return commonEng.PendingTxs, nil } } // waitForNeedToBuild waits until needToBuild returns true. // It returns the last time a block was built. -func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (bool, error) { +func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (time.Time, error) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() for !b.needToBuild() { if err := b.pendingSignal.Wait(ctx); err != nil { - return false, err + return time.Time{}, err } } - return b.isRetry, nil + return b.lastBuildTime, nil } diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index eea558cf57..446c0bde1b 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -910,7 +910,7 @@ func (vm *VM) buildBlockWithContext(_ context.Context, proposerVMBlockCtx *block } block, err := vm.miner.GenerateBlock(predicateCtx) - vm.builder.handleGenerateBlock(vm.blockChain.CurrentHeader().Number.Uint64()) + vm.builder.handleGenerateBlock(vm.blockChain.CurrentHeader().ParentHash) if err != nil { return nil, fmt.Errorf("%w: %w", vmerrors.ErrGenerateBlockFailed, err) } From da7a8cbcca508f9a8df83415646e9bb140c2a12e Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Sep 2025 17:00:37 +0300 Subject: [PATCH 03/19] rename var --- plugin/evm/block_builder.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index a4020c09ed..52cbf54e2d 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -38,10 +38,10 @@ type blockBuilder struct { pendingSignal *lock.Cond buildBlockLock sync.Mutex - // lastBuiltParentHash is the parent hash of the last block was built. + // lastBuildParentHash is the parent hash of the last block was built. // This and lastBuildTime are used to ensure that we don't build blocks too frequently, // but at least after a minimum delay of minBlockBuildingRetryDelay. - lastBuiltParentHash common.Hash + lastBuildParentHash common.Hash lastBuildTime time.Time } @@ -64,12 +64,12 @@ func (b *blockBuilder) handleGenerateBlock(currentParentHash common.Hash) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() // if this is a retry, set the last build time to the current time - if b.lastBuiltParentHash == currentParentHash { + if b.lastBuildParentHash == currentParentHash { b.lastBuildTime = time.Now() } else { // else reset the timer if this is the first time we build this block b.lastBuildTime = time.Time{} } - b.lastBuiltParentHash = currentParentHash + b.lastBuildParentHash = currentParentHash } // needToBuild returns true if there are outstanding transactions to be issued From 5402155e40fb094fba70d000230673a2d7d0b6f5 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Sep 2025 19:15:51 +0300 Subject: [PATCH 04/19] pass current header to waitForEvent --- plugin/evm/block_builder.go | 31 +++++++++++++++---------------- plugin/evm/vm.go | 2 +- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index 52cbf54e2d..eb63b0aa14 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -11,6 +11,7 @@ import ( "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/avalanchego/utils/lock" "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/core/types" "github.com/ava-labs/libevm/log" "github.com/holiman/uint256" "go.uber.org/zap" @@ -63,12 +64,7 @@ func (vm *VM) NewBlockBuilder(extraMempool extension.BuilderMempool) *blockBuild func (b *blockBuilder) handleGenerateBlock(currentParentHash common.Hash) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() - // if this is a retry, set the last build time to the current time - if b.lastBuildParentHash == currentParentHash { - b.lastBuildTime = time.Now() - } else { // else reset the timer if this is the first time we build this block - b.lastBuildTime = time.Time{} - } + b.lastBuildTime = time.Now() b.lastBuildParentHash = currentParentHash } @@ -123,22 +119,25 @@ func (b *blockBuilder) awaitSubmittedTxs() { // waitForEvent waits until a block needs to be built. // It returns only after at least [minBlockBuildingRetryDelay] passed from the last time a block was built. -func (b *blockBuilder) waitForEvent(ctx context.Context) (commonEng.Message, error) { - lastBuildTime, err := b.waitForNeedToBuild(ctx) +func (b *blockBuilder) waitForEvent(ctx context.Context, currentHeader *types.Header) (commonEng.Message, error) { + lastBuildTime, lastBuildParentHash, err := b.waitForNeedToBuild(ctx) if err != nil { return 0, err } timeSinceLastBuildTime := time.Since(lastBuildTime) - // if this is the first time we try to build a block - // or if the time since the last build is greater than the minimum retry delay + isRetry := lastBuildParentHash == currentHeader.ParentHash + // 1. if this is the first time we try to build a block + // 2. if this is not a retry + // 3. if the time since the last build is greater than the minimum retry delay // then we can build a block immediately. - if lastBuildTime.IsZero() || timeSinceLastBuildTime >= MinBlockBuildingRetryDelay { - b.ctx.Log.Debug("Last time we built a block was long enough ago, no need to wait", + if !isRetry || lastBuildTime.IsZero() || timeSinceLastBuildTime >= MinBlockBuildingRetryDelay { + b.ctx.Log.Debug("Last time we built a block was long enough ago or this is not a retry, no need to wait", zap.Duration("timeSinceLastBuildTime", timeSinceLastBuildTime), + zap.Bool("isRetry", isRetry), ) return commonEng.PendingTxs, nil } - timeUntilNextBuild := MinBlockBuildingRetryDelay - time.Since(lastBuildTime) + timeUntilNextBuild := MinBlockBuildingRetryDelay - timeSinceLastBuildTime b.ctx.Log.Debug("Last time we built a block was too recent, waiting", zap.Duration("timeUntilNextBuild", timeUntilNextBuild), ) @@ -152,13 +151,13 @@ func (b *blockBuilder) waitForEvent(ctx context.Context) (commonEng.Message, err // waitForNeedToBuild waits until needToBuild returns true. // It returns the last time a block was built. -func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (time.Time, error) { +func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (time.Time, common.Hash, error) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() for !b.needToBuild() { if err := b.pendingSignal.Wait(ctx); err != nil { - return time.Time{}, err + return time.Time{}, common.Hash{}, err } } - return b.lastBuildTime, nil + return b.lastBuildTime, b.lastBuildParentHash, nil } diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 446c0bde1b..34a6a4f3e0 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -868,7 +868,7 @@ func (vm *VM) WaitForEvent(ctx context.Context) (commonEng.Message, error) { } } - return builder.waitForEvent(ctx) + return builder.waitForEvent(ctx, vm.blockChain.CurrentHeader()) } // Shutdown implements the snowman.ChainVM interface From a35d6fb20c4bb5ea3ee76fa0debed5620544d081 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Sep 2025 21:11:23 +0300 Subject: [PATCH 05/19] add delay tests --- plugin/evm/vm_test.go | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index ccaa275f86..c2d767c0f0 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -1650,11 +1650,10 @@ func TestWaitForEvent(t *testing.T) { }, }, { - name: "WaitForEvent waits some time after a block is built", + name: "WaitForEvent does not wait for new block to be built", testCase: func(t *testing.T, vm *VM) { signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) - lastBuildBlockTime := time.Now() - blk, err := vmtest.IssueTxsAndBuild([]*types.Transaction{signedTx}, vm) + blk, err := vmtest.IssueTxsAndSetPreference([]*types.Transaction{signedTx}, vm) require.NoError(t, err) require.NoError(t, blk.Accept(context.Background())) signedTx = newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 1, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) @@ -1665,7 +1664,33 @@ func TestWaitForEvent(t *testing.T) { var wg sync.WaitGroup wg.Add(1) + // this should not delay anything + lastBuildBlockTime := time.Now() + go func() { + defer wg.Done() + msg, err := vm.WaitForEvent(context.Background()) + assert.NoError(t, err) + assert.Equal(t, commonEng.PendingTxs, msg) + assert.Less(t, time.Since(lastBuildBlockTime), MinBlockBuildingRetryDelay) + }() + wg.Wait() + }, + }, + { + name: "WaitForEvent waits for a delay with a retry", + testCase: func(t *testing.T, vm *VM) { + lastBuildBlockTime := time.Now() + _, err := vm.BuildBlock(context.Background()) + require.NoError(t, err) + // we haven't accepted the previous built block, so this should be a retry + signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) + for _, err := range vm.txPool.AddRemotesSync([]*types.Transaction{signedTx}) { + require.NoError(t, err) + } + + var wg sync.WaitGroup + wg.Add(1) go func() { defer wg.Done() msg, err := vm.WaitForEvent(context.Background()) @@ -1673,7 +1698,6 @@ func TestWaitForEvent(t *testing.T) { assert.Equal(t, commonEng.PendingTxs, msg) assert.GreaterOrEqual(t, time.Since(lastBuildBlockTime), MinBlockBuildingRetryDelay) }() - wg.Wait() }, }, From 4f5b78ba113bf5fefebe0e164bdef349bc089461 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Sep 2025 21:28:13 +0300 Subject: [PATCH 06/19] add tests --- plugin/evm/atomic/vm/vm_test.go | 138 +------------------------------- plugin/evm/block_builder.go | 2 +- plugin/evm/vm_test.go | 7 +- 3 files changed, 6 insertions(+), 141 deletions(-) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index 334792d343..27beead9c4 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -1514,27 +1514,7 @@ func TestWaitForEvent(t *testing.T) { testCase func(*testing.T, *VM, common.Address, *ecdsa.PrivateKey) }{ { - name: "WaitForEvent with context cancelled returns 0", - testCase: func(t *testing.T, vm *VM, _ common.Address, _ *ecdsa.PrivateKey) { - ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) - defer cancel() - - var wg sync.WaitGroup - wg.Add(1) - - // We run WaitForEvent in a goroutine to ensure it can be safely called concurrently. - go func() { - defer wg.Done() - msg, err := vm.WaitForEvent(ctx) - assert.ErrorIs(t, err, context.DeadlineExceeded) - assert.Zero(t, msg) - }() - - wg.Wait() - }, - }, - { - name: "WaitForEvent returns when a transaction is added to the mempool", + name: "WaitForEvent returns when a atomic transaction is added to the mempool", testCase: func(t *testing.T, vm *VM, address common.Address, _ *ecdsa.PrivateKey) { importTx, err := vm.newImportTx(vm.Ctx.XChainID, address, vmtest.InitialBaseFee, []*secp256k1.PrivateKey{vmtest.TestKeys[0]}) require.NoError(t, err) @@ -1551,122 +1531,6 @@ func TestWaitForEvent(t *testing.T) { require.NoError(t, vm.AtomicMempool.AddLocalTx(importTx)) - wg.Wait() - }, - }, - { - name: "WaitForEvent doesn't return once a block is built and accepted", - testCase: func(t *testing.T, vm *VM, address common.Address, key *ecdsa.PrivateKey) { - importTx, err := vm.newImportTx(vm.Ctx.XChainID, address, vmtest.InitialBaseFee, []*secp256k1.PrivateKey{vmtest.TestKeys[0]}) - require.NoError(t, err) - - require.NoError(t, vm.AtomicMempool.AddLocalTx(importTx)) - - blk, err := vm.BuildBlock(context.Background()) - require.NoError(t, err) - - require.NoError(t, blk.Verify(context.Background())) - - require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) - - require.NoError(t, blk.Accept(context.Background())) - - ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) - defer cancel() - - var wg sync.WaitGroup - wg.Add(1) - - // We run WaitForEvent in a goroutine to ensure it can be safely called concurrently. - go func() { - defer wg.Done() - msg, err := vm.WaitForEvent(ctx) - assert.ErrorIs(t, err, context.DeadlineExceeded) - assert.Zero(t, msg) - }() - - wg.Wait() - - t.Log("WaitForEvent returns when regular transactions are added to the mempool") - - txs := make([]*types.Transaction, 10) - for i := 0; i < 10; i++ { - tx := types.NewTransaction(uint64(i), address, big.NewInt(10), 21000, big.NewInt(3*ap0.MinGasPrice), nil) - signedTx, err := types.SignTx(tx, types.NewEIP155Signer(vm.Ethereum().BlockChain().Config().ChainID), key) - require.NoError(t, err) - - txs[i] = signedTx - } - errs := vm.Ethereum().TxPool().Add(txs, false, false) - for _, err := range errs { - require.NoError(t, err) - } - - wg.Add(1) - - go func() { - defer wg.Done() - msg, err := vm.WaitForEvent(context.Background()) - assert.NoError(t, err) - assert.Equal(t, commonEng.PendingTxs, msg) - }() - - wg.Wait() - - // Build a block again to wipe out the subscription - blk, err = vm.BuildBlock(context.Background()) - require.NoError(t, err) - - require.NoError(t, blk.Verify(context.Background())) - - require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) - - require.NoError(t, blk.Accept(context.Background())) - }, - }, - { - name: "WaitForEvent waits some time after a block is built", - testCase: func(t *testing.T, vm *VM, address common.Address, key *ecdsa.PrivateKey) { - importTx, err := vm.newImportTx(vm.Ctx.XChainID, address, vmtest.InitialBaseFee, []*secp256k1.PrivateKey{vmtest.TestKeys[0]}) - require.NoError(t, err) - - require.NoError(t, vm.AtomicMempool.AddLocalTx(importTx)) - - lastBuildBlockTime := time.Now() - - blk, err := vm.BuildBlock(context.Background()) - require.NoError(t, err) - - require.NoError(t, blk.Verify(context.Background())) - - require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) - - require.NoError(t, blk.Accept(context.Background())) - - txs := make([]*types.Transaction, 10) - for i := 0; i < 10; i++ { - tx := types.NewTransaction(uint64(i), address, big.NewInt(10), 21000, big.NewInt(3*ap0.MinGasPrice), nil) - signedTx, err := types.SignTx(tx, types.NewEIP155Signer(vm.Ethereum().BlockChain().Config().ChainID), key) - require.NoError(t, err) - - txs[i] = signedTx - } - errs := vm.Ethereum().TxPool().Add(txs, false, false) - for _, err := range errs { - require.NoError(t, err) - } - - var wg sync.WaitGroup - wg.Add(1) - - go func() { - defer wg.Done() - msg, err := vm.WaitForEvent(context.Background()) - assert.NoError(t, err) - assert.Equal(t, commonEng.PendingTxs, msg) - assert.GreaterOrEqual(t, time.Since(lastBuildBlockTime), evm.MinBlockBuildingRetryDelay) - }() - wg.Wait() }, }, diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index eb63b0aa14..6ec7c5cd58 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -39,7 +39,7 @@ type blockBuilder struct { pendingSignal *lock.Cond buildBlockLock sync.Mutex - // lastBuildParentHash is the parent hash of the last block was built. + // lastBuildParentHash is the parent hash of the last block that was built. // This and lastBuildTime are used to ensure that we don't build blocks too frequently, // but at least after a minimum delay of minBlockBuildingRetryDelay. lastBuildParentHash common.Hash diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index c2d767c0f0..efe4c78176 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -1664,14 +1664,15 @@ func TestWaitForEvent(t *testing.T) { var wg sync.WaitGroup wg.Add(1) - // this should not delay anything - lastBuildBlockTime := time.Now() + go func() { defer wg.Done() + timeStart := time.Now() msg, err := vm.WaitForEvent(context.Background()) + timeSinceStart := time.Since(timeStart) assert.NoError(t, err) assert.Equal(t, commonEng.PendingTxs, msg) - assert.Less(t, time.Since(lastBuildBlockTime), MinBlockBuildingRetryDelay) + assert.Less(t, timeSinceStart, 50*time.Millisecond) }() wg.Wait() From 3c2d944834cf20e5f2917c0c6701e55ba78f4cdd Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Sep 2025 23:36:56 +0300 Subject: [PATCH 07/19] ensure mempool is empty --- plugin/evm/vm_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index efe4c78176..c1a18bf0ca 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -40,6 +40,7 @@ import ( "github.com/ava-labs/coreth/consensus/dummy" "github.com/ava-labs/coreth/constants" "github.com/ava-labs/coreth/core" + "github.com/ava-labs/coreth/core/txpool" "github.com/ava-labs/coreth/eth" "github.com/ava-labs/coreth/miner" "github.com/ava-labs/coreth/node" @@ -1584,7 +1585,7 @@ func TestWaitForEvent(t *testing.T) { }, }, { - name: "WaitForEvent doesn't return once a block is built and accepted", + name: "WaitForEvent doesn't return if mempool is empty", testCase: func(t *testing.T, vm *VM) { signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) @@ -1601,6 +1602,8 @@ func TestWaitForEvent(t *testing.T) { require.NoError(t, blk.Accept(context.Background())) + require.Zero(t, vm.txPool.PendingSize(txpool.PendingFilter{})) + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) defer cancel() From eef3650d1d329ffa33148c838fb3878728b83538 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Tue, 30 Sep 2025 23:40:21 +0300 Subject: [PATCH 08/19] comment --- plugin/evm/block_builder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index 6ec7c5cd58..45e99198cf 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -126,8 +126,8 @@ func (b *blockBuilder) waitForEvent(ctx context.Context, currentHeader *types.He } timeSinceLastBuildTime := time.Since(lastBuildTime) isRetry := lastBuildParentHash == currentHeader.ParentHash - // 1. if this is the first time we try to build a block - // 2. if this is not a retry + // 1. if this is not a retry + // 2. if this is the first time we try to build a block // 3. if the time since the last build is greater than the minimum retry delay // then we can build a block immediately. if !isRetry || lastBuildTime.IsZero() || timeSinceLastBuildTime >= MinBlockBuildingRetryDelay { From 49d1a78109f627f3e8ab53994f5d2a087e797209 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Wed, 1 Oct 2025 00:04:15 +0300 Subject: [PATCH 09/19] remove reduntant and flaky test --- plugin/evm/vm_test.go | 49 ------------------------------------------- 1 file changed, 49 deletions(-) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index c1a18bf0ca..383bdb5347 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -40,7 +40,6 @@ import ( "github.com/ava-labs/coreth/consensus/dummy" "github.com/ava-labs/coreth/constants" "github.com/ava-labs/coreth/core" - "github.com/ava-labs/coreth/core/txpool" "github.com/ava-labs/coreth/eth" "github.com/ava-labs/coreth/miner" "github.com/ava-labs/coreth/node" @@ -1587,23 +1586,6 @@ func TestWaitForEvent(t *testing.T) { { name: "WaitForEvent doesn't return if mempool is empty", testCase: func(t *testing.T, vm *VM) { - signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) - - for _, err := range vm.txPool.AddRemotesSync([]*types.Transaction{signedTx}) { - require.NoError(t, err) - } - - blk, err := vm.BuildBlock(context.Background()) - require.NoError(t, err) - - require.NoError(t, blk.Verify(context.Background())) - - require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) - - require.NoError(t, blk.Accept(context.Background())) - - require.Zero(t, vm.txPool.PendingSize(txpool.PendingFilter{})) - ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) defer cancel() @@ -1619,37 +1601,6 @@ func TestWaitForEvent(t *testing.T) { }() wg.Wait() - - t.Log("WaitForEvent returns when regular transactions are added to the mempool") - - time.Sleep(time.Second * 2) // sleep some time to let the gas capacity to refill - - signedTx = newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 1, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) - - for _, err := range vm.txPool.AddRemotesSync([]*types.Transaction{signedTx}) { - require.NoError(t, err) - } - - wg.Add(1) - - go func() { - defer wg.Done() - msg, err := vm.WaitForEvent(context.Background()) - assert.NoError(t, err) - assert.Equal(t, commonEng.PendingTxs, msg) - }() - - wg.Wait() - - // Build a block again to wipe out the subscription - blk, err = vm.BuildBlock(context.Background()) - require.NoError(t, err) - - require.NoError(t, blk.Verify(context.Background())) - - require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) - - require.NoError(t, blk.Accept(context.Background())) }, }, { From e998652a7bcb75e50706d084d06e3c25110a5acd Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 2 Oct 2025 01:22:06 +0300 Subject: [PATCH 10/19] use timeout for no-delay return test --- plugin/evm/vm_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 383bdb5347..a41014cbf9 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -1616,19 +1616,17 @@ func TestWaitForEvent(t *testing.T) { require.NoError(t, err) } + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) + defer cancel() + var wg sync.WaitGroup wg.Add(1) - go func() { defer wg.Done() - timeStart := time.Now() - msg, err := vm.WaitForEvent(context.Background()) - timeSinceStart := time.Since(timeStart) + msg, err := vm.WaitForEvent(ctx) assert.NoError(t, err) assert.Equal(t, commonEng.PendingTxs, msg) - assert.Less(t, timeSinceStart, 50*time.Millisecond) }() - wg.Wait() }, }, From 10e3bc00a2f7f9e20cce4a82853d8cd7f9f5ada6 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 2 Oct 2025 15:41:30 +0300 Subject: [PATCH 11/19] add acp-226 constraints to builder --- miner/worker.go | 5 +- plugin/evm/block_builder.go | 111 +++++++++++++--- plugin/evm/block_builder_test.go | 183 +++++++++++++++++++++++++++ plugin/evm/customheader/time.go | 30 +++-- plugin/evm/customheader/time_test.go | 65 +++++++--- plugin/evm/customtypes/block_ext.go | 8 ++ plugin/evm/vm_test.go | 22 ++-- 7 files changed, 366 insertions(+), 58 deletions(-) create mode 100644 plugin/evm/block_builder_test.go diff --git a/miner/worker.go b/miner/worker.go index 783c1cedd6..bcea2c9e5b 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(parent, chainExtra, 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/block_builder.go b/plugin/evm/block_builder.go index 45e99198cf..a4410663dc 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -10,6 +10,7 @@ import ( "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/avalanchego/utils/lock" + "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/core/types" "github.com/ava-labs/libevm/log" @@ -18,17 +19,29 @@ import ( "github.com/ava-labs/coreth/core" "github.com/ava-labs/coreth/core/txpool" + "github.com/ava-labs/coreth/params/extras" + "github.com/ava-labs/coreth/plugin/evm/customheader" + "github.com/ava-labs/coreth/plugin/evm/customtypes" "github.com/ava-labs/coreth/plugin/evm/extension" commonEng "github.com/ava-labs/avalanchego/snow/engine/common" ) -// Minimum amount of time to wait after building a block before attempting to build a block -// a second time without changing the contents of the mempool. -const MinBlockBuildingRetryDelay = 500 * time.Millisecond +const ( + // Minimum amount of time to wait after building a block before attempting to build a block + // a second time without changing the contents of the mempool. + PreGraniteMinBlockBuildingRetryDelay = 500 * time.Millisecond + // Minimum amount of time to wait after attempting/build a block before attempting to build another block + // This is only applied for retrying to build a block after a minimum delay has passed. + // The initial minimum delay is applied according to parent minDelayExcess (if available) + // TODO (ceyonur): Decide whether this a correct value. + PostGraniteMinBlockBuildingRetryDelay = 100 * time.Millisecond +) type blockBuilder struct { - ctx *snow.Context + clock *mockable.Clock + ctx *snow.Context + chainConfig *extras.ChainConfig txPool *txpool.TxPool extraMempool extension.BuilderMempool @@ -51,10 +64,12 @@ type blockBuilder struct { func (vm *VM) NewBlockBuilder(extraMempool extension.BuilderMempool) *blockBuilder { b := &blockBuilder{ ctx: vm.ctx, + chainConfig: vm.chainConfigExtra(), txPool: vm.txPool, extraMempool: extraMempool, shutdownChan: vm.shutdownChan, shutdownWg: &vm.shutdownWg, + clock: vm.clock, } b.pendingSignal = lock.NewCond(&b.buildBlockLock) return b @@ -64,7 +79,7 @@ func (vm *VM) NewBlockBuilder(extraMempool extension.BuilderMempool) *blockBuild func (b *blockBuilder) handleGenerateBlock(currentParentHash common.Hash) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() - b.lastBuildTime = time.Now() + b.lastBuildTime = b.clock.Time() b.lastBuildParentHash = currentParentHash } @@ -124,20 +139,19 @@ func (b *blockBuilder) waitForEvent(ctx context.Context, currentHeader *types.He if err != nil { return 0, err } - timeSinceLastBuildTime := time.Since(lastBuildTime) - isRetry := lastBuildParentHash == currentHeader.ParentHash - // 1. if this is not a retry - // 2. if this is the first time we try to build a block - // 3. if the time since the last build is greater than the minimum retry delay - // then we can build a block immediately. - if !isRetry || lastBuildTime.IsZero() || timeSinceLastBuildTime >= MinBlockBuildingRetryDelay { - b.ctx.Log.Debug("Last time we built a block was long enough ago or this is not a retry, no need to wait", - zap.Duration("timeSinceLastBuildTime", timeSinceLastBuildTime), - zap.Bool("isRetry", isRetry), - ) + timeUntilNextBuild, shouldBuildImmediately, err := b.calculateBlockBuildingDelay( + lastBuildTime, + lastBuildParentHash, + currentHeader, + ) + if err != nil { + return 0, err + } + if shouldBuildImmediately { + b.ctx.Log.Debug("Last time we built a block was long enough ago or this is not a retry, no need to wait") return commonEng.PendingTxs, nil } - timeUntilNextBuild := MinBlockBuildingRetryDelay - timeSinceLastBuildTime + b.ctx.Log.Debug("Last time we built a block was too recent, waiting", zap.Duration("timeUntilNextBuild", timeUntilNextBuild), ) @@ -161,3 +175,66 @@ func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (time.Time, commo } return b.lastBuildTime, b.lastBuildParentHash, nil } + +// getMinBlockBuildingDelays returns the initial min block building delay and the minimum retry delay. +// It implements the following logic: +// 1. If the current header is in Granite, return the remaining ACP-226 delay after the parent block time and the minimum retry delay. +// 2. If the current header is not in Granite, return 0 and the minimum retry delay. +func (b *blockBuilder) getMinBlockBuildingDelays(currentHeader *types.Header, config *extras.ChainConfig) (time.Duration, time.Duration, error) { + // TODO (ceyonur): this check can be removed after Granite is activated. + currentTimestamp := b.clock.Unix() + if !config.IsGranite(currentTimestamp) { + return 0, PreGraniteMinBlockBuildingRetryDelay, nil // Pre-Granite: no initial delay + } + + acp226DelayExcess, err := customheader.MinDelayExcess(config, currentHeader, currentTimestamp, nil) + if err != nil { + return 0, 0, err + } + acp226Delay := time.Duration(acp226DelayExcess.Delay()) * time.Millisecond + + // Calculate initial delay: time since parent minus ACP-226 delay (clamped to 0) + parentBlockTime := customtypes.BlockTime(currentHeader) + timeSinceParentBlock := b.clock.Time().Sub(parentBlockTime) + // TODO question (ceyonur): should we just use acp226Delay if timeSinceParentBlock is negative? + initialMinBlockBuildingDelay := acp226Delay - timeSinceParentBlock + if initialMinBlockBuildingDelay < 0 { + initialMinBlockBuildingDelay = 0 + } + + return initialMinBlockBuildingDelay, PostGraniteMinBlockBuildingRetryDelay, nil +} + +// calculateBlockBuildingDelay calculates the delay needed before building the next block. +// It returns the time to wait, a boolean indicating whether to build immediately, and any error. +// It implements the following logic: +// 1. If there is no initial min block building delay +// 2. if this is not a retry +// 3. if the time since the last build is greater than the minimum retry delay +// then we can build a block immediately. +func (b *blockBuilder) calculateBlockBuildingDelay( + lastBuildTime time.Time, + lastBuildParentHash common.Hash, + currentHeader *types.Header, +) (time.Duration, bool, error) { + initialMinBlockBuildingDelay, minBlockBuildingRetryDelay, err := b.getMinBlockBuildingDelays(currentHeader, b.chainConfig) + if err != nil { + return 0, false, err + } + + isRetry := lastBuildParentHash == currentHeader.ParentHash && !lastBuildTime.IsZero() // if last build time is zero, this is not a retry + + timeSinceLastBuildTime := b.clock.Time().Sub(lastBuildTime) + var remainingMinDelay time.Duration + if minBlockBuildingRetryDelay > timeSinceLastBuildTime { + remainingMinDelay = minBlockBuildingRetryDelay - timeSinceLastBuildTime + } + + if initialMinBlockBuildingDelay > 0 { + remainingMinDelay = max(initialMinBlockBuildingDelay, remainingMinDelay) + } else if !isRetry || remainingMinDelay == 0 { + return 0, true, nil // Build immediately + } + + return remainingMinDelay, false, nil // Need to wait +} diff --git a/plugin/evm/block_builder_test.go b/plugin/evm/block_builder_test.go new file mode 100644 index 0000000000..c17ff19e49 --- /dev/null +++ b/plugin/evm/block_builder_test.go @@ -0,0 +1,183 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package evm + +import ( + "testing" + "time" + + "github.com/ava-labs/avalanchego/utils/timer/mockable" + "github.com/ava-labs/avalanchego/vms/evm/acp226" + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/core/types" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/coreth/params/extras" + "github.com/ava-labs/coreth/plugin/evm/customtypes" +) + +func TestCalculateBlockBuildingDelay(t *testing.T) { + now := time.UnixMilli(10000) + nowSecUint64 := uint64(now.Unix()) + nowMilliUint64 := uint64(now.UnixMilli()) + clock := &mockable.Clock{} + clock.Set(now) + tests := []struct { + name string + config *extras.ChainConfig + currentHeader *types.Header + lastBuildTime time.Time + lastBuildParentHash common.Hash + expectedTimeToWait time.Duration + expectedShouldBuildNow bool + }{ + // Test cases from original TestGetMinBlockBuildingDelays + { + name: "pre_granite_returns_build_immediately_zero_time", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{1}, + Time: nowSecUint64, + }, + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedShouldBuildNow: true, + }, + { + name: "pre_granite_returns_build_immediately_different_parent_hash", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{2}, + Time: nowSecUint64, + }, + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedShouldBuildNow: true, + }, + { + name: "pre_granite_returns_build_delays_with_same_parent_hash", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{1}, + Time: nowSecUint64, + }, + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PreGraniteMinBlockBuildingRetryDelay, + expectedShouldBuildNow: false, + }, + { + name: "pre_granite_returns_build_returns_immediately_if_enough_time_passed", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{1}, + Time: nowSecUint64, + }, + lastBuildTime: now.Add(-PreGraniteMinBlockBuildingRetryDelay), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, // Same as current parent + expectedTimeToWait: 0, + expectedShouldBuildNow: true, + }, + { + name: "pre_granite_returns_build_delays_only_remaining_min_delay", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{1}, + Time: nowSecUint64, + }, + lastBuildTime: now.Add(-PreGraniteMinBlockBuildingRetryDelay / 2), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PreGraniteMinBlockBuildingRetryDelay / 2, + expectedShouldBuildNow: false, + }, + { + name: "granite_block_with_now_time", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 2000 * time.Millisecond, // should wait for initial delay + expectedShouldBuildNow: false, + }, + { + name: "granite_block_with_2_seconds_before_clock_no_retry", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 0, // should not wait for initial delay + expectedShouldBuildNow: true, + }, + { + name: "granite_block_with_2_seconds_before_clock_with_retry", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, acp226.InitialDelayExcess), + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, + expectedShouldBuildNow: false, + }, + { + name: "granite_with_2_seconds_before_clock_only_waits_for_retry_delay", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 + lastBuildTime: now, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, + expectedShouldBuildNow: false, + }, + { + name: "granite_with_2_seconds_before_clock_only_waits_for_remaining_retry_delay", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 + lastBuildTime: now.Add(-PostGraniteMinBlockBuildingRetryDelay / 2), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay / 2, + expectedShouldBuildNow: false, + }, + { + name: "granite_with_2_seconds_after_clock", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64+2000, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 4000 * time.Millisecond, + expectedShouldBuildNow: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := &blockBuilder{ + clock: clock, + chainConfig: tt.config, + } + + timeToWait, shouldBuildNow, err := b.calculateBlockBuildingDelay( + tt.lastBuildTime, + tt.lastBuildParentHash, + tt.currentHeader, + ) + + require.NoError(t, err) + require.Equal(t, tt.expectedTimeToWait, timeToWait) + require.Equal(t, tt.expectedShouldBuildNow, shouldBuildNow) + }) + } +} + +func createGraniteTestHeader(parentHash common.Hash, timeMilliseconds uint64, minDelayExcess acp226.DelayExcess) *types.Header { + header := &types.Header{ + Time: timeMilliseconds / 1000, + } + header.ParentHash = parentHash + + extra := &customtypes.HeaderExtra{ + TimeMilliseconds: &timeMilliseconds, + MinDelayExcess: &minDelayExcess, + } + customtypes.SetHeaderExtra(header, extra) + + return header +} diff --git a/plugin/evm/customheader/time.go b/plugin/evm/customheader/time.go index 75075d1ee4..6e2cc37a3a 100644 --- a/plugin/evm/customheader/time.go +++ b/plugin/evm/customheader/time.go @@ -24,11 +24,13 @@ 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(parent *types.Header, config *extras.ChainConfig, now time.Time) (uint64, uint64, error) { var ( timestamp = uint64(now.Unix()) timestampMS = uint64(now.UnixMilli()) @@ -37,17 +39,23 @@ func GetNextTimestamp(parent *types.Header, now time.Time) (uint64, uint64) { // 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 || (parentExtra.TimeMilliseconds != nil && *parentExtra.TimeMilliseconds >= timestampMS) { + // In pre-Granite, blocks are allowed to have the same timestamp as their parent. + // 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) { + return 0, 0, ErrGraniteClockBehindParent } + // Add the delay to the parent timestamp with seconds precision. + timestamp = parent.Time + // Actually we don't need to set timestampMS, because it will be not be set if this is not + // Granite, but setting here for consistency. + timestampMS = parent.Time * 1000 } - return timestamp, timestampMS + + return timestamp, timestampMS, nil } // VerifyTime verifies that the header's Time and TimeMilliseconds fields are diff --git a/plugin/evm/customheader/time_test.go b/plugin/evm/customheader/time_test.go index 375110e082..e8160e98f3 100644 --- a/plugin/evm/customheader/time_test.go +++ b/plugin/evm/customheader/time_test.go @@ -15,17 +15,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) @@ -162,9 +151,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 +172,81 @@ 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: nowMillis + 10, // parent's TimeMilliseconds + 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, }, } 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.parent, test.extraConfig, 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, + }, + ) +} diff --git a/plugin/evm/customtypes/block_ext.go b/plugin/evm/customtypes/block_ext.go index 4e66b132ce..669d00d4c0 100644 --- a/plugin/evm/customtypes/block_ext.go +++ b/plugin/evm/customtypes/block_ext.go @@ -6,6 +6,7 @@ package customtypes import ( "math/big" "slices" + "time" "github.com/ava-labs/avalanchego/vms/evm/acp226" "github.com/ava-labs/libevm/common" @@ -155,6 +156,13 @@ func CalcExtDataHash(extdata []byte) common.Hash { return ethtypes.RLPHash(extdata) } +func BlockTime(eth *ethtypes.Header) time.Time { + if t := GetHeaderExtra(eth).TimeMilliseconds; t != nil { + return time.UnixMilli(int64(*t)) + } + return time.Unix(int64(eth.Time), 0) +} + func NewBlockWithExtData( header *ethtypes.Header, txs []*ethtypes.Transaction, uncles []*ethtypes.Header, receipts []*ethtypes.Receipt, hasher ethtypes.TrieHasher, extdata []byte, recalc bool, diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index c76cef2a20..69ea28b514 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -1540,8 +1540,10 @@ func TestBuildBlockLargeTxStarvation(t *testing.T) { } func TestWaitForEvent(t *testing.T) { + fortunaFork := upgradetest.Fortuna for _, testCase := range []struct { name string + Fork *upgradetest.Fork testCase func(*testing.T, *VM) }{ { @@ -1605,8 +1607,10 @@ func TestWaitForEvent(t *testing.T) { wg.Wait() }, }, + // TODO (ceyonur): remove this test after Granite is activated. { - name: "WaitForEvent does not wait for new block to be built", + name: "WaitForEvent does not wait for new block to be built in fortuna", + Fork: &fortunaFork, testCase: func(t *testing.T, vm *VM) { signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) blk, err := vmtest.IssueTxsAndSetPreference([]*types.Transaction{signedTx}, vm) @@ -1632,8 +1636,10 @@ func TestWaitForEvent(t *testing.T) { wg.Wait() }, }, + // TODO (ceyonur): remove this test after Granite is activated. { - name: "WaitForEvent waits for a delay with a retry", + name: "WaitForEvent waits for a delay with a retry in fortuna", + Fork: &fortunaFork, testCase: func(t *testing.T, vm *VM) { lastBuildBlockTime := time.Now() _, err := vm.BuildBlock(context.Background()) @@ -1651,7 +1657,7 @@ func TestWaitForEvent(t *testing.T) { msg, err := vm.WaitForEvent(context.Background()) assert.NoError(t, err) assert.Equal(t, commonEng.PendingTxs, msg) - assert.GreaterOrEqual(t, time.Since(lastBuildBlockTime), MinBlockBuildingRetryDelay) + assert.GreaterOrEqual(t, time.Since(lastBuildBlockTime), PreGraniteMinBlockBuildingRetryDelay) }() wg.Wait() }, @@ -1659,6 +1665,9 @@ func TestWaitForEvent(t *testing.T) { } { t.Run(testCase.name, func(t *testing.T) { fork := upgradetest.Latest + if testCase.Fork != nil { + fork = *testCase.Fork + } vm := newDefaultTestVM() vmtest.SetupTestVM(t, vm, vmtest.TestVMConfig{ Fork: &fork, @@ -1892,14 +1901,9 @@ func TestDelegatePrecompile_BehaviorAcrossUpgrades(t *testing.T) { data := crypto.Keccak256([]byte("delegateSendHello()"))[:4] nonce := vm.txPool.Nonce(vmtest.TestEthAddrs[0]) signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), nonce, &contractAddr, big.NewInt(0), 100000, tt.txGasPrice, data) - for _, err := range vm.txPool.AddRemotesSync([]*types.Transaction{signedTx}) { - require.NoError(t, err) - } - blk, err := vm.BuildBlock(ctx) + blk, err := vmtest.IssueTxsAndSetPreference([]*types.Transaction{signedTx}, vm) require.NoError(t, err) - require.NoError(t, blk.Verify(ctx)) - require.NoError(t, vm.SetPreference(ctx, blk.ID())) require.NoError(t, blk.Accept(ctx)) ethBlock := blk.(*chain.BlockWrapper).Block.(*wrappedBlock).ethBlock From 6d0598147339a14703b41afd9ed2ac76776c0b61 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 2 Oct 2025 16:17:08 +0300 Subject: [PATCH 12/19] nit comments --- plugin/evm/block_builder_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin/evm/block_builder_test.go b/plugin/evm/block_builder_test.go index c17ff19e49..481094e4ba 100644 --- a/plugin/evm/block_builder_test.go +++ b/plugin/evm/block_builder_test.go @@ -32,7 +32,6 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { expectedTimeToWait time.Duration expectedShouldBuildNow bool }{ - // Test cases from original TestGetMinBlockBuildingDelays { name: "pre_granite_returns_build_immediately_zero_time", config: extras.TestFortunaChainConfig, // Pre-Granite config @@ -122,7 +121,7 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { name: "granite_with_2_seconds_before_clock_only_waits_for_retry_delay", config: extras.TestGraniteChainConfig, currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 - lastBuildTime: now, // Zero time means not a retry + lastBuildTime: now, lastBuildParentHash: common.Hash{1}, expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, expectedShouldBuildNow: false, From 3385ef14df115f9a2dd38683c618d8b1ac8bdc9c Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Thu, 2 Oct 2025 21:25:12 +0300 Subject: [PATCH 13/19] refactor GetNextTimestamp and add test for same sec different ms --- plugin/evm/customheader/time.go | 32 ++++++++++++---------------- plugin/evm/customheader/time_test.go | 9 ++++++++ 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/plugin/evm/customheader/time.go b/plugin/evm/customheader/time.go index 6e2cc37a3a..569db9ab62 100644 --- a/plugin/evm/customheader/time.go +++ b/plugin/evm/customheader/time.go @@ -35,27 +35,23 @@ func GetNextTimestamp(parent *types.Header, config *extras.ChainConfig, now time 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) { - // In pre-Granite, blocks are allowed to have the same timestamp as their parent. - // 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 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 } - // Add the delay to the parent timestamp with seconds precision. - timestamp = parent.Time - // Actually we don't need to set timestampMS, because it will be not be set if this is not - // Granite, but setting here for consistency. - timestampMS = parent.Time * 1000 + return timestamp, timestampMS, nil } - - return timestamp, timestampMS, nil + // 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 diff --git a/plugin/evm/customheader/time_test.go b/plugin/evm/customheader/time_test.go index e8160e98f3..c0e40f9aee 100644 --- a/plugin/evm/customheader/time_test.go +++ b/plugin/evm/customheader/time_test.go @@ -225,6 +225,15 @@ func TestGetNextTimestamp(t *testing.T) { 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, + }, } for _, test := range tests { From 76111f5f36317dfdc5dad8a64c60121b3ce2d03a Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 3 Oct 2025 14:38:56 +0300 Subject: [PATCH 14/19] nits --- miner/worker.go | 2 +- plugin/evm/customheader/time.go | 3 +-- plugin/evm/customheader/time_test.go | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/miner/worker.go b/miner/worker.go index bcea2c9e5b..946f9f5c63 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -149,7 +149,7 @@ func (w *worker) commitNewWork(predicateContext *precompileconfig.PredicateConte chainExtra = params.GetExtra(w.chainConfig) ) - timestamp, timestampMS, err := customheader.GetNextTimestamp(parent, chainExtra, tstart) + timestamp, timestampMS, err := customheader.GetNextTimestamp(chainExtra, parent, tstart) if err != nil { return nil, fmt.Errorf("failed to get next timestamp: %w", err) } diff --git a/plugin/evm/customheader/time.go b/plugin/evm/customheader/time.go index 569db9ab62..56e56e6173 100644 --- a/plugin/evm/customheader/time.go +++ b/plugin/evm/customheader/time.go @@ -24,13 +24,12 @@ 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 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, config *extras.ChainConfig, now time.Time) (uint64, uint64, error) { +func GetNextTimestamp(config *extras.ChainConfig, parent *types.Header, now time.Time) (uint64, uint64, error) { var ( timestamp = uint64(now.Unix()) timestampMS = uint64(now.UnixMilli()) diff --git a/plugin/evm/customheader/time_test.go b/plugin/evm/customheader/time_test.go index c0e40f9aee..09d1330408 100644 --- a/plugin/evm/customheader/time_test.go +++ b/plugin/evm/customheader/time_test.go @@ -241,7 +241,7 @@ func TestGetNextTimestamp(t *testing.T) { if test.extraConfig == nil { test.extraConfig = extras.TestChainConfig } - sec, millis, err := GetNextTimestamp(test.parent, test.extraConfig, test.now) + 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) From 049aefa79cb3d1925ea26963e5d253c2f20173ee Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 3 Oct 2025 15:06:26 +0300 Subject: [PATCH 15/19] nits --- plugin/evm/atomic/vm/vm_test.go | 22 ++++++++++++++++++++- plugin/evm/block_builder.go | 2 +- plugin/evm/customheader/min_delay_excess.go | 2 +- plugin/evm/vm_test.go | 4 ++-- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/plugin/evm/atomic/vm/vm_test.go b/plugin/evm/atomic/vm/vm_test.go index e59df730fc..7bb24104e7 100644 --- a/plugin/evm/atomic/vm/vm_test.go +++ b/plugin/evm/atomic/vm/vm_test.go @@ -1521,7 +1521,27 @@ func TestWaitForEvent(t *testing.T) { testCase func(*testing.T, *VM, common.Address, *ecdsa.PrivateKey) }{ { - name: "WaitForEvent returns when a atomic transaction is added to the mempool", + name: "WaitForEvent with context cancelled returns 0", + testCase: func(t *testing.T, vm *VM, _ common.Address, _ *ecdsa.PrivateKey) { + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) + defer cancel() + + var wg sync.WaitGroup + wg.Add(1) + + // We run WaitForEvent in a goroutine to ensure it can be safely called concurrently. + go func() { + defer wg.Done() + msg, err := vm.WaitForEvent(ctx) + assert.ErrorIs(t, err, context.DeadlineExceeded) + assert.Zero(t, msg) + }() + + wg.Wait() + }, + }, + { + name: "WaitForEvent returns when a transaction is added to the mempool", testCase: func(t *testing.T, vm *VM, address common.Address, _ *ecdsa.PrivateKey) { importTx, err := vm.newImportTx(vm.Ctx.XChainID, address, vmtest.InitialBaseFee, []*secp256k1.PrivateKey{vmtest.TestKeys[0]}) require.NoError(t, err) diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index a4410663dc..72d41595ef 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -181,7 +181,7 @@ func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (time.Time, commo // 1. If the current header is in Granite, return the remaining ACP-226 delay after the parent block time and the minimum retry delay. // 2. If the current header is not in Granite, return 0 and the minimum retry delay. func (b *blockBuilder) getMinBlockBuildingDelays(currentHeader *types.Header, config *extras.ChainConfig) (time.Duration, time.Duration, error) { - // TODO (ceyonur): this check can be removed after Granite is activated. + // TODO Cleanup (ceyonur): this check can be removed after Granite is activated. currentTimestamp := b.clock.Unix() if !config.IsGranite(currentTimestamp) { return 0, PreGraniteMinBlockBuildingRetryDelay, nil // Pre-Granite: no initial delay 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/vm_test.go b/plugin/evm/vm_test.go index bf3c34020c..b034be1c39 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -1616,7 +1616,7 @@ func TestWaitForEvent(t *testing.T) { wg.Wait() }, }, - // TODO (ceyonur): remove this test after Granite is activated. + // TODO Cleanup (ceyonur): remove this test after Granite is activated. { name: "WaitForEvent does not wait for new block to be built in fortuna", Fork: &fortunaFork, @@ -1645,7 +1645,7 @@ func TestWaitForEvent(t *testing.T) { wg.Wait() }, }, - // TODO (ceyonur): remove this test after Granite is activated. + // TODO Cleanup (ceyonur): remove this test after Granite is activated. { name: "WaitForEvent waits for a delay with a retry in fortuna", Fork: &fortunaFork, From ce38349af10857747905067df5f08352a05507a4 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 3 Oct 2025 15:20:12 +0300 Subject: [PATCH 16/19] revert worker changes --- miner/worker.go | 5 +- plugin/evm/customheader/min_delay_excess.go | 2 +- plugin/evm/customheader/time.go | 33 +++++---- plugin/evm/customheader/time_test.go | 74 ++++++--------------- 4 files changed, 37 insertions(+), 77 deletions(-) diff --git a/miner/worker.go b/miner/worker.go index 946f9f5c63..783c1cedd6 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -149,10 +149,7 @@ func (w *worker) commitNewWork(predicateContext *precompileconfig.PredicateConte chainExtra = params.GetExtra(w.chainConfig) ) - timestamp, timestampMS, err := customheader.GetNextTimestamp(chainExtra, parent, tstart) - if err != nil { - return nil, fmt.Errorf("failed to get next timestamp: %w", err) - } + timestamp, timestampMS := customheader.GetNextTimestamp(parent, tstart) 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 a4cd4975d0..0498fdc32c 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 Cleanup (ceyonur): this can be removed after Granite is activated. + // TODO (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 56e56e6173..75075d1ee4 100644 --- a/plugin/evm/customheader/time.go +++ b/plugin/evm/customheader/time.go @@ -24,33 +24,30 @@ 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") - 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 header based on the parent's timestamp and the current time. +// 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(config *extras.ChainConfig, parent *types.Header, now time.Time) (uint64, uint64, error) { +func GetNextTimestamp(parent *types.Header, now time.Time) (uint64, uint64) { var ( timestamp = uint64(now.Unix()) timestampMS = uint64(now.UnixMilli()) ) - 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 + // 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 } - return timestamp, timestampMS, nil } - // 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 + return timestamp, timestampMS } // VerifyTime verifies that the header's Time and TimeMilliseconds fields are diff --git a/plugin/evm/customheader/time_test.go b/plugin/evm/customheader/time_test.go index 09d1330408..375110e082 100644 --- a/plugin/evm/customheader/time_test.go +++ b/plugin/evm/customheader/time_test.go @@ -15,6 +15,17 @@ 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) @@ -151,11 +162,9 @@ 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", @@ -172,90 +181,47 @@ func TestGetNextTimestamp(t *testing.T) { expectedMillis: nowMillis, }, { - name: "current_time_equals_parent_time_no_milliseconds_pre_granite", + name: "current_time_equals_parent_time_no_milliseconds", 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_pre_granite", + name: "current_time_equals_parent_time_with_milliseconds", parent: generateHeader(nowSeconds, utils.NewUint64(nowMillis)), - extraConfig: extras.TestFortunaChainConfig, now: now, expectedSec: nowSeconds, - expectedMillis: nowSeconds * 1000, // parent.Time * 1000 + expectedMillis: nowMillis, // parent's TimeMilliseconds }, { - name: "current_time_before_parent_time_pre_granite", + name: "current_time_before_parent_time", 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_pre_granite", + name: "current_time_before_parent_time_with_milliseconds", parent: generateHeader(nowSeconds+10, utils.NewUint64(nowMillis)), - extraConfig: extras.TestFortunaChainConfig, now: now, expectedSec: nowSeconds + 10, - expectedMillis: (nowSeconds + 10) * 1000, // parent.Time * 1000 + expectedMillis: nowMillis, // parent's TimeMilliseconds }, { - name: "current_time_milliseconds_before_parent_time_milliseconds_pre_granite", + name: "current_time_milliseconds_before_parent_time_milliseconds", 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, + expectedMillis: nowMillis + 10, // parent's TimeMilliseconds }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - if test.extraConfig == nil { - test.extraConfig = extras.TestChainConfig - } - sec, millis, err := GetNextTimestamp(test.extraConfig, test.parent, test.now) - require.ErrorIs(t, err, test.expectedErr) + sec, millis := GetNextTimestamp(test.parent, test.now) 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, - }, - ) -} From aa9a2cc9fb5e242f2d499444abd85d05eead2cae Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 3 Oct 2025 16:16:10 +0300 Subject: [PATCH 17/19] apply min blok delay to block builder --- RELEASES.md | 8 +- plugin/evm/block_builder.go | 111 ++++++++++++++--- plugin/evm/block_builder_test.go | 182 ++++++++++++++++++++++++++++ plugin/evm/customtypes/block_ext.go | 8 ++ plugin/evm/vm_test.go | 24 ++-- 5 files changed, 303 insertions(+), 30 deletions(-) create mode 100644 plugin/evm/block_builder_test.go diff --git a/RELEASES.md b/RELEASES.md index 45bc5a4ba1..31ed556bc5 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -4,9 +4,11 @@ - Removed deprecated flags `coreth-admin-api-enabled`, `coreth-admin-api-dir`, `tx-regossip-frequency`, `tx-lookup-limit`. Use `admin-api-enabled`, `admin-api-dir`, `regossip-frequency`, `transaction-history` instead. - Enabled RPC batch limits by default, and configurable with `batch-request-limit` and `batch-max-response-size`. -- Implement ACP-226: Set expected block gas cost to 0 in Granite network upgrade, removing block gas cost requirements for block building. -- Implement ACP-226: Add `timeMilliseconds` (Unix uint64) timestamp to block header for Granite upgrade. -- Implement ACP-226: Add `minDelayExcess` (uint64) to block header for Granite upgrade. +- ACP-226: + - Set expected block gas cost to 0 in Granite network upgrade, removing block gas cost requirements for block building. + - 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 ACP-226 requirements to the block builder. - Update go version to 1.24.7 ## [v0.15.3](https://github.com/ava-labs/coreth/releases/tag/v0.15.3) diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index 45e99198cf..72d41595ef 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -10,6 +10,7 @@ import ( "github.com/ava-labs/avalanchego/snow" "github.com/ava-labs/avalanchego/utils/lock" + "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/core/types" "github.com/ava-labs/libevm/log" @@ -18,17 +19,29 @@ import ( "github.com/ava-labs/coreth/core" "github.com/ava-labs/coreth/core/txpool" + "github.com/ava-labs/coreth/params/extras" + "github.com/ava-labs/coreth/plugin/evm/customheader" + "github.com/ava-labs/coreth/plugin/evm/customtypes" "github.com/ava-labs/coreth/plugin/evm/extension" commonEng "github.com/ava-labs/avalanchego/snow/engine/common" ) -// Minimum amount of time to wait after building a block before attempting to build a block -// a second time without changing the contents of the mempool. -const MinBlockBuildingRetryDelay = 500 * time.Millisecond +const ( + // Minimum amount of time to wait after building a block before attempting to build a block + // a second time without changing the contents of the mempool. + PreGraniteMinBlockBuildingRetryDelay = 500 * time.Millisecond + // Minimum amount of time to wait after attempting/build a block before attempting to build another block + // This is only applied for retrying to build a block after a minimum delay has passed. + // The initial minimum delay is applied according to parent minDelayExcess (if available) + // TODO (ceyonur): Decide whether this a correct value. + PostGraniteMinBlockBuildingRetryDelay = 100 * time.Millisecond +) type blockBuilder struct { - ctx *snow.Context + clock *mockable.Clock + ctx *snow.Context + chainConfig *extras.ChainConfig txPool *txpool.TxPool extraMempool extension.BuilderMempool @@ -51,10 +64,12 @@ type blockBuilder struct { func (vm *VM) NewBlockBuilder(extraMempool extension.BuilderMempool) *blockBuilder { b := &blockBuilder{ ctx: vm.ctx, + chainConfig: vm.chainConfigExtra(), txPool: vm.txPool, extraMempool: extraMempool, shutdownChan: vm.shutdownChan, shutdownWg: &vm.shutdownWg, + clock: vm.clock, } b.pendingSignal = lock.NewCond(&b.buildBlockLock) return b @@ -64,7 +79,7 @@ func (vm *VM) NewBlockBuilder(extraMempool extension.BuilderMempool) *blockBuild func (b *blockBuilder) handleGenerateBlock(currentParentHash common.Hash) { b.buildBlockLock.Lock() defer b.buildBlockLock.Unlock() - b.lastBuildTime = time.Now() + b.lastBuildTime = b.clock.Time() b.lastBuildParentHash = currentParentHash } @@ -124,20 +139,19 @@ func (b *blockBuilder) waitForEvent(ctx context.Context, currentHeader *types.He if err != nil { return 0, err } - timeSinceLastBuildTime := time.Since(lastBuildTime) - isRetry := lastBuildParentHash == currentHeader.ParentHash - // 1. if this is not a retry - // 2. if this is the first time we try to build a block - // 3. if the time since the last build is greater than the minimum retry delay - // then we can build a block immediately. - if !isRetry || lastBuildTime.IsZero() || timeSinceLastBuildTime >= MinBlockBuildingRetryDelay { - b.ctx.Log.Debug("Last time we built a block was long enough ago or this is not a retry, no need to wait", - zap.Duration("timeSinceLastBuildTime", timeSinceLastBuildTime), - zap.Bool("isRetry", isRetry), - ) + timeUntilNextBuild, shouldBuildImmediately, err := b.calculateBlockBuildingDelay( + lastBuildTime, + lastBuildParentHash, + currentHeader, + ) + if err != nil { + return 0, err + } + if shouldBuildImmediately { + b.ctx.Log.Debug("Last time we built a block was long enough ago or this is not a retry, no need to wait") return commonEng.PendingTxs, nil } - timeUntilNextBuild := MinBlockBuildingRetryDelay - timeSinceLastBuildTime + b.ctx.Log.Debug("Last time we built a block was too recent, waiting", zap.Duration("timeUntilNextBuild", timeUntilNextBuild), ) @@ -161,3 +175,66 @@ func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (time.Time, commo } return b.lastBuildTime, b.lastBuildParentHash, nil } + +// getMinBlockBuildingDelays returns the initial min block building delay and the minimum retry delay. +// It implements the following logic: +// 1. If the current header is in Granite, return the remaining ACP-226 delay after the parent block time and the minimum retry delay. +// 2. If the current header is not in Granite, return 0 and the minimum retry delay. +func (b *blockBuilder) getMinBlockBuildingDelays(currentHeader *types.Header, config *extras.ChainConfig) (time.Duration, time.Duration, error) { + // TODO Cleanup (ceyonur): this check can be removed after Granite is activated. + currentTimestamp := b.clock.Unix() + if !config.IsGranite(currentTimestamp) { + return 0, PreGraniteMinBlockBuildingRetryDelay, nil // Pre-Granite: no initial delay + } + + acp226DelayExcess, err := customheader.MinDelayExcess(config, currentHeader, currentTimestamp, nil) + if err != nil { + return 0, 0, err + } + acp226Delay := time.Duration(acp226DelayExcess.Delay()) * time.Millisecond + + // Calculate initial delay: time since parent minus ACP-226 delay (clamped to 0) + parentBlockTime := customtypes.BlockTime(currentHeader) + timeSinceParentBlock := b.clock.Time().Sub(parentBlockTime) + // TODO question (ceyonur): should we just use acp226Delay if timeSinceParentBlock is negative? + initialMinBlockBuildingDelay := acp226Delay - timeSinceParentBlock + if initialMinBlockBuildingDelay < 0 { + initialMinBlockBuildingDelay = 0 + } + + return initialMinBlockBuildingDelay, PostGraniteMinBlockBuildingRetryDelay, nil +} + +// calculateBlockBuildingDelay calculates the delay needed before building the next block. +// It returns the time to wait, a boolean indicating whether to build immediately, and any error. +// It implements the following logic: +// 1. If there is no initial min block building delay +// 2. if this is not a retry +// 3. if the time since the last build is greater than the minimum retry delay +// then we can build a block immediately. +func (b *blockBuilder) calculateBlockBuildingDelay( + lastBuildTime time.Time, + lastBuildParentHash common.Hash, + currentHeader *types.Header, +) (time.Duration, bool, error) { + initialMinBlockBuildingDelay, minBlockBuildingRetryDelay, err := b.getMinBlockBuildingDelays(currentHeader, b.chainConfig) + if err != nil { + return 0, false, err + } + + isRetry := lastBuildParentHash == currentHeader.ParentHash && !lastBuildTime.IsZero() // if last build time is zero, this is not a retry + + timeSinceLastBuildTime := b.clock.Time().Sub(lastBuildTime) + var remainingMinDelay time.Duration + if minBlockBuildingRetryDelay > timeSinceLastBuildTime { + remainingMinDelay = minBlockBuildingRetryDelay - timeSinceLastBuildTime + } + + if initialMinBlockBuildingDelay > 0 { + remainingMinDelay = max(initialMinBlockBuildingDelay, remainingMinDelay) + } else if !isRetry || remainingMinDelay == 0 { + return 0, true, nil // Build immediately + } + + return remainingMinDelay, false, nil // Need to wait +} diff --git a/plugin/evm/block_builder_test.go b/plugin/evm/block_builder_test.go new file mode 100644 index 0000000000..481094e4ba --- /dev/null +++ b/plugin/evm/block_builder_test.go @@ -0,0 +1,182 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package evm + +import ( + "testing" + "time" + + "github.com/ava-labs/avalanchego/utils/timer/mockable" + "github.com/ava-labs/avalanchego/vms/evm/acp226" + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/core/types" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/coreth/params/extras" + "github.com/ava-labs/coreth/plugin/evm/customtypes" +) + +func TestCalculateBlockBuildingDelay(t *testing.T) { + now := time.UnixMilli(10000) + nowSecUint64 := uint64(now.Unix()) + nowMilliUint64 := uint64(now.UnixMilli()) + clock := &mockable.Clock{} + clock.Set(now) + tests := []struct { + name string + config *extras.ChainConfig + currentHeader *types.Header + lastBuildTime time.Time + lastBuildParentHash common.Hash + expectedTimeToWait time.Duration + expectedShouldBuildNow bool + }{ + { + name: "pre_granite_returns_build_immediately_zero_time", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{1}, + Time: nowSecUint64, + }, + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedShouldBuildNow: true, + }, + { + name: "pre_granite_returns_build_immediately_different_parent_hash", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{2}, + Time: nowSecUint64, + }, + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedShouldBuildNow: true, + }, + { + name: "pre_granite_returns_build_delays_with_same_parent_hash", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{1}, + Time: nowSecUint64, + }, + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PreGraniteMinBlockBuildingRetryDelay, + expectedShouldBuildNow: false, + }, + { + name: "pre_granite_returns_build_returns_immediately_if_enough_time_passed", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{1}, + Time: nowSecUint64, + }, + lastBuildTime: now.Add(-PreGraniteMinBlockBuildingRetryDelay), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, // Same as current parent + expectedTimeToWait: 0, + expectedShouldBuildNow: true, + }, + { + name: "pre_granite_returns_build_delays_only_remaining_min_delay", + config: extras.TestFortunaChainConfig, // Pre-Granite config + currentHeader: &types.Header{ + ParentHash: common.Hash{1}, + Time: nowSecUint64, + }, + lastBuildTime: now.Add(-PreGraniteMinBlockBuildingRetryDelay / 2), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PreGraniteMinBlockBuildingRetryDelay / 2, + expectedShouldBuildNow: false, + }, + { + name: "granite_block_with_now_time", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 2000 * time.Millisecond, // should wait for initial delay + expectedShouldBuildNow: false, + }, + { + name: "granite_block_with_2_seconds_before_clock_no_retry", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 0, // should not wait for initial delay + expectedShouldBuildNow: true, + }, + { + name: "granite_block_with_2_seconds_before_clock_with_retry", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, acp226.InitialDelayExcess), + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, + expectedShouldBuildNow: false, + }, + { + name: "granite_with_2_seconds_before_clock_only_waits_for_retry_delay", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, + expectedShouldBuildNow: false, + }, + { + name: "granite_with_2_seconds_before_clock_only_waits_for_remaining_retry_delay", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 + lastBuildTime: now.Add(-PostGraniteMinBlockBuildingRetryDelay / 2), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay / 2, + expectedShouldBuildNow: false, + }, + { + name: "granite_with_2_seconds_after_clock", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64+2000, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 4000 * time.Millisecond, + expectedShouldBuildNow: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := &blockBuilder{ + clock: clock, + chainConfig: tt.config, + } + + timeToWait, shouldBuildNow, err := b.calculateBlockBuildingDelay( + tt.lastBuildTime, + tt.lastBuildParentHash, + tt.currentHeader, + ) + + require.NoError(t, err) + require.Equal(t, tt.expectedTimeToWait, timeToWait) + require.Equal(t, tt.expectedShouldBuildNow, shouldBuildNow) + }) + } +} + +func createGraniteTestHeader(parentHash common.Hash, timeMilliseconds uint64, minDelayExcess acp226.DelayExcess) *types.Header { + header := &types.Header{ + Time: timeMilliseconds / 1000, + } + header.ParentHash = parentHash + + extra := &customtypes.HeaderExtra{ + TimeMilliseconds: &timeMilliseconds, + MinDelayExcess: &minDelayExcess, + } + customtypes.SetHeaderExtra(header, extra) + + return header +} diff --git a/plugin/evm/customtypes/block_ext.go b/plugin/evm/customtypes/block_ext.go index 4e66b132ce..669d00d4c0 100644 --- a/plugin/evm/customtypes/block_ext.go +++ b/plugin/evm/customtypes/block_ext.go @@ -6,6 +6,7 @@ package customtypes import ( "math/big" "slices" + "time" "github.com/ava-labs/avalanchego/vms/evm/acp226" "github.com/ava-labs/libevm/common" @@ -155,6 +156,13 @@ func CalcExtDataHash(extdata []byte) common.Hash { return ethtypes.RLPHash(extdata) } +func BlockTime(eth *ethtypes.Header) time.Time { + if t := GetHeaderExtra(eth).TimeMilliseconds; t != nil { + return time.UnixMilli(int64(*t)) + } + return time.Unix(int64(eth.Time), 0) +} + func NewBlockWithExtData( header *ethtypes.Header, txs []*ethtypes.Transaction, uncles []*ethtypes.Header, receipts []*ethtypes.Receipt, hasher ethtypes.TrieHasher, extdata []byte, recalc bool, diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 3edffbff12..b034be1c39 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -1549,8 +1549,10 @@ func TestBuildBlockLargeTxStarvation(t *testing.T) { } func TestWaitForEvent(t *testing.T) { + fortunaFork := upgradetest.Fortuna for _, testCase := range []struct { name string + Fork *upgradetest.Fork testCase func(*testing.T, *VM) }{ { @@ -1614,8 +1616,10 @@ func TestWaitForEvent(t *testing.T) { wg.Wait() }, }, + // TODO Cleanup (ceyonur): remove this test after Granite is activated. { - name: "WaitForEvent does not wait to build on a new block", + name: "WaitForEvent does not wait for new block to be built in fortuna", + Fork: &fortunaFork, testCase: func(t *testing.T, vm *VM) { signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) blk, err := vmtest.IssueTxsAndSetPreference([]*types.Transaction{signedTx}, vm) @@ -1641,13 +1645,15 @@ func TestWaitForEvent(t *testing.T) { wg.Wait() }, }, + // TODO Cleanup (ceyonur): remove this test after Granite is activated. { - name: "WaitForEvent waits for a delay with a retry", + name: "WaitForEvent waits for a delay with a retry in fortuna", + Fork: &fortunaFork, testCase: func(t *testing.T, vm *VM) { lastBuildBlockTime := time.Now() _, err := vm.BuildBlock(context.Background()) require.NoError(t, err) - // we haven't advanced the tip to include the previous built block, so this is a retry + // we haven't accepted the previous built block, so this should be a retry signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), 0, &vmtest.TestEthAddrs[1], big.NewInt(1), 21000, vmtest.InitialBaseFee, nil) for _, err := range vm.txPool.AddRemotesSync([]*types.Transaction{signedTx}) { require.NoError(t, err) @@ -1660,7 +1666,7 @@ func TestWaitForEvent(t *testing.T) { msg, err := vm.WaitForEvent(context.Background()) assert.NoError(t, err) assert.Equal(t, commonEng.PendingTxs, msg) - assert.GreaterOrEqual(t, time.Since(lastBuildBlockTime), MinBlockBuildingRetryDelay) + assert.GreaterOrEqual(t, time.Since(lastBuildBlockTime), PreGraniteMinBlockBuildingRetryDelay) }() wg.Wait() }, @@ -1668,6 +1674,9 @@ func TestWaitForEvent(t *testing.T) { } { t.Run(testCase.name, func(t *testing.T) { fork := upgradetest.Latest + if testCase.Fork != nil { + fork = *testCase.Fork + } vm := newDefaultTestVM() vmtest.SetupTestVM(t, vm, vmtest.TestVMConfig{ Fork: &fork, @@ -1901,14 +1910,9 @@ func TestDelegatePrecompile_BehaviorAcrossUpgrades(t *testing.T) { data := crypto.Keccak256([]byte("delegateSendHello()"))[:4] nonce := vm.txPool.Nonce(vmtest.TestEthAddrs[0]) signedTx := newSignedLegacyTx(t, vm.chainConfig, vmtest.TestKeys[0].ToECDSA(), nonce, &contractAddr, big.NewInt(0), 100000, tt.txGasPrice, data) - for _, err := range vm.txPool.AddRemotesSync([]*types.Transaction{signedTx}) { - require.NoError(t, err) - } - blk, err := vm.BuildBlock(ctx) + blk, err := vmtest.IssueTxsAndSetPreference([]*types.Transaction{signedTx}, vm) require.NoError(t, err) - require.NoError(t, blk.Verify(ctx)) - require.NoError(t, vm.SetPreference(ctx, blk.ID())) require.NoError(t, blk.Accept(ctx)) ethBlock := blk.(*chain.BlockWrapper).Block.(*wrappedBlock).ethBlock From 1503d87ce72de03854aa621bfb3241bd1204016f Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Sat, 4 Oct 2025 20:48:51 +0300 Subject: [PATCH 18/19] Update RELEASES.md Co-authored-by: Michael Kaplan <55204436+michaelkaplan13@users.noreply.github.com> Signed-off-by: Ceyhun Onur --- RELEASES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASES.md b/RELEASES.md index 31ed556bc5..8fd3b0dea9 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -8,7 +8,7 @@ - Set expected block gas cost to 0 in Granite network upgrade, removing block gas cost requirements for block building. - 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 ACP-226 requirements to the block builder. + - Add minimum block building delays to conform the block builder to ACP-226 requirements. - Update go version to 1.24.7 ## [v0.15.3](https://github.com/ava-labs/coreth/releases/tag/v0.15.3) From d96886bb5850eb8dc38dc4bc8bb816aeed896777 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Sat, 4 Oct 2025 21:06:21 +0300 Subject: [PATCH 19/19] comments --- plugin/evm/block_builder.go | 26 +++---- plugin/evm/block_builder_test.go | 127 ++++++++++++++----------------- 2 files changed, 71 insertions(+), 82 deletions(-) diff --git a/plugin/evm/block_builder.go b/plugin/evm/block_builder.go index 72d41595ef..1513d4907f 100644 --- a/plugin/evm/block_builder.go +++ b/plugin/evm/block_builder.go @@ -139,7 +139,7 @@ func (b *blockBuilder) waitForEvent(ctx context.Context, currentHeader *types.He if err != nil { return 0, err } - timeUntilNextBuild, shouldBuildImmediately, err := b.calculateBlockBuildingDelay( + timeUntilNextBuild, err := b.calculateBlockBuildingDelay( lastBuildTime, lastBuildParentHash, currentHeader, @@ -147,7 +147,7 @@ func (b *blockBuilder) waitForEvent(ctx context.Context, currentHeader *types.He if err != nil { return 0, err } - if shouldBuildImmediately { + if timeUntilNextBuild == 0 { b.ctx.Log.Debug("Last time we built a block was long enough ago or this is not a retry, no need to wait") return commonEng.PendingTxs, nil } @@ -176,11 +176,11 @@ func (b *blockBuilder) waitForNeedToBuild(ctx context.Context) (time.Time, commo return b.lastBuildTime, b.lastBuildParentHash, nil } -// getMinBlockBuildingDelays returns the initial min block building delay and the minimum retry delay. +// currentMinBlockBuildingDelays returns the initial min block building delay and the minimum retry delay. // It implements the following logic: // 1. If the current header is in Granite, return the remaining ACP-226 delay after the parent block time and the minimum retry delay. // 2. If the current header is not in Granite, return 0 and the minimum retry delay. -func (b *blockBuilder) getMinBlockBuildingDelays(currentHeader *types.Header, config *extras.ChainConfig) (time.Duration, time.Duration, error) { +func (b *blockBuilder) currentMinBlockBuildingDelays(currentHeader *types.Header, config *extras.ChainConfig) (time.Duration, time.Duration, error) { // TODO Cleanup (ceyonur): this check can be removed after Granite is activated. currentTimestamp := b.clock.Unix() if !config.IsGranite(currentTimestamp) { @@ -216,25 +216,25 @@ func (b *blockBuilder) calculateBlockBuildingDelay( lastBuildTime time.Time, lastBuildParentHash common.Hash, currentHeader *types.Header, -) (time.Duration, bool, error) { - initialMinBlockBuildingDelay, minBlockBuildingRetryDelay, err := b.getMinBlockBuildingDelays(currentHeader, b.chainConfig) +) (time.Duration, error) { + initialDelay, retryDelay, err := b.currentMinBlockBuildingDelays(currentHeader, b.chainConfig) if err != nil { - return 0, false, err + return 0, err } isRetry := lastBuildParentHash == currentHeader.ParentHash && !lastBuildTime.IsZero() // if last build time is zero, this is not a retry timeSinceLastBuildTime := b.clock.Time().Sub(lastBuildTime) var remainingMinDelay time.Duration - if minBlockBuildingRetryDelay > timeSinceLastBuildTime { - remainingMinDelay = minBlockBuildingRetryDelay - timeSinceLastBuildTime + if retryDelay > timeSinceLastBuildTime { + remainingMinDelay = retryDelay - timeSinceLastBuildTime } - if initialMinBlockBuildingDelay > 0 { - remainingMinDelay = max(initialMinBlockBuildingDelay, remainingMinDelay) + if initialDelay > 0 { + remainingMinDelay = max(initialDelay, remainingMinDelay) } else if !isRetry || remainingMinDelay == 0 { - return 0, true, nil // Build immediately + return 0, nil // Build immediately } - return remainingMinDelay, false, nil // Need to wait + return remainingMinDelay, nil // Need to wait } diff --git a/plugin/evm/block_builder_test.go b/plugin/evm/block_builder_test.go index 481094e4ba..33f11dd155 100644 --- a/plugin/evm/block_builder_test.go +++ b/plugin/evm/block_builder_test.go @@ -24,13 +24,12 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { clock := &mockable.Clock{} clock.Set(now) tests := []struct { - name string - config *extras.ChainConfig - currentHeader *types.Header - lastBuildTime time.Time - lastBuildParentHash common.Hash - expectedTimeToWait time.Duration - expectedShouldBuildNow bool + name string + config *extras.ChainConfig + currentHeader *types.Header + lastBuildTime time.Time + lastBuildParentHash common.Hash + expectedTimeToWait time.Duration }{ { name: "pre_granite_returns_build_immediately_zero_time", @@ -39,9 +38,9 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { ParentHash: common.Hash{1}, Time: nowSecUint64, }, - lastBuildTime: time.Time{}, // Zero time means not a retry - lastBuildParentHash: common.Hash{1}, - expectedShouldBuildNow: true, + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 0, }, { name: "pre_granite_returns_build_immediately_different_parent_hash", @@ -50,9 +49,9 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { ParentHash: common.Hash{2}, Time: nowSecUint64, }, - lastBuildTime: now, - lastBuildParentHash: common.Hash{1}, - expectedShouldBuildNow: true, + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 0, }, { name: "pre_granite_returns_build_delays_with_same_parent_hash", @@ -61,10 +60,9 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { ParentHash: common.Hash{1}, Time: nowSecUint64, }, - lastBuildTime: now, - lastBuildParentHash: common.Hash{1}, - expectedTimeToWait: PreGraniteMinBlockBuildingRetryDelay, - expectedShouldBuildNow: false, + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PreGraniteMinBlockBuildingRetryDelay, }, { name: "pre_granite_returns_build_returns_immediately_if_enough_time_passed", @@ -73,10 +71,9 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { ParentHash: common.Hash{1}, Time: nowSecUint64, }, - lastBuildTime: now.Add(-PreGraniteMinBlockBuildingRetryDelay), // Less than retry delay ago - lastBuildParentHash: common.Hash{1}, // Same as current parent - expectedTimeToWait: 0, - expectedShouldBuildNow: true, + lastBuildTime: now.Add(-PreGraniteMinBlockBuildingRetryDelay), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, // Same as current parent + expectedTimeToWait: 0, }, { name: "pre_granite_returns_build_delays_only_remaining_min_delay", @@ -85,64 +82,57 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { ParentHash: common.Hash{1}, Time: nowSecUint64, }, - lastBuildTime: now.Add(-PreGraniteMinBlockBuildingRetryDelay / 2), // Less than retry delay ago - lastBuildParentHash: common.Hash{1}, - expectedTimeToWait: PreGraniteMinBlockBuildingRetryDelay / 2, - expectedShouldBuildNow: false, + lastBuildTime: now.Add(-PreGraniteMinBlockBuildingRetryDelay / 2), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PreGraniteMinBlockBuildingRetryDelay / 2, }, { - name: "granite_block_with_now_time", - config: extras.TestGraniteChainConfig, - currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64, acp226.InitialDelayExcess), - lastBuildTime: time.Time{}, - lastBuildParentHash: common.Hash{1}, - expectedTimeToWait: 2000 * time.Millisecond, // should wait for initial delay - expectedShouldBuildNow: false, + name: "granite_block_with_now_time", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 2000 * time.Millisecond, // should wait for initial delay }, { - name: "granite_block_with_2_seconds_before_clock_no_retry", - config: extras.TestGraniteChainConfig, - currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, acp226.InitialDelayExcess), - lastBuildTime: time.Time{}, // Zero time means not a retry - lastBuildParentHash: common.Hash{1}, - expectedTimeToWait: 0, // should not wait for initial delay - expectedShouldBuildNow: true, + name: "granite_block_with_2_seconds_before_clock_no_retry", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 0, // should not wait for initial delay }, { - name: "granite_block_with_2_seconds_before_clock_with_retry", - config: extras.TestGraniteChainConfig, - currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, acp226.InitialDelayExcess), - lastBuildTime: now, - lastBuildParentHash: common.Hash{1}, - expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, - expectedShouldBuildNow: false, + name: "granite_block_with_2_seconds_before_clock_with_retry", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, acp226.InitialDelayExcess), + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, }, { - name: "granite_with_2_seconds_before_clock_only_waits_for_retry_delay", - config: extras.TestGraniteChainConfig, - currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 - lastBuildTime: now, - lastBuildParentHash: common.Hash{1}, - expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, - expectedShouldBuildNow: false, + name: "granite_with_2_seconds_before_clock_only_waits_for_retry_delay", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 + lastBuildTime: now, + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay, }, { - name: "granite_with_2_seconds_before_clock_only_waits_for_remaining_retry_delay", - config: extras.TestGraniteChainConfig, - currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 - lastBuildTime: now.Add(-PostGraniteMinBlockBuildingRetryDelay / 2), // Less than retry delay ago - lastBuildParentHash: common.Hash{1}, - expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay / 2, - expectedShouldBuildNow: false, + name: "granite_with_2_seconds_before_clock_only_waits_for_remaining_retry_delay", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64-2000, 0), // 0 means min delay excess which is 1 + lastBuildTime: now.Add(-PostGraniteMinBlockBuildingRetryDelay / 2), // Less than retry delay ago + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: PostGraniteMinBlockBuildingRetryDelay / 2, }, { - name: "granite_with_2_seconds_after_clock", - config: extras.TestGraniteChainConfig, - currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64+2000, acp226.InitialDelayExcess), - lastBuildTime: time.Time{}, // Zero time means not a retry - lastBuildParentHash: common.Hash{1}, - expectedTimeToWait: 4000 * time.Millisecond, - expectedShouldBuildNow: false, + name: "granite_with_2_seconds_after_clock", + config: extras.TestGraniteChainConfig, + currentHeader: createGraniteTestHeader(common.Hash{1}, nowMilliUint64+2000, acp226.InitialDelayExcess), + lastBuildTime: time.Time{}, // Zero time means not a retry + lastBuildParentHash: common.Hash{1}, + expectedTimeToWait: 4000 * time.Millisecond, }, } @@ -153,7 +143,7 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { chainConfig: tt.config, } - timeToWait, shouldBuildNow, err := b.calculateBlockBuildingDelay( + timeToWait, err := b.calculateBlockBuildingDelay( tt.lastBuildTime, tt.lastBuildParentHash, tt.currentHeader, @@ -161,7 +151,6 @@ func TestCalculateBlockBuildingDelay(t *testing.T) { require.NoError(t, err) require.Equal(t, tt.expectedTimeToWait, timeToWait) - require.Equal(t, tt.expectedShouldBuildNow, shouldBuildNow) }) } }