docs(aztec-nr): flag host-dependent ordering in external_functions_registry#23899
Closed
AztecBot wants to merge 1 commit into
Closed
docs(aztec-nr): flag host-dependent ordering in external_functions_registry#23899AztecBot wants to merge 1 commit into
AztecBot wants to merge 1 commit into
Conversation
…gistry The PUBLIC/PRIVATE/UTILITY registries hand back entries in the order #[external(...)] attribute macros were applied, which the noir compiler does not guarantee to be host-independent. Downstream macros emit code in that order, so identical nargo binaries on different hosts can produce different expand snapshots for the same contract source. This commit only documents the invariant. The fix (sort entries by a host-independent key on read, or fix the order upstream in noirc) is follow-up work.
Collaborator
Author
|
Automatically closing this stale claudebox draft PR (no updates for 5+ days). Re-open if still needed. |
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
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.
Background
PR #23896 (contract-snapshots regen for
merge-train/fairies-v5) failed CI on one snapshot (expand::test_avm_test_contract) even though the author's local run reported 60/61 → 61/61 pass. Diagnosis from that CI run is in the gist.This PR documents what the investigation found and opens the conversation on where to put the fix. The only change in code so far is a module-level doc note in
external_functions_registry.nr.What's actually happening
The
expand::test_avm_test_contractsnapshot diff is purely a function-pair swap (bodies byte-identical):nargo expandoutputavm_test_contract/src/main.nrnargo expandon this hostnested_call_large_calldata(839) thencall_fee_juice(850)Both runs use the same
nargo 1.0.0-beta.22+c57152f. Two back-to-back freshnargo expandinvocations on this host produce byte-identical output, so it isn't per-process flakiness. Different hosts get different answers.Root cause path
The impl-block macro is
calls_generation::external_functions::generate_external_function_callsinaztec-nr/aztec/src/macros/calls_generation/external_functions.nr. It iterates overexternal_functions_registry::get_public_functions(m)etc.That registry is
CHashMap<Module, [FunctionDefinition]>(utils/cmap.nr).CHashMapis backed by a plain slice — insertion-order preserving. Soget_*returns entries in the orderadd_*was called.add_public/add_private/add_utilityare called from the body of the#[external(...)]attribute macro (macros/functions/mod.nr:341). Which is once per#[external(...)]-decorated function, in whatever order noirc invokes the attribute macro across the contract module's functions.That iteration order — noirc's attribute application order across a module — is not guaranteed to be host-independent. Different hosts with the same
nargobinary apply the attributes in different orders, and the registry faithfully preserves whatever order it was given. The forward-decl section ofnargo expandhappens to traverse functions in source order via a different code path, which is why those forward decls are consistent across hosts while the impl-block sections aren't.Fix difficulty
Two ways to make this deterministic:
get_*_functionssort entries by a host-independent key (FunctionDefinition::name()or::location()) before returning. Requires a comptime ordering primitive that the current Noir stdlib doesn't expose:Quotedhas onlyEq, noOrd;Locationhas onlyEqandHash(no file/line accessors). A pure aztec-nr workaround is possible but messy: it would need to thread function names throughf"{...}"-style fmtstrs and implement a byte-level string compare for variable-length names, or sort byHash<Location>(which is itself host-dependent and so doesn't actually help).My recommendation is (2) upstream for the long-term fix, plus a separate trivial noir-stdlib PR adding
OrdonQuoted/LocationandLocation::file()/line()accessors so that aztec-nr-side sorts of comptime registries are cheap to write going forward.What this PR does
# Determinismblock to the module doc ofexternal_functions_registry.nrrecording the invariant ("the iteration order ofget_*_functionsis not guaranteed to be host-independent") so the next person who writes a snapshot test that depends on this order is forewarned.What this PR does NOT do
avm_test_contractexpand snapshot on a Linux x86_64 host so its order matches CI's (or by waiting on whichever fix path below lands first).Reproduction evidence
From this session, on Linux x86_64 with
nargo 1.0.0-beta.22+c57152f:Same binary, same source, two different impl-block orderings.
Suggested next steps
Quoted/Locationordering primitives, depending on which path the team chooses).Created by claudebox · group:
slackbot