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

Summary by CodeRabbit

• Tests

  • Added benchmark tests for header hashing performance in different cache scenarios.

• Performance

  • Optimized memory allocation in transaction processing.
  • Implemented object pooling for serialization operations to reduce allocation overhead.

• Bug Fixes

  • Fixed state serialization to use binary marshalling directly.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces memory optimization strategies across the codebase, including object pooling for protobuf messages and SHA256 hash instances, unsafe slice conversions to avoid allocations, replacement of protobuf marshalling with direct binary marshalling, and compile-time type safety checks. The changes span serialization, hashing, and execution layers.

Changes

Cohort / File(s)	Summary
Memory Pooling Infrastructure types/serialization.go, types/hashing.go	Introduces global sync.Pool instances for protobuf messages (pb.Header, pb.Version, pb.Data, pb.Metadata, pb.Signer, pb.SignedHeader, pb.State) and SHA256 hash instances. Updates MarshalBinary() methods to reuse pooled objects, reducing allocations and improving performance during hot serialization paths.
Marshalling Strategy Changes pkg/store/batch.go, types/serialization.go	Replaces protobuf-based marshalling with direct binary marshalling. UpdateState() now uses state.MarshalBinary() instead of proto.Marshal(state.ToProto()). Adds new (*State).MarshalBinary() method for consistent binary serialization.
Unsafe Slice Optimizations block/internal/executing/executor.go, types/hashing.go	Employs unsafe slice reinterpretation to avoid allocations when converting between slice types. Executor.ApplyBlock converts data.Txs using unsafe.Slice() instead of allocating a new slice. DACommitment() builds protobuf views over existing slice data.
Type Safety and Testing types/tx.go, types/serialization_test.go, types/hash_memo_bench_test.go	Adds compile-time size equality guards using unsafe.Sizeof in tx.go, new test cases for marshalling copy semantics and round-trip behavior, and benchmarks for header hash caching behavior with memoization.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Suggested reviewers

• julienrbrt
• tzdybal
• randygrok
• MSevey

Poem

🐰 Pooling objects with grace so fine,
Unsafe slices? No more allocations' climb!
Binary marshalling takes the lead,
Memory optimized with thoughtful speed,
Hop along, performance's in the deed! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description provides context and specific optimization details but lacks a formal structure matching the template's Overview section with clear explanations and issue references.	Add a properly formatted Overview section explaining the goals and rationale, and include issue references (e.g., 'Closes #') if applicable.
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main objective: performance improvements through reducing allocations via autoresearch-driven optimizations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 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 9, 2026, 3:09 PM

[claude]

Claude finished @tac0turtle's task in 5m 48s —— View job

---

Code Review

Tasks:

• Gather context and understand PR changes
• Review types/serialization.go (sync.Pool, unsafe operations)
• Review types/hashing.go
• Review types/tx.go
• Review block/internal/executing/executor.go
• Review pkg/store/batch.go
• Review test files
• Post final review

---

Good allocation-reduction work overall. The sync.Pool approach for protobuf structs and the sha256 hasher are solid choices for hot paths. However, there are a few correctness and maintainability concerns worth addressing before merge.

---

Critical / Correctness

1. SignedHeader.MarshalBinary() — Pool objects returned before psh.Reset() clears the alias

ph and pv are Put back into their pools while psh.Header = ph still aliases them. Between pbHeaderPool.Put(ph) and psh.Reset(), another goroutine could grab ph from the pool and start writing to it, while psh.Header still references it. This is safe today because nothing reads through psh.Header after proto.Marshal returns — but it's a latent trap for future refactoring.

serialization.go:253-262 (no-pubkey branch), serialization.go:266-275 (error branch), serialization.go:281-290 (success branch) all have this pattern. Recommend reordering: reset psh first, then put children back, or use defer.

Additionally, the cleanup block is copy-pasted three times. A panic in proto.Marshal leaks all four pooled objects. A defer-based helper would eliminate both issues:

func (sh *SignedHeader) MarshalBinary() ([]byte, error) {
    psh := pbSignedHeaderPool.Get().(*pb.SignedHeader)
    ph  := pbHeaderPool.Get().(*pb.Header)
    pv  := pbVersionPool.Get().(*pb.Version)
    psi := pbSignerPool.Get().(*pb.Signer)
    defer func() {
        psh.Reset(); pbSignedHeaderPool.Put(psh)
        ph.Reset();  pbHeaderPool.Put(ph)
        pv.Reset();  pbVersionPool.Put(pv)
        psi.Reset();  pbSignerPool.Put(psi)
    }()
    // ... fill and marshal ...
}

Fix this →

---

2. Data.MarshalBinary() — pm.LastDataHash aliases d.Metadata.LastDataHash (via embedding)

pm.LastDataHash = d.LastDataHash   // direct alias, no copy

d.LastDataHash is d.Metadata.LastDataHash (via Data embedding *Metadata). The pooled pm will hold a reference to the same backing array until the defer runs pm.Reset(). This is fine since marshal is synchronous and Reset() just nils the field. However, the corresponding Data.ToProto() uses d.LastDataHash[:] (explicit slice expression), and Metadata.ToProto() also uses m.LastDataHash[:]. Keeping consistent slice expressions avoids subtle divergence if the field type ever changes.

---

Moderate

3. ApplyBlock unsafe alias — undocumented contract on ExecuteTxs

// executor.go:801
rawTxs = unsafe.Slice((*[]byte)(unsafe.SliceData(data.Txs)), n)

rawTxs shares the backing array with data.Txs. If ExecuteTxs mutates individual elements (e.g., rawTxs[i] = otherSlice), it silently corrupts data.Txs. The comment explains the layout guarantee but should also note that ExecuteTxs must treat the slice as read-only. This is an implicit contract that future execution-client implementers might violate.

4. DACommitment() — missing nil guard vs. MarshalBinary() inconsistency

// hashing.go:130
pbData := pb.Data{Txs: unsafe.Slice((*[]byte)(unsafe.SliceData(d.Txs)), len(d.Txs))}

Data.MarshalBinary() guards with if d.Txs != nil, but DACommitment() does not. Go's unsafe.Slice(nil, 0) is valid per spec (returns nil), so there's no crash — but the inconsistency is confusing and worth aligning.

---

Minor

5. sha256Pool only covers two of four hash paths

Data.Hash() and DACommitment() use the pooled hasher. Header.HashSlim() and Header.HashLegacy() still call sha256.Sum256(bytes) (stack hasher, separate allocation). Since headers are hashed frequently, pooling there too would be consistent.

6. State.MarshalBinary() — timestamppb.Timestamp still allocates

pts := &timestamppb.Timestamp{...}   // heap allocation not eliminated

ps and pv are pooled but pts escapes to the heap. If state serialization is on a hot path, consider pooling timestamppb.Timestamp too (or embedding it by value in the pool struct). Minor, but noted for completeness.

7. No defer cleanup in Header.MarshalBinary() / State.MarshalBinary()

If proto.Marshal panics (rare but possible with malformed data via reflection), ph/pv and ps/pv are leaked from their pools. Using defer prevents this. Currently only Data.MarshalBinary() uses defer.

---

Testing

8. Missing safety test for the unsafe.Slice path in MarshalBinary

TestDataToProtoCopiesOuterTxSlice verifies ToProto() isolation (modifying d.Txs[0] after calling ToProto doesn't affect the proto). But MarshalBinary uses an unsafe alias of the outer slice. A paired test verifying that concurrent or post-marshal mutations don't cause data corruption in the pooled path would increase confidence.

9. b.Loop() API (Go 1.24+)

hash_memo_bench_test.go uses for b.Loop(). The go.mod declares go 1.25.7, so this is fine — just noting it for anyone back-porting.

---

Positive Notes

• The compile-time guard in tx.go (unsafe.Sizeof checks) is the right way to protect the []Tx → [][]byte reinterpretation. Well done.
• pkg/store/batch.go simplification (UpdateState now delegates to State.MarshalBinary()) correctly consolidates the serialization path.
• TestStateMarshalBinaryRoundTrip and the new nil/invalid-input tests are solid additions.
• The PR description's per-commit alloc count table is excellent documentation for the reviewer.

[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 93.16770% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.06%. Comparing base (3abebca) to head (45e1951).

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3225      +/-   ##
==========================================
+ Coverage   61.67%   62.06%   +0.38%     
==========================================
  Files         120      120              
  Lines       12635    12778     +143     
==========================================
+ Hits         7793     7931     +138     
- Misses       3968     3975       +7     
+ Partials      874      872       -2     

Flag	Coverage Δ
combined	62.06% <93.16%> (+0.38%)	⬆️

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.

[coderabbitai]

🧹 Nitpick comments (2)

types/hash_memo_bench_test.go (1)

35-42: Consider invalidating cache between iterations for consistent baseline.

In BenchmarkHeaderHash_Single, the header is created once outside the loop. While Hash() doesn't set cachedHash, if future refactoring changes this behavior, the benchmark would measure cached hits after the first iteration. Consider adding h.InvalidateHash() inside the loop for robustness:

♻️ Suggested improvement for robustness

 func BenchmarkHeaderHash_Single(b *testing.B) {
 	h := GetRandomHeader("bench-chain", GetRandomBytes(32))
 	b.ReportAllocs()
 	b.ResetTimer()
 	for b.Loop() {
+		h.InvalidateHash()
 		_ = h.Hash()
 	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@types/hash_memo_bench_test.go` around lines 35 - 42,
BenchmarkHeaderHash_Single currently constructs h once and calls h.Hash()
repeatedly; to ensure each iteration measures an uncached baseline, call
h.InvalidateHash() before each h.Hash() invocation (or recreate h inside the
loop using GetRandomHeader/GetRandomBytes) so any cachedHash is cleared; update
the loop in BenchmarkHeaderHash_Single to invoke h.InvalidateHash() (or
reinitialize h) on every iteration before calling h.Hash().

types/hashing.go (1)

128-135: Ignored marshal error could silently produce incorrect commitment.

Line 131 ignores the error from proto.Marshal. While this is consistent with the existing pattern in Hash() (line 120-121), a marshal failure would produce a hash of nil/empty bytes, potentially causing consensus issues. Consider at least logging failures in debug builds or returning an error.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@types/hashing.go` around lines 128 - 135, DACommitment currently ignores the
error from proto.Marshal which can yield an incorrect commitment; change
DACommitment to surface marshal failures instead of silently hashing nil by
returning (Hash, error) and propagate this change to callers (mirror the new
signature across any places invoking DACommitment), or if changing signature is
impractical, check the error and log it via the package logger in a debug build
and return a zero-value Hash; locate DACommitment and the related Hash() usage
to implement proper error handling and ensure any callers handle the returned
error or the logged failure.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@types/hash_memo_bench_test.go`:
- Around line 35-42: BenchmarkHeaderHash_Single currently constructs h once and
calls h.Hash() repeatedly; to ensure each iteration measures an uncached
baseline, call h.InvalidateHash() before each h.Hash() invocation (or recreate h
inside the loop using GetRandomHeader/GetRandomBytes) so any cachedHash is
cleared; update the loop in BenchmarkHeaderHash_Single to invoke
h.InvalidateHash() (or reinitialize h) on every iteration before calling
h.Hash().

In `@types/hashing.go`:
- Around line 128-135: DACommitment currently ignores the error from
proto.Marshal which can yield an incorrect commitment; change DACommitment to
surface marshal failures instead of silently hashing nil by returning (Hash,
error) and propagate this change to callers (mirror the new signature across any
places invoking DACommitment), or if changing signature is impractical, check
the error and log it via the package logger in a debug build and return a
zero-value Hash; locate DACommitment and the related Hash() usage to implement
proper error handling and ensure any callers handle the returned error or the
logged failure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12528a06-2158-4aad-8475-8990e96b4bcb

📥 Commits

Reviewing files that changed from the base of the PR and between 3abebca and 45e1951.

📒 Files selected for processing (7)

• block/internal/executing/executor.go
• pkg/store/batch.go
• types/hash_memo_bench_test.go
• types/hashing.go
• types/serialization.go
• types/serialization_test.go
• types/tx.go