Skip to content

Commit

Permalink
feat: Check initialization arguments in constructors (#5144)
Browse files Browse the repository at this point in the history
The responsibility to check that the initialization arguments used when
calling the initializer as correct is responsibility of the initializer
itself. Without this check, a deployer could commit to a set of init
args in the address preimage, but then call the initializer using
something totally different.

Fixes #5154

---------

Co-authored-by: Alex Gherghisan <[email protected]>
  • Loading branch information
spalladino and alexghr authored Mar 13, 2024
1 parent 3e68b49 commit d003bd6
Show file tree
Hide file tree
Showing 21 changed files with 301 additions and 181 deletions.
1 change: 1 addition & 0 deletions noir-projects/aztec-nr/aztec/src/context/interface.nr
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ trait ContextInterface {
fn chain_id(self) -> Field;
fn version(self) -> Field;
fn selector(self) -> FunctionSelector;
fn get_args_hash(self) -> Field;
fn get_header(self) -> Header;
}
4 changes: 4 additions & 0 deletions noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ impl ContextInterface for PrivateContext {
self.inputs.call_context.function_selector
}

fn get_args_hash(self) -> Field {
self.args_hash
}

// Returns the header of a block whose state is used during private execution (not the block the transaction is
// included in).
pub fn get_header(self) -> Header {
Expand Down
4 changes: 4 additions & 0 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ impl ContextInterface for PublicContext {
self.inputs.call_context.function_selector
}

fn get_args_hash(self) -> Field {
self.args_hash
}

fn get_header(self) -> Header {
self.historical_header
}
Expand Down
25 changes: 22 additions & 3 deletions noir-projects/aztec-nr/aztec/src/initializer.nr
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
use dep::protocol_types::hash::silo_nullifier;
use crate::context::{PrivateContext, PublicContext, ContextInterface};
use crate::history::nullifier_inclusion::prove_nullifier_inclusion;
use dep::protocol_types::{
hash::{silo_nullifier, pedersen_hash},
constants::GENERATOR_INDEX__CONSTRUCTOR,
abis::function_selector::FunctionSelector,
};

use crate::{
context::{PrivateContext, PublicContext, ContextInterface},
oracle::get_contract_instance::get_contract_instance,
history::nullifier_inclusion::prove_nullifier_inclusion,
};

pub fn mark_as_initialized<TContext>(context: &mut TContext) where TContext: ContextInterface {
let init_nullifier = compute_unsiloed_contract_initialization_nullifier(*context);
Expand All @@ -23,3 +31,14 @@ pub fn compute_contract_initialization_nullifier<TContext>(context: TContext) ->
pub fn compute_unsiloed_contract_initialization_nullifier<TContext>(context: TContext) -> Field where TContext: ContextInterface {
context.this_address().to_field()
}

pub fn assert_initialization_args_match_address_preimage<TContext>(context: TContext) where TContext: ContextInterface {
let address = context.this_address();
let instance = get_contract_instance(address);
let expected_init = compute_initialization_hash(context.selector(), context.get_args_hash());
assert(instance.initialization_hash == expected_init, "Initialization hash does not match");
}

pub fn compute_initialization_hash(init_selector: FunctionSelector, init_args_hash: Field) -> Field {
pedersen_hash([init_selector.to_field(), init_args_hash], GENERATOR_INDEX__CONSTRUCTOR)
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ contract StatefulTest {
use dep::aztec::{
deploy::{deploy_contract as aztec_deploy_contract}, context::{PublicContext, Context},
oracle::get_contract_instance::get_contract_instance,
initializer::{mark_as_initialized, assert_is_initialized}
initializer::assert_is_initialized,
};

struct Storage {
Expand Down
23 changes: 23 additions & 0 deletions noir/noir-repo/aztec_macros/src/transforms/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ pub fn transform_function(
func.def.body.0.insert(0, init_check);
}

// Add assertion for initialization arguments
if is_initializer {
let assert_init_args = create_assert_init_args();
func.def.body.0.insert(0, assert_init_args);
}

// Add access to the storage struct
if storage_defined {
let storage_def = abstract_storage(&ty.to_lowercase(), false);
Expand Down Expand Up @@ -205,6 +211,23 @@ fn create_internal_check(fname: &str) -> Statement {
)))
}

/// Creates a call to assert_initialization_args_match_address_preimage to ensure
/// the initialization arguments used in the init call match the address preimage.
///
/// ```noir
/// assert_initialization_args_match_address_preimage(context);
/// ```
fn create_assert_init_args() -> Statement {
make_statement(StatementKind::Expression(call(
variable_path(chained_dep!(
"aztec",
"initializer",
"assert_initialization_args_match_address_preimage"
)),
vec![variable("context")],
)))
}

/// Creates the private context object to be accessed within the function, the parameters need to be extracted to be
/// appended into the args hash object.
///
Expand Down
15 changes: 6 additions & 9 deletions yarn-project/aztec.js/src/account_manager/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CompleteAddress, GrumpkinPrivateKey, PXE } from '@aztec/circuit-types';
import { EthAddress, PublicKey, getContractInstanceFromDeployParams } from '@aztec/circuits.js';
import { PublicKey, getContractInstanceFromDeployParams } from '@aztec/circuits.js';
import { Fr } from '@aztec/foundation/fields';
import { ContractInstanceWithAddress } from '@aztec/types/contracts';

Expand Down Expand Up @@ -77,14 +77,11 @@ export class AccountManager {
public getInstance(): ContractInstanceWithAddress {
if (!this.instance) {
const encryptionPublicKey = generatePublicKey(this.encryptionPrivateKey);
const portalAddress = EthAddress.ZERO;
this.instance = getContractInstanceFromDeployParams(
this.accountContract.getContractArtifact(),
this.accountContract.getDeploymentArgs(),
this.salt,
encryptionPublicKey,
portalAddress,
);
this.instance = getContractInstanceFromDeployParams(this.accountContract.getContractArtifact(), {
constructorArgs: this.accountContract.getDeploymentArgs(),
salt: this.salt,
publicKey: encryptionPublicKey,
});
}
return this.instance;
}
Expand Down
11 changes: 7 additions & 4 deletions yarn-project/aztec.js/src/contract/deploy_method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,13 @@ export class DeployMethod<TContract extends ContractBase = Contract> extends Bas
*/
public getInstance(options: DeployOptions = {}): ContractInstanceWithAddress {
if (!this.instance) {
const portalContract = options.portalContract ?? EthAddress.ZERO;
const contractAddressSalt = options.contractAddressSalt ?? Fr.random();
const deployParams = [this.artifact, this.args, contractAddressSalt, this.publicKey, portalContract] as const;
this.instance = getContractInstanceFromDeployParams(...deployParams);
this.instance = getContractInstanceFromDeployParams(this.artifact, {
constructorArgs: this.args,
salt: options.contractAddressSalt,
portalAddress: options.portalContract,
publicKey: this.publicKey,
constructorName: this.constructorArtifact.name,
});
}
return this.instance;
}
Expand Down
29 changes: 17 additions & 12 deletions yarn-project/circuits.js/src/contract/contract_instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,32 @@ import {
computeInitializationHash,
computePublicKeysHash,
} from './contract_address.js';
import { isConstructor } from './contract_tree/contract_tree.js';

/**
* Generates a Contract Instance from the deployment params.
* @param artifact - The account contract build artifact.
* @param args - The args to the account contract constructor
* @param contractAddressSalt - The salt to be used in the contract address derivation
* @param publicKey - The account public key
* @param portalContractAddress - The portal contract address
* @param opts - Options for the deployment.
* @returns - The contract instance
*/
export function getContractInstanceFromDeployParams(
artifact: ContractArtifact,
args: any[] = [],
contractAddressSalt: Fr = Fr.random(),
publicKey: PublicKey = Point.ZERO,
portalContractAddress: EthAddress = EthAddress.ZERO,
opts: {
constructorName?: string;
constructorArgs?: any[];
salt?: Fr;
publicKey?: PublicKey;
portalAddress?: EthAddress;
},
): ContractInstanceWithAddress {
const constructorArtifact = artifact.functions.find(isConstructor);
const args = opts.constructorArgs ?? [];
const salt = opts.salt ?? Fr.random();
const publicKey = opts.publicKey ?? Point.ZERO;
const portalContractAddress = opts.portalAddress ?? EthAddress.ZERO;
const constructorName = opts.constructorName ?? 'constructor';

const constructorArtifact = artifact.functions.find(fn => fn.name === constructorName);
if (!constructorArtifact) {
throw new Error('Cannot find constructor in the artifact.');
throw new Error(`Cannot find constructor with name ${constructorName} in the artifact.`);
}
if (!constructorArtifact.verificationKey) {
throw new Error('Missing verification key for the constructor.');
Expand All @@ -47,7 +52,7 @@ export function getContractInstanceFromDeployParams(
initializationHash,
portalContractAddress,
publicKeysHash,
salt: contractAddressSalt,
salt,
version: 1,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function hashVKStr(vk: string) {
* Determine if the given function is a constructor.
* This utility function checks if the 'name' property of the input object is "constructor".
* Returns true if the function is a constructor, false otherwise.
*
* TODO(palla/purge-old-contract-deploy): Remove me
* @param Object - An object containing a 'name' property.
* @returns Boolean indicating if the function is a constructor.
*/
Expand Down
12 changes: 5 additions & 7 deletions yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
generatePublicKey,
getContractInstanceFromDeployParams,
} from '@aztec/aztec.js';
import { EthAddress, computePartialAddress } from '@aztec/circuits.js';
import { computePartialAddress } from '@aztec/circuits.js';
import { InclusionProofsContract } from '@aztec/noir-contracts.js';
import { ClaimContract } from '@aztec/noir-contracts.js/Claim';
import { CrowdfundingContract, CrowdfundingContractArtifact } from '@aztec/noir-contracts.js/Crowdfunding';
Expand Down Expand Up @@ -108,13 +108,11 @@ describe('e2e_crowdfunding_and_claim', () => {
const salt = Fr.random();

const args = [donationToken.address, operatorWallet.getAddress(), deadline];
const deployInfo = getContractInstanceFromDeployParams(
CrowdfundingContractArtifact,
args,
const deployInfo = getContractInstanceFromDeployParams(CrowdfundingContractArtifact, {
constructorArgs: args,
salt,
crowdfundingPublicKey,
EthAddress.ZERO,
);
publicKey: crowdfundingPublicKey,
});
await pxe.registerAccount(crowdfundingPrivateKey, computePartialAddress(deployInfo));

crowdfundingContract = await CrowdfundingContract.deployWithPublicKey(
Expand Down
Loading

0 comments on commit d003bd6

Please sign in to comment.