From 4550f2596b51a985d6677191fd83bb2e621c5bc3 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Tue, 5 Mar 2024 14:27:06 -0300 Subject: [PATCH] feat: Add init check by default to public fns (#4897) 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... --- .../app_subscription_contract/src/main.nr | 1 + .../easy_private_voting_contract/src/main.nr | 5 +- .../contracts/fpc_contract/src/main.nr | 1 + .../inclusion_proofs_contract/src/main.nr | 1 + .../stateful_test_contract/src/main.nr | 2 +- .../token_blacklist_contract/src/main.nr | 1 + .../token_bridge_contract/src/main.nr | 1 + .../contracts/token_contract/src/main.nr | 1 + noir/noir-repo/aztec_macros/src/lib.rs | 10 ---- yarn-project/merkle-tree/src/index.ts | 1 + .../test/standard_indexed_tree_with_append.ts | 2 +- .../simulator/src/public/index.test.ts | 47 ++++++++++++++++--- 12 files changed, 53 insertions(+), 20 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr index b8312729da1..f983dbf3836 100644 --- a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr @@ -87,6 +87,7 @@ contract AppSubscription { #[aztec(public)] #[aztec(internal)] + #[aztec(noinitcheck)] fn init( target_address: AztecAddress, subscription_token_address: AztecAddress, diff --git a/noir-projects/noir-contracts/contracts/easy_private_voting_contract/src/main.nr b/noir-projects/noir-contracts/contracts/easy_private_voting_contract/src/main.nr index 76028481588..60cbf894ef1 100644 --- a/noir-projects/noir-contracts/contracts/easy_private_voting_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/easy_private_voting_contract/src/main.nr @@ -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); } diff --git a/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr b/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr index 9ab6846b076..7207f0ef308 100644 --- a/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr @@ -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); diff --git a/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr b/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr index 0deda6433a1..b026448a1b0 100644 --- a/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr @@ -38,6 +38,7 @@ contract InclusionProofs { #[aztec(public)] #[aztec(internal)] + #[aztec(noinitcheck)] fn _initialize(value: Field) { storage.public_value.write(value); } diff --git a/noir-projects/noir-contracts/contracts/stateful_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/stateful_test_contract/src/main.nr index 242e6f7f016..dddb482211e 100644 --- a/noir-projects/noir-contracts/contracts/stateful_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/stateful_test_contract/src/main.nr @@ -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); diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr index fe9bec5f4fe..1e26124a5bb 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr @@ -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); diff --git a/noir-projects/noir-contracts/contracts/token_bridge_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_bridge_contract/src/main.nr index e6d37db870e..1870537b2b9 100644 --- a/noir-projects/noir-contracts/contracts/token_bridge_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_bridge_contract/src/main.nr @@ -139,6 +139,7 @@ contract TokenBridge { #[aztec(public)] #[aztec(internal)] + #[aztec(noinitcheck)] fn _initialize(token: AztecAddress) { storage.token.write(token); } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr index 1321ad290f2..ed6ff0dfceb 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr @@ -314,6 +314,7 @@ contract Token { // docs:start:initialize #[aztec(public)] #[aztec(internal)] + #[aztec(noinitcheck)] fn _initialize( new_admin: AztecAddress, name: FieldCompressedString, diff --git a/noir/noir-repo/aztec_macros/src/lib.rs b/noir/noir-repo/aztec_macros/src/lib.rs index 9afedd39245..0c84c00820d 100644 --- a/noir/noir-repo/aztec_macros/src/lib.rs +++ b/noir/noir-repo/aztec_macros/src/lib.rs @@ -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; } @@ -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); } diff --git a/yarn-project/merkle-tree/src/index.ts b/yarn-project/merkle-tree/src/index.ts index 45ae1771bbf..ca08070bc8e 100644 --- a/yarn-project/merkle-tree/src/index.ts +++ b/yarn-project/merkle-tree/src/index.ts @@ -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'; diff --git a/yarn-project/merkle-tree/src/standard_indexed_tree/test/standard_indexed_tree_with_append.ts b/yarn-project/merkle-tree/src/standard_indexed_tree/test/standard_indexed_tree_with_append.ts index 60ff4a9ecca..a772f1fc5fb 100644 --- a/yarn-project/merkle-tree/src/standard_indexed_tree/test/standard_indexed_tree_with_append.ts +++ b/yarn-project/merkle-tree/src/standard_indexed_tree/test/standard_indexed_tree_with_append.ts @@ -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 diff --git a/yarn-project/simulator/src/public/index.test.ts b/yarn-project/simulator/src/public/index.test.ts index 810c0745c01..3c8396547c9 100644 --- a/yarn-project/simulator/src/public/index.test.ts +++ b/yarn-project/simulator/src/public/index.test.ts @@ -1,4 +1,4 @@ -import { L1ToL2Message, SiblingPath } from '@aztec/circuit-types'; +import { L1ToL2Message, NullifierMembershipWitness, SiblingPath } from '@aztec/circuit-types'; import { AppendOnlyTreeSnapshot, CallContext, @@ -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'; @@ -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(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); @@ -126,7 +162,6 @@ describe('ACIR public execution simulator', () => { }); describe('transfer', () => { - let contractAddress: AztecAddress; let transferArtifact: FunctionArtifact; let functionData: FunctionData; let args: Fr[]; @@ -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(); @@ -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)];