Skip to content

Commit

Permalink
chore(tests): change error messages in preparation for AVM (#6422)
Browse files Browse the repository at this point in the history
  • Loading branch information
fcarreiro authored May 15, 2024
1 parent 3e7e094 commit 6616dc6
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,20 @@ contract ImportTest {
Test::at(target).get_this_address().call(&mut context)
}

// Calls the create_nullifier_public on the Test contract at the target address
// Calls the emit_nullifier_public on the Test contract at the target address
// Used for testing calling an open function
// See yarn-project/end-to-end/src/e2e_nested_contract.test.ts
#[aztec(private)]
fn call_open_fn(target: AztecAddress) {
Test::at(target).create_nullifier_public(1, 2).enqueue(&mut context);
Test::at(target).emit_nullifier_public(1).enqueue(&mut context);
}

// Calls the create_nullifier_public on the Test contract at the target address
// Calls the emit_nullifier_public on the Test contract at the target address
// Used for testing calling an open function from another open function
// See yarn-project/end-to-end/src/e2e_nested_contract.test.ts
#[aztec(public)]
fn pub_call_open_fn(target: AztecAddress) {
Test::at(target).create_nullifier_public(1, 2).call(&mut context);
Test::at(target).emit_nullifier_public(1).call(&mut context);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,8 @@ contract Test {

// Purely exists for testing
#[aztec(public)]
fn create_nullifier_public(amount: Field, secret_hash: Field) {
// Create a commitment to the amount
let note = DummyNote::new(amount, secret_hash);

// Public oracle call to emit new commitment.
context.push_new_nullifier(note.get_commitment(), 0);
fn emit_nullifier_public(nullifier: Field) {
context.push_new_nullifier(nullifier, 0);
}

// Forcefully emits a nullifier (for testing purposes)
Expand Down
5 changes: 5 additions & 0 deletions yarn-project/end-to-end/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
"get-port": "^7.1.0",
"glob": "^10.3.10",
"jest": "^29.5.0",
"jest-extended": "^4.0.2",
"jest-mock-extended": "^3.0.5",
"koa": "^2.14.2",
"koa-static": "^5.0.0",
Expand Down Expand Up @@ -89,6 +90,7 @@
"@types/jest": "^29.5.0",
"concurrently": "^7.6.0",
"jest": "^29.5.0",
"jest-extended": "^4.0.2",
"ts-node": "^10.9.1",
"typescript": "^5.0.4"
},
Expand All @@ -103,6 +105,9 @@
},
"jest": {
"slowTestThreshold": 300,
"setupFilesAfterEnv": [
"jest-extended/all"
],
"extensionsToTreatAsEsm": [
".ts"
],
Expand Down
158 changes: 97 additions & 61 deletions yarn-project/end-to-end/src/e2e_block_building.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,21 @@ import { getSchnorrAccount } from '@aztec/accounts/schnorr';
import {
type AztecAddress,
type AztecNode,
BatchCall,
ContractDeployer,
ContractFunctionInteraction,
type DebugLogger,
Fr,
type PXE,
type SentTx,
type TxReceipt,
TxStatus,
type Wallet,
deriveKeys,
} from '@aztec/aztec.js';
import { times } from '@aztec/foundation/collection';
import { pedersenHash } from '@aztec/foundation/crypto';
import { StatefulTestContractArtifact } from '@aztec/noir-contracts.js';
import { TestContract } from '@aztec/noir-contracts.js/Test';
import { TokenContract } from '@aztec/noir-contracts.js/Token';

import 'jest-extended';

import { TaggedNote } from '../../circuit-types/src/logs/l1_note_payload/tagged_note.js';
import { setup } from './fixtures/utils.js';

Expand Down Expand Up @@ -117,6 +114,9 @@ describe('e2e_block_building', () => {
describe('double-spends', () => {
let contract: TestContract;
let teardown: () => Promise<void>;
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/5818): clean up
// Under current public, we expect 'dropped', under the AVM, we expect 'reverted'.
const DUPLICATE_NULLIFIER_ERROR = /dropped|reverted/;

beforeAll(async () => {
({ teardown, pxe, logger, wallet: owner } = await setup(1));
Expand All @@ -127,63 +127,104 @@ describe('e2e_block_building', () => {
afterAll(() => teardown());

// Regressions for https://github.com/AztecProtocol/aztec-packages/issues/2502
describe('in the same block', () => {
it('drops tx with private nullifier already emitted on the same block', async () => {
// Note that the order in which the TX are processed is not guaranteed.
describe('in the same block, different tx', () => {
it('private <-> private', async () => {
const nullifier = Fr.random();
const calls = times(2, () => contract.methods.emit_nullifier(nullifier));
for (const call of calls) {
await call.prove();
}
const [tx1, tx2] = calls.map(call => call.send());
await expectXorTx(tx1, tx2);
const txs = await sendAndWait([
contract.methods.emit_nullifier(nullifier),
contract.methods.emit_nullifier(nullifier),
]);

// One transaction should succeed, the other should fail, but in any order.
expect(txs).toIncludeSameMembers([
{ status: 'fulfilled', value: expect.anything() },
{
status: 'rejected',
reason: expect.objectContaining({ message: expect.stringMatching(DUPLICATE_NULLIFIER_ERROR) }),
},
]);
});

it('drops tx with public nullifier already emitted on the same block', async () => {
const secret = Fr.random();
const calls = times(2, () => contract.methods.create_nullifier_public(140n, secret));
for (const call of calls) {
await call.prove();
}
const [tx1, tx2] = calls.map(call => call.send());
await expectXorTx(tx1, tx2);
it('public -> public', async () => {
const nullifier = Fr.random();
const txs = await sendAndWait([
contract.methods.emit_nullifier_public(nullifier),
contract.methods.emit_nullifier_public(nullifier),
]);

// One transaction should succeed, the other should fail, but in any order.
expect(txs).toIncludeSameMembers([
{ status: 'fulfilled', value: expect.anything() },
{
status: 'rejected',
reason: expect.objectContaining({ message: expect.stringMatching(DUPLICATE_NULLIFIER_ERROR) }),
},
]);
});

it('drops tx with two equal nullifiers', async () => {
it('private -> public', async () => {
const nullifier = Fr.random();
const calls = times(2, () => contract.methods.emit_nullifier(nullifier).request());
await expect(new BatchCall(owner, calls).send().wait()).rejects.toThrow(/dropped/);
const txs = await sendAndWait([
contract.methods.emit_nullifier(nullifier),
contract.methods.emit_nullifier_public(nullifier),
]);

// One transaction should succeed, the other should fail, but in any order.
expect(txs).toIncludeSameMembers([
{ status: 'fulfilled', value: expect.anything() },
{
status: 'rejected',
reason: expect.objectContaining({ message: expect.stringMatching(DUPLICATE_NULLIFIER_ERROR) }),
},
]);
});

it('drops tx with private nullifier already emitted from public on the same block', async () => {
const secret = Fr.random();
// See yarn-project/simulator/src/public/index.test.ts 'Should be able to create a nullifier from the public context'
const emittedPublicNullifier = pedersenHash([new Fr(140), secret]);

const calls = [
contract.methods.create_nullifier_public(140n, secret),
contract.methods.emit_nullifier(emittedPublicNullifier),
];

for (const call of calls) {
await call.prove();
}
const [tx1, tx2] = calls.map(call => call.send());
await expectXorTx(tx1, tx2);
it('public -> private', async () => {
const nullifier = Fr.random();
const txs = await sendAndWait([
contract.methods.emit_nullifier_public(nullifier),
contract.methods.emit_nullifier(nullifier),
]);

// One transaction should succeed, the other should fail, but in any order.
expect(txs).toIncludeSameMembers([
{ status: 'fulfilled', value: expect.anything() },
{
status: 'rejected',
reason: expect.objectContaining({ message: expect.stringMatching(DUPLICATE_NULLIFIER_ERROR) }),
},
]);
});
});

describe('across blocks', () => {
it('drops a tx that tries to spend a nullifier already emitted on a previous block', async () => {
const secret = Fr.random();
const emittedPublicNullifier = pedersenHash([new Fr(140), secret]);

await expect(contract.methods.create_nullifier_public(140n, secret).send().wait()).resolves.toEqual(
expect.objectContaining({
status: TxStatus.MINED,
}),
it('private -> private', async () => {
const nullifier = Fr.random();
await contract.methods.emit_nullifier(nullifier).send().wait();
await expect(contract.methods.emit_nullifier(nullifier).send().wait()).rejects.toThrow('dropped');
});

it('public -> public', async () => {
const nullifier = Fr.random();
await contract.methods.emit_nullifier_public(nullifier).send().wait();
await expect(contract.methods.emit_nullifier_public(nullifier).send().wait()).rejects.toThrow(
DUPLICATE_NULLIFIER_ERROR,
);
});

it('private -> public', async () => {
const nullifier = Fr.random();
await contract.methods.emit_nullifier(nullifier).send().wait();
await expect(contract.methods.emit_nullifier_public(nullifier).send().wait()).rejects.toThrow(
DUPLICATE_NULLIFIER_ERROR,
);
});

await expect(contract.methods.emit_nullifier(emittedPublicNullifier).send().wait()).rejects.toThrow(/dropped/);
it('public -> private', async () => {
const nullifier = Fr.random();
await contract.methods.emit_nullifier_public(nullifier).send().wait();
await expect(contract.methods.emit_nullifier(nullifier).send().wait()).rejects.toThrow('dropped');
});
});
});
Expand Down Expand Up @@ -243,17 +284,12 @@ describe('e2e_block_building', () => {
});
});

/**
* Checks that only one of the two provided transactions succeeds.
* @param tx1 - A transaction.
* @param tx2 - Another transaction.
*/
async function expectXorTx(tx1: SentTx, tx2: SentTx) {
const receipts = await Promise.allSettled([tx1.wait(), tx2.wait()]);
const succeeded = receipts.find((r): r is PromiseSettledResult<TxReceipt> => r.status === 'fulfilled');
const failed = receipts.find((r): r is PromiseRejectedResult => r.status === 'rejected');

expect(succeeded).toBeDefined();
expect(failed).toBeDefined();
expect((failed?.reason as Error).message).toMatch(/dropped/);
async function sendAndWait(calls: ContractFunctionInteraction[]) {
return await Promise.allSettled(
calls
// First we send them all.
.map(call => call.send())
// Onlt then we wait.
.map(p => p.wait()),
);
}
4 changes: 3 additions & 1 deletion yarn-project/simulator/src/avm/journal/journal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,9 @@ export class AvmPersistableStateManager {
*/
public async writeNullifier(storageAddress: Fr, nullifier: Fr) {
// TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit
this.transitionalExecutionResult.newNullifiers.push(new Nullifier(nullifier, this.trace.accessCounter, Fr.ZERO));
this.transitionalExecutionResult.newNullifiers.push(
new Nullifier(nullifier, this.trace.accessCounter, /*noteHash=*/ Fr.ZERO),
);

this.log.debug(`nullifiers(${storageAddress}) += ${nullifier}.`);
// Cache pending nullifiers for later access
Expand Down
11 changes: 4 additions & 7 deletions yarn-project/simulator/src/public/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,10 +405,11 @@ describe('ACIR public execution simulator', () => {

it('Should be able to create a nullifier from the public context', async () => {
const createNullifierPublicArtifact = TestContractArtifact.functions.find(
f => f.name === 'create_nullifier_public',
f => f.name === 'emit_nullifier_public',
)!;

const args = encodeArguments(createNullifierPublicArtifact, params);
const nullifier = new Fr(1234);
const args = encodeArguments(createNullifierPublicArtifact, [nullifier]);

const callContext = makeCallContext(contractAddress);

Expand All @@ -417,11 +418,7 @@ describe('ACIR public execution simulator', () => {
const execution: PublicExecution = { contractAddress, functionData, args, callContext };
const result = await simulate(execution, globalVariables);

// Assert the l2 to l1 message was created
expect(result.newNullifiers.length).toEqual(1);

const expectedNewMessageValue = pedersenHash(params);
expect(result.newNullifiers[0].value).toEqual(expectedNewMessageValue);
expect(result.newNullifiers).toEqual([expect.objectContaining({ value: nullifier })]);
});

describe('L1 to L2 messages', () => {
Expand Down
20 changes: 18 additions & 2 deletions yarn-project/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ __metadata:
get-port: ^7.1.0
glob: ^10.3.10
jest: ^29.5.0
jest-extended: ^4.0.2
jest-mock-extended: ^3.0.5
koa: ^2.14.2
koa-static: ^5.0.0
Expand Down Expand Up @@ -9048,7 +9049,7 @@ __metadata:
languageName: node
linkType: hard

"jest-diff@npm:^29.7.0":
"jest-diff@npm:^29.0.0, jest-diff@npm:^29.7.0":
version: 29.7.0
resolution: "jest-diff@npm:29.7.0"
dependencies:
Expand Down Expand Up @@ -9096,7 +9097,22 @@ __metadata:
languageName: node
linkType: hard

"jest-get-type@npm:^29.6.3":
"jest-extended@npm:^4.0.2":
version: 4.0.2
resolution: "jest-extended@npm:4.0.2"
dependencies:
jest-diff: ^29.0.0
jest-get-type: ^29.0.0
peerDependencies:
jest: ">=27.2.5"
peerDependenciesMeta:
jest:
optional: true
checksum: afdc255eec7caa173f9e805e94562273d8b8aa4c7ab9b396668f018c18ea5236270a6ac499ca84b8c60e90ccbe9ccb4aebf998daef13aec9542c426df1df6079
languageName: node
linkType: hard

"jest-get-type@npm:^29.0.0, jest-get-type@npm:^29.6.3":
version: 29.6.3
resolution: "jest-get-type@npm:29.6.3"
checksum: 88ac9102d4679d768accae29f1e75f592b760b44277df288ad76ce5bf038c3f5ce3719dea8aa0f035dac30e9eb034b848ce716b9183ad7cc222d029f03e92205
Expand Down

0 comments on commit 6616dc6

Please sign in to comment.