Force invalid state access check on SSTORE

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

@@ -1216,6 +1216,12 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager {
     )
         internal
     {
+        // We need to make sure that the transaction isn't trying to access storage that hasn't
+        // been provided to the OVM_StateManager. We'll immediately abort if this is the case.
+        _checkInvalidStateAccess(
+            ovmStateManager.hasContractStorage(_contract, _key)
+        );
+
         // Check whether the slot has been changed before and mark it as changed if not. We need
         // this because "nuisance gas" only applies to the first time that a slot is changed.
         (

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_StateManager.sol

@@ -366,7 +366,14 @@ contract OVM_StateManager is iOVM_StateManager {
         authenticated
     {
         contractStorage[_contract][_key] = _value;
-        verifiedContractStorage[_contract][_key] = true;
+
+        // Only used when initially populating the contract storage. OVM_ExecutionManager will
+        // perform a `hasContractStorage` INVALID_STATE_ACCESS check before putting any contract
+        // storage because writing to zero when the actual value is nonzero causes a gas
+        // discrepancy.
+        if (verifiedContractStorage[_contract][_key] == false) {
+            verifiedContractStorage[_contract][_key] = true;
+        }
     }
 
     /**

packages/contracts/package.json

@@ -7,7 +7,7 @@
     "build": "yarn run build:contracts",
     "build:contracts": "buidler compile",
     "test": "yarn run test:contracts",
-    "test:contracts": "buidler test \"test/contracts/OVM/execution/OVM_ExecutionManager/ovmDELEGATECALL.spec.ts\"",
+    "test:contracts": "buidler test \"test/contracts/OVM/execution/OVM_ExecutionManager/run.spec.ts\"",
     "lint": "tslint --format stylish --project .",
     "fix": "prettier --config prettier-config.json --write \"buidler.config.ts\" \"{src,test}/**/*.ts\""
   },

packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/run.spec.ts

+/* Internal Imports */
+import {
+  ExecutionManagerTestRunner,
+  TestDefinition,
+  GAS_LIMIT,
+  NON_NULL_BYTES32,
+  REVERT_FLAGS,
+  ZERO_ADDRESS,
+  VERIFIED_EMPTY_CONTRACT_HASH,
+} from '../../../../helpers'
+
+const DUMMY_REVERT_DATA =
+  '0xdeadbeef1e5420deadbeef1e5420deadbeef1e5420deadbeef1e5420deadbeef1e5420'
+const GAS_METADATA_ADDRESS = '0x06a506a506a506a506a506a506a506a506a506a5'
+
+enum GasMetadataKey {
+  CURRENT_EPOCH_START_TIMESTAMP,
+  CUMULATIVE_SEQUENCER_QUEUE_GAS,
+  CUMULATIVE_L1TOL2_QUEUE_GAS,
+  PREV_EPOCH_SEQUENCER_QUEUE_GAS,
+  PREV_EPOCH_L1TOL2_QUEUE_GAS
+}
+
+const keyToBytes32 = (key: GasMetadataKey): string => {
+  return '0x' + `0${key}`.padStart(64, '0')
+}
+
+const test_run: TestDefinition = {
+  name: 'Basic tests for ovmCALL',
+  preState: {
+    ExecutionManager: {
+      ovmStateManager: '$OVM_STATE_MANAGER',
+      ovmSafetyChecker: '$OVM_SAFETY_CHECKER',
+      messageRecord: {
+        nuisanceGasLeft: GAS_LIMIT,
+      },
+    },
+    StateManager: {
+      owner: '$OVM_EXECUTION_MANAGER',
+      accounts: {
+        $DUMMY_OVM_ADDRESS_1: {
+          codeHash: NON_NULL_BYTES32,
+          ethAddress: '$OVM_CALL_HELPER',
+        },
+        $DUMMY_OVM_ADDRESS_2: {
+          codeHash: NON_NULL_BYTES32,
+          ethAddress: '$OVM_CALL_HELPER',
+        },
+        $DUMMY_OVM_ADDRESS_3: {
+          codeHash: VERIFIED_EMPTY_CONTRACT_HASH,
+          ethAddress: '0x' + '00'.repeat(20),
+        },
+      },
+      contractStorage: {
+        [GAS_METADATA_ADDRESS]: {
+          [keyToBytes32(GasMetadataKey.CURRENT_EPOCH_START_TIMESTAMP)]: 1,
+          [keyToBytes32(GasMetadataKey.CUMULATIVE_SEQUENCER_QUEUE_GAS)]: 0,
+          [keyToBytes32(GasMetadataKey.CUMULATIVE_L1TOL2_QUEUE_GAS)]: 0,
+          [keyToBytes32(GasMetadataKey.PREV_EPOCH_SEQUENCER_QUEUE_GAS)]: 0,
+          [keyToBytes32(GasMetadataKey.PREV_EPOCH_L1TOL2_QUEUE_GAS)]: 0
+        }
+      },
+      verifiedContractStorage: {
+        [GAS_METADATA_ADDRESS]: {
+          [keyToBytes32(GasMetadataKey.CURRENT_EPOCH_START_TIMESTAMP)]: true,
+          [keyToBytes32(GasMetadataKey.CUMULATIVE_SEQUENCER_QUEUE_GAS)]: true,
+          [keyToBytes32(GasMetadataKey.CUMULATIVE_L1TOL2_QUEUE_GAS)]: true,
+          [keyToBytes32(GasMetadataKey.PREV_EPOCH_SEQUENCER_QUEUE_GAS)]: true,
+          [keyToBytes32(GasMetadataKey.PREV_EPOCH_L1TOL2_QUEUE_GAS)]: true,
+        }
+      }
+    },
+  },
+  parameters: [
+    {
+      name: 'run => ovmCALL(ADDRESS_1) => ovmADDRESS',
+      focus: true,
+      steps: [
+        {
+          functionName: 'run',
+          functionParams: {
+            timestamp: 0,
+            queueOrigin: 0,
+            entrypoint: '$OVM_CALL_HELPER',
+            origin: ZERO_ADDRESS,
+            msgSender: ZERO_ADDRESS,
+            gasLimit: GAS_LIMIT,
+            subSteps: [
+              {
+                functionName: 'ovmCALL',
+                functionParams: {
+                  gasLimit: GAS_LIMIT,
+                  target: '$DUMMY_OVM_ADDRESS_1',
+                  subSteps: [
+                    {
+                      functionName: 'ovmADDRESS',
+                      expectedReturnValue: '$DUMMY_OVM_ADDRESS_1',
+                    },
+                  ],
+                },
+                expectedReturnStatus: true,
+              },
+            ]
+          }
+        }
+      ],
+    },
+  ]
+}
+
+const runner = new ExecutionManagerTestRunner()
+runner.run(test_run)

packages/contracts/test/helpers/storage/contract-storage.ts

@@ -166,29 +166,57 @@ export const setContractStorage = async (
     const inputSlots = parseInputSlots(layout, layoutMap.type)
 
     const slot = parseInt(layoutMap.slot, 10)
-    const depth = (layoutMap.type.match(/t_mapping/g) || []).length
+    let depth = (layoutMap.type.match(/t_mapping/g) || []).length
 
     if (typeof value !== 'object') {
       const slotHash = getStorageSlotHash(slot, depth, value)
       await contract.__setStorageSlot(slotHash, toHexString32(value as string))
     } else {
-      for (const [subKey, subValue] of Object.entries(value)) {
-        const baseSlotHash = getStorageSlotHash(slot, depth, {
-          [subKey]: subValue,
-        })
-        const slotValues = getFlattenedValues(depth, {
-          [subKey]: subValue,
-        })
-
-        for (const slotValue of slotValues) {
-          const slotIndex = inputSlots.find((inputSlot) => {
-            return inputSlot.label === slotValue.label
-          }).slot
-          const slotHash = toHexString32(
-            BigNumber.from(baseSlotHash).add(slotIndex)
-          )
-
-          await contract.__setStorageSlot(slotHash, slotValue.value)
+      if (key === 'contractStorage' || key === 'verifiedContractStorage') {
+        for (const [subKey1, subValue1] of Object.entries(value)) {
+          for (const [subKey, subValue] of Object.entries(subValue1)) {
+            const baseSlotHash = getStorageSlotHash(slot, depth, {
+              [subKey1]: {
+                [subKey]: subValue,
+              }
+            })
+            const slotValues = getFlattenedValues(depth, {
+              [subKey1]: {
+                [subKey]: subValue,
+              }
+            })
+    
+            for (const slotValue of slotValues) {
+              const slotIndex = inputSlots.find((inputSlot) => {
+                return inputSlot.label === slotValue.label
+              }).slot
+              const slotHash = toHexString32(
+                BigNumber.from(baseSlotHash).add(slotIndex)
+              )
+    
+              await contract.__setStorageSlot(slotHash, slotValue.value)
+            }
+          }
+        }
+      } else {
+        for (const [subKey, subValue] of Object.entries(value)) {
+          const baseSlotHash = getStorageSlotHash(slot, depth, {
+            [subKey]: subValue,
+          })
+          const slotValues = getFlattenedValues(depth, {
+            [subKey]: subValue,
+          })
+
+          for (const slotValue of slotValues) {
+            const slotIndex = inputSlots.find((inputSlot) => {
+              return inputSlot.label === slotValue.label
+            }).slot
+            const slotHash = toHexString32(
+              BigNumber.from(baseSlotHash).add(slotIndex)
+            )
+
+            await contract.__setStorageSlot(slotHash, slotValue.value)
+          }
         }
       }
     }
@@ -223,28 +251,62 @@ export const checkContractStorage = async (
         )
       }
     } else {
-      for (const [subKey, subValue] of Object.entries(value)) {
-        const baseSlotHash = getStorageSlotHash(slot, depth, {
-          [subKey]: subValue,
-        })
-        const slotValues = getFlattenedValues(depth, {
-          [subKey]: subValue,
-        })
-
-        for (const slotValue of slotValues) {
-          const slotIndex = inputSlots.find((inputSlot) => {
-            return inputSlot.label === slotValue.label
-          }).slot
-          const slotHash = toHexString32(
-            BigNumber.from(baseSlotHash).add(slotIndex)
-          )
-
-          const retSlotValue = await contract.__getStorageSlot(slotHash)
-
-          if (retSlotValue !== slotValue.value) {
-            throw new Error(
-              `Resulting state of ${slotValue.label} (${retSlotValue}) did not match expected state (${slotValue.value}).`
+      if (key === 'contractStorage' || key === 'verifiedContractStorage') {
+        for (const [subKey1, subValue1] of Object.entries(value)) {
+          for (const [subKey, subValue] of Object.entries(subValue1)) {
+            const baseSlotHash = getStorageSlotHash(slot, depth, {
+              [subKey1]: {
+                [subKey]: subValue,
+              }
+            })
+            const slotValues = getFlattenedValues(depth, {
+              [subKey1]: {
+                [subKey]: subValue,
+              }
+            })
+    
+            for (const slotValue of slotValues) {
+              const slotIndex = inputSlots.find((inputSlot) => {
+                return inputSlot.label === slotValue.label
+              }).slot
+              const slotHash = toHexString32(
+                BigNumber.from(baseSlotHash).add(slotIndex)
+              )
+
+              const retSlotValue = await contract.__getStorageSlot(slotHash)
+  
+              if (retSlotValue !== slotValue.value) {
+                throw new Error(
+                  `Resulting state of ${slotValue.label} (${retSlotValue}) did not match expected state (${slotValue.value}).`
+                )
+              }
+            }
+          }
+        }
+      } else {
+        for (const [subKey, subValue] of Object.entries(value)) {
+          const baseSlotHash = getStorageSlotHash(slot, depth, {
+            [subKey]: subValue,
+          })
+          const slotValues = getFlattenedValues(depth, {
+            [subKey]: subValue,
+          })
+
+          for (const slotValue of slotValues) {
+            const slotIndex = inputSlots.find((inputSlot) => {
+              return inputSlot.label === slotValue.label
+            }).slot
+            const slotHash = toHexString32(
+              BigNumber.from(baseSlotHash).add(slotIndex)
             )
+
+            const retSlotValue = await contract.__getStorageSlot(slotHash)
+
+            if (retSlotValue !== slotValue.value) {
+              throw new Error(
+                `Resulting state of ${slotValue.label} (${retSlotValue}) did not match expected state (${slotValue.value}).`
+              )
+            }
           }
         }
       }

packages/contracts/test/helpers/test-utils/test-runner.ts

@@ -8,17 +8,20 @@ import { cloneDeep } from 'lodash'
 /* Internal Imports */
 import {
   TestDefinition,
+  ParsedTestStep,
+  TestParameter,
   TestStep,
+  TestStep_CALL,
+  TestStep_Run,
+  isRevertFlagError,
   isTestStep_SSTORE,
   isTestStep_SLOAD,
   isTestStep_CALL,
   isTestStep_CREATE,
   isTestStep_CREATE2,
   isTestStep_Context,
-  ParsedTestStep,
-  isRevertFlagError,
-  TestParameter,
   isTestStep_evm,
+  isTestStep_Run,
   isTestStep_EXTCODESIZE,
   isTestStep_EXTCODEHASH,
   isTestStep_EXTCODECOPY,
@@ -190,15 +193,47 @@ export class ExecutionManagerTestRunner {
     return ret
   }
 
-  private async runTestStep(step: TestStep) {
-    await this.contracts.OVM_ExecutionManager.ovmCALL(
-      GAS_LIMIT / 2,
-      this.contracts.Helper_TestRunner.address,
-      this.contracts.Helper_TestRunner.interface.encodeFunctionData(
-        'runSingleTestStep',
-        [this.parseTestStep(step)]
+  private async runTestStep(step: TestStep | TestStep_Run) {
+    if (isTestStep_Run(step)) {
+      let calldata: string
+      if (step.functionParams.data) {
+        calldata = step.functionParams.data
+      } else {
+        const runStep: TestStep_CALL = {
+          functionName: 'ovmCALL',
+          functionParams: {
+            gasLimit: GAS_LIMIT,
+            target: this.contracts.Helper_TestRunner.address,
+            subSteps: step.functionParams.subSteps
+          },
+          expectedReturnStatus: true
+        }
+
+        calldata = this.encodeFunctionData(runStep)
+      }
+
+      await this.contracts.OVM_ExecutionManager.run(
+        {
+          timestamp: step.functionParams.timestamp,
+          queueOrigin: step.functionParams.queueOrigin,
+          entrypoint: step.functionParams.entrypoint,
+          origin: step.functionParams.origin,
+          msgSender: step.functionParams.msgSender,
+          gasLimit: step.functionParams.gasLimit,
+          data: calldata
+        },
+        this.contracts.OVM_StateManager.address
       )
-    )
+    } else {
+      await this.contracts.OVM_ExecutionManager.ovmCALL(
+        GAS_LIMIT / 2,
+        this.contracts.Helper_TestRunner.address,
+        this.contracts.Helper_TestRunner.interface.encodeFunctionData(
+          'runSingleTestStep',
+          [this.parseTestStep(step)]
+        )
+      )
+    }
   }
 
   private parseTestStep(step: TestStep): ParsedTestStep {

packages/contracts/test/helpers/test-utils/test.types.ts

@@ -83,7 +83,7 @@ interface TestStep_SLOAD {
   expectedReturnValue: string | RevertFlagError
 }
 
-interface TestStep_CALL {
+export interface TestStep_CALL {
   functionName: CallOpcode
   functionParams: {
     gasLimit: number | BigNumber
@@ -116,6 +116,20 @@ interface TestStep_CREATE2 {
   expectedReturnValue: string | RevertFlagError
 }
 
+export interface TestStep_Run {
+  functionName: 'run',
+  functionParams: {
+    timestamp: number
+    queueOrigin: number
+    entrypoint: string
+    origin: string
+    msgSender: string
+    gasLimit: number
+    data?: string
+    subSteps?: TestStep[]
+  }
+}
+
 export type TestStep =
   | TestStep_Context
   | TestStep_SSTORE
@@ -164,33 +178,33 @@ export const isTestStep_Context = (
 }
 
 export const isTestStep_SSTORE = (step: TestStep): step is TestStep_SSTORE => {
-  return step.functionName == 'ovmSSTORE'
+  return step.functionName === 'ovmSSTORE'
 }
 
 export const isTestStep_SLOAD = (step: TestStep): step is TestStep_SLOAD => {
-  return step.functionName == 'ovmSLOAD'
+  return step.functionName === 'ovmSLOAD'
 }
 
 export const isTestStep_EXTCODESIZE = (
   step: TestStep
 ): step is TestStep_EXTCODESIZE => {
-  return step.functionName == 'ovmEXTCODESIZE'
+  return step.functionName === 'ovmEXTCODESIZE'
 }
 
 export const isTestStep_EXTCODEHASH = (
   step: TestStep
 ): step is TestStep_EXTCODEHASH => {
-  return step.functionName == 'ovmEXTCODEHASH'
+  return step.functionName === 'ovmEXTCODEHASH'
 }
 
 export const isTestStep_EXTCODECOPY = (
   step: TestStep
 ): step is TestStep_EXTCODECOPY => {
-  return step.functionName == 'ovmEXTCODECOPY'
+  return step.functionName === 'ovmEXTCODECOPY'
 }
 
 export const isTestStep_REVERT = (step: TestStep): step is TestStep_REVERT => {
-  return step.functionName == 'ovmREVERT'
+  return step.functionName === 'ovmREVERT'
 }
 
 export const isTestStep_CALL = (step: TestStep): step is TestStep_CALL => {
@@ -200,13 +214,19 @@ export const isTestStep_CALL = (step: TestStep): step is TestStep_CALL => {
 }
 
 export const isTestStep_CREATE = (step: TestStep): step is TestStep_CREATE => {
-  return step.functionName == 'ovmCREATE'
+  return step.functionName === 'ovmCREATE'
 }
 
 export const isTestStep_CREATE2 = (
   step: TestStep
 ): step is TestStep_CREATE2 => {
-  return step.functionName == 'ovmCREATE2'
+  return step.functionName === 'ovmCREATE2'
+}
+
+export const isTestStep_Run = (
+  step: TestStep | TestStep_Run
+): step is TestStep_Run => {
+  return step.functionName === 'run'
 }
 
 interface TestState {
@@ -216,7 +236,7 @@ interface TestState {
 
 export interface TestParameter {
   name: string
-  steps: TestStep[]
+  steps: Array<TestStep | TestStep_Run>
   expectInvalidStateAccess?: boolean
   focus?: boolean
 }