Skip to content

Commit 135f1e1

Browse files
authored
fix(ci): run noir-projects nargo fmt check in CI (#24003)
## Problem The nargo fmt check for noir-projects lives only in the aggregate `noir-projects/bootstrap.sh` `prep` function. CI stopped invoking that script when the build moved to the root Makefile (90b7a95, "feat: build with makefile"): the Makefile calls each subproject bootstrap directly. Since then no nargo fmt check has run in CI, which is how unformatted `.nr` files have been landing and showing up as unrelated fmt diffs in later PRs. ## Fix Extract the check into a `format_check` function in `noir-projects/bootstrap.sh` and wire it as a `noir-projects-format-check` Makefile target, mirroring `bb-cpp-format-check`. It is a prerequisite of all four noir-projects subproject builds, preserving the old `prep` ordering: the check doubles as the nargo git dependency download, which must finish before the subproject builds run nargo in parallel. Also add `protocol-fuzzer/contracts` to the check and to the precommit hook. It is a Nargo workspace that no check covered, and it had already accumulated fmt drift. The nargo path in both loops is now anchored (repo root in the bootstrap, absolute in the precommit) since the previous relative form only resolved for workspaces one level under noir-projects. ## Red green The branch tip deliberately leaves 5 unformatted files untouched (3 from the recent delivery module PRs, 2 in protocol-fuzzer/contracts), so CI on this PR should fail in `noir-projects-format-check`. A follow up commit will run `nargo fmt` to turn it green.
1 parent 539b1d9 commit 135f1e1

5 files changed

Lines changed: 83 additions & 56 deletions

File tree

Makefile

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -287,19 +287,30 @@ claude-tests:
287287
# Noir Projects
288288
#==============================================================================
289289

290-
noir-protocol-circuits: noir bb-cpp-native
290+
# Generates the noir-protocol-circuits workspace files (Nargo.toml, autogenerated crates),
291+
# which are git-ignored and must exist before nargo can run in that workspace. Needs only
292+
# yarn/node, so no prerequisites: it runs in parallel with the noir build.
293+
noir-protocol-circuits-variants:
294+
$(call build,$@,noir-projects,generate_variants)
295+
296+
# Format check. Also warms the nargo dependency cache, so it must complete before the
297+
# subproject builds to avoid parallel nargo runs tripping over each other downloading.
298+
noir-projects-format-check: noir noir-protocol-circuits-variants
299+
$(call build,$@,noir-projects,format_check)
300+
301+
noir-protocol-circuits: noir bb-cpp-native noir-projects-format-check
291302
$(call build,$@,noir-projects/noir-protocol-circuits)
292303

293304
noir-protocol-circuits-tests: noir noir-protocol-circuits
294305
$(call test,$@,noir-projects/noir-protocol-circuits)
295306

296-
mock-protocol-circuits: noir bb-cpp-native
307+
mock-protocol-circuits: noir bb-cpp-native noir-projects-format-check
297308
$(call build,$@,noir-projects/mock-protocol-circuits)
298309

299-
noir-contracts: noir bb-cpp-native
310+
noir-contracts: noir bb-cpp-native noir-projects-format-check
300311
$(call build,$@,noir-projects/noir-contracts)
301312

302-
aztec-nr: noir bb-cpp-native
313+
aztec-nr: noir bb-cpp-native noir-projects-format-check
303314
$(call build,$@,noir-projects/aztec-nr)
304315

305316
# These tests are not included in the dep tree.
@@ -308,7 +319,7 @@ noir-projects-txe-tests:
308319
$(call test,$@,noir-projects/aztec-nr)
309320
$(call test,$@,noir-projects/noir-contracts)
310321

311-
contract-snapshots-tests: noir
322+
contract-snapshots-tests: noir noir-projects-format-check
312323
$(call test,$@,noir-projects/contract-snapshots)
313324

314325
# Noir Projects - Aggregate target (builds all sub-projects)

noir-projects/bootstrap.sh

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,29 @@
11
#!/usr/bin/env bash
22
source $(git rev-parse --show-toplevel)/ci3/source_bootstrap
33

4+
# Generates the noir-protocol-circuits workspace files (Nargo.toml, crates/autogenerated),
5+
# which are git-ignored and must exist before nargo can run in that workspace. CI runs
6+
# this via the root Makefile's noir-protocol-circuits-variants target.
7+
function generate_variants {
8+
set -eu
9+
(cd noir-protocol-circuits && yarn && node ./scripts/generate_variants.js)
10+
}
11+
12+
# Format check across all workspaces. CI runs this via the root Makefile's
13+
# noir-projects-format-check target, ordered after generate_variants and before the
14+
# subproject builds so the nargo dependency cache is warm by the time they run.
15+
function format_check {
16+
# nargo downloads its git dependencies (e.g. noir-lang/poseidon) on first use.
17+
# Under heavy parallel CI load the VPC DNS resolver drops lookups
18+
# ("Could not resolve host: github.com"), leaving a half-cloned dependency dir
19+
# that then fails with "Cannot read file .../Nargo.toml". Retry the download,
20+
# wiping the partial dependency cache after a failure so the next attempt
21+
# re-clones cleanly. A warm cache is left intact on success.
22+
local nargo=$root/noir/noir-repo/target/release/nargo
23+
local fmt_check="( set -e; for dir in noir-contracts noir-protocol-circuits mock-protocol-circuits aztec-nr protocol-fuzzer/contracts; do (cd \"\$dir\" && \"$nargo\" fmt --check); done )"
24+
RETRY_SLEEP=10 retry "$fmt_check || { rm -rf \"\$HOME/nargo\"; exit 1; }"
25+
}
26+
427
function build {
528
echo_header "noir-projects build"
629

@@ -9,17 +32,10 @@ function build {
932
# Also doubles up as our formatting check.
1033
function prep {
1134
set -eu
12-
(cd noir-protocol-circuits && yarn && node ./scripts/generate_variants.js)
13-
# nargo downloads its git dependencies (e.g. noir-lang/poseidon) on first use.
14-
# Under heavy parallel CI load the VPC DNS resolver drops lookups
15-
# ("Could not resolve host: github.com"), leaving a half-cloned dependency dir
16-
# that then fails with "Cannot read file .../Nargo.toml". Retry the download,
17-
# wiping the partial dependency cache after a failure so the next attempt
18-
# re-clones cleanly. A warm cache is left intact on success.
19-
local fmt_check='( set -e; for dir in noir-contracts noir-protocol-circuits mock-protocol-circuits aztec-nr; do (cd "$dir" && ../../noir/noir-repo/target/release/nargo fmt --check); done )'
20-
RETRY_SLEEP=10 retry "$fmt_check || { rm -rf \"\$HOME/nargo\"; exit 1; }"
35+
generate_variants
36+
format_check
2137
}
22-
export -f prep
38+
export -f prep generate_variants format_check
2339

2440
denoise prep
2541

noir-projects/precommit.sh

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ export FORCE_COLOR=true
1313

1414
# Path to nargo binary
1515
NARGO_PATH="../noir/noir-repo/target/release/nargo"
16+
# Absolute path for use inside the per-workspace loop, where a relative path would
17+
# resolve differently depending on each workspace's depth.
18+
NARGO_ABS="$PWD/$NARGO_PATH"
1619

1720
# Check if there are staged .nr files
1821
staged_nr_files=$(git diff --cached --name-only --diff-filter=d | grep '\.nr$' || true)
@@ -50,10 +53,10 @@ if [[ -n "$staged_nr_files" ]]; then
5053
exit 0
5154
fi
5255

53-
for dir in noir-contracts noir-protocol-circuits mock-protocol-circuits aztec-nr; do
56+
for dir in noir-contracts noir-protocol-circuits mock-protocol-circuits aztec-nr protocol-fuzzer/contracts; do
5457
if [[ -d "$dir" ]]; then
5558
echo "Formatting in $dir..."
56-
(cd "$dir" && "../$NARGO_PATH" fmt) || echo "Warning: Formatting failed in $dir, but continuing..."
59+
(cd "$dir" && "$NARGO_ABS" fmt) || echo "Warning: Formatting failed in $dir, but continuing..."
5760
else
5861
echo "Warning: Directory $dir not found, skipping..."
5962
fi

noir-projects/protocol-fuzzer/contracts/parent_contract/src/main.nr

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ pub contract Parent {
55
use aztec::{
66
macros::functions::{external, initializer},
77
protocol::{
8-
abis::function_selector::FunctionSelector,
9-
address::AztecAddress,
10-
traits::ToField,
8+
abis::function_selector::FunctionSelector, address::AztecAddress, traits::ToField,
119
},
1210
};
1311

@@ -36,48 +34,34 @@ pub contract Parent {
3634
// Forward a call to emit_nullifier(Field) on the target contract.
3735
#[external("private")]
3836
fn forward_emit_nullifier(target: AztecAddress, nullifier: Field) {
39-
let selector =
40-
comptime { FunctionSelector::from_signature("emit_nullifier(Field)") };
37+
let selector = comptime { FunctionSelector::from_signature("emit_nullifier(Field)") };
4138
let _ = self.context.call_private_function(target, selector, [nullifier]);
4239
}
4340

4441
// Forward a call to test_settled_nullifier_inclusion(Field) on the target contract.
4542
#[external("private")]
4643
fn forward_test_settled_nullifier_inclusion(target: AztecAddress, nullifier: Field) {
47-
let selector =
48-
comptime { FunctionSelector::from_signature("test_settled_nullifier_inclusion(Field)") };
44+
let selector = comptime {
45+
FunctionSelector::from_signature("test_settled_nullifier_inclusion(Field)")
46+
};
4947
let _ = self.context.call_private_function(target, selector, [nullifier]);
5048
}
5149

5250
// Forward a call to call_destroy_note(AztecAddress, Field) on the target contract.
5351
#[external("private")]
54-
fn forward_call_destroy_note(
55-
target: AztecAddress,
56-
owner: AztecAddress,
57-
storage_slot: Field,
58-
) {
52+
fn forward_call_destroy_note(target: AztecAddress, owner: AztecAddress, storage_slot: Field) {
5953
let selector =
6054
comptime { FunctionSelector::from_signature("call_destroy_note((Field),Field)") };
61-
let _ = self.context.call_private_function(
62-
target,
63-
selector,
64-
[owner.to_field(), storage_slot],
65-
);
55+
let _ =
56+
self.context.call_private_function(target, selector, [owner.to_field(), storage_slot]);
6657
}
6758

6859
// Forward a call to test_note_inclusion(AztecAddress, Field) on the target contract.
6960
#[external("private")]
70-
fn forward_test_note_inclusion(
71-
target: AztecAddress,
72-
owner: AztecAddress,
73-
storage_slot: Field,
74-
) {
61+
fn forward_test_note_inclusion(target: AztecAddress, owner: AztecAddress, storage_slot: Field) {
7562
let selector =
7663
comptime { FunctionSelector::from_signature("test_note_inclusion((Field),Field)") };
77-
let _ = self.context.call_private_function(
78-
target,
79-
selector,
80-
[owner.to_field(), storage_slot],
81-
);
64+
let _ =
65+
self.context.call_private_function(target, selector, [owner.to_field(), storage_slot]);
8266
}
8367
}

noir-projects/protocol-fuzzer/contracts/side_effect_contract/src/main.nr

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@ pub contract SideEffect {
2626
use aztec::{
2727
hash::compute_siloed_nullifier,
2828
history::nullifier::assert_nullifier_existed_by,
29-
macros::{
30-
functions::{external, initializer, noinitcheck},
31-
storage::storage,
32-
},
29+
macros::{functions::{external, initializer, noinitcheck}, storage::storage},
3330
messages::delivery::MessageDelivery,
3431
note::{
3532
ConfirmedNote,
@@ -122,18 +119,26 @@ pub contract SideEffect {
122119
active_or_nullified: bool,
123120
offset: u32,
124121
) -> [u128; 2] {
125-
let mut options = NoteViewerOptions::new()
126-
.set_owner(owner)
127-
.set_offset(offset)
128-
.sort(PropertySelector { index: 0, offset: 0, length: 32 }, SortOrder.ASC);
122+
let mut options = NoteViewerOptions::new().set_owner(owner).set_offset(offset).sort(
123+
PropertySelector { index: 0, offset: 0, length: 32 },
124+
SortOrder.ASC,
125+
);
129126
if (active_or_nullified) {
130127
options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED);
131128
}
132129

133130
let notes: BoundedVec<UintNote, MAX_NOTES_PER_PAGE> = view_notes(storage_slot, options);
134131

135-
let v0 = if notes.len() > 0 { notes.get(0).value } else { 0 };
136-
let v1 = if notes.len() > 1 { notes.get(1).value } else { 0 };
132+
let v0 = if notes.len() > 0 {
133+
notes.get(0).value
134+
} else {
135+
0
136+
};
137+
let v1 = if notes.len() > 1 {
138+
notes.get(1).value
139+
} else {
140+
0
141+
};
137142
[v0, v1]
138143
}
139144

@@ -154,8 +159,16 @@ pub contract SideEffect {
154159

155160
let confirmed_notes = get_notes(self.context, storage_slot, options);
156161

157-
let v0 = if confirmed_notes.len() > 0 { confirmed_notes.get(0).note.value } else { 0 };
158-
let v1 = if confirmed_notes.len() > 1 { confirmed_notes.get(1).note.value } else { 0 };
162+
let v0 = if confirmed_notes.len() > 0 {
163+
confirmed_notes.get(0).note.value
164+
} else {
165+
0
166+
};
167+
let v1 = if confirmed_notes.len() > 1 {
168+
confirmed_notes.get(1).note.value
169+
} else {
170+
0
171+
};
159172
[v0, v1]
160173
}
161174

0 commit comments

Comments
 (0)