Skip to content

Commit

Permalink
fix: avoid recomputing contractclassid (#11783)
Browse files Browse the repository at this point in the history
TXE was spending a lot of time on that

---------

Co-authored-by: saleel <[email protected]>
  • Loading branch information
Thunkar and saleel authored Feb 6, 2025
1 parent 952615b commit f8448bf
Show file tree
Hide file tree
Showing 25 changed files with 158 additions and 138 deletions.
2 changes: 1 addition & 1 deletion docs/docs/developers/guides/smart_contracts/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ Unconstrained functions can be directly called from the contract interface. Noti
The test environment provides two different ways of creating accounts, depending on the testing needs. For most cases, it is only necessary to obtain a valid `AztecAddress` that represents the user's account contract. For this, is is enough to do:

```rust
let mocked_account_address = env.create_account();
let mocked_account_address = env.create_account(secret);
```

These accounts also create the necessary keys to ensure notes can be created/nullified, etc.
Expand Down
2 changes: 1 addition & 1 deletion noir-projects/aztec-nr/aztec/src/keys/getters/test.nr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ global KEY_ORACLE_RESPONSE_LENGTH: u32 = 13; // 12 fields for the keys, one fiel

unconstrained fn setup() -> TestAccount {
let _ = TestEnvironment::new();
let account = cheatcodes::create_account();
let account = cheatcodes::create_account(1);

account
}
Expand Down
6 changes: 3 additions & 3 deletions noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ pub unconstrained fn direct_storage_write<let N: u32>(
let _hash = direct_storage_write_oracle(contract_address, storage_slot, fields);
}

pub unconstrained fn create_account() -> TestAccount {
oracle_create_account()
pub unconstrained fn create_account(secret: Field) -> TestAccount {
oracle_create_account(secret)
}

pub unconstrained fn add_account(secret: Field) -> TestAccount {
Expand Down Expand Up @@ -121,7 +121,7 @@ unconstrained fn direct_storage_write_oracle<let N: u32>(
) -> [Field; N] {}

#[oracle(createAccount)]
unconstrained fn oracle_create_account() -> TestAccount {}
unconstrained fn oracle_create_account(secret: Field) -> TestAccount {}

#[oracle(addAccount)]
unconstrained fn oracle_add_account(secret: Field) -> TestAccount {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ impl TestEnvironment {
PrivateContext::new(inputs, 0)
}

pub unconstrained fn create_account(_self: Self) -> AztecAddress {
let test_account = cheatcodes::create_account();
pub unconstrained fn create_account(_self: Self, secret: Field) -> AztecAddress {
let test_account = cheatcodes::create_account(secret);
test_account.address
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use crate::Auth;
pub unconstrained fn setup() -> (&mut TestEnvironment, AztecAddress, AztecAddress, AztecAddress, AztecAddress) {
let mut env = TestEnvironment::new();

let admin = env.create_account();
let to_authorize = env.create_account();
let other = env.create_account();
let admin = env.create_account(1);
let to_authorize = env.create_account(2);
let other = env.create_account(3);

let initializer_call_interface = Auth::interface().constructor(admin);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ pub contract Counter {
unconstrained fn test_increment() {
// Setup env, generate keys
let mut env = TestEnvironment::new();
let owner = env.create_account();
let sender = env.create_account();
let owner = env.create_account(1);
let sender = env.create_account(2);
let initial_value: Field = 5;
env.impersonate(owner);
// Deploy contract and initialize
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ pub unconstrained fn setup(
) -> (&mut TestEnvironment, AztecAddress, AztecAddress, AztecAddress) {
// Setup env, generate keys
let mut env = TestEnvironment::new();
let owner = env.create_account();
let sender = env.create_account();
let owner = env.create_account(1);
let sender = env.create_account(2);
env.impersonate(owner);
// Deploy contract and initialize
let initializer = Counter::interface().initialize(initial_value as u64, owner);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ unconstrained fn test_end_vote() {
#[test(should_fail)]
unconstrained fn test_fail_end_vote_by_non_admin() {
let (env, voting_contract_address, _) = utils::setup();
let alice = env.create_account();
let alice = env.create_account(2);

env.impersonate(alice);
EasyPrivateVoting::at(voting_contract_address).end_vote().call(&mut env.public());
Expand All @@ -53,7 +53,7 @@ unconstrained fn test_fail_end_vote_by_non_admin() {
#[test]
unconstrained fn test_cast_vote() {
let (env, voting_contract_address, _) = utils::setup();
let alice = env.create_account();
let alice = env.create_account(2);
env.impersonate(alice);

let candidate = 1;
Expand All @@ -72,8 +72,8 @@ unconstrained fn test_cast_vote() {
#[test]
unconstrained fn test_cast_vote_with_separate_accounts() {
let (env, voting_contract_address, _) = utils::setup();
let alice = env.create_account();
let bob = env.create_account();
let alice = env.create_account(2);
let bob = env.create_account(3);

let candidate = 101;

Expand All @@ -97,7 +97,7 @@ unconstrained fn test_cast_vote_with_separate_accounts() {
#[test(should_fail)]
unconstrained fn test_fail_vote_twice() {
let (env, voting_contract_address, _) = utils::setup();
let alice = env.create_account();
let alice = env.create_account(2);

let candidate = 101;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::EasyPrivateVoting;
pub unconstrained fn setup() -> (&mut TestEnvironment, AztecAddress, AztecAddress) {
let mut env = TestEnvironment::new();

let admin = env.create_account();
let admin = env.create_account(1);

let initializer_call_interface = EasyPrivateVoting::interface().constructor(admin);
let voting_contract = env.deploy_self("EasyPrivateVoting").with_public_void_initializer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ unconstrained fn transfer_in_private_to_non_deployed_account() {
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
let (env, nft_contract_address, sender, _, token_id) =
utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false);
let not_deployed = cheatcodes::create_account();
let not_deployed = cheatcodes::create_account(999);

// Transfer the NFT to the recipient
NFT::at(nft_contract_address)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ pub unconstrained fn setup(
let recipient = env.create_account_contract(2);
(owner, recipient)
} else {
let owner = env.create_account();
let recipient = env.create_account();
let owner = env.create_account(1);
let recipient = env.create_account(2);
(owner, recipient)
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ pub contract Parent {
unconstrained fn test_private_call() {
// Setup env, generate keys
let mut env = TestEnvironment::new();
let owner = env.create_account();
let owner = env.create_account(1);
// Deploy parent contract
let parent_contract = env.deploy_self("Parent").without_initializer();
let parent_contract_address = parent_contract.to_address();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ unconstrained fn transfer_private_to_non_deployed_account() {
// Setup without account contracts. We are not using authwits here, so dummy accounts are enough
let (env, token_contract_address, owner, _, mint_amount) =
utils::setup_and_mint_to_private(/* with_account_contracts */ false);
let not_deployed = cheatcodes::create_account();
let not_deployed = cheatcodes::create_account(999);
// Transfer tokens
let transfer_amount = U128::from_integer(1000);
Token::at(token_contract_address).transfer(not_deployed.address, transfer_amount).call(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ pub unconstrained fn setup(
let recipient = env.create_account_contract(2);
(owner, recipient)
} else {
let owner = env.create_account();
let recipient = env.create_account();
let owner = env.create_account(1);
let recipient = env.create_account(2);
(owner, recipient)
};

Expand Down
3 changes: 1 addition & 2 deletions yarn-project/circuit-types/src/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
PrivateToPublicAccumulatedDataBuilder,
SerializableContractInstance,
computeContractAddressFromInstance,
computeContractClassId,
getContractClassFromArtifact,
} from '@aztec/circuits.js';
import { computeVarArgsHash } from '@aztec/circuits.js/hash';
Expand Down Expand Up @@ -226,7 +225,7 @@ export const randomContractInstanceWithAddress = async (

export const randomDeployedContract = async () => {
const artifact = randomContractArtifact();
const contractClassId = await computeContractClassId(await getContractClassFromArtifact(artifact));
const { id: contractClassId } = await getContractClassFromArtifact(artifact);
return { artifact, instance: await randomContractInstanceWithAddress({ contractClassId }) };
};

Expand Down
4 changes: 1 addition & 3 deletions yarn-project/circuits.js/src/contract/contract_instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { BufferReader, numToUInt8, serializeToBuffer } from '@aztec/foundation/s
import { type FieldsOf } from '@aztec/foundation/types';

import { getContractClassFromArtifact } from '../contract/contract_class.js';
import { computeContractClassId } from '../contract/contract_class_id.js';
import { PublicKeys } from '../types/public_keys.js';
import {
computeContractAddressFromInstance,
Expand Down Expand Up @@ -114,7 +113,6 @@ export async function getContractInstanceFromDeployParams(
const constructorArtifact = getConstructorArtifact(artifact, opts.constructorArtifact);
const deployer = opts.deployer ?? AztecAddress.ZERO;
const contractClass = await getContractClassFromArtifact(artifact);
const contractClassId = await computeContractClassId(contractClass);
const initializationHash =
constructorArtifact && opts?.skipArgsDecoding
? await computeInitializationHashFromEncodedArgs(
Expand All @@ -125,7 +123,7 @@ export async function getContractInstanceFromDeployParams(
const publicKeys = opts.publicKeys ?? PublicKeys.default();

const instance: ContractInstance = {
contractClassId,
contractClassId: contractClass.id,
initializationHash,
publicKeys,
salt,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
deployInstance,
registerContractClass,
} from '@aztec/aztec.js/deployment';
import { type ContractClassIdPreimage, PublicKeys, computeContractClassId } from '@aztec/circuits.js';
import { type ContractClassIdPreimage, PublicKeys } from '@aztec/circuits.js';
import { FunctionSelector, FunctionType } from '@aztec/foundation/abi';
import { writeTestData } from '@aztec/foundation/testing/files';
import { StatefulTestContract } from '@aztec/noir-contracts.js/StatefulTest';
Expand Down Expand Up @@ -76,7 +76,7 @@ describe('e2e_deploy_contract contract class registration', () => {

// TODO(#10007) Remove this test as well.
it('starts archiver with pre-registered common contracts', async () => {
const classId = await computeContractClassId(await getContractClassFromArtifact(TokenContractArtifact));
const { id: classId } = await getContractClassFromArtifact(TokenContractArtifact);
// The node checks the registration nullifier
expect(await aztecNode.getContractClass(classId)).toBeUndefined();
// But the archiver does not
Expand Down
15 changes: 5 additions & 10 deletions yarn-project/pxe/src/pxe_service/pxe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,7 @@ import type {
PartialAddress,
PrivateKernelTailCircuitPublicInputs,
} from '@aztec/circuits.js';
import {
computeContractAddressFromInstance,
computeContractClassId,
getContractClassFromArtifact,
} from '@aztec/circuits.js/contract';
import { computeContractAddressFromInstance, getContractClassFromArtifact } from '@aztec/circuits.js/contract';
import { computeNoteHashNonce, siloNullifier } from '@aztec/circuits.js/hash';
import { computeAddressSecret } from '@aztec/circuits.js/keys';
import {
Expand Down Expand Up @@ -240,7 +236,7 @@ export class PXEService implements PXE {
}

public async registerContractClass(artifact: ContractArtifact): Promise<void> {
const contractClassId = await computeContractClassId(await getContractClassFromArtifact(artifact));
const { id: contractClassId } = await getContractClassFromArtifact(artifact);
await this.db.addContractArtifact(contractClassId, artifact);
this.log.info(`Added contract class ${artifact.name} with id ${contractClassId}`);
}
Expand All @@ -252,18 +248,17 @@ export class PXEService implements PXE {
if (artifact) {
// If the user provides an artifact, validate it against the expected class id and register it
const contractClass = await getContractClassFromArtifact(artifact);
const contractClassId = await computeContractClassId(contractClass);
if (!contractClassId.equals(instance.contractClassId)) {
if (!contractClass.id.equals(instance.contractClassId)) {
throw new Error(
`Artifact does not match expected class id (computed ${contractClassId} but instance refers to ${instance.contractClassId})`,
`Artifact does not match expected class id (computed ${contractClass.id} but instance refers to ${instance.contractClassId})`,
);
}
const computedAddress = await computeContractAddressFromInstance(instance);
if (!computedAddress.equals(instance.address)) {
throw new Error('Added a contract in which the address does not match the contract instance.');
}

await this.db.addContractArtifact(contractClassId, artifact);
await this.db.addContractArtifact(contractClass.id, artifact);

const publicFunctionSignatures = artifact.functions
.filter(fn => fn.functionType === FunctionType.PUBLIC)
Expand Down
1 change: 0 additions & 1 deletion yarn-project/txe/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
"@aztec/protocol-contracts": "workspace:^",
"@aztec/pxe": "workspace:^",
"@aztec/simulator": "workspace:^",
"@aztec/telemetry-client": "workspace:^",
"@aztec/types": "workspace:^",
"@aztec/world-state": "workspace:^",
"zod": "^3.23.8"
Expand Down
Loading

0 comments on commit f8448bf

Please sign in to comment.