No actionable comments were generated in the recent review. 🎉

sequenceDiagram
    participant Test as TestMixedWorkload
    participant Suite as Benchmark Suite
    participant Spammers as Spammer Nodes
    participant Metrics as Spamoor Metrics
    participant Chain as Blockchain

    Test->>Suite: Setup benchmark environment
    Test->>Suite: Remove stale spammers
    Test->>Test: Calculate spammer distribution<br/>(4 workload types)
    
    loop For each workload type
        Test->>Spammers: CreateSpammer (ERC20/Uniswap/Gas/Storage)
    end
    
    Test->>Test: Wait for spammers running
    
    Test->>Metrics: Poll spamoor_transactions_sent_total<br/>(deployment target)
    Metrics-->>Test: Metrics received
    
    Test->>Metrics: Poll spamoor_transactions_sent_total<br/>(warmup target)
    Metrics-->>Test: Warmup complete
    
    Test->>Chain: Capture start block header
    
    Test->>Metrics: Poll until steady-state<br/>transaction count reached
    Metrics-->>Test: Target reached
    
    Test->>Chain: Drain pending transactions
    Test->>Chain: Capture end block header
    
    Test->>Chain: Collect block-level metrics<br/>for range
    Chain-->>Test: Metrics data
    
    Test->>Suite: Gather execution traces
    Suite-->>Test: Trace data
    
    Test->>Metrics: Fetch final spamoor metrics
    Metrics-->>Test: Final metrics (sent/failed)
    
    Test->>Test: Assert results (duration > 0,<br/>sent > 0, failed == 0)

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 @coderabbitai help to get the list of available commands and usage tips.}

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Claude finished @chatton's task in 2m 13s —— View job

Code Review: chore: mixed benchmark test

• Gather context and read changed files
• Review mixed_workload_test.go
• Review distribute_spammers_test.go
• Review helpers.go and config.go changes
• Post comprehensive review feedback

Overall this is well-structured code with good documentation. A few things worth addressing before merge:

Issues

1. Missing non-negative validation for BENCH_MIX_*_PCT env vars (mixed_workload_test.go:52)

The test validates that the four percentages sum to 100, but doesn't guard against negative values. A configuration like 150, -50, 50, 50 would satisfy the sum check but cause distributeSpammers to produce incorrect (or even negative) counts before the 1-minimum floor applies. Fix this →

// current
s.Require().Equal(100, pctSum, "BENCH_MIX_*_PCT values must sum to 100, got %d", pctSum)
// also add:
for i, p := range pcts {
    s.Require().GreaterOrEqual(p, 0, "BENCH_MIX_*_PCT[%d] must be non-negative, got %d", i, p)
}

2. distributeSpammers doesn't validate pcts for negative values (mixed_workload_test.go:212)

The panic guard correctly rejects total < 4, but negative percentages in pcts will silently produce wrong distributions (negative floor, compensated by the 1-minimum, but still incorrect). Consider adding a guard or documenting the assumption that all pcts values are ≥ 0.

3. requireSpammersRunning uses a hardcoded 5-second timeout (helpers.go:201)

In slow CI environments (particularly when deploying contracts for the first time), 5 seconds may be insufficient to confirm all spammers have transitioned to a running state.

}, 5*time.Second, 100*time.Millisecond)

This is especially relevant for the mixed workload, which starts 4+ spammers (vs 2 previously). Consider using cfg.WaitTimeout or at least a longer fixed value. Fix this →

4. envInt silently ignores invalid values (config.go:83)

If a user sets BENCH_MIX_ERC20_PCT=abc, it silently falls back to the default 40 without any log message. For a benchmark that's typically run with explicit configuration, a warning log would improve debuggability. Minor DX issue.

Observations

waitForMetricTarget: t.Logf → t.Fatalf (good fix) (helpers.go diff)

The change from non-fatal logging to t.Fatalf on timeout/context cancellation is the right call — without it, timed-out benchmarks would silently pass with incomplete data.

distributeSpammers algorithm and tests are solid (distribute_spammers_test.go)

The largest-remainder method is the correct approach for this kind of proportional allocation. Table-driven tests cover edge cases well (minimum total, uneven splits, large totals). The panic-on-total < 4 is tested explicitly.

config.go default bump from 2 → 4 spammers

Necessary for the mixed workload (1 per type minimum). Note this also changes the default for all other benchmark tests (ERC20, gas burner, etc.), which now get 4 spammers by default. Intended?

collectBlockMetrics makes O(N) sequential RPC calls (helpers.go:485)

One HeaderByNumber + one TransactionCount per block in the range. For a 10-minute benchmark run at 100ms block time that's ~6000 blocks / ~12000 RPC calls made serially. Not a correctness issue (benchmark teardown phase), but worth noting for very long runs.

Log message uses cfg.NumSpammers as total (mixed_workload_test.go:56)

t.Logf("mix distribution: erc20=%d, defi=%d, gasburner=%d, state=%d (total=%d)",
    counts[0], counts[1], counts[2], counts[3], cfg.NumSpammers)

Since distributeSpammers guarantees sum(counts) == total, this is equivalent, but using counts[0]+counts[1]+counts[2]+counts[3] would be more self-evident that we're logging the actual distributed sum.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.45%. Comparing base (cc9f965) to head (8410d69).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3223      +/-   ##
==========================================
+ Coverage   61.43%   61.45%   +0.01%     
==========================================
  Files         120      120              
  Lines       12504    12504              
==========================================
+ Hits         7682     7684       +2     
+ Misses       3960     3959       -1     
+ Partials      862      861       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.