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

chore: simulator utils cleanup #4507

Merged
merged 3 commits into from
Feb 8, 2024
Merged
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
22 changes: 11 additions & 11 deletions yarn-project/simulator/src/client/private_execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ describe('Private Execution test suite', () => {

expect(result.newNotes).toHaveLength(1);
const newNote = result.newNotes[0];
expect(newNote.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner.toField()));
expect(newNote.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner));

const newCommitments = sideEffectArrayToValueArray(
nonEmptySideEffects(result.callStackItem.publicInputs.newCommitments),
Expand All @@ -336,7 +336,7 @@ describe('Private Execution test suite', () => {

expect(result.newNotes).toHaveLength(1);
const newNote = result.newNotes[0];
expect(newNote.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner.toField()));
expect(newNote.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner));

const newCommitments = sideEffectArrayToValueArray(
nonEmptySideEffects(result.callStackItem.publicInputs.newCommitments),
Expand All @@ -353,8 +353,8 @@ describe('Private Execution test suite', () => {
const amountToTransfer = 100n;
const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create');

const storageSlot = computeSlotForMapping(new Fr(1n), owner.toField());
const recipientStorageSlot = computeSlotForMapping(new Fr(1n), recipient.toField());
const storageSlot = computeSlotForMapping(new Fr(1n), owner);
const recipientStorageSlot = computeSlotForMapping(new Fr(1n), recipient);

const notes = [buildNote(60n, owner, storageSlot), buildNote(80n, owner, storageSlot)];
oracle.getNotes.mockResolvedValue(notes);
Expand Down Expand Up @@ -407,7 +407,7 @@ describe('Private Execution test suite', () => {
const balance = 160n;
const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create');

const storageSlot = computeSlotForMapping(new Fr(1n), owner.toField());
const storageSlot = computeSlotForMapping(new Fr(1n), owner);

const notes = [buildNote(balance, owner, storageSlot)];
oracle.getNotes.mockResolvedValue(notes);
Expand Down Expand Up @@ -905,7 +905,7 @@ describe('Private Execution test suite', () => {

expect(result.newNotes).toHaveLength(1);
const noteAndSlot = result.newNotes[0];
expect(noteAndSlot.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner.toField()));
expect(noteAndSlot.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner));

expect(noteAndSlot.note.items[0]).toEqual(new Fr(amountToTransfer));

Expand All @@ -915,7 +915,7 @@ describe('Private Execution test suite', () => {
expect(newCommitments).toHaveLength(1);

const commitment = newCommitments[0];
const storageSlot = computeSlotForMapping(new Fr(1n), owner.toField());
const storageSlot = computeSlotForMapping(new Fr(1n), owner);
const innerNoteHash = await acirSimulator.computeInnerNoteHash(contractAddress, storageSlot, noteAndSlot.note);
expect(commitment).toEqual(innerNoteHash);

Expand Down Expand Up @@ -986,7 +986,7 @@ describe('Private Execution test suite', () => {

expect(execInsert.newNotes).toHaveLength(1);
const noteAndSlot = execInsert.newNotes[0];
expect(noteAndSlot.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner.toField()));
expect(noteAndSlot.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner));

expect(noteAndSlot.note.items[0]).toEqual(new Fr(amountToTransfer));

Expand All @@ -996,7 +996,7 @@ describe('Private Execution test suite', () => {
expect(newCommitments).toHaveLength(1);

const commitment = newCommitments[0];
const storageSlot = computeSlotForMapping(new Fr(1n), owner.toField());
const storageSlot = computeSlotForMapping(new Fr(1n), owner);
const innerNoteHash = await acirSimulator.computeInnerNoteHash(contractAddress, storageSlot, noteAndSlot.note);
expect(commitment).toEqual(innerNoteHash);

Expand Down Expand Up @@ -1042,7 +1042,7 @@ describe('Private Execution test suite', () => {

expect(result.newNotes).toHaveLength(1);
const noteAndSlot = result.newNotes[0];
expect(noteAndSlot.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner.toField()));
expect(noteAndSlot.storageSlot).toEqual(computeSlotForMapping(new Fr(1n), owner));

expect(noteAndSlot.note.items[0]).toEqual(new Fr(amountToTransfer));

Expand All @@ -1052,7 +1052,7 @@ describe('Private Execution test suite', () => {
expect(newCommitments).toHaveLength(1);

const commitment = newCommitments[0];
const storageSlot = computeSlotForMapping(new Fr(1n), owner.toField());
const storageSlot = computeSlotForMapping(new Fr(1n), owner);
expect(commitment).toEqual(
await acirSimulator.computeInnerNoteHash(contractAddress, storageSlot, noteAndSlot.note),
);
Expand Down
8 changes: 4 additions & 4 deletions yarn-project/simulator/src/public/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('ACIR public execution simulator', () => {
const execution: PublicExecution = { contractAddress, functionData, args, callContext };
const result = await executor.simulate(execution, GlobalVariables.empty());

const recipientBalanceStorageSlot = computeSlotForMapping(new Fr(6n), recipient.toField());
const recipientBalanceStorageSlot = computeSlotForMapping(new Fr(6n), recipient);
const totalSupplyStorageSlot = new Fr(4n);

const expectedBalance = new Fr(previousBalance.value + mintAmount);
Expand All @@ -110,7 +110,7 @@ describe('ACIR public execution simulator', () => {
]);

const mintersStorageSlot = new Fr(2n);
const isMinterStorageSlot = computeSlotForMapping(mintersStorageSlot, msgSender.toField());
const isMinterStorageSlot = computeSlotForMapping(mintersStorageSlot, msgSender);
// Note: There is only 1 storage read (for the isMinter value) because the other 2 reads get overwritten by
// the updates
expect(result.contractStorageReads).toEqual([
Expand Down Expand Up @@ -152,8 +152,8 @@ describe('ACIR public execution simulator', () => {
startSideEffectCounter: 0,
});

recipientStorageSlot = computeSlotForMapping(new Fr(6n), recipient.toField());
senderStorageSlot = computeSlotForMapping(new Fr(6n), Fr.fromBuffer(sender.toBuffer()));
recipientStorageSlot = computeSlotForMapping(new Fr(6n), recipient);
senderStorageSlot = computeSlotForMapping(new Fr(6n), sender);

publicContracts.getBytecode.mockResolvedValue(Buffer.from(transferArtifact.bytecode, 'base64'));

Expand Down
41 changes: 9 additions & 32 deletions yarn-project/simulator/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,18 @@
import { GrumpkinPrivateKey } from '@aztec/circuits.js';
import { Grumpkin } from '@aztec/circuits.js/barretenberg';
import { pedersenHash } from '@aztec/foundation/crypto';
import { Fr } from '@aztec/foundation/fields';

/**
* A point in the format that Aztec.nr uses.
*/
export type NoirPoint = {
/** The x coordinate. */
x: bigint;
/** The y coordinate. */
y: bigint;
};

/**
* Computes the resulting storage slot for an entry in a mapping.
* @param mappingSlot - The slot of the mapping within state.
* @param owner - The key of the mapping.
* @param key - The key of the mapping.
* @returns The slot in the contract storage where the value is stored.
*/
export function computeSlotForMapping(mappingSlot: Fr, owner: NoirPoint | Fr) {
const isFr = (owner: NoirPoint | Fr): owner is Fr => typeof (owner as Fr).value === 'bigint';
const ownerField = isFr(owner) ? owner : new Fr(owner.x);

return Fr.fromBuffer(pedersenHash([mappingSlot, ownerField].map(f => f.toBuffer())));
}

/**
* Computes the public key for a private key.
* @param privateKey - The private key.
* @param grumpkin - The grumpkin instance.
* @returns The public key.
*/
export function toPublicKey(privateKey: GrumpkinPrivateKey, grumpkin: Grumpkin): NoirPoint {
const point = grumpkin.mul(Grumpkin.generator, privateKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this simply unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

return {
x: point.x.value,
y: point.y.value,
};
export function computeSlotForMapping(
mappingSlot: Fr,
key: {
/** Serialize to a field. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there not be some limiter on the length of the key? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Added the check by instead accepting object with toField. That is a better way to do it since mapping keys in Noir are fields anyway. 21d29b6

toField: () => Fr;
},
) {
return Fr.fromBuffer(pedersenHash([mappingSlot, key.toField()].map(field => field.toBuffer())));
}
Loading