chore: Accumulated backports to v4-next#21654
Merged
Conversation
Threads ChainInfo through some methods that externals needed to protect against replay attacks. While at it, protects our own entrypoints from them too Closes: #21572 --------- Co-authored-by: Jan Beneš <janbenes1234@gmail.com>
3 tasks
Calling `Array.from({length})` allocates length immediately. We were
calling this method in the context of deserialization with untrusted
input.
This PR changes it so we use `new Array(size)` for untrusted input. A
bit less efficient, but more secure.
3 tasks
…shipWitness (#21472) As I was going through TODOs I found this ancient TODO of mine (from November 2023): ``` * Note: This function returns the membership witness of the nullifier itself and not the low nullifier when * the nullifier already exists in the tree. This is because the `getPreviousValueIndex` function returns the * index of the nullifier itself when it already exists in the tree. * TODO: This is a confusing behavior and we should eventually address that. ``` In this PR i handle it by instead throwing an error in this scenario. This doesn't modify the interface so it was not urgent to do but felt like it makes sense to do now anyway so I went with it. (Pinged this PR to alpha team) ## Summary - `getLowNullifierMembershipWitness` now throws a descriptive error when the queried nullifier already exists in the tree, instead of silently returning the nullifier's own witness (which is wrong for a non-inclusion proof) - Removes the long-standing TODO about confusing behavior (open since Nov 2023) - Adds `@throws` JSDoc to both the interface and implementation - Adds unit tests for the throw and undefined-return paths ## Context Previously, `getPreviousValueIndex` returns the nullifier's own index when `alreadyPresent: true`. The method just logged a warning and returned that witness anyway. The Noir circuit would catch this implicitly (the `low < target` assertion fails), but the error surfaced as a cryptic circuit assertion rather than a clear "nullifier already exists" message. ## Test plan - Unit tests added in `server.test.ts` covering both the throw-on-exists and return-undefined paths - All 34 existing tests in `server.test.ts` continue to pass - Build, format, and lint pass 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This was referenced Mar 17, 2026
## Motivation
Errors during `readMessage` (oversized snappy responses, corrupt data,
etc.) were caught and silently converted to `{ status: UNKNOWN }` return
values instead of re-throwing. Since `sendRequestToPeer` only calls
`handleResponseError` in its own catch block, none of these errors
resulted in peer penalties. The request was simply retried with another
peer, allowing a malicious peer to waste bandwidth indefinitely.
## Approach
Re-throw non-protocol errors from `readMessage` so they propagate to
`sendRequestToPeer`'s catch block where `handleResponseError` applies
peer penalties. Additionally, introduce a dedicated
`OversizedSnappyResponseError` class so oversized responses get a
harsher `LowToleranceError` penalty (score -50, banned after 2 offenses)
instead of falling through to the generic `HighToleranceError`
catch-all.
## Changes
- **p2p (reqresp)**: Changed `readMessage` catch block to only return
status for `ReqRespStatusError` and re-throw all other errors, so they
reach `handleResponseError` for penalization
- **p2p (encoding)**: Added `OversizedSnappyResponseError` class for
explicit categorization
- **p2p (reqresp)**: Added `OversizedSnappyResponseError` handling in
`categorizeResponseError` with `LowToleranceError` severity
I simply asked Claude to go through our code and find bugs, and it found this ## Summary - Fixes an out-of-bounds array access in `extract_property_value_from_selector` when `PropertySelector.offset > 0`. The formula `31 + offset - i` produces index >= 32 at `i = 0`; corrected to `31 - offset - i`. - Adds a regression test exercising a nonzero offset. ## Note The bug was dormant -- every `PropertySelector` in the codebase uses `offset: 0` (the macro hardcodes it). But anyone trying to use sub-field byte selection would hit a runtime panic.
…21520) ## Motivation When building multiple blocks within a single checkpoint, the `CheckpointBuilder` was creating a new `PublicContractsDB` instance for each block. This meant that contracts deployed in an earlier block within the same checkpoint were not visible to subsequent blocks, causing calls to newly deployed contracts to fail. ## Approach Move the `PublicContractsDB` instance to be a persistent field on `CheckpointBuilder`, initialized once in the constructor and shared across all blocks in the checkpoint. Wrap block building in checkpoint/commit/revert semantics on the contracts DB so that failed blocks don't leak state. ## Changes - **validator-client**: Promote `contractsDB` from a local variable in `makeBlockBuilderDeps` to a class field on `CheckpointBuilder`. Wrap `buildBlock` in `createCheckpoint`/`commitCheckpoint`/`revertCheckpoint` calls on the contracts DB. - **validator-client (tests)**: Add tests verifying that the contracts DB checkpoint lifecycle is correctly managed across successful and failed block builds. - **end-to-end (tests)**: Add e2e test that deploys a contract and calls it in separate blocks within the same slot, validating cross-block contract visibility within a checkpoint. Fixes A-658 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
AztecBot
requested review from
dbanks12,
fcarreiro and
sirasistant
as code owners
Fixing issue reproted by @just-mitch on [slack](https://aztecprotocol.slack.com/archives/C04PUD9AA4W/p1773715408859609). ## AI Summary Fixes a TypeScript compilation error when running `aztec-builder codegen` on contracts where every function is void (most notably, a blank `#[aztec] contract Main {}`). The `#[aztec]` macro injects lifecycle functions like `process_message` and `sync_state` into every contract. These are void, so the Noir compiler outputs `"return_type": null` for them. Our `NoirFunctionAbi` type only accepted a non-null object for `return_type`, which caused a type error on the `as NoirCompiledContract` cast in the generated TS. For contracts with at least one non-void function, TypeScript infers the JSON array element type as a union (`null | { abi_type, visibility }`), which has enough overlap with the expected type for the `as` cast to succeed. But when *every* function is void, the inferred type is just `null` — zero overlap — so the cast fails. The runtime code in `contract_artifact.ts` already handled the `null` case correctly. Only the type definition was out of sync with the compiler's actual output. Repro: https://github.com/just-mitch/mytoken ## Test plan - Verified `yarn build` passes with no new type errors - Cloned the repro, confirmed the TS error, patched `node_modules/@aztec/stdlib` with the fix, confirmed clean compilation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
EmbeddedWallet now simulates before sending in order to estimate gas and capture autwitness data. This PR also adds validation to captured authwitnesses from offchaineffects (ensuring the inner hash matches the emitted preimage) and fee payer handling to kernelless simulations Closes: https://linear.app/aztec-labs/issue/F-402/estimate-gas-limits-for-a-tx-if-not-provided-by-the-caller Closes: https://linear.app/aztec-labs/issue/F-403/compute-gas-limits-for-private-only-txs --------- Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
… setups (backport #21603) (#21659) ## Summary Backport of #21603 to v4-next. In an HA setup, two nodes sharing the same validator keys would ignore each other's block proposals because `validateBlockProposal` returned early for self-proposals. This PR: 1. **Removes self-filtering** in `validateBlockProposal` so HA peers process proposals from their own validator keys 2. **Reorders checkpoint_proposal_job** to sign before syncing to archiver, preventing stale blocks in HA setups 3. **Adds shared slashing protection** for testing via `createSharedSlashingProtectionDb` (in-memory impl for v4-next) 4. **Threads `slashingProtectionDb`** through validator creation chain for HA test injection ### Conflict resolution The cherry-pick had conflicts in `validator.ts` due to different variable naming on v4-next (`validatorKeyStore`/`haSigner` vs `slashingProtectionSigner` on next). Resolved by adapting the new `slashingProtectionDb` branch to use v4-next's naming pattern. ### Build adaptation v4-next doesn't have `LmdbSlashingProtectionDatabase`, `HASignerMetrics`, or `CreateLocalSignerWithProtectionDeps` that exist on `next`. Created `InMemorySlashingProtectionDatabase` as a lightweight alternative for testing, and simplified `createSignerFromSharedDb` to work with v4-next's 2-arg `ValidatorHASigner` constructor. ## Commits 1. **Cherry-pick with conflicts** — raw cherry-pick of 5eedb7d with conflict markers 2. **Conflict resolution** — resolve import and constructor conflicts in validator.ts 3. **Build fixes** — adapt factory functions for v4-next (in-memory DB, simplified deps) ClaudeBox log: https://claudebox.work/s/5fffc97ddc02fa23?run=1 --------- Co-authored-by: Jan Beneš <janbenes1234@gmail.com> Co-authored-by: Santiago Palladino <santiago@aztec-labs.com>
The pool should never reject a tx that passed validation. However, in case it does, we now add a warning and penalize the peer that sent us the invalid tx.
… (A-677) (#21747) ## Motivation The `sendRequests` method in the sequencer publisher correctly filters L1 publish requests by `lastValidL2Slot` to discard expired ones. However, gas and blob configs were extracted from the unfiltered request list, meaning expired requests' gas configurations leaked into the aggregated gas limit calculation. This could over-estimate gas and overpay for L1 transactions. Fixes A-677 ## Approach Changed the gas and blob config extraction to use the filtered `validRequests` list instead of the unfiltered `requestsToProcess` list, so only non-expired requests contribute to the aggregated gas limit. ## Changes - **sequencer-client**: Use `validRequests` instead of `requestsToProcess` when extracting `gasConfigs` and `blobConfigs` in `sendRequests` - **sequencer-client (tests)**: Added test verifying that expired requests' gas configs are excluded from the aggregated gas limit Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
…trap.sh build arg (#21748) ## Summary Backport of #21744 to v4-next. Replaces all occurrences of the dead `BOOTSTRAP_TO=yarn-project ./bootstrap.sh` env var with `./bootstrap.sh build yarn-project`, which actually calls `prep` then `make yarn-project`. ## Conflicts resolved - **bootstrap.sh**: v4-next had already removed the `BOOTSTRAP_TO` prefix but left bare `./bootstrap.sh` — resolved to `./bootstrap.sh build yarn-project` - **.claude/skills/backport/SKILL.md**: doesn't exist on v4-next (deleted in modify/delete conflict) ## Changes - `bootstrap.sh`: fix ci-docs build command - `container-builds/avm-fuzzing-container/src/Dockerfile`: fix build step - `yarn-project/CLAUDE.md`: update developer instructions - `yarn-project/.claude/skills/{fix-pr,rebase-pr}/SKILL.md`: update skill instructions ClaudeBox log: https://claudebox.work/s/e50c1cdccd277aa9?run=1 --------- Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com>
…ed, receipt.blockNumber)
…1134, #21072, #21186, #21189, #21229, #21228, #21234, #21639) (#21745) ## Summary Combined backport of 12 PRs to v4-next, cherry-picked in chronological merge order: 1. #20512 — docs: add delayedpublicmutable apiref, fix misc docs 2. #20379 — feat: custom message handlers in Aztec.nr 3. #20831 — feat!: make unused msg disco fns private 4. #21024 — feat: add compile-time size check for events and error code links 5. #21134 — chore: add warning on invalid recipients 6. #21072 — feat: add aztecaddress::is_valid 7. #21186 — chore: use returns `true` for boolean fns 8. #21189 — feat: add note hash and nullifier helper functions with domain separation 9. #21229 — docs: small delayedpubmut update 10. #21228 — test: restore pubmut tests 11. #21234 — fix: claim contract & improve nullif docs 12. #21639 — feat!: split compute note hash and nullifier to reduce hashing Each PR is a separate commit (raw cherry-pick with conflicts left in), followed by a single final commit that resolves all conflicts — making it easy to review the conflict resolution independently. ## Conflict resolution (last commit) - `delayed_public_mutable.nr` / `public_immutable.nr` — merged doc comments from both sides - `aztec.nr` macros — integrated custom message handler + split compute functions - `discovery/mod.nr` / `process_message.nr` — merged offchain inbox sync + custom handler dispatch + split compute - `constants.nr` / `constants_tests.nr` — added new domain separators - `Nargo.toml` — added custom_message_contract - `netlify.toml` — merged error code redirects - `auth_contract` — kept v4-next delay value - `note_metadata.nr` — doc comment reformat - `migration_notes.md` — merged migration notes from both sides - Deleted `traits.nr` (v4-next uses `traits/` directory) ClaudeBox log: https://claudebox.work/s/3145d1bd30977c20?run=1 --------- Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
## Summary Backports #21716 to v4-next via `backport-to-v4-next-staging`. Based on latest `backport-to-v4-next-staging` which has #21646 (gas estimations on send), so this PR contains only #21716's actual changes (NO_FROM pattern). Cherry-pick conflicts resolved: - `bot/src/factory.ts` — `AztecAddress.ZERO` → `NO_FROM` - `e2e_fees/account_init.test.ts` — `AztecAddress.ZERO` → `NO_FROM` - `docs/examples/ts/recursive_verification/index.ts` — imports + data loading updated - `docs/examples/ts/aztecjs_connection/index.ts` — `AztecAddress.ZERO` → `NO_FROM` in existing fee juice section, discarded duplicate sections from cherry-pick - `spartan/block_capacity.test.ts` — deleted (already removed on v4-next) ## Key changes - Introduces `NO_FROM` pattern replacing `AztecAddress.ZERO` sentinel for bypassing account contract entrypoint - Removes `SignerlessAccountContract` - Uses `DefaultEntrypoint` directly when `from: NO_FROM` - Deshrines `MulticallEntrypoint` from wallet internals --------- Co-authored-by: Gregorio Juliana <gregojquiros@gmail.com>
Collaborator
Author
|
🤖 Auto-merge enabled after 8 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
9 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.
BEGIN_COMMIT_OVERRIDE
feat: entrypoint replay protection (#21649)
feat: guard BoundedVec oracle returns against dirty trailing storage (#21589)
fix: add bounds when allocating arrays in deserialization (#21622)
feat: implement manual Packable for structs with sub-Field members (#21576)
fix(aztec-node): throw on existing nullifier in getLowNullifierMembershipWitness (#21472)
fix: use trait dispatch for array Packable::unpack in card_game_contract (#21683)
fix(p2p): penalize peers for errors during response reading (#21680)
fix: update nullifier non-inclusion test expectations after early oracle throw (backport #21600) (#21615)
fix(aztec-nr): fix OOB index with nonzero offset (#21613)
fix(builder): persist contractsDB across blocks within a checkpoint (#21520)
fix(stdlib): accept null return_type for void Noir functions (#21647)
feat: gas estimations on send (#21646)
fix(validator): process block proposals from own validator keys in HA setups (backport #21603) (#21659)
fix(p2p): penalize peer on tx rejected by pool (#21677)
fix(sequencer): fix checkpoint budget redistribution for multi-block slots (#21692)
feat: sync cache invalidation oracle (backport #21459) (#21730)
feat!: make AES128 decrypt oracle return Option (backport #21696) (#21735)
feat!: include init_hash in private initialization nullifier (backport #21427) (#21736)
fix(sequencer): extract gas and blob configs from valid requests only (A-677) (#21747)
chore: backport #21744 — replace dead BOOTSTRAP_TO env var with bootstrap.sh build arg (#21748)
refactor: revert remove assert_bounded_vec_trimmed (#21758)
END_COMMIT_OVERRIDE