Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Initializer checks across txs #4842

Merged
merged 6 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions noir-projects/aztec-nr/aztec/src/initializer.nr
Original file line number Diff line number Diff line change
@@ -1,29 +1,23 @@
use dep::protocol_types::hash::silo_nullifier;
use crate::context::PrivateContext;
use crate::history::nullifier_inclusion::prove_nullifier_inclusion;

pub fn mark_as_initialized(context: &mut PrivateContext) {
let init_nullifier = compute_unsiloed_contract_initialization_nullifier(context);
let init_nullifier = compute_unsiloed_contract_initialization_nullifier(*context);
context.push_new_nullifier(init_nullifier, 0);

// We push a commitment as well and use this value to check initialization,
// since we cannot yet read a nullifier from the same tx in which it was emitted.
// Eventually, when that's supported, we should delete this note_hash and
// have all checks rely on reading the nullifier directly.
// TODO(@spalladino) Remove when possible.
context.push_new_note_hash(init_nullifier);
}

// TODO(@spalladino): Add a variant using PublicContext once we can read nullifiers or note hashes from public-land.
pub fn assert_is_initialized(context: &mut PrivateContext) {
let init_nullifier = compute_contract_initialization_nullifier(context);
context.push_read_request(init_nullifier);
let init_nullifier = compute_contract_initialization_nullifier(*context);
prove_nullifier_inclusion(init_nullifier, *context);
}

pub fn compute_contract_initialization_nullifier(context: &mut PrivateContext) -> Field {
pub fn compute_contract_initialization_nullifier(context: PrivateContext) -> Field {
let address = context.this_address();
silo_nullifier(address, compute_unsiloed_contract_initialization_nullifier(context))
}

pub fn compute_unsiloed_contract_initialization_nullifier(context: &mut PrivateContext) -> Field {
pub fn compute_unsiloed_contract_initialization_nullifier(context: PrivateContext) -> Field {
context.this_address().to_field()
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract StatefulTest {

#[aztec(private)]
fn constructor(owner: AztecAddress, value: Field) {
let selector = FunctionSelector::from_signature("internal_create_note((Field),Field)");
let selector = FunctionSelector::from_signature("create_note_no_init_check((Field),Field)");
let _res = context.call_private_function(context.this_address(), selector, [owner.to_field(), value]);
mark_as_initialized(&mut context);
}
Expand All @@ -34,7 +34,7 @@ contract StatefulTest {
}

#[aztec(private)]
internal fn internal_create_note(owner: AztecAddress, value: Field) {
fn create_note_no_init_check(owner: AztecAddress, value: Field) {
if (value != 0) {
let loc = storage.notes.at(owner);
increment(loc, value, owner);
Expand All @@ -53,6 +53,18 @@ contract StatefulTest {
increment(recipient_notes, amount, recipient);
}

#[aztec(private)]
fn destroy_and_create_no_init_check(recipient: AztecAddress, amount: Field) {
let sender = context.msg_sender();

let sender_notes = storage.notes.at(sender);
decrement(sender_notes, amount, sender);

let recipient_notes = storage.notes.at(recipient);
increment(recipient_notes, amount, recipient);
}


#[aztec(public)]
fn increment_public_value(owner: AztecAddress, value: Field) {
let loc = storage.public_values.at(owner);
Expand All @@ -69,10 +81,4 @@ contract StatefulTest {
unconstrained fn get_public_value(owner: AztecAddress) -> pub Field {
storage.public_values.at(owner).read()
}

// This method is here because we've hit the 32 function limit for the TestContract
#[aztec(private)]
fn deploy_contract(target: AztecAddress) {
aztec_deploy_contract(&mut context, target);
}
}
}
14 changes: 10 additions & 4 deletions noir-projects/noir-contracts/contracts/test_contract/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ contract Test {
context::{Context, inputs::private_context_inputs::PrivateContextInputs}, hash::pedersen_hash,
context::PrivateContext,
note::{
note_header::NoteHeader, utils as note_utils, lifecycle::{create_note, destroy_note},
note_getter::{get_notes, view_notes}, note_getter_options::{NoteGetterOptions, NoteStatus},
note_viewer_options::NoteViewerOptions
},
note_header::NoteHeader, utils as note_utils, lifecycle::{create_note, destroy_note},
note_getter::{get_notes, view_notes}, note_getter_options::{NoteGetterOptions, NoteStatus},
note_viewer_options::NoteViewerOptions
},
deploy::{deploy_contract as aztec_deploy_contract},
oracle::{get_public_key::get_public_key as get_public_key_oracle, context::get_portal_address, rand::rand},
state_vars::PrivateImmutable, log::emit_unencrypted_log_from_private
};
Expand Down Expand Up @@ -349,6 +350,11 @@ contract Test {
assert(context.historical_header.hash() == header_hash, "Invalid header hash");
}

#[aztec(private)]
fn deploy_contract(target: AztecAddress) {
aztec_deploy_contract(&mut context, target);
}

unconstrained fn get_constant() -> pub Field {
let constant = storage.example_constant.view_note();
constant.value
Expand Down
28 changes: 22 additions & 6 deletions yarn-project/end-to-end/src/e2e_deploy_contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,16 @@ describe('e2e_deploy_contract', () => {
const owner = await registerRandomAccount(pxe);
const initArgs: StatefulContractCtorArgs = [owner, 42];
const contract = await registerContract(testWallet, StatefulTestContract, initArgs);
logger.info(`Calling the constructor for ${contract.address}`);
await contract.methods
.constructor(...initArgs)
.send()
.wait();
logger.info(`Checking if the constructor was run for ${contract.address}`);
expect(await contract.methods.summed_values(owner).view()).toEqual(42n);
logger.info(`Calling a function that requires initialization on ${contract.address}`);
await contract.methods.create_note(owner, 10).send().wait();
expect(await contract.methods.summed_values(owner).view()).toEqual(52n);
},
30_000,
);
Expand All @@ -214,6 +219,20 @@ describe('e2e_deploy_contract', () => {
expect(await contracts[1].methods.summed_values(owner).view()).toEqual(52n);
}, 30_000);

// TODO(@spalladino): This won't work until we can read a nullifier in the same tx in which it was emitted.
it.skip('initializes and calls a private function in a single tx', async () => {
const owner = await registerRandomAccount(pxe);
const initArgs: StatefulContractCtorArgs = [owner, 42];
const contract = await registerContract(wallet, StatefulTestContract, initArgs);
const batch = new BatchCall(wallet, [
contract.methods.constructor(...initArgs).request(),
contract.methods.create_note(owner, 10).request(),
]);
logger.info(`Executing constructor and private function in batch at ${contract.address}`);
await batch.send().wait();
expect(await contract.methods.summed_values(owner).view()).toEqual(52n);
});

it('refuses to initialize a contract twice', async () => {
const owner = await registerRandomAccount(pxe);
const initArgs: StatefulContractCtorArgs = [owner, 42];
Expand All @@ -234,12 +253,9 @@ describe('e2e_deploy_contract', () => {
const owner = await registerRandomAccount(pxe);
const initArgs: StatefulContractCtorArgs = [owner, 42];
const contract = await registerContract(wallet, StatefulTestContract, initArgs);
// TODO(@spalladino): It'd be nicer to be able to fail the assert with a more descriptive message,
// but the best we can do for now is pushing a read request to the kernel and wait for it to fail.
// Maybe we need an unconstrained check for the read request that runs within the app circuit simulation
// so we can bail earlier with a more descriptive error? I should create an issue for this.
// TODO(@spalladino): It'd be nicer to be able to fail the assert with a more descriptive message.
await expect(contract.methods.create_note(owner, 10).send().wait()).rejects.toThrow(
/The read request.*does not match/,
/nullifier witness not found/i,
);
});

Expand Down Expand Up @@ -350,7 +366,7 @@ describe('e2e_deploy_contract', () => {
// Register the instance to be deployed in the pxe
await wallet.addContracts([{ artifact, instance }]);
// Set up the contract that calls the deployer (which happens to be the StatefulTestContract) and call it
const deployer = await registerContract(wallet, StatefulTestContract, [accounts[0].address, 48]);
const deployer = await registerContract(wallet, TestContract, [accounts[0].address, 48]);
await deployer.methods.deploy_contract(instance.address).send().wait();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,10 @@ describe('e2e_inclusion_proofs_contract', () => {

await expect(
contract.methods.test_nullifier_inclusion(randomNullifier, true, blockNumber).send().wait(),
).rejects.toThrow(`Low nullifier witness not found for nullifier ${randomNullifier.toString()} at block`);
).rejects.toThrow(`Nullifier witness not found for nullifier ${randomNullifier.toString()} at block`);

await expect(contract.methods.test_nullifier_inclusion(randomNullifier, false, 0n).send().wait()).rejects.toThrow(
`Low nullifier witness not found for nullifier ${randomNullifier.toString()} at block`,
`Nullifier witness not found for nullifier ${randomNullifier.toString()} at block`,
);
});
});
Expand Down
4 changes: 1 addition & 3 deletions yarn-project/simulator/src/acvm/oracle/oracle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,7 @@ export class Oracle {

const witness = await this.typedOracle.getNullifierMembershipWitness(parsedBlockNumber, parsedNullifier);
if (!witness) {
throw new Error(
`Low nullifier witness not found for nullifier ${parsedNullifier} at block ${parsedBlockNumber}.`,
);
throw new Error(`Nullifier witness not found for nullifier ${parsedNullifier} at block ${parsedBlockNumber}.`);
}
return witness.toFields().map(toACVMField);
}
Expand Down
9 changes: 4 additions & 5 deletions yarn-project/simulator/src/client/private_execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ describe('Private Execution test suite', () => {
});

it('should run the create_note function', async () => {
const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'create_note');
const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'create_note_no_init_check');

const result = await runSimulator({ args: [owner, 140], artifact });

Expand All @@ -364,7 +364,7 @@ describe('Private Execution test suite', () => {

it('should run the destroy_and_create function', async () => {
const amountToTransfer = 100n;
const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create');
const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create_no_init_check');

const storageSlot = computeSlotForMapping(new Fr(1n), owner);
const recipientStorageSlot = computeSlotForMapping(new Fr(1n), recipient);
Expand Down Expand Up @@ -411,8 +411,7 @@ describe('Private Execution test suite', () => {
expect(changeNote.note.items[0]).toEqual(new Fr(40n));

const readRequests = sideEffectArrayToValueArray(
// We remove the first element which is the read request for the initialization commitment
nonEmptySideEffects(result.callStackItem.publicInputs.readRequests.slice(1)),
nonEmptySideEffects(result.callStackItem.publicInputs.readRequests),
);

expect(readRequests).toHaveLength(consumedNotes.length);
Expand All @@ -422,7 +421,7 @@ describe('Private Execution test suite', () => {
it('should be able to destroy_and_create with dummy notes', async () => {
const amountToTransfer = 100n;
const balance = 160n;
const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create');
const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create_no_init_check');

const storageSlot = computeSlotForMapping(new Fr(1n), owner);
const noteTypeId = new Fr(869710811710178111116101n); // ValueNote
Expand Down
Loading