Skip to content

Commit 7e82b89

Browse files
authored
Merge branch 'main' into julien/cache
2 parents 2c0b4aa + f0eba0d commit 7e82b89

File tree

5 files changed

+256
-132
lines changed

5 files changed

+256
-132
lines changed

.github/workflows/benchmark.yml

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ jobs:
103103
# single job to push all results to gh-pages sequentially, avoiding race conditions
104104
publish-benchmarks:
105105
name: Publish Benchmark Results
106-
needs: [evm-benchmark, spamoor-benchmark]
106+
needs: [evm-benchmark]
107107
runs-on: ubuntu-latest
108108
permissions:
109109
contents: write
@@ -115,11 +115,6 @@ jobs:
115115
uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3 # v8.0.0
116116
with:
117117
name: evm-benchmark-results
118-
- name: Download Spamoor benchmark results
119-
uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3 # v8.0.0
120-
with:
121-
name: spamoor-benchmark-results
122-
path: test/e2e/benchmark/
123118

124119
# only update the benchmark baseline on push/dispatch, not on PRs
125120
- name: Store EVM Contract Roundtrip result
@@ -154,22 +149,3 @@ jobs:
154149
alert-threshold: '150%'
155150
fail-on-alert: true
156151
comment-on-alert: true
157-
158-
# delete local gh-pages so the next benchmark action step fetches fresh from remote
159-
- name: Reset local gh-pages branch
160-
if: always()
161-
run: git branch -D gh-pages || true
162-
163-
- name: Store Spamoor Trace result
164-
if: always()
165-
uses: benchmark-action/github-action-benchmark@a7bc2366eda11037936ea57d811a43b3418d3073 # v1.21.0
166-
with:
167-
name: Spamoor Trace Benchmarks
168-
tool: 'customSmallerIsBetter'
169-
output-file-path: test/e2e/benchmark/spamoor_bench.json
170-
auto-push: ${{ github.event_name != 'pull_request' }}
171-
save-data-file: ${{ github.event_name != 'pull_request' }}
172-
github-token: ${{ secrets.GITHUB_TOKEN }}
173-
alert-threshold: '150%'
174-
fail-on-alert: false
175-
comment-on-alert: true

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2323
By using one or the other, you are losing the ability to rollback or replay transactions earlier than `HEAD-n`.
2424
When using _classic pruning_, you aren't able to fetch blocks prior to `HEAD-n`.
2525

26+
### Fixed
27+
28+
- Fix block timer to account for execution time. Previously, the block timer reset to the full `block_time` duration after `ProduceBlock` completed, making the effective interval `block_time + execution_time`. Now the timer subtracts elapsed execution time so blocks are produced at the configured cadence.
29+
2630
### Changes
2731

2832
- Store pending blocks separately from executed blocks key. [#3073](https://github.com/evstack/ev-node/pull/3073)

block/internal/executing/executor.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,12 +415,18 @@ func (e *Executor) executionLoop() {
415415
continue
416416
}
417417

418+
start := time.Now()
418419
if err := e.blockProducer.ProduceBlock(e.ctx); err != nil {
419420
e.logger.Error().Err(err).Msg("failed to produce block")
420421
}
421422
txsAvailable = false
422-
// Always reset block timer to keep ticking
423-
blockTimer.Reset(e.config.Node.BlockTime.Duration)
423+
// reset timer accounting for time spent producing the block
424+
elapsed := time.Since(start)
425+
remaining := e.config.Node.BlockTime.Duration - elapsed
426+
if remaining <= 0 {
427+
remaining = 0
428+
}
429+
blockTimer.Reset(remaining)
424430

425431
case <-lazyTimerCh:
426432
e.logger.Debug().Msg("Lazy timer triggered block production")

block/internal/executing/executor_logic_test.go

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import (
44
"context"
55
crand "crypto/rand"
66
"errors"
7+
"sync"
78
"testing"
89
"testing/synctest"
910
"time"
1011

1112
"github.com/ipfs/go-datastore"
12-
"github.com/ipfs/go-datastore/sync"
13+
dssync "github.com/ipfs/go-datastore/sync"
1314
"github.com/libp2p/go-libp2p/core/crypto"
1415
"github.com/rs/zerolog"
1516
"github.com/stretchr/testify/assert"
@@ -245,7 +246,7 @@ type executorTestFixture struct {
245246
func setupTestExecutor(t *testing.T, pendingLimit uint64) executorTestFixture {
246247
t.Helper()
247248

248-
ds := sync.MutexWrap(datastore.NewMapDatastore())
249+
ds := dssync.MutexWrap(datastore.NewMapDatastore())
249250
memStore := store.New(ds)
250251

251252
cacheManager, err := cache.NewManager(config.DefaultConfig(), memStore, zerolog.Nop())
@@ -342,3 +343,86 @@ func assertProduceBlockInvariantWithTxs(t *testing.T, txs [][]byte) {
342343
err = prevState.AssertValidForNextState(header, data)
343344
require.NoError(t, err, "Produced block failed AssertValidForNextState")
344345
}
346+
347+
// stubBlockProducer records ProduceBlock call times and simulates execution by sleeping.
348+
type stubBlockProducer struct {
349+
mu sync.Mutex
350+
callTimes []time.Time
351+
execTime time.Duration
352+
}
353+
354+
func (s *stubBlockProducer) ProduceBlock(ctx context.Context) error {
355+
s.mu.Lock()
356+
s.callTimes = append(s.callTimes, time.Now())
357+
s.mu.Unlock()
358+
timer := time.NewTimer(s.execTime)
359+
defer timer.Stop()
360+
select {
361+
case <-timer.C:
362+
case <-ctx.Done():
363+
}
364+
return nil
365+
}
366+
367+
func (s *stubBlockProducer) RetrieveBatch(context.Context) (*BatchData, error) {
368+
panic("unexpected call")
369+
}
370+
371+
func (s *stubBlockProducer) CreateBlock(context.Context, uint64, *BatchData) (*types.SignedHeader, *types.Data, error) {
372+
panic("unexpected call")
373+
}
374+
375+
func (s *stubBlockProducer) ApplyBlock(context.Context, types.Header, *types.Data) (types.State, error) {
376+
panic("unexpected call")
377+
}
378+
379+
func (s *stubBlockProducer) getCalls() []time.Time {
380+
s.mu.Lock()
381+
defer s.mu.Unlock()
382+
out := make([]time.Time, len(s.callTimes))
383+
copy(out, s.callTimes)
384+
return out
385+
}
386+
387+
// TestExecutionLoop_BlockInterval verifies that the block timer accounts for
388+
// execution time so that the interval between block starts equals blockTime,
389+
// not blockTime + executionTime.
390+
func TestExecutionLoop_BlockInterval(t *testing.T) {
391+
synctest.Test(t, func(t *testing.T) {
392+
const (
393+
blockTime = 100 * time.Millisecond
394+
executionTime = 50 * time.Millisecond
395+
wantBlocks = 5
396+
)
397+
398+
fx := setupTestExecutor(t, 1000)
399+
defer fx.Cancel()
400+
401+
fx.Exec.config.Node.BlockTime = config.DurationWrapper{Duration: blockTime}
402+
403+
stub := &stubBlockProducer{execTime: executionTime}
404+
fx.Exec.SetBlockProducer(stub)
405+
406+
done := make(chan struct{})
407+
go func() {
408+
fx.Exec.executionLoop()
409+
close(done)
410+
}()
411+
412+
// advance fake time enough for wantBlocks blocks
413+
time.Sleep(blockTime + time.Duration(wantBlocks)*blockTime)
414+
415+
fx.Cancel()
416+
<-done
417+
418+
calls := stub.getCalls()
419+
require.GreaterOrEqual(t, len(calls), wantBlocks,
420+
"expected at least %d blocks, got %d", wantBlocks, len(calls))
421+
422+
for i := 1; i < len(calls); i++ {
423+
interval := calls[i].Sub(calls[i-1])
424+
assert.Equal(t, blockTime, interval,
425+
"block %d: interval between block starts should equal blockTime", i)
426+
}
427+
})
428+
}

0 commit comments

Comments
 (0)