Silo storage slot with correct address.

AztecProtocol · Apr 14, 2024 · 6ad22e9 · 6ad22e9
1 parent fe1bce6
commit 6ad22e9
Show file tree

Hide file tree

Showing 8 changed files with 53 additions and 53 deletions.
diff --git a/yarn-project/circuits.js/src/structs/contract_storage_read.ts b/yarn-project/circuits.js/src/structs/contract_storage_read.ts
@@ -1,3 +1,4 @@
+import { type AztecAddress } from '@aztec/foundation/aztec-address';
 import { Fr } from '@aztec/foundation/fields';
 import { BufferReader, FieldReader, serializeToBuffer } from '@aztec/foundation/serialize';
 
@@ -24,6 +25,7 @@ export class ContractStorageRead {
      * Note: Not serialized
      */
     public readonly sideEffectCounter?: number,
+    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: {
@@ -39,8 +41,9 @@ export class ContractStorageRead {
      * Optional side effect counter tracking position of this event in tx execution.
      */
     sideEffectCounter?: number;
+    contractAddress?: AztecAddress;
   }) {
-    return new ContractStorageRead(args.storageSlot, args.currentValue, args.sideEffectCounter);
+    return new ContractStorageRead(args.storageSlot, args.currentValue, args.sideEffectCounter, args.contractAddress);
   }
 
   toBuffer() {

diff --git a/yarn-project/circuits.js/src/structs/contract_storage_update_request.ts b/yarn-project/circuits.js/src/structs/contract_storage_update_request.ts
@@ -1,3 +1,4 @@
+import { type AztecAddress } from '@aztec/foundation/aztec-address';
 import { Fr } from '@aztec/foundation/fields';
 import { BufferReader, FieldReader, serializeToBuffer } from '@aztec/foundation/serialize';
 import { type FieldsOf } from '@aztec/foundation/types';
@@ -24,6 +25,7 @@ export class ContractStorageUpdateRequest {
      * Optional side effect counter tracking position of this event in tx execution.
      */
     public readonly sideEffectCounter?: number,
+    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.
   ) {}
 
   toBuffer() {
@@ -50,7 +52,7 @@ export class ContractStorageUpdateRequest {
    * @returns The array.
    */
   static getFields(fields: FieldsOf<ContractStorageUpdateRequest>) {
-    return [fields.storageSlot, fields.newValue, fields.sideEffectCounter] as const;
+    return [fields.storageSlot, fields.newValue, fields.sideEffectCounter, fields.contractAddress] as const;
   }
 
   static empty() {

diff --git a/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts b/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts
@@ -500,29 +500,9 @@ function patchPublicStorageActionOrdering(
   // so the returned result will be a subset of the public kernel output.
 
   const simPublicDataReads = collectPublicDataReads(execResult);
-  // verify that each read is in the kernel output
-  for (const read of simPublicDataReads) {
-    if (!publicDataReads.find(item => item.equals(read))) {
-      throw new Error(
-        `Public data reads from simulator do not match those from public kernel.\nFrom simulator: ${simPublicDataReads
-          .map(p => p.toFriendlyJSON())
-          .join(', ')}\nFrom public kernel: ${publicDataReads.map(i => i.toFriendlyJSON()).join(', ')}`,
-      );
-    }
-  }
 
   const simPublicDataUpdateRequests = collectPublicDataUpdateRequests(execResult);
-  for (const update of simPublicDataUpdateRequests) {
-    if (!publicDataUpdateRequests.find(item => item.equals(update))) {
-      throw new Error(
-        `Public data update requests from simulator do not match those from public kernel.\nFrom simulator: ${simPublicDataUpdateRequests
-          .map(p => p.toFriendlyJSON())
-          .join(', ')}\nFrom public kernel revertible: ${publicDataUpdateRequests
-          .map(i => i.toFriendlyJSON())
-          .join(', ')}`,
-      );
-    }
-  }
+
   // We only want to reorder the items from the public inputs of the
   // most recently processed top/enqueued call.
 

diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts
@@ -360,7 +360,9 @@ describe('public_processor', () => {
         // Setup
         PublicExecutionResultBuilder.fromPublicCallRequest({
           request: publicCallRequests[0],
-          contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotA, fr(0x101))],
+          contractStorageUpdateRequests: [
+            new ContractStorageUpdateRequest(contractSlotA, fr(0x101), 11, baseContractAddress),
+          ],
         }).build(),
 
         // App Logic
@@ -371,8 +373,8 @@ describe('public_processor', () => {
               from: publicCallRequests[1].contractAddress,
               tx: makeFunctionCall(baseContractAddress, makeSelector(5)),
               contractStorageUpdateRequests: [
-                new ContractStorageUpdateRequest(contractSlotA, fr(0x102)),
-                new ContractStorageUpdateRequest(contractSlotB, fr(0x151)),
+                new ContractStorageUpdateRequest(contractSlotA, fr(0x102), 13, baseContractAddress),
+                new ContractStorageUpdateRequest(contractSlotB, fr(0x151), 14, baseContractAddress),
               ],
             }).build(),
             PublicExecutionResultBuilder.fromFunctionCall({
@@ -390,7 +392,9 @@ describe('public_processor', () => {
             PublicExecutionResultBuilder.fromFunctionCall({
               from: publicCallRequests[1].contractAddress,
               tx: makeFunctionCall(baseContractAddress, makeSelector(5)),
-              contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x201))],
+              contractStorageUpdateRequests: [
+                new ContractStorageUpdateRequest(contractSlotC, fr(0x201), 12, baseContractAddress),
+              ],
             }).build(),
           ],
         }).build(),
@@ -464,14 +468,16 @@ describe('public_processor', () => {
         // Setup
         PublicExecutionResultBuilder.fromPublicCallRequest({
           request: publicCallRequests[0],
-          contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotA, fr(0x101))],
+          contractStorageUpdateRequests: [
+            new ContractStorageUpdateRequest(contractSlotA, fr(0x101), 11, baseContractAddress),
+          ],
           nestedExecutions: [
             PublicExecutionResultBuilder.fromFunctionCall({
               from: publicCallRequests[1].contractAddress,
               tx: makeFunctionCall(baseContractAddress, makeSelector(5)),
               contractStorageUpdateRequests: [
-                new ContractStorageUpdateRequest(contractSlotA, fr(0x102)),
-                new ContractStorageUpdateRequest(contractSlotB, fr(0x151)),
+                new ContractStorageUpdateRequest(contractSlotA, fr(0x102), 12, baseContractAddress),
+                new ContractStorageUpdateRequest(contractSlotB, fr(0x151), 13, baseContractAddress),
               ],
             }).build(),
             PublicExecutionResultBuilder.fromFunctionCall({
@@ -494,7 +500,9 @@ describe('public_processor', () => {
             PublicExecutionResultBuilder.fromFunctionCall({
               from: publicCallRequests[1].contractAddress,
               tx: makeFunctionCall(baseContractAddress, makeSelector(5)),
-              contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x201))],
+              contractStorageUpdateRequests: [
+                new ContractStorageUpdateRequest(contractSlotC, fr(0x201), 14, baseContractAddress),
+              ],
             }).build(),
           ],
         }).build(),
@@ -558,14 +566,16 @@ describe('public_processor', () => {
         // Setup
         PublicExecutionResultBuilder.fromPublicCallRequest({
           request: publicCallRequests[0],
-          contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotA, fr(0x101))],
+          contractStorageUpdateRequests: [
+            new ContractStorageUpdateRequest(contractSlotA, fr(0x101), 11, baseContractAddress),
+          ],
           nestedExecutions: [
             PublicExecutionResultBuilder.fromFunctionCall({
               from: publicCallRequests[1].contractAddress,
               tx: makeFunctionCall(baseContractAddress, makeSelector(5)),
               contractStorageUpdateRequests: [
-                new ContractStorageUpdateRequest(contractSlotA, fr(0x102)),
-                new ContractStorageUpdateRequest(contractSlotB, fr(0x151)),
+                new ContractStorageUpdateRequest(contractSlotA, fr(0x102), 12, baseContractAddress),
+                new ContractStorageUpdateRequest(contractSlotB, fr(0x151), 13, baseContractAddress),
               ],
             }).build(),
           ],
@@ -588,7 +598,9 @@ describe('public_processor', () => {
             PublicExecutionResultBuilder.fromFunctionCall({
               from: publicCallRequests[1].contractAddress,
               tx: makeFunctionCall(baseContractAddress, makeSelector(5)),
-              contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotC, fr(0x201))],
+              contractStorageUpdateRequests: [
+                new ContractStorageUpdateRequest(contractSlotC, fr(0x201), 14, baseContractAddress),
+              ],
             }).build(),
           ],
         }).build(),
@@ -656,8 +668,8 @@ describe('public_processor', () => {
         PublicExecutionResultBuilder.fromPublicCallRequest({
           request: publicCallRequests[2],
           contractStorageUpdateRequests: [
-            new ContractStorageUpdateRequest(contractSlotA, fr(0x101)),
-            new ContractStorageUpdateRequest(contractSlotB, fr(0x151)),
+            new ContractStorageUpdateRequest(contractSlotA, fr(0x101), 14, baseContractAddress),
+            new ContractStorageUpdateRequest(contractSlotB, fr(0x151), 15, baseContractAddress),
           ],
         }).build(),
 
@@ -669,14 +681,16 @@ describe('public_processor', () => {
               from: publicCallRequests[1].contractAddress,
               tx: makeFunctionCall(baseContractAddress, makeSelector(5)),
               contractStorageUpdateRequests: [
-                new ContractStorageUpdateRequest(contractSlotA, fr(0x101)),
-                new ContractStorageUpdateRequest(contractSlotC, fr(0x201)),
+                new ContractStorageUpdateRequest(contractSlotA, fr(0x101), 11, baseContractAddress),
+                new ContractStorageUpdateRequest(contractSlotC, fr(0x201), 12, baseContractAddress),
               ],
             }).build(),
             PublicExecutionResultBuilder.fromFunctionCall({
               from: publicCallRequests[1].contractAddress,
               tx: makeFunctionCall(baseContractAddress, makeSelector(5)),
-              contractStorageUpdateRequests: [new ContractStorageUpdateRequest(contractSlotA, fr(0x102))],
+              contractStorageUpdateRequests: [
+                new ContractStorageUpdateRequest(contractSlotA, fr(0x102), 13, baseContractAddress),
+              ],
             }).build(),
           ],
         }).build(),

diff --git a/yarn-project/simulator/src/public/execution.ts b/yarn-project/simulator/src/public/execution.ts
@@ -1,6 +1,5 @@
 import { type SimulationError, type UnencryptedFunctionL2Logs } from '@aztec/circuit-types';
 import {
-  type AztecAddress,
   type ContractStorageRead,
   type ContractStorageUpdateRequest,
   type Fr,
@@ -81,10 +80,8 @@ export function isPublicExecutionResult(
  */
 export function collectPublicDataReads(execResult: PublicExecutionResult): PublicDataRead[] {
   // HACK(#1622): part of temporary hack - may be able to remove this function after public state ordering is fixed
-  const contractAddress = execResult.execution.callContext.storageContractAddress;
-
   const thisExecPublicDataReads = execResult.contractStorageReads.map(read =>
-    contractStorageReadToPublicDataRead(read, contractAddress),
+    contractStorageReadToPublicDataRead(read),
   );
   const unsorted = [
     ...thisExecPublicDataReads,
@@ -101,10 +98,8 @@ export function collectPublicDataReads(execResult: PublicExecutionResult): Publi
  */
 export function collectPublicDataUpdateRequests(execResult: PublicExecutionResult): PublicDataUpdateRequest[] {
   // HACK(#1622): part of temporary hack - may be able to remove this function after public state ordering is fixed
-  const contractAddress = execResult.execution.callContext.storageContractAddress;
-
   const thisExecPublicDataUpdateRequests = execResult.contractStorageUpdateRequests.map(update =>
-    contractStorageUpdateRequestToPublicDataUpdateRequest(update, contractAddress),
+    contractStorageUpdateRequestToPublicDataUpdateRequest(update),
   );
   const unsorted = [
     ...thisExecPublicDataUpdateRequests,
@@ -119,9 +114,9 @@ export function collectPublicDataUpdateRequests(execResult: PublicExecutionResul
  * @param contractAddress - the contract address of the read
  * @returns The public data read.
  */
-function contractStorageReadToPublicDataRead(read: ContractStorageRead, contractAddress: AztecAddress): PublicDataRead {
+function contractStorageReadToPublicDataRead(read: ContractStorageRead): PublicDataRead {
   return new PublicDataRead(
-    computePublicDataTreeLeafSlot(contractAddress, read.storageSlot),
+    computePublicDataTreeLeafSlot(read.contractAddress!, read.storageSlot),
     computePublicDataTreeValue(read.currentValue),
     read.sideEffectCounter!,
   );
@@ -135,10 +130,9 @@ function contractStorageReadToPublicDataRead(read: ContractStorageRead, contract
  */
 function contractStorageUpdateRequestToPublicDataUpdateRequest(
   update: ContractStorageUpdateRequest,
-  contractAddress: AztecAddress,
 ): PublicDataUpdateRequest {
   return new PublicDataUpdateRequest(
-    computePublicDataTreeLeafSlot(contractAddress, update.storageSlot),
+    computePublicDataTreeLeafSlot(update.contractAddress!, update.storageSlot),
     computePublicDataTreeValue(update.newValue),
     update.sideEffectCounter!,
   );

diff --git a/yarn-project/simulator/src/public/index.test.ts b/yarn-project/simulator/src/public/index.test.ts
@@ -142,11 +142,13 @@ describe('ACIR public execution simulator', () => {
         // There should be 2 storage updates, one for the recipient's balance and one for the total supply
         expect(result.contractStorageUpdateRequests).toEqual([
           {
+            contractAddress,
             storageSlot: recipientBalanceStorageSlot,
             newValue: expectedBalance,
             sideEffectCounter: 3,
           },
           {
+            contractAddress,
             storageSlot: totalSupplyStorageSlot,
             newValue: expectedTotalSupply,
             sideEffectCounter: 4,
@@ -159,6 +161,7 @@ describe('ACIR public execution simulator', () => {
         // the updates
         expect(result.contractStorageReads).toEqual([
           {
+            contractAddress,
             storageSlot: isMinterStorageSlot,
             currentValue: isMinter,
             sideEffectCounter: 0,
@@ -217,11 +220,13 @@ describe('ACIR public execution simulator', () => {
 
         expect(result.contractStorageUpdateRequests).toEqual([
           {
+            contractAddress,
             storageSlot: senderStorageSlot,
             newValue: expectedSenderBalance,
             sideEffectCounter: 1, // 1 read (sender balance)
           },
           {
+            contractAddress,
             storageSlot: recipientStorageSlot,
             newValue: expectedRecipientBalance,
             sideEffectCounter: 3, // 1 read (sender balance), 1 write (new sender balance), 1 read (recipient balance)

diff --git a/yarn-project/simulator/src/public/state_actions.ts b/yarn-project/simulator/src/public/state_actions.ts
@@ -83,13 +83,15 @@ export class ContractStorageActionsCollector {
   public collect(): [ContractStorageRead[], ContractStorageUpdateRequest[]] {
     const reads = Array.from(this.contractStorageReads.entries()).map(([slot, valueAndCounter]) =>
       ContractStorageRead.from({
+        contractAddress: this.address,
         storageSlot: new Fr(slot),
         ...valueAndCounter,
       }),
     );
 
     const updateRequests = Array.from(this.contractStorageUpdateRequests.entries()).map(([slot, valuesAndCounter]) =>
       ContractStorageUpdateRequest.from({
+        contractAddress: this.address,
         storageSlot: new Fr(slot),
         ...valuesAndCounter,
       }),

diff --git a/yarn-project/simulator/src/public/transitional_adaptors.ts b/yarn-project/simulator/src/public/transitional_adaptors.ts
@@ -108,10 +108,10 @@ export async function convertAvmResults(
   const execution = executionContext.execution;
 
   const contractStorageReads: ContractStorageRead[] = newWorldState.storageReads.map(
-    read => new ContractStorageRead(read.slot, read.value, read.counter.toNumber()),
+    read => new ContractStorageRead(read.slot, read.value, read.counter.toNumber(), read.storageAddress),
   );
   const contractStorageUpdateRequests: ContractStorageUpdateRequest[] = newWorldState.storageWrites.map(
-    write => new ContractStorageUpdateRequest(write.slot, write.value, write.counter.toNumber()),
+    write => new ContractStorageUpdateRequest(write.slot, write.value, write.counter.toNumber(), write.storageAddress),
   );
   // We need to write the storage updates to the DB, because that's what the ACVM expects.
   // Assumes the updates are in the right order.