Skip to content

refactor(pxe): type and audit legacy oracle mappings#21569

Merged
nchamo merged 1 commit intomerge-train/fairiesfrom
fix/f-451-type-legacy-oracle-mappings
Mar 16, 2026
Merged

refactor(pxe): type and audit legacy oracle mappings#21569
nchamo merged 1 commit intomerge-train/fairiesfrom
fix/f-451-type-legacy-oracle-mappings

Conversation

@nchamo
Copy link
Contributor

@nchamo nchamo commented Mar 14, 2026

Summary

  • Added explicit parameter and return type annotations to all entries in buildLegacyOracleCallbacks, so that changes to Oracle method signatures produce compile-time errors instead of silently breaking at runtime.
  • Removed 5 unused legacy entries that were not used by any pinned protocol contract
    • privateGetNextAppTagAsSender
    • privateGetSenderForTags
    • privateSetSenderForTags
    • utilityCheckNullifierExists
    • utilityGetRandomField

Closes F-451

@nchamo nchamo self-assigned this Mar 14, 2026
@nchamo nchamo added ci-draft Run CI on draft PRs. and removed ci-draft Run CI on draft PRs. labels Mar 14, 2026
@nchamo nchamo marked this pull request as ready for review March 14, 2026 14:33
@nchamo
Copy link
Contributor Author

nchamo commented Mar 14, 2026

@benesjan , I run Claude a few times and it said that those 5 functions in the description were not being used. I'm not sure how to check exactly, can you guide me through it so that I can check if those are used or not?

Copy link
Contributor

@benesjan benesjan left a comment

Choose a reason for hiding this comment

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

so that changes to Oracle method signatures produce compile-time errors instead of silently breaking at runtime.

Regarding this ^ note that the change in this PR will not trigger the compile time check because it is being run on the functions on the Oracle class (see here).

But I wonder why do we check about the compile time check here? I thought these mappings are a no-touch zone until v5 at which point we just delete them.

I run Claude a few times and it said that those 5 functions in the description were not being used. I'm not sure how to check exactly, can you guide me through it so that I can check if those are used or not?

Regarding this ^. I accidentally did not delete the legacy callbacks when I unpinned the SponsoredFPC contract (I was checking if there were some only used by SponsoredFPC but I missed these when looking through the table that AI outputted).

But I re-verified with an AI prompt and it told me only aztec_prv_notifyRevertiblePhaseStart is used by SponsoredFPC and not used by any of the other pinned contracts so not sure if your AI output is correct.

But anyway I propose that you also open this PR against v4-next (or backport-to-v4-next-staging) and just see if CI passes. I don't think there is any other better way to verify that.

But would also note that this is relevant only until v5 so I would not worry too much about this as if we have some redundant callback mappings it doesn't matter much.

@nchamo
Copy link
Contributor Author

nchamo commented Mar 16, 2026

Ok, I created #21607 to make sure those 5 functions were not being used, and the CI passed. So I think we are good to go to merge these changes

Copy link
Contributor

@mverzilli mverzilli left a comment

Choose a reason for hiding this comment

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

nice!

@nchamo nchamo merged commit 659d476 into merge-train/fairies Mar 16, 2026
27 of 31 checks passed
@nchamo nchamo deleted the fix/f-451-type-legacy-oracle-mappings branch March 16, 2026 15:41
@AztecBot
Copy link
Collaborator

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

github-merge-queue bot pushed a commit that referenced this pull request Mar 16, 2026
BEGIN_COMMIT_OVERRIDE
feat: add public log filtering by tag (#21561)
feat: default to kernelless simulations (#21575)
fix(aztec-node): throw on existing nullifier in
getLowNullifierMembershipWitness (#21472)
fix: update nullifier non-inclusion test expectations after early oracle
throw (#21600)
refactor(pxe): type and audit legacy oracle mappings (#21569)
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.

4 participants