Skip to content

Commit

Permalink
fix: Check class registration nullifier in node before returning class (
Browse files Browse the repository at this point in the history
#11074)

Since #10000 we
are not broadcasting public bytecode. To work around this in some
networks, we're manually adding some common contracts (Token) to
archivers on startup.

Since an Aztec node returns that a class is publicly registered as long
as it's on its archiver, clients were not sending txs to register the
Token class.

However, we will soon check for registration nullifiers. But since
clients think the Token class is registered, they never send the
registration tx. So this will cause all interactions with the Token to
break.

This attempts to fix this by only telling clients that a class is
registered if the nullifier is present.
  • Loading branch information
spalladino authored Jan 7, 2025
1 parent b1b67b0 commit 649b590
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 4 deletions.
24 changes: 21 additions & 3 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ import {
type PrivateLog,
type ProtocolContractAddresses,
type PublicDataTreeLeafPreimage,
REGISTERER_CONTRACT_ADDRESS,
} from '@aztec/circuits.js';
import { computePublicDataTreeLeafSlot } from '@aztec/circuits.js/hash';
import { computePublicDataTreeLeafSlot, siloNullifier } from '@aztec/circuits.js/hash';
import { EpochCache } from '@aztec/epoch-cache';
import { type L1ContractAddresses, createEthereumChain } from '@aztec/ethereum';
import { AztecAddress } from '@aztec/foundation/aztec-address';
Expand Down Expand Up @@ -348,8 +349,25 @@ export class AztecNodeService implements AztecNode, Traceable {
return Promise.resolve(this.l1ChainId);
}

public getContractClass(id: Fr): Promise<ContractClassPublic | undefined> {
return this.contractDataSource.getContractClass(id);
public async getContractClass(id: Fr): Promise<ContractClassPublic | undefined> {
const klazz = await this.contractDataSource.getContractClass(id);

// TODO(#10007): Remove this check. This is needed only because we're manually registering
// some contracts in the archiver so they are available to all nodes (see `registerCommonContracts`
// in `archiver/src/factory.ts`), but we still want clients to send the registration tx in order
// to emit the corresponding nullifier, which is now being checked. Note that this method
// is only called by the PXE to check if a contract is publicly registered.
if (klazz) {
const classNullifier = siloNullifier(AztecAddress.fromNumber(REGISTERER_CONTRACT_ADDRESS), id);
const worldState = await this.#getWorldState('latest');
const [index] = await worldState.findLeafIndices(MerkleTreeId.NULLIFIER_TREE, [classNullifier.toBuffer()]);
this.log.debug(`Registration nullifier ${classNullifier} for contract class ${id} found at index ${index}`);
if (index === undefined) {
return undefined;
}
}

return klazz;
}

public getContract(address: AztecAddress): Promise<ContractInstanceWithAddress | undefined> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { type AztecNodeService } from '@aztec/aztec-node';
import {
AztecAddress,
type AztecNode,
Expand Down Expand Up @@ -76,7 +77,11 @@ 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 = computeContractClassId(getContractClassFromArtifact(TokenContractArtifact));
expect(await aztecNode.getContractClass(classId)).not.toBeUndefined();
// The node checks the registration nullifier
expect(await aztecNode.getContractClass(classId)).toBeUndefined();
// But the archiver does not
const archiver = (aztecNode as AztecNodeService).getContractDataSource();
expect(await archiver.getContractClass(classId)).toBeDefined();
});

it('registers the contract class on the node', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type PXE,
type Wallet,
createPXEClient,
getContractClassFromArtifact,
makeFetch,
} from '@aztec/aztec.js';
import { CounterContract } from '@aztec/noir-contracts.js/Counter';
Expand Down Expand Up @@ -38,6 +39,16 @@ describe('e2e_deploy_contract deploy method', () => {
logger.debug(`Calling public method on stateful test contract at ${contract.address.toString()}`);
await contract.methods.increment_public_value(owner, 84).send().wait();
expect(await contract.methods.get_public_value(owner).simulate()).toEqual(84n);
expect(await pxe.isContractClassPubliclyRegistered(contract.instance.contractClassId)).toBeTrue();
});

// TODO(#10007): Remove this test. Common contracts (ie token contracts) are only distinguished
// because we're manually adding them to the archiver to support provernet.
it('registers a contract class for a common contract', async () => {
const { id: tokenContractClass } = getContractClassFromArtifact(TokenContract.artifact);
expect(await pxe.isContractClassPubliclyRegistered(tokenContractClass)).toBeFalse();
await TokenContract.deploy(wallet, wallet.getAddress(), 'TOKEN', 'TKN', 18n).send().deployed();
expect(await pxe.isContractClassPubliclyRegistered(tokenContractClass)).toBeTrue();
});

it('publicly universally deploys and initializes a contract', async () => {
Expand Down

0 comments on commit 649b590

Please sign in to comment.