Skip to content

Commit dfcdea6

Browse files
nventuronventuro
authored
fix: dont panic on note msgs on contracts with no notes (#24852)
A contract with no notes might otherwise panic if e.g. it processed an offchain message related to one. I also made PXE skip the standard contracts that have no notes and events, both to avoid such a situation and because there's no need to do it.
1 parent 3b4ba4a commit dfcdea6

12 files changed

Lines changed: 251 additions & 32 deletions

File tree

‎noir-projects/aztec-nr/aztec/src/macros/aztec/compute_note_hash_and_nullifier.nr‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,12 @@ pub(crate) comptime fn generate_contract_library_methods_compute_note_hash_and_n
4444

4545
comptime fn generate_contract_library_method_compute_note_hash() -> Quoted {
4646
if NOTES.len() == 0 {
47-
// Contracts with no notes still implement this function to avoid having special-casing, the implementation
48-
// simply throws immediately.
47+
// Contracts with no notes still implement this function to avoid having special-casing. Since the contract
48+
// declares no note types there is never a note to hash, so we return `Option::none()`. This will cause any
49+
// messages related to (non-existing) notes to be simply skipped, mirroring the unknown-note-type branch of the
50+
// with-notes implementation.
4951
quote {
50-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
52+
/// This contract does not use private notes, so this function always returns `Option::none()`.
5153
///
5254
/// This function is automatically injected by the `#[aztec]` macro.
5355
#[contract_library_method]
@@ -59,7 +61,7 @@ comptime fn generate_contract_library_method_compute_note_hash() -> Quoted {
5961
_contract_address: aztec::protocol::address::AztecAddress,
6062
_randomness: Field,
6163
) -> Option<Field> {
62-
panic(f"This contract does not use private notes")
64+
Option::none()
6365
}
6466
}
6567
} else {
@@ -155,10 +157,12 @@ comptime fn generate_contract_library_method_compute_note_hash() -> Quoted {
155157

156158
comptime fn generate_contract_library_method_compute_note_nullifier() -> Quoted {
157159
if NOTES.len() == 0 {
158-
// Contracts with no notes still implement this function to avoid having special-casing, the implementation
159-
// simply throws immediately.
160+
// Contracts with no notes still implement this function to avoid having special-casing. Since the contract
161+
// declares no note types there is never a note to nullify, so we return `Option::none()`. This will cause any
162+
// messages related to (non-existing) notes to be simply skipped, mirroring the unknown-note-type branch of the
163+
// with-notes implementation.
160164
quote {
161-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
165+
/// This contract does not use private notes, so this function always returns `Option::none()`.
162166
///
163167
/// This function is automatically injected by the `#[aztec]` macro.
164168
#[contract_library_method]
@@ -171,7 +175,7 @@ comptime fn generate_contract_library_method_compute_note_nullifier() -> Quoted
171175
_contract_address: aztec::protocol::address::AztecAddress,
172176
_randomness: Field,
173177
) -> Option<Field> {
174-
panic(f"This contract does not use private notes")
178+
Option::none()
175179
}
176180
}
177181
} else {

‎noir-projects/contract-snapshots/tests/snapshots/expand/avm_gadgets_test_contract/snapshots__expanded.snap‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
source: tests/snapshots.rs
33
expression: stdout
44
---
5-
65
use aztec::macros::aztec;
76
use aztec::macros::aztec;
87

@@ -119,20 +118,20 @@ contract AvmGadgetsTest {
119118
})
120119
}
121120

122-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
121+
/// This contract does not use private notes, so this function always returns `Option::none()`.
123122
///
124123
/// This function is automatically injected by the `#[aztec]` macro.
125124
#[contract_library_method]
126125
unconstrained fn _compute_note_hash(_packed_note: BoundedVec<Field, 8>, _owner: aztec::protocol::address::AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: aztec::protocol::address::AztecAddress, _randomness: Field) -> Option<Field> {
127-
panic(f"This contract does not use private notes")
126+
Option::<Field>::none()
128127
}
129128

130-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
129+
/// This contract does not use private notes, so this function always returns `Option::none()`.
131130
///
132131
/// This function is automatically injected by the `#[aztec]` macro.
133132
#[contract_library_method]
134133
unconstrained fn _compute_note_nullifier(_unique_note_hash: Field, _packed_note: BoundedVec<Field, 8>, _owner: aztec::protocol::address::AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: aztec::protocol::address::AztecAddress, _randomness: Field) -> Option<Field> {
135-
panic(f"This contract does not use private notes")
134+
Option::<Field>::none()
136135
}
137136

138137
/// Receives offchain messages into this contract's offchain inbox for subsequent processing.

‎noir-projects/contract-snapshots/tests/snapshots/expand/avm_test_contract/snapshots__expanded.snap‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
source: tests/snapshots.rs
33
expression: stdout
44
---
5-
65
use aztec::macros::aztec;
76
use aztec::macros::aztec;
87

@@ -713,20 +712,20 @@ pub contract AvmTest {
713712
})
714713
}
715714

716-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
715+
/// This contract does not use private notes, so this function always returns `Option::none()`.
717716
///
718717
/// This function is automatically injected by the `#[aztec]` macro.
719718
#[contract_library_method]
720719
unconstrained fn _compute_note_hash(_packed_note: BoundedVec<Field, 8>, _owner: AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: AztecAddress, _randomness: Field) -> Option<Field> {
721-
panic(f"This contract does not use private notes")
720+
Option::<Field>::none()
722721
}
723722

724-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
723+
/// This contract does not use private notes, so this function always returns `Option::none()`.
725724
///
726725
/// This function is automatically injected by the `#[aztec]` macro.
727726
#[contract_library_method]
728727
unconstrained fn _compute_note_nullifier(_unique_note_hash: Field, _packed_note: BoundedVec<Field, 8>, _owner: AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: AztecAddress, _randomness: Field) -> Option<Field> {
729-
panic(f"This contract does not use private notes")
728+
Option::<Field>::none()
730729
}
731730

732731
/// Receives offchain messages into this contract's offchain inbox for subsequent processing.
@@ -1765,7 +1764,7 @@ pub contract AvmTest {
17651764

17661765
unconstrained fn sync_state(scope: AztecAddress) {
17671766
let address: AztecAddress = aztec::context::UtilityContext::new().this_address();
1768-
aztec::messages::discovery::do_sync_state(address, _compute_note_hash, _compute_note_nullifier, Option::<unconstrained fn(AztecAddress, u64, u64, BoundedVec<Field, 11>, aztec::messages::processing::MessageContext, AztecAddress)>::none(), Option::<unconstrained fn(AztecAddress, AztecAddress) -> aztec::unconstrained_array::UnconstrainedArray<aztec::messages::processing::OffchainMessageWithContext, aztec::ephemeral::EphemeralOracles>>::some(aztec::messages::processing::offchain::sync_inbox), scope);
1767+
aztec::messages::discovery::do_sync_state(address, _compute_note_hash, _compute_note_nullifier, Option::<unconstrained fn(AztecAddress, u64, u64, BoundedVec<Field, 11>, aztec::oracle::tx_resolution::ResolvedTx, AztecAddress)>::none(), Option::<unconstrained fn(AztecAddress, AztecAddress) -> aztec::unconstrained_array::UnconstrainedArray<aztec::messages::processing::OffchainMessageWithTx, aztec::ephemeral::EphemeralOracles>>::some(aztec::messages::processing::offchain::sync_inbox), scope);
17691768
}
17701769

17711770
pub struct offchain_receive_parameters {

‎noir-projects/contract-snapshots/tests/snapshots/expand/public_fns_with_emit_repro_contract/snapshots__expanded.snap‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
source: tests/snapshots.rs
33
expression: stdout
44
---
5-
65
use aztec::macros::aztec;
76
use aztec::macros::aztec;
87

@@ -140,20 +139,20 @@ pub contract PublicFnsWithEmitRepro {
140139
})
141140
}
142141

143-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
142+
/// This contract does not use private notes, so this function always returns `Option::none()`.
144143
///
145144
/// This function is automatically injected by the `#[aztec]` macro.
146145
#[contract_library_method]
147146
unconstrained fn _compute_note_hash(_packed_note: BoundedVec<Field, 8>, _owner: aztec::protocol::address::AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: aztec::protocol::address::AztecAddress, _randomness: Field) -> Option<Field> {
148-
panic(f"This contract does not use private notes")
147+
Option::<Field>::none()
149148
}
150149

151-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
150+
/// This contract does not use private notes, so this function always returns `Option::none()`.
152151
///
153152
/// This function is automatically injected by the `#[aztec]` macro.
154153
#[contract_library_method]
155154
unconstrained fn _compute_note_nullifier(_unique_note_hash: Field, _packed_note: BoundedVec<Field, 8>, _owner: aztec::protocol::address::AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: aztec::protocol::address::AztecAddress, _randomness: Field) -> Option<Field> {
156-
panic(f"This contract does not use private notes")
155+
Option::<Field>::none()
157156
}
158157

159158
/// Receives offchain messages into this contract's offchain inbox for subsequent processing.

‎noir-projects/contract-snapshots/tests/snapshots/expand/storage_proof_test_contract/snapshots__expanded.snap‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
source: tests/snapshots.rs
33
expression: stdout
44
---
5-
65
use aztec::macros::aztec;
76
use aztec::macros::aztec;
87

@@ -86,20 +85,20 @@ contract StorageProofTest {
8685
})
8786
}
8887

89-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
88+
/// This contract does not use private notes, so this function always returns `Option::none()`.
9089
///
9190
/// This function is automatically injected by the `#[aztec]` macro.
9291
#[contract_library_method]
9392
unconstrained fn _compute_note_hash(_packed_note: BoundedVec<Field, 8>, _owner: AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: AztecAddress, _randomness: Field) -> Option<Field> {
94-
panic(f"This contract does not use private notes")
93+
Option::<Field>::none()
9594
}
9695

97-
/// This contract does not use private notes, so this function should never be called as it will unconditionally fail.
96+
/// This contract does not use private notes, so this function always returns `Option::none()`.
9897
///
9998
/// This function is automatically injected by the `#[aztec]` macro.
10099
#[contract_library_method]
101100
unconstrained fn _compute_note_nullifier(_unique_note_hash: Field, _packed_note: BoundedVec<Field, 8>, _owner: AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: AztecAddress, _randomness: Field) -> Option<Field> {
102-
panic(f"This contract does not use private notes")
101+
Option::<Field>::none()
103102
}
104103

105104
/// Receives offchain messages into this contract's offchain inbox for subsequent processing.

‎noir-projects/noir-contracts/Nargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ members = [
6060
"contracts/test/nested_utility_contract",
6161
"contracts/test/no_constructor_contract",
6262
"contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract",
63+
"contracts/test/no_notes_contract",
6364
"contracts/test/note_getter_contract",
6465
"contracts/test/offchain_effect_contract",
6566
"contracts/test/offchain_payment_contract",

‎noir-projects/noir-contracts/contracts/test/no_notes_contract/Nargo.toml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[package]
2+
name = "no_notes_contract"
3+
authors = [""]
4+
compiler_version = ">=0.25.0"
5+
type = "contract"
6+
7+
[dependencies]
8+
aztec = { path = "../../../../aztec-nr/aztec" }

‎noir-projects/noir-contracts/contracts/test/no_notes_contract/src/main.nr‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
mod test;
2+
3+
use aztec::macros::aztec;
4+
5+
/// A minimal contract that declares no note types, used to test the macro-generated no-notes
6+
/// `_compute_note_hash`, `_compute_note_nullifier` and (deprecated) `_compute_note_hash_and_nullifier` functions.
7+
#[aztec]
8+
pub contract NoNotes {
9+
use aztec::{
10+
messages::{discovery::NoteHashAndNullifier as NoteHashAndNullifierResult, logs::note::MAX_NOTE_PACKED_LEN},
11+
protocol::address::AztecAddress,
12+
};
13+
14+
#[contract_library_method]
15+
pub unconstrained fn test_compute_note_hash(
16+
packed_note: BoundedVec<Field, MAX_NOTE_PACKED_LEN>,
17+
owner: AztecAddress,
18+
storage_slot: Field,
19+
note_type_id: Field,
20+
contract_address: AztecAddress,
21+
randomness: Field,
22+
) -> Option<Field> {
23+
_compute_note_hash(
24+
packed_note,
25+
owner,
26+
storage_slot,
27+
note_type_id,
28+
contract_address,
29+
randomness,
30+
)
31+
}
32+
33+
#[contract_library_method]
34+
pub unconstrained fn test_compute_note_nullifier(
35+
unique_note_hash: Field,
36+
packed_note: BoundedVec<Field, MAX_NOTE_PACKED_LEN>,
37+
owner: AztecAddress,
38+
storage_slot: Field,
39+
note_type_id: Field,
40+
contract_address: AztecAddress,
41+
randomness: Field,
42+
) -> Option<Field> {
43+
_compute_note_nullifier(
44+
unique_note_hash,
45+
packed_note,
46+
owner,
47+
storage_slot,
48+
note_type_id,
49+
contract_address,
50+
randomness,
51+
)
52+
}
53+
54+
#[contract_library_method]
55+
pub unconstrained fn test_compute_note_hash_and_nullifier(
56+
packed_note: BoundedVec<Field, MAX_NOTE_PACKED_LEN>,
57+
owner: AztecAddress,
58+
storage_slot: Field,
59+
note_type_id: Field,
60+
contract_address: AztecAddress,
61+
randomness: Field,
62+
note_nonce: Field,
63+
) -> Option<NoteHashAndNullifierResult> {
64+
_compute_note_hash_and_nullifier(
65+
packed_note,
66+
owner,
67+
storage_slot,
68+
note_type_id,
69+
contract_address,
70+
randomness,
71+
note_nonce,
72+
)
73+
}
74+
}

‎noir-projects/noir-contracts/contracts/test/no_notes_contract/src/test.nr‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
use crate::NoNotes;
2+
use aztec::protocol::address::AztecAddress;
3+
4+
// The contract declares no note types, so there is never a note to hash or nullify: every input must be skipped
5+
// (return `Option::none()`) regardless of its contents, so that malicious messages about non-existent notes are
6+
// ignored.
7+
8+
#[test]
9+
unconstrained fn compute_note_hash_skips() {
10+
let result = NoNotes::test_compute_note_hash(
11+
BoundedVec::from_array([42]),
12+
AztecAddress::zero(),
13+
0,
14+
0xdeadbeef,
15+
AztecAddress::zero(),
16+
0,
17+
);
18+
19+
assert(result.is_none());
20+
}
21+
22+
#[test]
23+
unconstrained fn compute_note_nullifier_skips() {
24+
let result = NoNotes::test_compute_note_nullifier(
25+
0,
26+
BoundedVec::from_array([42]),
27+
AztecAddress::zero(),
28+
0,
29+
0xdeadbeef,
30+
AztecAddress::zero(),
31+
0,
32+
);
33+
34+
assert(result.is_none());
35+
}
36+
37+
#[test]
38+
unconstrained fn compute_note_hash_and_nullifier_skips() {
39+
let result = NoNotes::test_compute_note_hash_and_nullifier(
40+
BoundedVec::from_array([42]),
41+
AztecAddress::zero(),
42+
0,
43+
0xdeadbeef,
44+
AztecAddress::zero(),
45+
0,
46+
0,
47+
);
48+
49+
assert(result.is_none());
50+
}

‎yarn-project/pxe/src/contract/helpers.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import { isProtocolContract } from '@aztec/protocol-contracts';
21
import type { FunctionCall, FunctionSelector } from '@aztec/stdlib/abi';
32
import type { AztecAddress } from '@aztec/stdlib/aztec-address';
43
import type { BlockHeader } from '@aztec/stdlib/tx';
54

65
import type { ContractStore } from '../storage/contract_store/contract_store.js';
76
import type { ContractClassService } from './contract_class_service.js';
7+
import { isSkipSyncContract } from './skip_sync_contracts.js';
88

99
export async function syncScope(
1010
contractAddress: AztecAddress,
@@ -15,8 +15,8 @@ export async function syncScope(
1515
utilityExecutor: (privateSyncCall: FunctionCall, scopes: AztecAddress[]) => Promise<any>,
1616
scope: AztecAddress,
1717
) {
18-
// Protocol contracts don't have private state to sync
19-
if (isProtocolContract(contractAddress)) {
18+
// Some canonical contracts hold no private state, so there is nothing to sync (see `skipSyncContracts`).
19+
if (isSkipSyncContract(contractAddress)) {
2020
return;
2121
}
2222

0 commit comments

Comments
 (0)