Skip to content

Commit

Permalink
chore: avm simulator uses regular ContractStorageRead type
Browse files Browse the repository at this point in the history
  • Loading branch information
@dbanks12
dbanks12 committed Jun 26, 2024
1 parent 1542fa6 commit 5636e43
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 101 deletions.
9 changes: 9 additions & 0 deletions yarn-project/circuits.js/src/structs/avm/avm.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ export class AvmContractInstanceHint {
}
}

// TODO(dbanks12): rename AvmCircuitHints
export class AvmExecutionHints {
public readonly storageValues: Vector<AvmKeyValueHint>;
public readonly noteHashExists: Vector<AvmKeyValueHint>;
Expand All @@ -267,6 +268,14 @@ export class AvmExecutionHints {
this.contractInstances = new Vector(contractInstances);
}

/**
* Return an empty instance.
* @returns an empty instance.
*/
empty() {
return new AvmExecutionHints([], [], [], [], [], []);
}

/**
* Serializes the inputs to a buffer.
* @returns - The inputs serialized to a buffer.
Expand Down
28 changes: 11 additions & 17 deletions yarn-project/circuits.js/src/structs/contract_storage_read.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,30 +23,24 @@ export class ContractStorageRead {
/**
* Side effect counter tracking position of this event in tx execution.
*/
public readonly sideEffectCounter: number,
public readonly counter: number,
/**
* Contract address whose storage is being read.
*/
public contractAddress?: AztecAddress, // TODO: Should not be optional. This is a temporary hack to silo the storage slot with the correct address for nested executions.
) {}

static from(args: {
/**
* Storage slot we are reading from.
*/
storageSlot: Fr;
/**
* Value read from the storage slot.
*/
currentValue: Fr;
/**
* Side effect counter tracking position of this event in tx execution.
*/
sideEffectCounter: number;
counter: number;
contractAddress?: AztecAddress;
}) {
return new ContractStorageRead(args.storageSlot, args.currentValue, args.sideEffectCounter, args.contractAddress);
return new ContractStorageRead(args.storageSlot, args.currentValue, args.counter, args.contractAddress);
}

toBuffer() {
return serializeToBuffer(this.storageSlot, this.currentValue, new Fr(this.sideEffectCounter));
return serializeToBuffer(this.storageSlot, this.currentValue, new Fr(this.counter));
}

static fromBuffer(buffer: Buffer | BufferReader) {
Expand All @@ -59,15 +53,15 @@ export class ContractStorageRead {
}

isEmpty() {
return this.storageSlot.isZero() && this.currentValue.isZero() && this.sideEffectCounter == 0;
return this.storageSlot.isZero() && this.currentValue.isZero() && this.counter == 0;
}

toFriendlyJSON() {
return `Slot=${this.storageSlot.toFriendlyJSON()}: ${this.currentValue.toFriendlyJSON()}`;
}

toFields(): Fr[] {
const fields = [this.storageSlot, this.currentValue, new Fr(this.sideEffectCounter)];
const fields = [this.storageSlot, this.currentValue, new Fr(this.counter)];
if (fields.length !== CONTRACT_STORAGE_READ_LENGTH) {
throw new Error(
`Invalid number of fields for ContractStorageRead. Expected ${CONTRACT_STORAGE_READ_LENGTH}, got ${fields.length}`,
Expand All @@ -81,8 +75,8 @@ export class ContractStorageRead {

const storageSlot = reader.readField();
const currentValue = reader.readField();
const sideEffectCounter = reader.readField().toNumber();
const counter = reader.readField().toNumber();

return new ContractStorageRead(storageSlot, currentValue, sideEffectCounter);
return new ContractStorageRead(storageSlot, currentValue, counter);
}
}
22 changes: 11 additions & 11 deletions yarn-project/simulator/src/avm/journal/journal.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class AvmPersistableStateManager {
l1ToL2MsgReadRequests: [],
newNoteHashes: [],
newL2ToL1Messages: [],
startSideEffectCounter: this.trace.accessCounter,
startSideEffectCounter: this.trace.counter,
newNullifiers: [],
contractStorageReads: [],
contractStorageUpdateRequests: [],
Expand Down Expand Up @@ -150,7 +150,7 @@ export class AvmPersistableStateManager {

// TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit
this.transitionalExecutionResult.contractStorageUpdateRequests.push(
new ContractStorageUpdateRequest(slot, value, this.trace.accessCounter, storageAddress),
new ContractStorageUpdateRequest(slot, value, this.trace.counter, storageAddress),
);

// Trace all storage writes (even reverted ones)
Expand All @@ -172,7 +172,7 @@ export class AvmPersistableStateManager {

// TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit
this.transitionalExecutionResult.contractStorageReads.push(
new ContractStorageRead(slot, value, this.trace.accessCounter, storageAddress),
new ContractStorageRead(slot, value, this.trace.counter, storageAddress),
);

// We want to keep track of all performed reads (even reverted ones)
Expand All @@ -195,7 +195,7 @@ export class AvmPersistableStateManager {
this.log.debug(`noteHashes(${storageAddress})@${noteHash} ?? leafIndex: ${leafIndex}, exists: ${exists}.`);

// TODO: include exists here also - This can for sure come from the trace???
this.transitionalExecutionResult.noteHashReadRequests.push(new ReadRequest(noteHash, this.trace.accessCounter));
this.transitionalExecutionResult.noteHashReadRequests.push(new ReadRequest(noteHash, this.trace.counter));

this.trace.traceNoteHashCheck(storageAddress, noteHash, exists, leafIndex);
return Promise.resolve(exists);
Expand All @@ -207,7 +207,7 @@ export class AvmPersistableStateManager {
*/
public writeNoteHash(storageAddress: Fr, noteHash: Fr) {
// TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit
this.transitionalExecutionResult.newNoteHashes.push(new NoteHash(noteHash, this.trace.accessCounter));
this.transitionalExecutionResult.newNoteHashes.push(new NoteHash(noteHash, this.trace.counter));

this.log.debug(`noteHashes(${storageAddress}) += @${noteHash}.`);
this.trace.traceNewNoteHash(storageAddress, noteHash);
Expand All @@ -227,10 +227,10 @@ export class AvmPersistableStateManager {

// TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit
if (exists) {
this.transitionalExecutionResult.nullifierReadRequests.push(new ReadRequest(nullifier, this.trace.accessCounter));
this.transitionalExecutionResult.nullifierReadRequests.push(new ReadRequest(nullifier, this.trace.counter));
} else {
this.transitionalExecutionResult.nullifierNonExistentReadRequests.push(
new ReadRequest(nullifier, this.trace.accessCounter),
new ReadRequest(nullifier, this.trace.counter),
);
}

Expand All @@ -246,7 +246,7 @@ 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, /*noteHash=*/ Fr.ZERO),
new Nullifier(nullifier, this.trace.counter, /*noteHash=*/ Fr.ZERO),
);

this.log.debug(`nullifiers(${storageAddress}) += ${nullifier}.`);
Expand All @@ -269,7 +269,7 @@ export class AvmPersistableStateManager {
`l1ToL2Messages(@${msgLeafIndex}) ?? exists: ${exists}, expected: ${msgHash}, found: ${valueAtIndex}.`,
);

this.transitionalExecutionResult.l1ToL2MsgReadRequests.push(new ReadRequest(msgHash, this.trace.accessCounter));
this.transitionalExecutionResult.l1ToL2MsgReadRequests.push(new ReadRequest(msgHash, this.trace.counter));

this.trace.traceL1ToL2MessageCheck(msgHash, msgLeafIndex, exists);
return Promise.resolve(exists);
Expand Down Expand Up @@ -305,7 +305,7 @@ export class AvmPersistableStateManager {
// this duplicates exactly what happens in the trace just for the purpose of transitional integration with the kernel
this.transitionalExecutionResult.unencryptedLogsHashes.push(
// TODO(6578): explain magic number 4 here
new LogHash(logHash, this.trace.accessCounter, new Fr(ulog.length + 4)),
new LogHash(logHash, this.trace.counter, new Fr(ulog.length + 4)),
);
// TODO(6206): likely need to track this here and not just in the transitional logic.

Expand Down Expand Up @@ -374,7 +374,7 @@ export class AvmPersistableStateManager {
currentStorageValue: this.publicStorage.getCache().cachePerContract,
storageReads: this.trace.publicStorageReads,
storageWrites: this.trace.publicStorageWrites,
sideEffectCounter: this.trace.accessCounter,
sideEffectCounter: this.trace.counter,
};
}
}
18 changes: 9 additions & 9 deletions yarn-project/simulator/src/avm/journal/trace.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('world state access trace', () => {
leafIndex: leafIndex,
},
]);
expect(trace.getAccessCounter()).toBe(1);
expect(trace.getCounter()).toBe(1);
});
it('Should trace note hashes', () => {
const contractAddress = new Fr(1);
Expand All @@ -42,7 +42,7 @@ describe('world state access trace', () => {
expect(trace.newNoteHashes).toEqual([
expect.objectContaining({ storageAddress: contractAddress, noteHash: utxo }),
]);
expect(trace.getAccessCounter()).toEqual(1);
expect(trace.getCounter()).toEqual(1);
});
it('Should trace nullifier checks', () => {
const contractAddress = new Fr(1);
Expand All @@ -62,7 +62,7 @@ describe('world state access trace', () => {
leafIndex: leafIndex,
};
expect(trace.nullifierChecks).toEqual([expectedCheck]);
expect(trace.getAccessCounter()).toEqual(1);
expect(trace.getCounter()).toEqual(1);
});
it('Should trace nullifiers', () => {
const contractAddress = new Fr(1);
Expand All @@ -76,7 +76,7 @@ describe('world state access trace', () => {
// endLifetime: Fr.ZERO,
};
expect(trace.newNullifiers).toEqual([expectedNullifier]);
expect(trace.getAccessCounter()).toEqual(1);
expect(trace.getCounter()).toEqual(1);
});
it('Should trace L1ToL2 Message checks', () => {
const utxo = new Fr(2);
Expand All @@ -90,13 +90,13 @@ describe('world state access trace', () => {
counter: new Fr(0),
};
expect(trace.l1ToL2MessageChecks).toEqual([expectedCheck]);
expect(trace.getAccessCounter()).toEqual(1);
expect(trace.getCounter()).toEqual(1);
});
it('Should trace get contract instance', () => {
const instance = randomTracedContractInstance();
trace.traceGetContractInstance(instance);
expect(trace.gotContractInstances).toEqual([instance]);
expect(trace.getAccessCounter()).toEqual(1);
expect(trace.getCounter()).toEqual(1);
});
});

Expand Down Expand Up @@ -145,7 +145,7 @@ describe('world state access trace', () => {
counter++;
trace.traceGetContractInstance(instance);
counter++;
expect(trace.getAccessCounter()).toEqual(counter);
expect(trace.getCounter()).toEqual(counter);
});

it('Should merge two traces together', () => {
Expand Down Expand Up @@ -216,9 +216,9 @@ describe('world state access trace', () => {
childTrace.traceL1ToL2MessageCheck(msgHashT1, msgLeafIndexT1, msgExistsT1);
childTrace.traceGetContractInstance(instanceT1);

const childCounterBeforeMerge = childTrace.getAccessCounter();
const childCounterBeforeMerge = childTrace.getCounter();
trace.acceptAndMerge(childTrace);
expect(trace.getAccessCounter()).toEqual(childCounterBeforeMerge);
expect(trace.getCounter()).toEqual(childCounterBeforeMerge);

expect(trace.publicStorageReads).toEqual([
expect.objectContaining({
Expand Down
Loading

0 comments on commit 5636e43

Please sign in to comment.