Added more state transitioner tests

packages/contracts/bin/take-dump.ts

@@ -5,7 +5,6 @@ import * as mkdirp from 'mkdirp'
 
 /* Internal Imports */
 import { makeStateDump } from '../src'
-
 ;(async () => {
   const outdir = path.resolve(__dirname, '../build/dumps')
   const outfile = path.join(outdir, 'state-dump.latest.json')

packages/contracts/contracts/optimistic-ethereum/OVM/verification/OVM_StateTransitioner.sol

@@ -262,16 +262,27 @@ contract OVM_StateTransitioner is iOVM_StateTransitioner, Lib_AddressResolver {
             "Contract must be verified before proving a storage slot."
         );
 
-        require(
-            Lib_SecureMerkleTrie.verifyInclusionProof(
-                abi.encodePacked(_key),
-                abi.encodePacked(_value),
-                _storageTrieWitness,
-                ovmStateManager.getAccountStorageRoot(_ovmContractAddress)
-            ),
-            "Storage slot is invalid or invalid inclusion proof provided."
+        (
+            bool exists,
+            bytes memory value
+        ) = Lib_SecureMerkleTrie.get(
+            abi.encodePacked(_key),
+            _storageTrieWitness,
+            ovmStateManager.getAccountStorageRoot(_ovmContractAddress)
         );
 
+        if (exists == true) {
+            require(
+                keccak256(value) == keccak256(abi.encodePacked(_value)),
+                "Provided storage slot value is invalid."
+            );
+        } else {
+            require(
+                _value == bytes32(0),
+                "Provided storage slot value is invalid."
+            );
+        }
+
         ovmStateManager.putContractStorage(
             _ovmContractAddress,
             _key,

packages/contracts/contracts/optimistic-ethereum/libraries/codec/Lib_OVMCodec.sol

@@ -207,8 +207,8 @@ library Lib_OVMCodec {
         // index-by-index circumvents this issue.
         raw[0] = Lib_RLPWriter.writeUint(_account.nonce);
         raw[1] = Lib_RLPWriter.writeUint(_account.balance);
-        raw[2] = _account.storageRoot == 0 ? RLP_NULL_BYTES : Lib_RLPWriter.writeBytes(abi.encodePacked(_account.storageRoot));
-        raw[3] = _account.codeHash == 0 ? RLP_NULL_BYTES : Lib_RLPWriter.writeBytes(abi.encodePacked(_account.codeHash));
+        raw[2] = Lib_RLPWriter.writeBytes(abi.encodePacked(_account.storageRoot));
+        raw[3] = Lib_RLPWriter.writeBytes(abi.encodePacked(_account.codeHash));
 
         return Lib_RLPWriter.writeList(raw);
     }

packages/contracts/package.json

@@ -36,6 +36,7 @@
     "ganache-core": "^2.12.1",
     "lodash": "^4.17.20",
     "merkle-patricia-tree": "git+https://github.com/kfichter/merkle-patricia-tree",
+    "mkdirp": "^1.0.4",
     "mocha": "^8.1.1",
     "prettier": "^2.1.2",
     "random-bytes-seed": "^1.0.3",

packages/contracts/src/contract-dumps.ts

@@ -175,7 +175,7 @@ export const makeStateDump = async (): Promise<any> => {
   const changedAccounts = await getChangedAccounts(cStateManager)
 
   let deadAddressIndex = 0
-  let accounts: Accounts = []
+  const accounts: Accounts = []
 
   for (const originalAddress of changedAccounts) {
     const code = (
@@ -207,7 +207,7 @@ export const makeStateDump = async (): Promise<any> => {
     accounts.push({
       originalAddress,
       address,
-      code: code,
+      code,
     })
   }
 

packages/contracts/test/contracts/OVM/verification/OVM_StateTransitioner.spec.ts

@@ -15,10 +15,16 @@ import {
   TrieTestGenerator,
   ZERO_ADDRESS,
 } from '../../../helpers'
-import { MockContract, smockit } from '@eth-optimism/smock'
+import {
+  MockContract,
+  smockit,
+  ModifiableContract,
+  smoddit,
+  ModifiableContractFactory,
+} from '@eth-optimism/smock'
 import { keccak256 } from 'ethers/lib/utils'
 
-describe.skip('OVM_StateTransitioner', () => {
+describe('OVM_StateTransitioner', () => {
   let AddressManager: Contract
   before(async () => {
     AddressManager = await makeAddressManager()
@@ -57,14 +63,12 @@ describe.skip('OVM_StateTransitioner', () => {
     Mock__OVM_StateManager.smocked.putAccount.will.return()
   })
 
-  let Factory__OVM_StateTransitioner: ContractFactory
+  let Factory__OVM_StateTransitioner: ModifiableContractFactory
   before(async () => {
-    Factory__OVM_StateTransitioner = await ethers.getContractFactory(
-      'OVM_StateTransitioner'
-    )
+    Factory__OVM_StateTransitioner = await smoddit('OVM_StateTransitioner')
   })
 
-  let OVM_StateTransitioner: Contract
+  let OVM_StateTransitioner: ModifiableContract
   beforeEach(async () => {
     OVM_StateTransitioner = await Factory__OVM_StateTransitioner.deploy(
       AddressManager.address,
@@ -172,8 +176,8 @@ describe.skip('OVM_StateTransitioner', () => {
               BigNumber.from(account.balance),
               account.storageRoot,
               account.codeHash,
-              account.ethAddress,
-              account.isFresh,
+              ethContractAddress,
+              false,
             ],
           ])
         })
@@ -207,52 +211,81 @@ describe.skip('OVM_StateTransitioner', () => {
 
     describe('when the corresponding account is proven', () => {
       beforeEach(() => {
-        Mock__OVM_StateManager.smocked.hasAccount.will.return.with(false)
+        Mock__OVM_StateManager.smocked.hasAccount.will.return.with(true)
       })
 
       describe('when provided an invalid slot inclusion proof', () => {
-        let account: any
-        let proof: string
+        let key = keccak256('0x1234')
+        let val = keccak256('0x5678')
+        let proof = '0x'
         beforeEach(async () => {
-          account = {
-            nonce: 0,
-            balance: 0,
-            storageRoot: NULL_BYTES32,
-            codeHash: NULL_BYTES32,
-          }
-
-          const generator = await TrieTestGenerator.fromAccounts({
-            accounts: [
+          const generator = await TrieTestGenerator.fromNodes({
+            nodes: [
               {
-                ...account,
-                address: NON_ZERO_ADDRESS,
-                storage: [
-                  {
-                    key: keccak256('0x1234'),
-                    val: keccak256('0x5678'),
-                  },
-                ],
+                key,
+                val,
               },
             ],
             secure: true,
           })
 
-          const test = await generator.makeAccountProofTest(NON_ZERO_ADDRESS)
-
-          proof = test.accountTrieWitness
+          const test = await generator.makeInclusionProofTest(0)
 
-          OVM_StateTransitioner = await Factory__OVM_StateTransitioner.deploy(
-            AddressManager.address,
-            0,
-            test.accountTrieRoot,
-            NULL_BYTES32
+          Mock__OVM_StateManager.smocked.getAccountStorageRoot.will.return.with(
+            test.root
           )
         })
 
-        it('should revert', async () => {})
+        it('should revert', async () => {
+          await expect(
+            OVM_StateTransitioner.proveStorageSlot(
+              ZERO_ADDRESS,
+              key,
+              val,
+              proof
+            )
+          ).to.be.reverted
+        })
       })
 
-      describe('when provided a valid slot inclusion proof', () => {})
+      describe('when provided a valid slot inclusion proof', () => {
+        let key = keccak256('0x1234')
+        let val = keccak256('0x5678')
+        let proof: string
+        beforeEach(async () => {
+          const generator = await TrieTestGenerator.fromNodes({
+            nodes: [
+              {
+                key,
+                val,
+              },
+            ],
+            secure: true,
+          })
+
+          const test = await generator.makeInclusionProofTest(0)
+          proof = test.proof
+
+          Mock__OVM_StateManager.smocked.getAccountStorageRoot.will.return.with(
+            test.root
+          )
+        })
+
+        it('should insert the storage slot', async () => {
+          await expect(
+            OVM_StateTransitioner.proveStorageSlot(
+              ZERO_ADDRESS,
+              key,
+              val,
+              proof
+            )
+          ).to.not.be.reverted
+
+          expect(
+            Mock__OVM_StateManager.smocked.putContractStorage.calls[0]
+          ).to.deep.equal([ZERO_ADDRESS, key, val])
+        })
+      })
     })
   })
 
@@ -261,46 +294,280 @@ describe.skip('OVM_StateTransitioner', () => {
   })
 
   describe('commitContractState', () => {
-    describe('when the account was not changed', () => {
-      it('should revert', async () => {})
+    beforeEach(async () => {
+      OVM_StateTransitioner.smodify.set({
+        phase: 1,
+      })
     })
 
-    describe('when the account was changed', () => {
-      describe('when the account has not been committed', () => {
-        it('should commit the account and update the state', async () => {})
+    let ovmContractAddress = NON_ZERO_ADDRESS
+    let account: any
+    beforeEach(() => {
+      Mock__OVM_StateManager.smocked.hasAccount.will.return.with(false)
+      account = {
+        nonce: 0,
+        balance: 0,
+        storageRoot: NULL_BYTES32,
+        codeHash: NULL_BYTES32,
+      }
+    })
+
+    describe('when the account was not changed or has already been committed', () => {
+      before(() => {
+        Mock__OVM_StateManager.smocked.commitAccount.will.return.with(false)
+      })
+
+      it('should revert', async () => {
+        await expect(
+          OVM_StateTransitioner.commitContractState(
+            ovmContractAddress,
+            account,
+            '0x'
+          )
+        ).to.be.revertedWith(
+          'Account was not changed or has already been committed.'
+        )
+      })
+    })
+
+    describe('when the account was changed or has not already been committed', () => {
+      before(() => {
+        Mock__OVM_StateManager.smocked.commitAccount.will.return.with(true)
       })
 
-      describe('when the account was already committed', () => {
-        it('should revert', () => {})
+      describe('when given an valid update proof', () => {
+        let proof: string
+        let postStateRoot: string
+        beforeEach(async () => {
+          const generator = await TrieTestGenerator.fromAccounts({
+            accounts: [
+              {
+                ...account,
+                nonce: 10,
+                address: ovmContractAddress,
+              },
+            ],
+            secure: true,
+          })
+
+          const test = await generator.makeAccountUpdateTest(
+            ovmContractAddress,
+            account
+          )
+
+          proof = test.accountTrieWitness
+          postStateRoot = test.newAccountTrieRoot
+
+          OVM_StateTransitioner.smodify.put({
+            postStateRoot: test.accountTrieRoot,
+          })
+        })
+
+        it('should update the post state root', async () => {
+          await expect(
+            OVM_StateTransitioner.commitContractState(
+              ovmContractAddress,
+              account,
+              proof
+            )
+          ).to.not.be.reverted
+
+          expect(await OVM_StateTransitioner.getPostStateRoot()).to.equal(
+            postStateRoot
+          )
+        })
       })
     })
   })
 
   describe('commitStorageSlot', () => {
-    describe('when the slot was not changed', () => {
-      it('should revert', async () => {})
+    beforeEach(() => {
+      OVM_StateTransitioner.smodify.set({
+        phase: 1,
+      })
     })
 
-    describe('when the slot was changed', () => {
-      describe('when the slot has not been committed', () => {
-        it('should commit the slot and update the state', async () => {})
+    let ovmContractAddress = NON_ZERO_ADDRESS
+    let account: any
+    let key = keccak256('0x1234')
+    let val = keccak256('0x5678')
+    let newVal = keccak256('0x4321')
+    beforeEach(() => {
+      account = {
+        nonce: 0,
+        balance: 0,
+        storageRoot: NULL_BYTES32,
+        codeHash: NULL_BYTES32,
+      }
+
+      Mock__OVM_StateManager.smocked.getAccount.will.return.with({
+        ...account,
+        ethAddress: ZERO_ADDRESS,
+        isFresh: false,
       })
+    })
 
-      describe('when the slot was already committed', () => {
-        it('should revert', () => {})
+    describe('when the slot was not changed or was already committed', () => {
+      beforeEach(() => {
+        Mock__OVM_StateManager.smocked.commitContractStorage.will.return.with(
+          false
+        )
+      })
+
+      it('should revert', async () => {
+        await expect(
+          OVM_StateTransitioner.commitStorageSlot(
+            ovmContractAddress,
+            key,
+            val,
+            '0x',
+            '0x'
+          )
+        ).to.be.revertedWith(
+          'Storage slot was not changed or has already been committed.'
+        )
+      })
+    })
+
+    describe('when the slot was changed or not already committed', () => {
+      beforeEach(() => {
+        Mock__OVM_StateManager.smocked.commitContractStorage.will.return.with(
+          true
+        )
+      })
+
+      describe('with a valid proof', () => {
+        let accountTrieProof: string
+        let storageTrieProof: string
+        let postStateRoot: string
+        beforeEach(async () => {
+          const storageGenerator = await TrieTestGenerator.fromNodes({
+            nodes: [
+              {
+                key,
+                val,
+              },
+            ],
+            secure: true,
+          })
+
+          const storageTest = await storageGenerator.makeNodeUpdateTest(
+            key,
+            newVal
+          )
+
+          const generator = await TrieTestGenerator.fromAccounts({
+            accounts: [
+              {
+                ...account,
+                storageRoot: storageTest.root,
+                address: ovmContractAddress,
+              },
+            ],
+            secure: true,
+          })
+
+          const test = await generator.makeAccountUpdateTest(
+            ovmContractAddress,
+            {
+              ...account,
+              storageRoot: storageTest.newRoot,
+            }
+          )
+
+          Mock__OVM_StateManager.smocked.getAccount.will.return.with({
+            ...account,
+            storageRoot: storageTest.root,
+            ethAddress: ZERO_ADDRESS,
+            isFresh: false,
+          })
+
+          accountTrieProof = test.accountTrieWitness
+          storageTrieProof = storageTest.proof
+
+          postStateRoot = test.newAccountTrieRoot
+
+          OVM_StateTransitioner.smodify.put({
+            postStateRoot: test.accountTrieRoot,
+          })
+        })
+
+        it('should commit the slot and update the state', async () => {
+          await expect(
+            OVM_StateTransitioner.commitStorageSlot(
+              ovmContractAddress,
+              key,
+              newVal,
+              accountTrieProof,
+              storageTrieProof
+            )
+          ).to.not.be.reverted
+        })
       })
     })
   })
 
   describe('completeTransition', () => {
+    beforeEach(() => {
+      OVM_StateTransitioner.smodify.set({
+        phase: 1,
+      })
+    })
+
     describe('when there are uncommitted accounts', () => {
-      it('should revert', async () => {})
+      beforeEach(() => {
+        Mock__OVM_StateManager.smocked.getTotalUncommittedAccounts.will.return.with(
+          1
+        )
+        Mock__OVM_StateManager.smocked.getTotalUncommittedContractStorage.will.return.with(
+          0
+        )
+      })
+
+      it('should revert', async () => {
+        await expect(
+          OVM_StateTransitioner.completeTransition()
+        ).to.be.revertedWith(
+          'All accounts must be committed before completing a transition.'
+        )
+      })
     })
 
     describe('when there are uncommitted storage slots', () => {
-      it('should revert', async () => {})
+      beforeEach(() => {
+        Mock__OVM_StateManager.smocked.getTotalUncommittedAccounts.will.return.with(
+          0
+        )
+        Mock__OVM_StateManager.smocked.getTotalUncommittedContractStorage.will.return.with(
+          1
+        )
+      })
+
+      it('should revert', async () => {
+        await expect(
+          OVM_StateTransitioner.completeTransition()
+        ).to.be.revertedWith(
+          'All storage must be committed before completing a transition.'
+        )
+      })
     })
 
-    describe('when all state changes are committed', () => {})
+    describe('when all state changes are committed', () => {
+      beforeEach(() => {
+        Mock__OVM_StateManager.smocked.getTotalUncommittedAccounts.will.return.with(
+          0
+        )
+        Mock__OVM_StateManager.smocked.getTotalUncommittedContractStorage.will.return.with(
+          0
+        )
+      })
+
+      it('should complete the transition', async () => {
+        await expect(OVM_StateTransitioner.completeTransition()).to.not.be
+          .reverted
+
+        expect(await OVM_StateTransitioner.isComplete()).to.equal(true)
+      })
+    })
   })
 })

packages/contracts/yarn.lock

@@ -39,10 +39,10 @@
   resolved "https://registry.yarnpkg.com/@ensdomains/resolver/-/resolver-0.2.4.tgz#c10fe28bf5efbf49bff4666d909aed0265efbc89"
   integrity sha512-bvaTH34PMCbv6anRa9I/0zjLJgY4EuznbEMgbV77JBCQ9KNC46rzi0avuxpOfu+xDjPEtSFGqVEOr5GlUSGudA==
 
-"@eth-optimism/smock@^0.0.1":
-  version "0.0.1"
-  resolved "https://registry.yarnpkg.com/@eth-optimism/smock/-/smock-0.0.1.tgz#8ea7379072eccfe5cac327e7eb78384f0e38b18c"
-  integrity sha512-lJmdXaDkAQ7Y2T9GfEzrhF6lrJ3WiEb8HJyDBjr2r3Cd4/0b2RAQj4kqkKLFZyGDMnAZ0O+XnTTyaKhBXkDSXw==
+"@eth-optimism/smock@^0.0.2":
+  version "0.0.2"
+  resolved "https://registry.yarnpkg.com/@eth-optimism/smock/-/smock-0.0.2.tgz#58b9b28885fef1b08c5b13cb31bf614635027d90"
+  integrity sha512-6Wn4h8ZuDqZlNq2lF1Mov78b41S7g4xXWXBOqqS5T+d0yC2V6Ao9XIJBheBS6l1cCN8tFNR47zdlmOCRSDX7sw==
   dependencies:
     ethereum-waffle "^3"
     ethers "^5"