Skip to content

Commit

Permalink
feat: Add init check by default to public fns (#4897)
Browse files Browse the repository at this point in the history
Adds the init check by default to all public functions, unless they are
flagged as `noinitcheck`.

This should break all contracts that call a public function from their
constructor. Let's see...
  • Loading branch information
spalladino committed Mar 5, 2024
1 parent 7b06895 commit 4550f25
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ contract AppSubscription {

#[aztec(public)]
#[aztec(internal)]
#[aztec(noinitcheck)]
fn init(
target_address: AztecAddress,
subscription_token_address: AztecAddress,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ contract EasyPrivateVoting {
// docs:end:constructor
// docs:start:initialize
#[aztec(public)] // annotation to mark function as public and expose public context
#[aztec(internal)]
fn _initialize(admin: AztecAddress) { // internal - can only be called by contract
#[aztec(internal)] // internal - can only be called by contract
#[aztec(noinitcheck)]
fn _initialize(admin: AztecAddress) {
storage.admin.write(admin);
storage.voteEnded.write(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ contract FPC {

#[aztec(public)]
#[aztec(internal)]
#[aztec(noinitcheck)]
fn _initialize(other_asset: AztecAddress, fee_asset: AztecAddress) {
storage.other_asset.initialize(other_asset);
storage.fee_asset.initialize(fee_asset);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ contract InclusionProofs {

#[aztec(public)]
#[aztec(internal)]
#[aztec(noinitcheck)]
fn _initialize(value: Field) {
storage.public_value.write(value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ contract StatefulTest {

#[aztec(public)]
fn increment_public_value(owner: AztecAddress, value: Field) {
assert_is_initialized(&mut context);
let loc = storage.public_values.at(owner);
loc.write(loc.read() + value);
}

#[aztec(public)]
#[aztec(noinitcheck)]
fn increment_public_value_no_init_check(owner: AztecAddress, value: Field) {
let loc = storage.public_values.at(owner);
loc.write(loc.read() + value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ contract TokenBlacklist {
///////
#[aztec(public)]
#[aztec(internal)]
#[aztec(noinitcheck)]
fn _initialize(new_admin: AztecAddress, slow_updates_contract: AztecAddress) {
assert(!new_admin.is_zero(), "invalid admin");
storage.admin.write(new_admin);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ contract TokenBridge {

#[aztec(public)]
#[aztec(internal)]
#[aztec(noinitcheck)]
fn _initialize(token: AztecAddress) {
storage.token.write(token);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ contract Token {
// docs:start:initialize
#[aztec(public)]
#[aztec(internal)]
#[aztec(noinitcheck)]
fn _initialize(
new_admin: AztecAddress,
name: FieldCompressedString,
Expand Down
10 changes: 0 additions & 10 deletions noir/noir-repo/aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,6 @@ fn transform_module(
is_internal = true;
} else if is_custom_attribute(&secondary_attribute, "aztec(public)") {
is_public = true;
insert_init_check = false;
} else if is_custom_attribute(&secondary_attribute, "aztec(public-vm)") {
is_public_vm = true;
}
Expand Down Expand Up @@ -673,15 +672,6 @@ fn transform_function(

// Add initialization check
if insert_init_check {
if ty == "Public" {
let error = AztecMacroError::UnsupportedAttributes {
span: func.def.name.span(),
secondary_message: Some(
"public functions do not yet support initialization check".to_owned(),
),
};
return Err(error);
}
let init_check = create_init_check();
func.def.body.0.insert(0, init_check);
}
Expand Down
1 change: 1 addition & 0 deletions yarn-project/merkle-tree/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export * from './interfaces/update_only_tree.js';
export * from './pedersen.js';
export * from './sparse_tree/sparse_tree.js';
export { StandardIndexedTree } from './standard_indexed_tree/standard_indexed_tree.js';
export { StandardIndexedTreeWithAppend } from './standard_indexed_tree/test/standard_indexed_tree_with_append.js';
export * from './standard_tree/standard_tree.js';
export { INITIAL_LEAF, getTreeMeta } from './tree_base.js';
export { newTree } from './new_tree.js';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { StandardIndexedTree } from '../../index.js';
import { StandardIndexedTree } from '../standard_indexed_tree.js';

/**
* A testing utility which is here to store the original implementation of StandardIndexedTree.appendLeaves method
Expand Down
47 changes: 41 additions & 6 deletions yarn-project/simulator/src/public/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { L1ToL2Message, SiblingPath } from '@aztec/circuit-types';
import { L1ToL2Message, NullifierMembershipWitness, SiblingPath } from '@aztec/circuit-types';
import {
AppendOnlyTreeSnapshot,
CallContext,
Expand All @@ -7,13 +7,19 @@ import {
Header,
L1_TO_L2_MSG_TREE_HEIGHT,
L2ToL1Message,
NULLIFIER_TREE_HEIGHT,
NullifierLeaf,
NullifierLeafPreimage,
} from '@aztec/circuits.js';
import { siloNullifier } from '@aztec/circuits.js/hash';
import { makeHeader } from '@aztec/circuits.js/testing';
import { FunctionArtifact, FunctionSelector, encodeArguments } from '@aztec/foundation/abi';
import { AztecAddress } from '@aztec/foundation/aztec-address';
import { pedersenHash } from '@aztec/foundation/crypto';
import { EthAddress } from '@aztec/foundation/eth-address';
import { Fr } from '@aztec/foundation/fields';
import { openTmpStore } from '@aztec/kv-store/utils';
import { Pedersen, StandardIndexedTreeWithAppend } from '@aztec/merkle-tree';
import { ChildContractArtifact } from '@aztec/noir-contracts.js/Child';
import { ParentContractArtifact } from '@aztec/noir-contracts.js/Parent';
import { TestContractArtifact } from '@aztec/noir-contracts.js/Test';
Expand Down Expand Up @@ -50,16 +56,46 @@ describe('ACIR public execution simulator', () => {
executor = new PublicExecutor(publicState, publicContracts, commitmentsDb, header);
}, 10000);

const mockInitializationNullifierCallback = async (contractAddress: AztecAddress) => {
// We create a nullifier tree just to get the membership witness for the token contract
// initialization nullifier, which is checked by all of the Token contract functions.
const nullifierTree = new StandardIndexedTreeWithAppend(
openTmpStore(),
new Pedersen(),
'nullifier',
NULLIFIER_TREE_HEIGHT,
0n,
NullifierLeafPreimage,
NullifierLeaf,
);
await nullifierTree.init(1);
const initializationNullifier = siloNullifier(contractAddress, contractAddress.toField());
await nullifierTree.appendLeaves([initializationNullifier.toBuffer()]);
header.state.partial.nullifierTree.root = Fr.fromBuffer(nullifierTree.getRoot(true));
commitmentsDb.getNullifierMembershipWitnessAtLatestBlock.mockImplementation(async nullifier => {
if (nullifier.equals(initializationNullifier)) {
const index = 1n;
const preimage = nullifierTree.getLatestLeafPreimageCopy(index, true);
const siblingPath = await nullifierTree.getSiblingPath<typeof NULLIFIER_TREE_HEIGHT>(index, true);
return new NullifierMembershipWitness(index, preimage as NullifierLeafPreimage, siblingPath);
} else {
throw new Error(`Unexpected nullifier witness request for ${nullifier}`);
}
});
};

describe('Token contract', () => {
let recipient: AztecAddress;
let contractAddress: AztecAddress;

beforeEach(() => {
beforeEach(async () => {
recipient = AztecAddress.random();
contractAddress = AztecAddress.random();
await mockInitializationNullifierCallback(contractAddress);
});

describe('mint', () => {
it('should run the mint_public function', async () => {
const contractAddress = AztecAddress.random();
const mintArtifact = TokenContractArtifact.functions.find(f => f.name === 'mint_public')!;
const functionData = FunctionData.fromAbi(mintArtifact);

Expand Down Expand Up @@ -126,7 +162,6 @@ describe('ACIR public execution simulator', () => {
});

describe('transfer', () => {
let contractAddress: AztecAddress;
let transferArtifact: FunctionArtifact;
let functionData: FunctionData;
let args: Fr[];
Expand All @@ -137,7 +172,6 @@ describe('ACIR public execution simulator', () => {
let execution: PublicExecution;

beforeEach(() => {
contractAddress = AztecAddress.random();
transferArtifact = TokenContractArtifact.functions.find(f => f.name === 'transfer_public')!;
functionData = new FunctionData(FunctionSelector.empty(), false, false, false);
sender = AztecAddress.random();
Expand Down Expand Up @@ -295,8 +329,9 @@ describe('ACIR public execution simulator', () => {
let amount: Fr;
let params: Fr[];

beforeEach(() => {
beforeEach(async () => {
contractAddress = AztecAddress.random();
await mockInitializationNullifierCallback(contractAddress);
functionData = new FunctionData(FunctionSelector.empty(), false, false, false);
amount = new Fr(1);
params = [amount, new Fr(1)];
Expand Down

0 comments on commit 4550f25

Please sign in to comment.