Skip to content

feat(archiver): validate contract instance addresses before storing#21787

Merged
PhilWindle merged 1 commit into
merge-train/spartanfrom
palla/archiver-validate-contract-address
Mar 19, 2026
Merged

feat(archiver): validate contract instance addresses before storing#21787
PhilWindle merged 1 commit into
merge-train/spartanfrom
palla/archiver-validate-contract-address

Conversation

@spalladino

Copy link
Copy Markdown
Contributor

Motivation

The archiver blindly stores contract instance data extracted from private logs without verifying that the claimed address matches the instance fields. While the p2p tx validator (#21771) catches this at the network layer, blocks received via req/resp or proposals skip that check. This adds a second line of defense at the storage layer.

Approach

Before storing contract instances, the archiver now recomputes the address from the instance fields via computeContractAddressFromInstance and filters out any with mismatched addresses. The check is skipped during delete operations since we need to remove instances regardless.

Changes

  • archiver: Validate contract instance addresses in updateDeployedContractInstances before storing, filtering out mismatches with a warning log
  • stdlib (tests): Add elapsed timing to computeContractAddressFromInstance test (~1.3ms per call)

Adds a defense-in-depth check in the archiver's data store updater that
recomputes contract instance addresses from their fields before storing
them, filtering out any instances with mismatched addresses.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@PhilWindle PhilWindle merged commit ce0db76 into merge-train/spartan Mar 19, 2026
23 checks passed
@PhilWindle PhilWindle deleted the palla/archiver-validate-contract-address branch March 19, 2026 14:16
AztecBot pushed a commit that referenced this pull request Mar 19, 2026
…21787)

## Motivation

The archiver blindly stores contract instance data extracted from
private logs without verifying that the claimed address matches the
instance fields. While the p2p tx validator (#21771) catches this at the
network layer, blocks received via req/resp or proposals skip that
check. This adds a second line of defense at the storage layer.

## Approach

Before storing contract instances, the archiver now recomputes the
address from the instance fields via
`computeContractAddressFromInstance` and filters out any with mismatched
addresses. The check is skipped during delete operations since we need
to remove instances regardless.

## Changes

- **archiver**: Validate contract instance addresses in
`updateDeployedContractInstances` before storing, filtering out
mismatches with a warning log
- **stdlib (tests)**: Add elapsed timing to
`computeContractAddressFromInstance` test (~1.3ms per call)
@AztecBot

Copy link
Copy Markdown
Collaborator

✅ Successfully backported to backport-to-v4-staging #21783.

github-merge-queue Bot pushed a commit that referenced this pull request Mar 20, 2026
BEGIN_COMMIT_OVERRIDE
feat(p2p): add tx validator for contract instance deployment addresses
(#21771)
fix: always deploy IRM for testnet (#21755)
fix: avoid mutating caller's array via splice in snapshot sync (A-718)
(#21759)
chore: update network logs skill (#21785)
feat(archiver): validate contract instance addresses before storing
(#21787)
fix: ensure no division by 0 (#21786)
feat: support private fork releases via ci-release (#21778)
fix: restrict scenario deployments to only nightly (#21798)
fix(stdlib): zero-pad bufferFromFields when declared length exceeds
payload (#21802)
test(protocol-contracts): verify max-size bytecode fits in contract
class log (#21818)
fix: wire BOT_DA_GAS_LIMIT through helm/terraform for staging-public
(#21809)
fix: remove jest-mock-extended from worker processes + fix
parallelize_strict silent failures (#21821)
fix(archiver): throw on duplicate contract class or instance additions
(#21799)
chore: remove broadcasted function events (#21805)
fix: sync dateProvider from anvil stdout on every mined block (#21829)
fix(sequencer): use wall-clock time instead of L1 block timestamp for
slot estimation (#21769)
fix: use correct EthCheatCodes method name in epochs_missed_l1_slot test
(#21848)
feat(p2p): add tx validator for contract class id verification (#21788)
feat: publisher funding (#21631)
feat: batch chonk verifier TS integration (#21823)
fix(sequencer): remove l1 block timestamp check (#21853)
fix: use local IVC inputs for batch_verifier bench test (#21857)
fix(p2p): centralize gossipsub penalization and fix inconsistencies
(#21863)
chore: publish GitHub releases to AztecProtocol/barretenberg (#21775)
END_COMMIT_OVERRIDE
spalladino added a commit that referenced this pull request Mar 26, 2026
## Summary
Merges v4 branch into backport-to-v4-next-staging to bring in 17 commits
from v4 that were missing.

Key changes from v4:
- fix: verify accumulated pairing points in native ChonkVerifier
(#21975)
- chore: Properly compute finalized block (#21795, #21850)
- feat(archiver): validate contract instance addresses before storing
(#21787)
- feat(p2p): add tx validator for contract instance deployment addresses
(#21771)
- Various accumulated backports and bug fixes

## Conflicts resolved
6 files had merge conflicts, all resolved:
- `archiver/src/factory.ts` — kept `ContractClassPublicWithCommitment`
type from staging
- `archiver/src/modules/data_store_updater.ts` — merged imports from
both branches
- `archiver/src/store/kv_archiver_store.test.ts` — took v4's richer
describe block structure, adapted to singular `addProposedBlock` API
- `aztec-node/src/aztec-node/server.ts` — kept staging's block 0 check
and `getWorldState` method name
- `stdlib/src/tx/validator/error_texts.ts` — kept staging's contract
class error constants
- `world-state/src/synchronizer/server_world_state_synchronizer.ts` —
kept verbose log level

## Test plan
- CI should verify the merge compiles and tests pass
- Once merged, this branch can be merged into the backport-to-v4-next
branch

ClaudeBox log: https://claudebox.work/s/0150633f06bfb379?run=1
github-merge-queue Bot pushed a commit that referenced this pull request Apr 13, 2026
### Changes

During address derivation, the AVM assumes that all input public keys
are non-infinite points, and hardcodes an `is_infinity` flag as zero. In
reviewing how address derivation changes now that noir no longer uses a
flag, we [found
that](https://github.com/AztecProtocol/aztec-packages/pull/22393/changes/BASE..6ce45576564c12e39a4a0ea138f33e11def435f3#r3063596289)
we actually couldn't rely on that assumption (I thought we could due to
#7529 and comments around the repo). This PR explicity checks all keys
are non-infinity.

Edit: Some keys are checked in the [private
context](https://github.com/AztecProtocol/aztec-packages/blob/e88ecdf581dbb7ade7bdc883d7da2ba86137f8e0/noir-projects/aztec-nr/aztec/src/context/private_context.nr#L778),
but since this isn't in a protocol contract/the rollup circuits, we
still require this change for the avm!

### Notes

The DoS (for before or after removing the `inf` flag from noir) seems to
be protected against at the archiver level
(#21787) but this is
an explicit, constrained solution.

Before removing the `inf` flag, the AVM hardcoded it as zero in the hash
preimage(s) to the address. Now it does not exist in noir, we simply
omit the flag (see
[diff](https://github.com/AztecProtocol/aztec-packages/pull/22393/changes/BASE..6ce45576564c12e39a4a0ea138f33e11def435f3#diff-f8cc678527e3ed7de63f64b12494329eb9ad389c1dbfce9bea7b480199220836))
to match the new address derivation.

However, we still rely on this assumption for the `ivpk` in
`#[IVK_ON_CURVE_CHECK]` (which would fail in the AVM for an inf point
with `(0,0)` coordinates, but not in noir), and assume it is non-inf in
the call to `ecc` for `address_point = preaddress_public_key +
incoming_viewing_key`.
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.

4 participants