Skip to content

feat(protocol,taiko-client): add tdx verifiers#21724

Open
Gohnnyman wants to merge 12 commits into
mainfrom
feat/tdx
Open

feat(protocol,taiko-client): add tdx verifiers#21724
Gohnnyman wants to merge 12 commits into
mainfrom
feat/tdx

Conversation

@Gohnnyman

Copy link
Copy Markdown
Collaborator

What's new

Adds TDX (Trust Domain Extensions) prover support for the Shasta hardfork, pairing a TDX proof with an existing ZK proof for on-chain verification.

Changes

protocol

  • AzureTdxVerifier — on-chain registry + verifier; registerInstance runs Azure vTPM + Automata DCAP attestation, verifyProof checks 89-byte ECDSA proof against registered instance
  • TdxAndZkVerifier — compose verifier requiring [ZK(5/6), TDX(7)] sub-proofs in ascending VerifierID order
  • ComposeVerifier.VerifierType extended with TDX_RETH = 7
  • docs/tdx_deployment.md

taiko-client

  • TdxZkComposeProofProducer — fetches TDX and ZK proofs from raiko2 in parallel

@github-actions

github-actions Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

🐋 DeepSeek Code Review

🔴 Critical Issues

None. No identified vulnerability that directly leads to fund loss or chain halts.

🟡 Warnings

  • AzureTdxVerifier._validateAttestationOutput – missing bounds check on PCR index
    The function writes into a fixed-size pcrs[24] array using _attestation.pcrs[i].index without checking it is < 24. A malicious caller could supply a crafted attestation with an out-of-range index, causing a panic-based revert. While it is unlikely with a valid Azure attestation, a bounds check (require(index < 24)) should be added for correctness and to avoid unexpected reverts.

  • Attestation output offsets must be verified
    Both AzureTdxVerifier and GcpTdxVerifier parse the Automata DCAP V4 output at hardcoded offsets (e.g., TEE_TCB_SVN at 11, MRTD at 147, RTMR0 at 339, REPORT_DATA at 531). If these offsets do not match the actual Automata DCAP output format for TDX V4 quotes, the verifier would silently accept invalid attestations or reject valid ones. Thorough cross-checking with the Automata DCAP contract source and a set of real attestations is required before production.

  • Go – missing context cancellation in parallel proof fetching
    TdxZkComposeProofProducer.RequestProof and Aggregate use errgroup.Group (not errgroup.WithContext). If one sub-proof request fails, the other may continue to run until its own timeout, wasting resources. Use errgroup.WithContext to cancel the remaining goroutine on first error.

  • Misleading proof-generation state flags
    The TDX proof producer uses IsGethProofGenerated() and GethProofGenerated/GethProofAggregationGenerated fields (originally for SGX) to track whether a TDX proof has been generated. This is confusing and could lead to logical errors if these flags are later reused for both TEE types. The flags should be renamed to a generic TEE placeholder or the TDX-specific state stored separately.

  • Solidity version inconsistency
    TdxAndZkVerifier.sol uses ^0.8.24 while the new TDX verifier contracts use ^0.8.26. This is a minor consistency issue; use a uniform version across the project.

🔵 Suggestions

  • Add NatSpec documentation for all external/public functions in AzureTdxVerifier and GcpTdxVerifier (especially registerInstance and verifyProof).
  • Optimise _popcount24 with a bit-count library (e.g., Solady’s LibBit.popCount) to save gas.
  • Consider emitting an event for setTrustedParams in both verifiers (already done in GcpTdxVerifier but verify both).
  • In GcpTdxVerifier, the constant OFF_REPORT_DATA = 531 may be more readable as OFF_RTMR0 + 4*RTMR_LEN to reflect its derivation.
  • The AzureTdxVerifier’s _validateAttestationOutput could avoid a full 24-element array allocation by only storing and comparing the required PCRs from the attestation.
  • In the Go code, replace the manual error join in TdxZkComposeProofProducer.RequestProof with errors.Join(err, zkProofError) for clarity.

🟢 What Looks Good

  • The composite verifier pattern (TdxAndZkVerifier, ComposeVerifier) cleanly separates concerns and respects ascending VerifierType ordering (ZK before TDX).
  • The verifyProof function correctly binds the signature to the verifier address, instance, and chain ID via LibPublicInput, preventing cross-verifier replay.
  • Instance registry with expiry (INSTANCE_EXPIRY = 365 days) and nonce-based attestation replay protection is well designed.
  • The deployment scripts are thorough, idempotent, and include auto-detection of TCB data from Intel APIs.
  • Tests cover happy path and error cases for both verifier variants, including a real bootstrap quote prefix.
  • The Go TdxZkComposeProofProducer correctly encodes sub-proofs in [ZK, TDX] order, matching the on-chain requirement.

Automatically triggered on PR update • model: deepseek-v4-pro

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ee57aa252b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


g.Go(func() error {
if s.Dummy {
proofType = s.ProofType

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Return concrete ZK proof type in TDX dummy flow

When s.Dummy is enabled, RequestProof stores s.ProofType (zk_any) into ProofResponse.ProofType, but Aggregate only accepts verifier IDs for risc0/sp1 and rejects unknown types. In a TDX+ZK setup running with dummy proofs (common in local/dev experiments), aggregation will consistently fail with unknown ZK proof type from raiko: zk_any, so no proof batches can be submitted.

Useful? React with 👍 / 👎.

/// @param _instance The address to check
/// @return True if the address has at least one live instance entry.
function isInstanceRegistered(address _instance) external view returns (bool) {
return addressRegistered[_instance];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Report only live instances in registration status API

isInstanceRegistered is documented as checking whether an instance is currently registered and non-expired, but it returns addressRegistered, which is an "ever attested" flag that is never cleared in deleteInstances and does not consider expiry. This can cause operators/automation to treat deleted or expired prover keys as active and skip re-registration, resulting in avoidable on-chain proof verification failures later.

Useful? React with 👍 / 👎.

@codecov

codecov Bot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 34.52381% with 220 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.75%. Comparing base (12de017) to head (0c23dc7).
⚠️ Report is 45 commits behind head on main.

Files with missing lines Patch % Lines
...er/proof_producer/tdx_zk_compose_proof_producer.go 45.10% 94 Missing and 7 partials ⚠️
...client/prover/proof_producer/tdx_proof_producer.go 20.33% 94 Missing ⚠️
packages/taiko-client/prover/init.go 5.26% 15 Missing and 3 partials ⚠️
...ient/prover/proof_submitter/transaction/builder.go 50.00% 6 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
packages/taiko-client/prover/config.go 75.00% <100.00%> (+0.39%) ⬆️
...es/taiko-client/prover/proof_producer/interface.go 14.28% <ø> (ø)
...ient/prover/proof_submitter/transaction/builder.go 64.77% <50.00%> (ø)
packages/taiko-client/prover/init.go 62.33% <5.26%> (-7.52%) ⬇️
...client/prover/proof_producer/tdx_proof_producer.go 20.33% <20.33%> (ø)
...er/proof_producer/tdx_zk_compose_proof_producer.go 45.10% <45.10%> (ø)

... and 4 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49efe2a...0c23dc7. Read the comment docs.

🚀 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.

@davidtaikocha

davidtaikocha commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Will test in our devnet once the changes to raiko2 are finalized and we can actually deploy, put the do-not-merge tag here for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants