perf: use autoresearch to reduce allocations

[tac0turtle]

Overview

This pr used autoresearch to run a loop on reducing allocations

3 a8b6352 78allocs 22,996 33,091 keep Unsafe reinterpret cast of Txs to [][]byte in ApplyBlock — eliminates make([][]byte, n) allocation
4 823aa62 77allocs 20,276 32,480 keep Direct pb.Data serialization in DACommitment — avoids pruned Data wrapper and txsToByteSlices allocations
5 0720b44 74allocs 12,192 31,624 keep unsafe.Slice in Data.ToProto() — eliminates txsToByteSlices [][]byte allocation
6 — 74allocs 12,187 31,217 discard Reverted hand-written HashSlim wire encoder — produced different hashes than MarshalBinary
7 ccbc2e4 64allocs 11,130 31,570 keep sync.Pool for protobuf message structs in MarshalBinary — eliminates 10 allocs per block
8 805672e 56allocs 10,217 31,037 keep Pooled SignedHeader.MarshalBinary — reuse pb.SignedHeader/pb.Header/pb.Signer/pb.Version structs

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ecbbef0b-bfd6-49cc-8df4-75ee3e7e0bef

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch autoresearch/block-perf-2026-04-04

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

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

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Apr 7, 2026, 3:12 PM

[claude]

Claude finished @tac0turtle's task in 2m 32s —— View job

---

Code Review — perf: reduce allocations

• Gather context and read changed files
• Review types/serialization.go — unsafe usage, sync.Pool patterns
• Review types/hashing.go — allocation optimizations
• Review block/internal/executing/executor.go — ApplyBlock changes
• Review pkg/store/batch.go — batch changes
• Review benchmark test
• Summarize findings

---

Overview

The PR reduces per-block allocations significantly (78→56 allocs per benchmark iteration) using three techniques: sync.Pool for short-lived protobuf message structs, unsafe.Slice/unsafe.SliceData for zero-copy []Tx → [][]byte reinterpretation, and sha256 instance pooling. The overall approach is sound, but there are several correctness and maintainability concerns that warrant attention before merging.

---

Issues

1. Pool cleanup without defer — fragile error path management

types/serialization.go, Header.MarshalBinary() (line 75) and SignedHeader.MarshalBinary() (line 222)

Pool objects are returned manually at each return site instead of via defer. SignedHeader.MarshalBinary() has three distinct exit points and repeats the same four-line cleanup block at each one. This is brittle: a future edit adding a new return path will silently leak pool objects (they won't be GC'd, but the pool will grow unboundedly under load). defer is the conventional and correct pattern here:

func (sh *SignedHeader) MarshalBinary() ([]byte, error) {
    psh := pbSignedHeaderPool.Get().(*pb.SignedHeader)
    psh.Reset()
    defer func() {
        psh.Reset()
        pbSignedHeaderPool.Put(psh)
    }()
    ph := pbHeaderPool.Get().(*pb.Header)
    defer func() { ph.Reset(); pbHeaderPool.Put(ph) }()
    // ... etc

The Data.MarshalBinary() method does use defer for pm but manually handles pd, making it inconsistent. Fix this →

---

2. Data.ToProto() returns an aliased pb.Data.Txs

types/serialization.go, Data.ToProto() (line 505)

txBytes := unsafe.Slice((*[]byte)(unsafe.SliceData(d.Txs)), len(d.Txs))
return &pb.Data{Metadata: mProto, Txs: txBytes}

The returned *pb.Data holds a Txs slice that directly aliases d.Txs. Any mutation of d.Txs after ToProto() returns (or vice versa) will corrupt the result. The original code used txsToByteSlices which copied. This is safe today because ToProto() is immediately marshaled in every call site, but the method signature is public and makes no guarantee that the result is ephemeral. At minimum, this deserves a doc comment: // The returned pb.Data.Txs aliases d.Txs; callers must not modify d.Txs until the returned value is no longer used.

---

3. pv (pb.Version) is not Reset() before populating

types/serialization.go, Header.MarshalBinary() (line 78) and SignedHeader.MarshalBinary() (line 230)

pv := pbVersionPool.Get().(*pb.Version)
pv.Block, pv.App = h.Version.Block, h.Version.App  // Reset() not called first

pb.Version only has two fields, so overwriting both is functionally equivalent to Reset(). However, if a future proto change adds a third field (e.g. Patch), the pooled object would silently carry over the previous value without Reset(). Following the same Reset()-first pattern used for all other pooled messages would be safer and more consistent.

---

4. &pb.Signer{} allocation not pooled in SignedHeader.MarshalBinary()

types/serialization.go, line 247

if sh.Signer.PubKey == nil {
    psh.Signer = &pb.Signer{}  // escapes to heap, not pooled

The empty-signer path allocates a new pb.Signer rather than reusing pbSignerPool. Since this is the "no signer" fast path (e.g. based sequencer), it may run frequently. Using pbSignerPool here would be consistent with the rest of the function.

---

5. Unsafe []Tx → [][]byte reinterpretation — layout assumption

block/internal/executing/executor.go, ApplyBlock() (line 801)
types/serialization.go, Data.MarshalBinary() (line 142)
types/hashing.go, DACommitment() (line 130)

rawTxs = unsafe.Slice((*[]byte)(unsafe.SliceData(data.Txs)), n)

type Tx []byte is a defined type, not a type alias (=). Go's spec guarantees that a defined type and its underlying type have the same memory layout, so this cast is technically valid today. However, this is a load-bearing assumption that is nowhere documented near the type definition. If Tx were ever changed (even to add a method set or change to a struct), this code would silently produce wrong results rather than failing to compile. A compile-time layout assertion guard would make the assumption explicit:

// Ensure Tx and []byte share the same memory layout.
var _ = [0]struct{}{ [unsafe.Sizeof(Tx{})]struct{}{}, [unsafe.Sizeof([]byte{})]struct{}{} }

Or simply add a //go:linkname-style comment near type Tx []byte noting the unsafe dependency. Fix this →

---

6. Benchmark uses b.Loop() (Go 1.24+)

types/hash_memo_bench_test.go, lines 13, 26, 38

for b.Loop() {

testing.B.Loop() was introduced in Go 1.24. This is fine if the module's go directive is ≥ 1.24, but it's worth verifying this is intentional and the CI Go version satisfies it. The more widely-compatible idiom is for i := 0; i < b.N; i++.

---

7. State.MarshalBinary() not covered by tests (Codecov: 32 missing lines)

The new pooled serialization paths (State.MarshalBinary, Header.MarshalBinary, Data.MarshalBinary) appear to lack direct unit tests based on the Codecov report (77.39% patch coverage). Serialization round-trip tests for these methods would catch regression if protobuf field layout changes.

---

Non-issues / Confirmations

• Pool lifecycle correctness: pd.Reset() before pbDataPool.Put(pd) correctly clears the pd.Txs alias before returning to pool. The defer for pm in Data.MarshalBinary fires after pd is already reset and returned — lifecycle is correct.
• sha256Pool usage: leafHashOpt calls s.Reset() before use, so pooled instances are always clean. s.Sum(nil) allocates a new slice for the result, so the returned hash is not aliased to the pooled hasher.
• pkg/store/batch.go: Simplification of UpdateState to call state.MarshalBinary() directly is clean and correct.
• DACommitment nil safety: unsafe.Slice(nil, 0) produces a nil slice, matching the original behavior.

---

Summary

The optimization strategy is solid and the allocation wins are real. The main actionable items are: (1) switch pool cleanup to defer to prevent future leaks, (2) add a compile-time layout assertion for the Tx↔[]byte unsafe cast, (3) document the aliasing contract on Data.ToProto(), and (4) add round-trip tests for the new MarshalBinary implementations.

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://evstack.github.io/docs-preview/pr-3225/
Built to branch main at 2026-04-04 10:13 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[codecov]

Codecov Report

❌ Patch coverage is 78.75000% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.64%. Comparing base (ff88b95) to head (8f9ba3b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
types/serialization.go	77.39%	32 Missing and 1 partial ⚠️
pkg/store/batch.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3225      +/-   ##
==========================================
+ Coverage   61.45%   61.64%   +0.19%     
==========================================
  Files         120      120              
  Lines       12504    12692     +188     
==========================================
+ Hits         7684     7824     +140     
- Misses       3959     4005      +46     
- Partials      861      863       +2     

Flag	Coverage Δ
combined	61.64% <78.75%> (+0.19%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.