feat: merge-train/fairies - #25215
Merged
Merged
Conversation
…ulator (#24968) ## Problem The public-tx simulator contract benchmarks (dashboard: `yarn-project/simulator/<contract tests>`) lost their "instructions executed" metric when the TS AVM simulator was removed only total duration and mana remained. This is a regression in our ability to track opcode-level work (e.g. the public `dispatch`/macro optimizations that `public_fns_with_emit_repro_contract` tracked in the benchmark). ## Root cause `recordEnqueuedCallSimulation` (which carried the per-call instruction count from the old TS interpreter) has no callers once simulation routes through the C++ simulator, so `TestExecutorMetrics.stopRecordingTxSimulation` summed an always-empty `enqueuedCalls` list and reported a flat `0`. `manaUsed` hit the same wall but was rescued earlier by reading the tx-level `gasUsed`; the instruction count was not, because the C++ result carried no such field. (For clarity: the public-bytecode-*size* metric was not lost — it moved to a dedicated noir-contracts bench and lives on the dashboard under `noir-projects/noir-contracts/artifact-size/<contract>/`.) ## Change - **C++:** add `total_instructions_executed` to `TxSimulationResult`, populated in both result builders (`simulate_fast_internal` and `simulate_for_witgen_internal`) from the AVM execution-id delta. The execution loop advances the execution id exactly once per instruction it processes — across all enqueued and nested calls — so the delta is the number of instructions executed, i.e. the number of AVM execution trace rows. This is a simulation-output field only: no proving / VK impact. - **TS:** thread it through `PublicTxResult` and record it as the tx-level `TestExecutorMetrics.totalInstructionsExecuted`, which the benchmark JSON already emits — so the dashboard series is repopulated with no dashboard-side change. - Added the field to the AVM fuzzer's fast-vs-witgen result comparison so the two paths can't silently diverge on it. The metric counts AVM execution steps: one per instruction the VM processes, including a terminal halting/failing instruction. For successful executions — which is all of the benchmarked contracts — this equals the number of instructions executed.
Fixes [F-548](https://linear.app/aztec-labs/issue/F-548/audit-147-walletdbstoreaccount-writes-non-atomically-partial-data-on) `WalletDB.storeAccount` wrote each account field (alias, type, sk, salt, signingKey) as a separate awaited `set`, so a failure partway through left a corrupted partial account: `retrieveAccount` would resolve with `signingKey: undefined`, or throw on a missing salt, and `listAccounts` would show a phantom entry. - Wrap all `storeAccount` writes in `store.transactionAsync()` so they commit atomically. On failure nothing persists; nested calls join the parent transaction. - Add an atomicity test that injects a write failure on the final field and asserts no trace of the account remains (no secret key, not listed, no alias entry). Red before the fix (retrieveAccount resolved with a corrupt partial account), green after. `deleteAccount` has the same non-atomicity (parallel deletes plus separate alias cleanup); left as a follow-up since this issue is scoped to account creation.
…25184) Fixes [F-619](https://linear.app/aztec-labs/issue/F-619/feature-export-noir-contract-globals-in-artifacts) Resolves #21761 - Preserve `#[abi(tag)]` global names in contract artifacts (`outputs.globals` entries are `{name, value}`; new `getNamedContractGlobals` accessor). Hard cutover: artifacts compiled before Noir exported names are rejected on load. - Re-pin standard contracts (new addresses/class IDs) and bump PXE data schema 13->14. Migration note included. - In `getGlobalsByTag` we collected names in a Map so global names that collide with Object.prototype properties (e.g. `toString`) are neither flagged as duplicates nor mishandled (`__proto__`) when building the record.
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See merge-train-readme.md.
This is a merge-train.