feat: add TestStatePressure benchmark for state trie stress test#3188
feat: add TestStatePressure benchmark for state trie stress test#3188
Conversation
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
|
Claude finished @chatton's task in 2m 15s —— View job Code Review
The test is well-structured and closely follows the established pattern from
|
📝 WalkthroughWalkthroughAdded a GitHub Actions job "State Pressure Benchmark" and a new E2E benchmark test (build tag Changes
Sequence DiagramsequenceDiagram
participant Test as State Pressure Test
participant Spamoor as Spamoor Spammers
participant EVM as EVM Node
participant Metrics as Metrics Collector
rect rgba(220, 220, 255, 0.5)
Test->>Test: Setup env (ev-node-state-pressure)\nConfigure spammer params
end
Test->>Spamoor: Create/start configured spammers
Spamoor->>EVM: Send transaction stream
rect rgba(255, 200, 100, 0.5)
Note over Test,Metrics: Warmup phase
Spamoor->>Metrics: Increment spamoor_transactions_sent_total
Test->>Metrics: Wait for warmup target\nReset trace start time
end
Test->>Metrics: Record start block header
rect rgba(100, 200, 255, 0.5)
Note over Test,Metrics: Measurement phase
Spamoor->>EVM: Continue sending to target count
EVM->>Metrics: Record block gas/tx metrics
Test->>Metrics: Wait for overall target reached
end
Test->>Test: Measure duration\nRecord end block header
Test->>Metrics: Collect metrics & traces
Test->>Test: Build and validate StatePressure result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@4d04d5d9486b7bd6fa91e7baf45bbb4f8b9deedd # v4.0.0 | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3188 +/- ##
=======================================
Coverage 61.14% 61.14%
=======================================
Files 117 117
Lines 12082 12082
=======================================
Hits 7387 7387
Misses 3868 3868
Partials 827 827
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/benchmark.yml (1)
119-123: Consider adding artifact upload for benchmark results.The
evm-benchmarkandspamoor-benchmarkjobs upload their results as artifacts, but this job (likeerc20-benchmark) does not. If the benchmark produces useful output (e.g., viaBENCH_JSON_OUTPUT), consider adding an upload step for result persistence and comparison across runs.📦 Optional: Add artifact upload
- name: Run state pressure test run: | - cd test/e2e && go test -tags evm \ + cd test/e2e && BENCH_JSON_OUTPUT=state_pressure_bench.json go test -tags evm \ -run='^TestSpamoorSuite$/^TestStatePressure$' -v -timeout=15m \ ./benchmark/ --evm-binary=../../../build/evm + - name: Upload benchmark results + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: state-pressure-benchmark-results + path: test/e2e/benchmark/state_pressure_bench.json🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml around lines 119 - 123, The "Run state pressure test" step currently runs the benchmark but doesn't persist results; add an upload-artifact step similar to the evm-benchmark/spamoor-benchmark jobs to save the benchmark output (e.g., the BENCH_JSON_OUTPUT file or benchmark directory). Specifically, after the run block in the "Run state pressure test" job, add a step named like "Upload state pressure benchmark results" that uses actions/upload-artifact to upload the BENCH_JSON_OUTPUT (or the generated JSON/bench directory) with a clear name (e.g., state-pressure-results-${{ github.run_id }}), so outputs are retained for later download and comparison.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/benchmark.yml:
- Around line 115-116: Replace the unpinned GitHub Action reference uses:
extractions/setup-just@v3 with the same action pinned to a specific commit SHA
so the workflow uses a fixed revision; locate the line referencing the
setup-just action (uses: extractions/setup-just@v3) and update it to the
commit-pinned form (the action's repository@<commit-sha>) matching how other
actions are pinned in this workflow.
---
Nitpick comments:
In @.github/workflows/benchmark.yml:
- Around line 119-123: The "Run state pressure test" step currently runs the
benchmark but doesn't persist results; add an upload-artifact step similar to
the evm-benchmark/spamoor-benchmark jobs to save the benchmark output (e.g., the
BENCH_JSON_OUTPUT file or benchmark directory). Specifically, after the run
block in the "Run state pressure test" job, add a step named like "Upload state
pressure benchmark results" that uses actions/upload-artifact to upload the
BENCH_JSON_OUTPUT (or the generated JSON/bench directory) with a clear name
(e.g., state-pressure-results-${{ github.run_id }}), so outputs are retained for
later download and comparison.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a0f6a69-98c1-4a47-a97c-f21b9d439a6f
📒 Files selected for processing (2)
.github/workflows/benchmark.ymltest/e2e/benchmark/spamoor_state_pressure_test.go
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the commit SHA for extractions/setup-just v3 tag
curl -s "https://api.github.com/repos/extractions/setup-just/git/refs/tags/v3" | jq -r '.object.sha'Repository: evstack/ev-node
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Try alternative method to get the commit SHA for the v3 tag
# Using the commits API instead
curl -s "https://api.github.com/repos/extractions/setup-just/releases/tags/v3" | jq -r '.target_commitish'
echo "---"
# Also try getting commit info directly
curl -s "https://api.github.com/repos/extractions/setup-just/git/refs/tags/v3" | jq '.'Repository: evstack/ev-node
Length of output: 448
🏁 Script executed:
#!/bin/bash
# Check if the repository exists and list recent releases
curl -s "https://api.github.com/repos/extractions/setup-just" | jq -r '.name, .full_name, .archived'
echo "---"
# Get all releases
curl -s "https://api.github.com/repos/extractions/setup-just/releases" | jq -r '.[0:3] | .[] | "\(.tag_name): \(.target_commitish)"'Repository: evstack/ev-node
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Check the benchmark.yml file around lines 115-116
sed -n '110,120p' .github/workflows/benchmark.ymlRepository: evstack/ev-node
Length of output: 501
Pin setup-just action to a commit hash.
The extractions/setup-just@v3 action is not pinned to a specific commit hash, unlike other actions in this workflow (e.g., actions/setup-go, docker/setup-buildx-action). Pinning to a commit hash improves supply chain security.
🔒 Proposed fix to pin the action
- name: Install just
- uses: extractions/setup-just@v3
+ uses: extractions/setup-just@f8a3cce218d9f83db3a2ecd90e41ac3de6cdfd9b # v3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install just | |
| uses: extractions/setup-just@v3 | |
| - name: Install just | |
| uses: extractions/setup-just@f8a3cce218d9f83db3a2ecd90e41ac3de6cdfd9b # v3 |
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 116-116: Unpinned tag for a non-immutable Action in workflow
Unpinned 3rd party Action 'Benchmarks' step Uses Step uses 'extractions/setup-just' with ref 'v3', not a pinned commit hash
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/benchmark.yml around lines 115 - 116, Replace the unpinned
GitHub Action reference uses: extractions/setup-just@v3 with the same action
pinned to a specific commit SHA so the workflow uses a fixed revision; locate
the line referencing the setup-just action (uses: extractions/setup-just@v3) and
update it to the commit-pinned form (the action's repository@<commit-sha>)
matching how other actions are pinned in this workflow.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/benchmark.yml (1)
116-117:⚠️ Potential issue | 🟠 MajorPin
setup-justto a commit SHA (still unpinned).Line [117] uses a mutable tag (
extractions/setup-just@v3). Please pin to an immutable commit hash to reduce supply-chain risk.🔒 Proposed fix
- - name: Install just - uses: extractions/setup-just@v3 + - name: Install just + uses: extractions/setup-just@<full-40-char-commit-sha> # v3#!/bin/bash # Verify unpinned uses in this workflow rg -nP 'uses:\s*extractions/setup-just@(?![0-9a-f]{40}\b)' .github/workflows/benchmark.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml around lines 116 - 117, The workflow uses an unpinned third-party action reference "extractions/setup-just@v3"; replace this mutable tag with a specific commit SHA for the extractions/setup-just action (e.g., extractions/setup-just@<40-char-commit-sha>) so the step named "Install just" is pinned to an immutable reference; update the uses value accordingly and verify the SHA corresponds to the intended release commit in the setup-just repository.
🧹 Nitpick comments (1)
.github/workflows/benchmark.yml (1)
120-124: Persist state-pressure benchmark output for later comparison.The new job runs the test but does not persist structured results, so trend/regression analysis is harder. Consider emitting JSON and uploading an artifact (same pattern as
spamoor-benchmark).📊 Suggested update
- name: Run state pressure test run: | - cd test/e2e && go test -tags evm \ + cd test/e2e && BENCH_JSON_OUTPUT=state_pressure_bench.json go test -tags evm \ -run='^TestSpamoorSuite$/^TestStatePressure$' -v -timeout=15m \ ./benchmark/ --evm-binary=../../../build/evm + - name: Upload benchmark results + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: state-pressure-benchmark-results + path: test/e2e/benchmark/state_pressure_bench.json🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml around lines 120 - 124, The "Run state pressure test" job currently runs the Go test but doesn't persist structured results; change the test invocation (the go test call in the Run state pressure test step) to emit JSON output (use go test -json or the test's JSON flag) and redirect it to a file (e.g. state-pressure-results.json) in the workspace, then add a following step that uploads that file as an artifact (mirror the pattern used by the spamoor-benchmark artifact upload, using actions/upload-artifact and a descriptive name like state-pressure-benchmark) so results are stored for later trend/regression analysis.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/benchmark.yml:
- Around line 116-117: The workflow uses an unpinned third-party action
reference "extractions/setup-just@v3"; replace this mutable tag with a specific
commit SHA for the extractions/setup-just action (e.g.,
extractions/setup-just@<40-char-commit-sha>) so the step named "Install just" is
pinned to an immutable reference; update the uses value accordingly and verify
the SHA corresponds to the intended release commit in the setup-just repository.
---
Nitpick comments:
In @.github/workflows/benchmark.yml:
- Around line 120-124: The "Run state pressure test" job currently runs the Go
test but doesn't persist structured results; change the test invocation (the go
test call in the Run state pressure test step) to emit JSON output (use go test
-json or the test's JSON flag) and redirect it to a file (e.g.
state-pressure-results.json) in the workspace, then add a following step that
uploads that file as an artifact (mirror the pattern used by the
spamoor-benchmark artifact upload, using actions/upload-artifact and a
descriptive name like state-pressure-benchmark) so results are stored for later
trend/regression analysis.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66bc87a2-5106-4c3d-8996-880f4fd85dcb
📒 Files selected for processing (2)
.github/workflows/benchmark.ymltest/e2e/benchmark/spamoor_state_pressure_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/benchmark/spamoor_state_pressure_test.go
- use cfg.GasUnitsToBurn instead of re-reading env var with different default - add comment explaining time.Sleep before requireSpammersRunning - add post-run assertions for sent > 0 and failed == 0 - add TODO comment for CI result publishing
chatton
force-pushed
the
cian/add-state-pressure-benchmark
branch
from
4853272 to
b6b0af4
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/benchmark.yml (1)
138-139:⚠️ Potential issue | 🟠 MajorPin
extractions/setup-justto an immutable commit SHA.Line 139 uses a movable tag (
@v3), which weakens workflow supply-chain guarantees.🔒 Suggested change
- - name: Install just - uses: extractions/setup-just@v3 + - name: Install just + uses: extractions/setup-just@f8a3cce218d9f83db3a2ecd90e41ac3de6cdfd9b # v3#!/bin/bash set -euo pipefail # Verify the current target object for tag v3 before pinning. curl -s https://api.github.com/repos/extractions/setup-just/git/ref/tags/v3 | jq -r '.object.sha'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml around lines 138 - 139, Replace the movable tag on the GitHub Action usage so the workflow uses an immutable commit SHA instead of "extractions/setup-just@v3": locate the job step that sets up just (the step with uses: "extractions/setup-just@v3") and change the reference to the repository@<commit-sha> where <commit-sha> is the full SHA obtained from the repo (e.g., the object SHA for tag v3); ensure you paste the full 40-char commit SHA and update the workflow commit message accordingly.
🧹 Nitpick comments (1)
test/e2e/benchmark/spamoor_state_pressure_test.go (1)
58-60: Replace fixed sleep with condition-based readiness wait.Line 59 adds a hard delay that can be flaky under variable CI load and makes runtime less deterministic. Prefer polling readiness directly (or rely on
requireSpammersRunningif it already handles retries) instead of sleeping.♻️ Suggested change
- // allow spamoor time to initialise spammer goroutines before polling status - time.Sleep(3 * time.Second) requireSpammersRunning(t, e.spamoorAPI, spammerIDs)As per coding guidelines "Ensure tests are deterministic in Go".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/benchmark/spamoor_state_pressure_test.go` around lines 58 - 60, Replace the fixed time.Sleep(3 * time.Second) with a condition-based wait: remove the hard sleep and instead poll the spamoor readiness by calling or extending requireSpammersRunning(t, e.spamoorAPI, spammerIDs) until it succeeds (with a short interval and overall timeout) or use a dedicated helper like waitForSpammersReady(spamoorAPI, spammerIDs, timeout) that retries under a timeout; references for change: the existing time.Sleep call and the requireSpammersRunning function should be updated so the test waits deterministically for spammers to be running rather than sleeping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/benchmark.yml:
- Around line 138-139: Replace the movable tag on the GitHub Action usage so the
workflow uses an immutable commit SHA instead of "extractions/setup-just@v3":
locate the job step that sets up just (the step with uses:
"extractions/setup-just@v3") and change the reference to the
repository@<commit-sha> where <commit-sha> is the full SHA obtained from the
repo (e.g., the object SHA for tag v3); ensure you paste the full 40-char commit
SHA and update the workflow commit message accordingly.
---
Nitpick comments:
In `@test/e2e/benchmark/spamoor_state_pressure_test.go`:
- Around line 58-60: Replace the fixed time.Sleep(3 * time.Second) with a
condition-based wait: remove the hard sleep and instead poll the spamoor
readiness by calling or extending requireSpammersRunning(t, e.spamoorAPI,
spammerIDs) until it succeeds (with a short interval and overall timeout) or use
a dedicated helper like waitForSpammersReady(spamoorAPI, spammerIDs, timeout)
that retries under a timeout; references for change: the existing time.Sleep
call and the requireSpammersRunning function should be updated so the test waits
deterministically for spammers to be running rather than sleeping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c6f05cd-7c2d-4905-909d-7855f1bd01db
📒 Files selected for processing (2)
.github/workflows/benchmark.ymltest/e2e/benchmark/spamoor_state_pressure_test.go
* main: refactor(tools/da-debug): improve da debug (#3199) fix(pkg/da): fix json rpc types (#3200) chore: demote warn log as debug (#3198) build(deps): Bump the all-go group across 1 directory with 2 updates (#3191) refactor: display block source in sync log (#3193) chore: remove cache analyzer (#3192) feat: add TestStatePressure benchmark for state trie stress test (#3188)
2 participants
Summary
TestStatePressurebenchmark measuring SSTORE-heavy throughputbenchConfigwithBENCH_*env vars, addBENCH_GAS_UNITS_TO_BURNPart of #2288
Results , local docker
Best sustained: 325 MGas/s (run 8). Highest ev-reth rate: 0.824 GGas/s (run 11).
Storage writes ~40% slower than pure compute (325 vs ~383 MGas/s gasburner).
ev-node overhead: 0.2-1.1% in clean runs. 50ms blocks not achievable under load.
Summary by CodeRabbit