Skip to content

Commit

Permalink
fix: disallow contract registration in pxe of contract with duplicate…
Browse files Browse the repository at this point in the history
… private function selectors (#9773)

Related to #4433

This PR adds pretty naive validation when adding a contract artifact to
the pxe database. Specifically it rejects adding contract artifacts to
the PXE with duplicate private function selectors.

This also assumes the absence of partial artifacts.
  • Loading branch information
sklppy88 authored Nov 20, 2024
1 parent aba568a commit 2587ad5
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export interface ContractArtifactDatabase {
* Adds a new contract artifact to the database or updates an existing one.
* @param id - Id of the corresponding contract class.
* @param contract - Contract artifact to add.
* @throws - If there are duplicate private function selectors.
*/
addContractArtifact(id: Fr, contract: ContractArtifact): Promise<void>;
/**
Expand Down
15 changes: 14 additions & 1 deletion yarn-project/pxe/src/database/kv_pxe_database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
SerializableContractInstance,
computePoint,
} from '@aztec/circuits.js';
import { type ContractArtifact } from '@aztec/foundation/abi';
import { type ContractArtifact, FunctionSelector, FunctionType } from '@aztec/foundation/abi';
import { toBufferBE } from '@aztec/foundation/bigint-buffer';
import { Fr } from '@aztec/foundation/fields';
import {
Expand Down Expand Up @@ -142,6 +142,19 @@ export class KVPxeDatabase implements PxeDatabase {
}

public async addContractArtifact(id: Fr, contract: ContractArtifact): Promise<void> {
const privateSelectors = contract.functions
.filter(functionArtifact => functionArtifact.functionType === FunctionType.PRIVATE)
.map(privateFunctionArtifact =>
FunctionSelector.fromNameAndParameters(
privateFunctionArtifact.name,
privateFunctionArtifact.parameters,
).toString(),
);

if (privateSelectors.length !== new Set(privateSelectors).size) {
throw new Error('Repeated function selectors of private functions');
}

await this.#contractArtifacts.set(id.toString(), contractArtifactToBuffer(contract));
}

Expand Down
15 changes: 15 additions & 0 deletions yarn-project/pxe/src/database/pxe_database_test_suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import {
computePoint,
} from '@aztec/circuits.js';
import { makeHeader } from '@aztec/circuits.js/testing';
import { FunctionType } from '@aztec/foundation/abi';
import { randomInt } from '@aztec/foundation/crypto';
import { Fr, Point } from '@aztec/foundation/fields';
import { BenchmarkingContractArtifact } from '@aztec/noir-contracts.js/Benchmarking';
import { TestContractArtifact } from '@aztec/noir-contracts.js/Test';

import { IncomingNoteDao } from './incoming_note_dao.js';
import { OutgoingNoteDao } from './outgoing_note_dao.js';
Expand Down Expand Up @@ -439,6 +441,19 @@ export function describePxeDatabase(getDatabase: () => PxeDatabase) {
await expect(database.getContractArtifact(id)).resolves.toEqual(artifact);
});

it('does not store a contract artifact with a duplicate private function selector', async () => {
const artifact = TestContractArtifact;
const index = artifact.functions.findIndex(fn => fn.functionType === FunctionType.PRIVATE);

const copiedFn = structuredClone(artifact.functions[index]);
artifact.functions.push(copiedFn);

const id = Fr.random();
await expect(database.addContractArtifact(id, artifact)).rejects.toThrow(
'Repeated function selectors of private functions',
);
});

it('stores a contract instance', async () => {
const address = AztecAddress.random();
const instance = SerializableContractInstance.random().withAddress(address);
Expand Down

0 comments on commit 2587ad5

Please sign in to comment.