fix(compression): bound gzip decoding

[rodrodros]

No description provided.

[github-actions]

Claude finished @rodrodros's task in 6m 50s —— View job

---

Review: bound gzip decoding

• Read the diff and all Gzip64Decode call sites
• Audit the bounding logic in utils/compression/compression.go
• Trace blast radius of the new limit through sync / VM / RPC paths
• Build + vet affected packages
• Post inline findings

The core idea is right and the boundary arithmetic is correct: the +1 read distinguishes "exactly at limit" from "one over", the readLimit < math.MaxInt64 guard prevents the overflow that would otherwise flip the limit negative, and I confirmed every non-test Gzip64Decode call site was updated (core/class_hash.go, adapters/core2sn/class.go, rpc/v8/class.go, rpc/v9/class.go) — no unbounded decode paths left behind. go vet passes on all affected packages.

My concern is not the mechanism, it's the value and where it's applied.

Blocker

MaxDeprecatedClassProgramSize = 4 MiB needs empirical justification before merge — core/class.go:27. The doc comment justifies the number with the sequencer's class-size cap, but that cap applies to the compressed class object while this constant bounds the decompressed program — quantities that differ by the gzip ratio (5–10× for Cairo 0 program JSON). Cairo 0's max_contract_bytecode_size of 81,290 felts puts the data array alone at ~5.5 MB before hints and identifiers, so the ceiling for a valid class is plausibly above 4 MiB.

That matters because the same constant is used on paths where the class is already on chain and rejecting it is unrecoverable: Hash() on the sync path, and core2sn → vm/class.go:25 for starknet_call/estimateFee/traceTransaction on historical blocks. A false positive there is a permanent sync halt, not a retryable error.

The bound doesn't need to be tight to work: the untrusted entry is JSON-RPC at MaxRequestBodySize = 10 MiB, so worst-case expansion is ~10 GB. A 64 MiB bound kills the bomb just as dead with three orders of magnitude of headroom. Since Cairo 0 declares have been disabled since v0.11, the class set is finite and historical — the observed max is exactly measurable, and recording it in the comment would settle this.

Important

• gzipReader.Close() is now skipped on two error paths — compression.go:169-183. Benign with klauspost today, but perf(jsonrpc): gzip responses with klauspost at BestSpeed #3957 just pooled the writers in this same file; if readers follow, this becomes a leak.
• The test table never exercises a compression bomb — compression_test.go:62-74. All six payloads are ≤ 1025 bytes, so the property the PR exists to enforce (tiny input, huge expansion, rejected without materialising it) is untested and would survive a refactor that reads everything before checking.

Nit

• Name the "unbounded" sentinel instead of spelling math.MaxInt64 at six call sites; export the over-limit error so RPC handlers can errors.Is it rather than string-matching. Also diferentiat-ing → differentiating, and a negative limit produces a confusing message.
• core/class_hash.go:11-17 — wrap the error so an over-limit stall on the sync path is diagnosable.

Not verified

I could not run the test suite — go test is blocked by the sandbox in this environment (go build/go vet were allowed). I also could not measure real decompression ratios against the core/testdata/cairo0_hash_regressions fixtures for the same reason, so the sizing argument above rests on the felt-count math rather than measurement. Adding Bash(go test) to --allowedTools would let me check both.
· branch rdr/encode-fix

[codecov]

Codecov Report

❌ Patch coverage is 61.53846% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.44%. Comparing base (518345c) to head (c6ad6ac).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
rpc/v8/class.go	0.00%	4 Missing ⚠️
rpc/v9/class.go	0.00%	4 Missing ⚠️
core/class_hash.go	60.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3976      +/-   ##
==========================================
- Coverage   74.58%   74.44%   -0.14%     
==========================================
  Files         462      462              
  Lines       41061    41079      +18     
==========================================
- Hits        30625    30582      -43     
- Misses       8335     8394      +59     
- Partials     2101     2103       +2     

Flag	Coverage Δ
jsonv2	77.15% <23.07%> (-0.08%)	⬇️

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

☔ View full report in Codecov by Harness.
📢 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Claude finished @rodrodros's task in 6m 47s —— View job

---

Re-review: bound gzip decoding

• Diff the new chore: address review commit
• Re-check the resolved threads (limit value, NoLimit, error wrapping, tests)
• Audit the new tests for flakiness / correctness
• Build + vet affected packages
• Post inline findings

Most of the previous round is genuinely addressed. Specifically verified:

• NoLimit is a real improvement over the magic value. The readLimit < NoLimit guard now reads as intent rather than as arithmetic defence, and all 8 opt-out call sites use it.
• The new tests close the gap I raised. TestGzip64DecodeCompressionBomb is the case that was missing — and the TotalAlloc assertion is the right shape, because it fails if someone later "simplifies" the check into a post-hoc len(io.ReadAll(gzipReader)). TestGzip64DecodeCorruptStream covers the propagation-through-LimitReader gap. One detail worth calling out as correct rather than accidental: the test imports klauspost/compress/gzip, not stdlib — with stdlib the assert.ErrorIs(err, gzip.ErrChecksum) would compare two distinct error values with identical messages and always fail.
• All 4 non-test call sites remain bounded, and no new Gzip64Decode callers slipped in.
• go build ./... and go vet are clean on every affected package (only cmd/juno fails, at link time, for missing Rust artifacts — unrelated).

Important

• The bound is still the open question — core/class.go:27. 16 MiB is 4× better than 4 MiB, but it doesn't clearly escape the original arithmetic. The sequencer cap (~4,089,446 bytes) is on the compressed class object, so a gzip ratio of just 4.2× on the program puts a legitimate class over 16 MiB — and Cairo 0 program JSON routinely compresses 4–8×. That matters only because Hash() (sync) and core2sn (starknet_call/estimateFee/traceTransaction on historical blocks) share the constant, where a false positive is an unrecoverable stall rather than a rejected request. Since Cairo 0 declares have been off since v0.11 the true max is finite and measurable; either record it in the comment, or give the trusted paths a separate, much larger bound. Splitting is safe: nothing reaches the DB without passing Hash() first, so a looser bound downstream adds no exposure.

Nit

• core/class_hash.go:17 — the new wrap says "compressing Cairo Zero class"; it's decompressing. The wrap exists for diagnosability, so the verb should match.
• compression_test.go:128 — the comment states the reader can't reach the footer, but the test passes precisely because it can. The +1 read is also what preserves trailer validation at exactly-the-limit (Close() does not re-validate), which is worth documenting correctly.
• compression.go:154 — no doc on the new parameter (the bound is inclusive; NoLimit opts out), and the over-limit error is still unexported, so tests string-match it and RPC handlers can't errors.Is it.

On the two threads you pushed back on: agreed on the Close() one — the suggested diff had the same hole, and it's a hypothetical until readers get pooled. And int64 matching the io API is the right call; uint64 would just move the conversions.

Not verified

go test and go run are both blocked by the sandbox here (go build/go vet were permitted), so I could not run the new tests, nor decompress the core/testdata/cairo0_hash_regressions fixtures to report real Cairo 0 gzip ratios. The sizing argument above therefore rests on the sequencer-cap arithmetic, not on measurement. Adding Bash(go test) and Bash(go run) to --allowedTools would let me settle the 16 MiB question with data instead of inference.
· branch rdr/encode-fix

[github-actions]

Claude finished @rodrodros's task in 3m 48s —— View job

---

Re-review: bound gzip decoding

• Diff the new chore: minor improveents commit
• Re-check previously-raised threads
• Audit the newly-added utils/async + utils/queue files
• Build + vet affected packages
• Post inline findings

The gzip work is done — but this push also added two unrelated packages that don't compile, so go build ./... is currently red on this branch.

Blocker

utils/async/ and utils/queue/ (~230 new lines, no callers anywhere, no tests) arrived in a commit titled chore: minor improveents on a PR titled "bound gzip decoding". They look like a WIP branch picked up by accident, and they break the build three ways:

1. utils/async/async_test.go is a 0-byte file → expected 'package', found 'EOF'. Fails go build, go vet, and go test for the package.
2. async.go:108 — cannot range over e.queue (variable of type *queue.Queue[IdentifiedAction[R]]). Looks like leftover code from when queue was a channel (the struct doc still calls it one).
3. queue.go:15-22 — New never sets size, so almostFull()/nextPosition() do % 0. Every Push panics with an integer divide-by-zero, for every constructor argument. No test file, so nothing caught it.

Cleanest resolution is to git rm all three and land them separately once they build and have tests. If they're meant to stay, the other findings below apply.

Important

Only relevant if utils/async/utils/queue stay in this PR:

• queue.go:51-60 — Len/Cap read start/end/size/buffer with no lock on a type whose doc says "safe for concurrent use"; -race will fire on any concurrent Len+Pop. Separately, Len() returns q.size rather than 0 for an empty queue, because the start == end case falls into the wraparound branch.
• async.go:54-61 — Run only waits on ctx.Done(); no worker is ever started and doAction is never called, so Go + Await blocks until the context expires. doAction/workersAvailable are dead code that unused will flag.

Nit

• compression.go:155 — the new doc starts with Gzip64Encode but sits on Gzip64Decode, so godoc renders "Gzip64Encode decompress data"; also retured → returned.

Verified as resolved from the previous round

• "compressing Cairo Zero class" → "decompressing" (@brbrr's pre-merge ask) — fixed at core/class_hash.go:17.
• The misleading corrupt-stream test comment is gone; both subtests now read correctly.
• The Gzip64Decode bound itself is unchanged and correct: the +1 read distinguishes at-limit from over-limit, the readLimit < NoLimit guard prevents overflow, and all four non-test call sites (core/class_hash.go, adapters/core2sn/class.go, rpc/v8/class.go, rpc/v9/class.go) pass MaxDeprecatedClassProgramSize. No unbounded decode paths remain.
• On the 16 MiB question — you've measured that the largest real Cairo 0 class is well under 4 MiB, which is the data point I was asking for. That settles it; the only thing I'd still suggest is putting that sentence in the comment next to the constant, so the next reader doesn't redo this thread. Not blocking.

Not verified

go test is blocked by the sandbox in this environment (go build and go vet were permitted), so the divide-by-zero in queue.New and the range failure in async.go are confirmed by compiler output and by reading, not by execution. The compression tests themselves I could not run.
· branch rdr/encode-fix

[github-actions]

Claude finished @rodrodros's task in 3m 58s —— View job

---

Re-review: bound gzip decoding

• Diff the force-pushed chore: minor improvements commit
• Confirm utils/async / utils/queue are gone and the build is green again
• Re-check previously-raised threads
• Build + vet affected packages
• Post findings

The blocker from the last round is gone. The force-push dropped utils/async/async.go, utils/async/async_test.go and utils/queue/queue.go, so the PR is back to the 9 files it should be (+174/-14) and go build ./... is clean again — the only failure is cmd/juno at link time for missing Rust artifacts (-ljuno_starknet_rs, -lbz2), which is environmental and unrelated. go vet passes on utils/compression, core, adapters/core2sn, rpc/v8, rpc/v9, rpc/v10.

Everything the async/queue findings covered (divide-by-zero in queue.New, range over a *queue.Queue, the empty test file, the unlocked Len/Cap, the no-op Run) is moot now — none of that code is on the branch.

No blockers

Re-verified on the current head:

• All four non-test Gzip64Decode call sites are bounded — core/class_hash.go:13, adapters/core2sn/class.go:119, rpc/v8/class.go:70, rpc/v9/class.go:76 — and the eight opt-out sites all use compression.NoLimit. I also checked for decode paths that bypass this helper: the only other gzip.NewReader calls in the tree are jsonrpc/http_test.go:370 and clients/gateway/test_gateway.go:40, both test-only. No unbounded production decode remains.
• The bounding logic is correct. The +1 read distinguishes at-limit from over-limit, readLimit < NoLimit prevents the increment from wrapping negative, and the check is against len(decompressedBytes) after a bounded read — so an over-limit stream is rejected without materialising the expansion.
• decompressing Cairo Zero class — @brbrr's pre-merge ask is fixed at core/class_hash.go:15, and the sibling unmarshalDeprecatedCairoProgram error got wrapped too.
• The 16 MiB question is settled by your measurement that the largest real Cairo 0 class is well under 4 MiB, plus the custom-network headroom rationale. Only remaining suggestion is to put that sentence in the comment next to the constant so the next reader doesn't reopen the thread — not blocking.
• The corrupt-stream test comment now reads correctly, and the test still imports klauspost/compress/gzip rather than stdlib, which is what makes assert.ErrorIs(err, gzip.ErrChecksum) meaningful.

Nit

• compression.go:155 — retured → returned, decompress → decompresses, and the doc could say the bound is inclusive.

Two earlier nits are still open but I won't push on either: the over-limit error is an unexported fmt.Errorf, so tests string-match it and the RPC declare paths can't errors.Is it; and the constant's comment doesn't record the measured max. Both are fine to leave.

Not verified

go test is blocked by the sandbox here (go build and go vet were permitted), so I confirmed the build state from compiler output and read the new tests rather than executing them — including the runtime.MemStats assertion in TestGzip64DecodeCompressionBomb, whose 8 MiB headroom I reasoned about but did not measure. Adding Bash(go test) to --allowedTools would close that gap.
· branch rdr/encode-fix