Skip to content

fix(prover): align manifest transaction decoding - #60

Merged
davidtaikocha merged 1 commit into
mainfrom
fix/align-manifest-decoding
Sep 29, 2026
Merged

davidtaikocha merged 1 commit into
mainfrom
fix/align-manifest-decoding

Conversation

@smtmfft

@smtmfft smtmfft commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • align gaiko2's Shasta manifest decoder with taikoxyz/taiko-mono#22173
  • bump taiko-geth to the v2.7.0 commit used by that change (8e98046bfd6a)
  • reject transaction encodings that the execution engine cannot decode before retaining a source
  • add all 136 canonical manifest-decoding vectors plus whole-source fallback and source-isolation coverage

The validation is intentionally not fork-gated. If any transaction in a decoded source has an incompatible encoding, the entire source follows the existing default-source path; signer recovery and execution validity remain the execution engine's responsibility.

No custom zlib behavior or decompression cap is added. The shared fixtures cover complete/truncated streams, dictionaries, trailing bytes and second streams, strict RLP framing, numeric boundaries, typed parity, and blob sidecar wrappers.

Validation

  • RED against the previous decoder: 37 canonical F9 cases retained incompatible transactions; the whole-source fallback assertion also failed
  • go test -count=1 ./internal/prover -run '^TestManifestVectors' — passed, all 136 vectors
  • go test -count=1 ./internal/prover — passed
  • go test -count=1 ./... — passed, all five packages
  • go vet ./... — passed
  • go mod verify — passed
  • repository-wide gofmt -l — clean
  • go test -race -count=1 ./cmd/gaiko2 ./internal/api ./internal/tee — passed
  • git diff --check — passed
  • independent adversarial review — passed with no findings
  • independent behavioral verification — passed, including representative zlib/framing/overflow/sidecar cases and existing blob-integrity hard errors

This PR only changes source and tests. It does not build, register, or deploy a new TEE image; the resulting enclave measurement will be handled by the release flow.


Note

Medium Risk
Changes L2 derivation/prover manifest handling on the critical path from blob data to executed blocks; behavior shifts for previously retained hostile encodings, though extensive vectors and default-source fallback limit blast radius.

Overview
Aligns Shasta blob manifest decoding with taiko-mono by bumping the taiko-geth replace to commit 8e98046bfd6a and adding post-RLP transaction encoding checks in decodeManifestPayload.

New validateManifestTransactions rejects sources whose txs use unsupported types, out-of-range numeric fields, invalid legacy v / typed parity, or blob sidecar wrappers—one bad tx invalidates the whole source, which then follows the existing default-manifest path (same as other decode failures). Signer recovery and execution validity are still left to the engine.

Adds 136 canonical derivation vectors (manifest_cases.json plus TestManifestVectors and follow-on tests for whole-source defaulting, neighbor preservation, forced-inclusion multi-block behavior, and late invalid txs).

Reviewed by Cursor Bugbot for commit a88ae8b. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved: Cursor Bugbot completed successfully with no findings that need human review. No additional reviewers were assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router

@davidtaikocha davidtaikocha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against taiko-mono#22173 (merge commit aa3a24f5). Approving.

Verified

  • internal/prover/transaction_encoding.go has the same logic as upstream packages/taiko-client/driver/chain_syncer/event/derivation/transaction_encoding.go. Only names, blank lines and doc comments differ.
  • The call site is equivalent to upstream source_fetcher.go:
    • every validation failure falls back to the default source (upstream also logs a warning);
    • host-integrity blob errors stay hard errors;
    • decodeManifestPayload is still the only place source bytes are decoded.
  • testdata/derivation_vectors/manifest_cases.json is byte-identical to upstream (sha256 c4c040c6…4f62d223).
  • taiko-geth 4e001283f488 → 8e98046bfd6a is the v2.7.0 tag and a direct child of the old pin. The diff is only CHANGELOG.md and .release-please-manifest.json, so none of the linked go-ethereum packages change.
  • go test ./internal/prover passes on go1.24.11, go1.26.0 and go1.27.1, with 142/142 vector tests each time. The compress/zlib and flate reader code is effectively identical across those versions. ego's 1.26.1 was not run directly.
  • Mutation testing: removing any check, or moving its bound by one, fails the vector tests. The only exceptions are branches the geth decoder already enforces: the default: type arm, v < 35, and Sign() < 0.

Non-blocking follow-ups (test-only, so no enclave measurement impact)

  1. Positional coverage. Every invalid vector puts the bad tx last in the last block. A regression that checks only the last tx or block, or the classic err = overwrite in the loop, would pass all 383 prover tests. These cases would catch it, ideally added upstream in generate.go so all three decoders get them:
    • bad tx then valid tx in the same block
    • bad block 0 then valid block 1
    • sidecar tx then valid tx
  2. EIP-155 legacy overflow. All legacy overflow vectors use v=27, so EIP-155 legacy txs aren't exercised for the fee, value and r/s bounds.
  3. Forced-inclusion variants. Upstream's isolation test also runs type2_parity_2, type2_control and late_bad_transaction as forced-inclusion sources. Here only two_valid_blocks runs as forced.
  4. Nits:
    • The doc comment on validateManifestTransactionEncoding drops upstream's precise wording ("the Rust manifest decoder and alethia-reth's execution transaction decoder"). In gaiko2 the manifest decoder is geth, which accepts all of these encodings, so the generic wording is misleading.
    • The fixture could record its provenance (upstream commit + sha256).
    • ...DefaultWholeSourceAndPreserveNeighbor doesn't put two sources in one proposal.
    • vector := vector is redundant on Go ≥ 1.22.

Rollout notes

  • No fork gate, consistent with upstream. Upstream validated local driver behaviour, not historical replay. Before registering the new MRENCLAVE, it's worth scanning mainnet/Hoodi Shasta proposals for any source that would now fall back to the default.
  • raiko2 pin. raiko2 release/providers.toml (#268) pins gaiko2 a88ae8b4, this PR's head. Its release tooling runs git clone then git checkout <sha>. Since this PR is being squash-merged, please keep the fix/align-manifest-decoding branch until raiko2 is re-pinned to the squash commit on main. The tree is identical, so the MRENCLAVE is the same.

@davidtaikocha
davidtaikocha merged commit a666927 into main Sep 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants