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

feat: pay fee when deploying account contracts #5540

Closed
Closed
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
17 changes: 15 additions & 2 deletions noir-projects/aztec-nr/authwit/src/account.nr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use dep::aztec::protocol_types::{address::AztecAddress, abis::function_selector:
use crate::entrypoint::{app::AppPayload, fee::FeePayload};
use crate::auth::{IS_VALID_SELECTOR, compute_outer_authwit_hash};

use crate::capsule::pop_capsule;

struct AccountActions {
context: Context,
is_valid_impl: fn(&mut PrivateContext, Field) -> bool,
Expand Down Expand Up @@ -54,6 +56,17 @@ impl AccountActions {
)
}

pub fn initialize_account_contract(self) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rename this to something like "execute_fee_payload_for_init", since it's not actually initializing the contract.

let valid_fn = self.is_valid_impl;
let mut private_context = self.context.private.unwrap();

let fee_payload = FeePayload::deserialize(pop_capsule());
let fee_hash = fee_payload.hash();
Comment on lines +63 to +64
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm starting to agree with Joe on this use case for capsules. The fee_payload should be an argument to this function and not something that shows up magically, and what we really need is a way to say "this argument is not part of the initialization hash". But we can change that in a future PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tasks for this would be:

  • Add a noinitargcheck attribute in Noir for the argument to be skipped
  • Make sure the attribute is emitted in the abi
  • Compute a separate args hash for initialization preimage check if there are any noinitargcheck
  • Change the address computation so it ignores args marked as noinitargcheck when computing the initialization hash in the address preimage (this will be the most annoying bit I think)
  • Ensure the deploy method does not ignore them when actually executing the initialization

It's quite a lot unfortunately, but feels cleaner..?

Copy link
Contributor Author

@alexghr alexghr Apr 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm starting to agree with Joe on this use case for capsules

lol same, but I was actually thinking of going the protocol contract route instead of adding a new macro 😅

assert(valid_fn(private_context, fee_hash));
fee_payload.execute_calls(private_context);
private_context.capture_min_revertible_side_effect_counter();
}

// docs:start:entrypoint
pub fn entrypoint(self, app_payload: AppPayload, fee_payload: FeePayload) {
let valid_fn = self.is_valid_impl;
Expand All @@ -73,7 +86,7 @@ impl AccountActions {
// docs:start:spend_private_authwit
pub fn spend_private_authwit(self, inner_hash: Field) -> Field {
let context = self.context.private.unwrap();
// The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can
// The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can
// consume the message.
// This ensures that contracts cannot consume messages that are not intended for them.
let message_hash = compute_outer_authwit_hash(
Expand All @@ -92,7 +105,7 @@ impl AccountActions {
// docs:start:spend_public_authwit
pub fn spend_public_authwit(self, inner_hash: Field) -> Field {
let context = self.context.public.unwrap();
// The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can
// The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can
// consume the message.
// This ensures that contracts cannot consume messages that are not intended for them.
let message_hash = compute_outer_authwit_hash(
Expand Down
10 changes: 10 additions & 0 deletions noir-projects/aztec-nr/authwit/src/capsule.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copied from noir-contracts/contracts/slow_tree_contract/src/capsule.nr
// We should extract this to a shared lib in aztec-nr once we settle on a design for capsules

#[oracle(popCapsule)]
fn pop_capsule_oracle<N>() -> [Field; N] {}

// A capsule is a "blob" of data that is passed to the contract through an oracle.
unconstrained pub fn pop_capsule<N>() -> [Field; N] {
pop_capsule_oracle()
}
16 changes: 15 additions & 1 deletion noir-projects/aztec-nr/authwit/src/entrypoint/fee.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use dep::aztec::prelude::PrivateContext;
use dep::aztec::protocol_types::{constants::GENERATOR_INDEX__FEE_PAYLOAD, hash::pedersen_hash, traits::{Hash, Serialize}};
use dep::aztec::protocol_types::utils::reader::Reader;
use dep::aztec::protocol_types::{constants::GENERATOR_INDEX__FEE_PAYLOAD, hash::pedersen_hash, traits::{Hash, Serialize, Deserialize}};
use crate::entrypoint::function_call::FunctionCall;

// 2 * 4 (function call) + 1
Expand Down Expand Up @@ -29,6 +30,19 @@ impl Serialize<FEE_PAYLOAD_SIZE> for FeePayload {
}
}

impl Deserialize<FEE_PAYLOAD_SIZE> for FeePayload {
// Deserializes the entrypoint struct
fn deserialize(fields: [Field; FEE_PAYLOAD_SIZE]) -> Self {
let mut reader = Reader::new(fields);
let payload = FeePayload {
function_calls: reader.read_struct_array(FunctionCall::deserialize, [FunctionCall::empty(); MAX_FEE_FUNCTION_CALLS]),
nonce: reader.read(),
};
reader.finish();
payload
}
}

impl Hash for FeePayload {
fn hash(self) -> Field {
pedersen_hash(
Expand Down
27 changes: 26 additions & 1 deletion noir-projects/aztec-nr/authwit/src/entrypoint/function_call.nr
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use dep::aztec::protocol_types::{abis::function_selector::FunctionSelector, address::AztecAddress, traits::Serialize};
use dep::aztec::protocol_types::{
abis::function_selector::FunctionSelector, address::AztecAddress,
traits::{Serialize, Deserialize, Empty}
};

// 1 (ARGS_HASH) + 1 (FUNCTION_SELECTOR) + 1 (TARGET_ADDRESS) + 1 (IS_PUBLIC)
global FUNCTION_CALL_SIZE: Field = 4;
Expand All @@ -18,6 +21,28 @@ impl Serialize<FUNCTION_CALL_SIZE> for FunctionCall {
}
}

impl Deserialize<FUNCTION_CALL_SIZE> for FunctionCall {
fn deserialize(data: [Field; FUNCTION_CALL_SIZE]) -> Self {
FunctionCall {
args_hash: data[0],
function_selector: FunctionSelector::from_field(data[1]),
target_address: AztecAddress::from_field(data[2]),
is_public: data[3] as bool,
}
}
}

impl Empty for FunctionCall {
fn empty() -> Self {
FunctionCall {
args_hash: 0,
function_selector: FunctionSelector::zero(),
target_address: AztecAddress::empty(),
is_public: false,
}
}
}

impl FunctionCall {
fn to_be_bytes(self) -> [u8; FUNCTION_CALL_SIZE_IN_BYTES] {
let mut bytes: [u8; FUNCTION_CALL_SIZE_IN_BYTES] = [0; FUNCTION_CALL_SIZE_IN_BYTES];
Expand Down
1 change: 1 addition & 0 deletions noir-projects/aztec-nr/authwit/src/lib.nr
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ mod account;
mod auth_witness;
mod auth;
mod entrypoint;
mod capsule;
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ contract EcdsaAccount {
let this = context.this_address();
let mut pub_key_note = EcdsaPublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this);
storage.public_key.initialize(&mut pub_key_note, true);

let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl);
actions.initialize_account_contract();
}

// Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ contract SchnorrAccount {
let mut pub_key_note = PublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this);
storage.signing_public_key.initialize(&mut pub_key_note, true);
// docs:end:initialize

let actions = AccountActions::private(
&mut context,
storage.approved_actions.storage_slot,
is_valid_impl
);
actions.initialize_account_contract();
}

// Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts file
Expand Down Expand Up @@ -117,9 +124,9 @@ contract SchnorrAccount {
* @param block_number The block number to check the nullifier against
* @param check_private Whether to check the validity of the authwitness in private state or not
* @param message_hash The message hash of the message to check the validity
* @return An array of two booleans, the first is the validity of the authwitness in the private state,
* @return An array of two booleans, the first is the validity of the authwitness in the private state,
* the second is the validity of the authwitness in the public state
* Both values will be `false` if the nullifier is spent
* Both values will be `false` if the nullifier is spent
*/
unconstrained fn lookup_validity(
myself: AztecAddress,
Expand Down Expand Up @@ -147,7 +154,7 @@ contract SchnorrAccount {
let valid_in_public = storage.approved_actions.at(message_hash).read();

// Compute the nullifier and check if it is spent
// This will BLINDLY TRUST the oracle, but the oracle is us, and
// This will BLINDLY TRUST the oracle, but the oracle is us, and
// it is not as part of execution of the contract, so we are good.
let siloed_nullifier = compute_siloed_nullifier(myself, message_hash);
let lower_wit = get_low_nullifier_membership_witness(block_number, siloed_nullifier);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ contract SchnorrHardcodedAccount {

global ACCOUNT_ACTIONS_STORAGE_SLOT = 1;

#[aztec(private)]
#[aztec(initializer)]
fn constructor() {
let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl);
actions.initialize_account_contract();
}
Comment on lines +17 to +22
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this: this account contract does not need initialization at all.


// Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts
#[aztec(private)]
fn entrypoint(app_payload: pub AppPayload, fee_payload: pub FeePayload) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ contract SchnorrSingleKeyAccount {

global ACCOUNT_ACTIONS_STORAGE_SLOT = 1;

#[aztec(private)]
#[aztec(initializer)]
fn constructor() {
let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl);
actions.initialize_account_contract();
}
Comment on lines +13 to +18
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this one as well


// Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts
#[aztec(private)]
fn entrypoint(app_payload: pub AppPayload, fee_payload: pub FeePayload) {
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/accounts/src/single_key/account_contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export class SingleKeyAccountContract extends DefaultAccountContract {
super(SchnorrSingleKeyAccountContractArtifact as ContractArtifact);
}

getDeploymentArgs(): undefined {
return undefined;
getDeploymentArgs(): Array<void> {
return [];
}

getAuthWitnessProvider({ partialAddress }: CompleteAddress): AuthWitnessProvider {
Expand Down
40 changes: 21 additions & 19 deletions yarn-project/accounts/src/testing/configuration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { generatePublicKey } from '@aztec/aztec.js';
import { type DeploySentTx, generatePublicKey } from '@aztec/aztec.js';
import { type AccountWalletWithPrivateKey } from '@aztec/aztec.js/wallet';
import { type PXE } from '@aztec/circuit-types';
import { Fr, GrumpkinScalar } from '@aztec/foundation/fields';
Expand Down Expand Up @@ -58,24 +58,26 @@ export async function deployInitialTestAccounts(pxe: PXE) {
privateKey,
};
});
// Attempt to get as much parallelism as possible
const deployMethods = await Promise.all(
accounts.map(async x => {
const deployMethod = await x.account.getDeployMethod();
await deployMethod.create({
contractAddressSalt: x.account.salt,
skipClassRegistration: true,
skipPublicDeployment: true,
universalDeploy: true,
});
await deployMethod.prove({});
return deployMethod;
}),
);
// Send tx together to try and get them in the same rollup
const sentTxs = deployMethods.map(dm => {
return dm.send();
});

const sentTxs: DeploySentTx[] = [];
for (const { account } of accounts) {
const deploymentMethod = account.getDeployMethod();

// pxe needs to prove txs one-by-one
// this is because the tx use capsules and the capsule stack is a shared resource
// TODO #5556 parallelize this back
await deploymentMethod.prove({
contractAddressSalt: account.salt,
});

// the txs can be processed in parallel by the sequencer though
sentTxs.push(
deploymentMethod.send({
contractAddressSalt: account.salt,
}),
);
}

await Promise.all(
sentTxs.map(async (tx, i) => {
const wallet = await accounts[i].account.getWallet();
Expand Down
14 changes: 5 additions & 9 deletions yarn-project/accounts/src/testing/create_account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,15 @@ export async function createAccounts(pxe: PXE, numberOfAccounts = 1): Promise<Ac
// Unfortunately the function below is not stateless and we call it here because it takes a long time to run and
// the results get stored within the account object. By calling it here we increase the probability of all the
// accounts being deployed in the same block because it makes the deploy() method basically instant.
await account.getDeployMethod().then(d =>
d.prove({
contractAddressSalt: account.salt,
skipClassRegistration: true,
skipPublicDeployment: true,
universalDeploy: true,
}),
);
const d = account.getDeployMethod();
await d.prove({
contractAddressSalt: account.salt,
});
accounts.push(account);
}

// Send them and await them to be mined
const txs = await Promise.all(accounts.map(account => account.deploy()));
const txs = accounts.map(account => account.deploy());
await Promise.all(txs.map(tx => tx.wait({ interval: 0.1 })));
return Promise.all(accounts.map(account => account.getWallet()));
}
8 changes: 7 additions & 1 deletion yarn-project/aztec.js/src/account/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { type CompleteAddress } from '@aztec/circuit-types';
import { type ContractArtifact } from '@aztec/foundation/abi';
import { type NodeInfo } from '@aztec/types/interfaces';

import { type AccountInterface } from './interface.js';
import { type AccountInterface, type AuthWitnessProvider } from './interface.js';

// docs:start:account-contract-interface
/**
Expand All @@ -29,5 +29,11 @@ export interface AccountContract {
* @returns An account interface instance for creating tx requests and authorizing actions.
*/
getInterface(address: CompleteAddress, nodeInfo: NodeInfo): AccountInterface;

/**
* Returns the auth witness provider for the given address.
* @param address - Address for which to create auth witnesses.
*/
getAuthWitnessProvider(address: CompleteAddress): AuthWitnessProvider;
}
// docs:end:account-contract-interface
Loading
Loading