Skip to content

refactor(pxe): avoid expensive toTx() call when computing tx hash#23136

Merged
nchamo merged 3 commits into
merge-train/fairiesfrom
nchamo/to-tx-improvement
May 11, 2026
Merged

refactor(pxe): avoid expensive toTx() call when computing tx hash#23136
nchamo merged 3 commits into
merge-train/fairiesfrom
nchamo/to-tx-improvement

Conversation

@nchamo
Copy link
Copy Markdown
Contributor

@nchamo nchamo commented May 10, 2026

Summary

  • Replace (await txProvingResult.toTx()).txHash with await Tx.computeTxHash({ data: publicInputs }) in proveTx, and remove the corresponding TODO.

Why this is safe and worth doing

TxProvingResult.toTx() passes this.publicInputs directly as data to Tx.create (proven_tx.ts:28-29). Tx.create computes the hash via Tx.computeTxHash(fields) (tx.ts:159), which only reads fields.data (tx.ts:151-155). So calling Tx.computeTxHash({ data: publicInputs }) produces the same hash while skipping work that doesn't contribute to it:

  • Walking the nested PrivateCallExecutionResult tree to collect and sort contract class logs
  • Constructing a full Tx (with proof, logs, calldata) only to read one field and discard the rest

A short version of this rationale lives inline in pxe.ts.

@nchamo nchamo self-assigned this May 10, 2026
@nchamo nchamo force-pushed the nchamo/to-tx-improvement branch from 295dc4c to 4369cc0 Compare May 10, 2026 23:10
@nchamo nchamo requested review from dbanks12 and mverzilli and removed request for mverzilli May 11, 2026 11:37
@dbanks12
Copy link
Copy Markdown
Contributor

@nchamo I think this is a bit leaky. PXE needs to know a bit about how tx hashes get computed (use public inputs, pass in via the data field).

Maybe we could add a simple helper to the TxProvingRequest instead?
image

@dbanks12
Copy link
Copy Markdown
Contributor

That being said, this PR is a very small but meaningful change. So if my suggestion proves to be a rabbit hole, let's just merge this

@nchamo
Copy link
Copy Markdown
Contributor Author

nchamo commented May 11, 2026

Like the suggestion @dbanks12 , makes a lot of sense

@nchamo nchamo merged commit cd903d9 into merge-train/fairies May 11, 2026
14 checks passed
@nchamo nchamo deleted the nchamo/to-tx-improvement branch May 11, 2026 18:35
@AztecBot
Copy link
Copy Markdown
Collaborator

✅ Successfully backported to backport-to-v4-next-staging #23142.

benesjan added a commit that referenced this pull request May 12, 2026
BEGIN_COMMIT_OVERRIDE
fix(sequencer): bounded sweep instead of event scan for governance
proposal check (#22989)
fix(sequencer): bounded sweep instead of event scan for governance
proposal check (#22989) (#23001)
chore: route backport CI failure notifications to #backports channel
(#21779)
fix: (A-589) epochs l1 reorgs test (#20999)
chore: Accumulated backports to v4 (#23065)
fix(bb-prover): use temp directory for avm_verify (#23138)
chore: notify on v4-next sync (#23139)
refactor(pxe): use findLeavesIndexes for read request verification
(#23123)
refactor(pxe): skip storage reads for never-updated contracts (#23131)
fix(pxe): skip registerContractFunctionSignatures when no public fns
(#23134)
chore: Update Noir to nightly-2026-04-15 (#22572)
chore: Update Noir to nightly-2026-04-16 (#22594)
chore: Update Noir to nightly-2026-04-17 (#22633)
chore: Update Noir to nightly-2026-04-23 (#22653)
chore: Update Noir to nightly-2026-04-28 (#22755)
chore: Update Noir to nightly-2026-05-01 (#22836)
chore: Update Noir to nightly-2026-05-05 (#22911)
chore: Update Noir to nightly-2026-05-11 (#23023)
chore: backport noir sync PRs to backport-to-v4-next-staging (#23148)
refactor(pxe): prefetch updated class id hints per unique contract
(#23130)
chore(aztec-nr): Public self constructor function to prevent static byte
code size blow up (#23062)
chore: merge v4 into backport-to-v4-next-staging (#23140)
chore(aztec-nr): Public self constructor function (backport #23062)
(#23156)
refactor(pxe): avoid expensive toTx() call when computing tx hash
(#23136)
END_COMMIT_OVERRIDE
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