Require height specification in SCC (#27)

* Require height specification in SCC

* Added tests for new functionality

* Removed console imports in contracts

packages/contracts/contracts/optimistic-ethereum/OVM/bridge/OVM_L1CrossDomainMessenger.sol

@@ -15,9 +15,6 @@ import { iOVM_StateCommitmentChain } from "../../iOVM/chain/iOVM_StateCommitment
 /* Contract Imports */
 import { OVM_BaseCrossDomainMessenger } from "./OVM_BaseCrossDomainMessenger.sol";
 
-/* Logging Imports */
-import { console } from "@nomiclabs/buidler/console.sol";
-
 /**
  * @title OVM_L1CrossDomainMessenger
  */

packages/contracts/contracts/optimistic-ethereum/OVM/bridge/OVM_L2CrossDomainMessenger.sol

@@ -13,9 +13,6 @@ import { iOVM_L2ToL1MessagePasser } from "../../iOVM/precompiles/iOVM_L2ToL1Mess
 /* Contract Imports */
 import { OVM_BaseCrossDomainMessenger } from "./OVM_BaseCrossDomainMessenger.sol";
 
-/* Logging Imports */
-import { console } from "@nomiclabs/buidler/console.sol";
-
 /**
  * @title OVM_L2CrossDomainMessenger
  * @dev L2 CONTRACT (COMPILED)

packages/contracts/contracts/optimistic-ethereum/OVM/chain/OVM_CanonicalTransactionChain.sol

@@ -14,9 +14,6 @@ import { iOVM_CanonicalTransactionChain } from "../../iOVM/chain/iOVM_CanonicalT
 /* Contract Imports */
 import { OVM_ExecutionManager } from "../execution/OVM_ExecutionManager.sol";
 
-/* Logging Imports */
-import { console } from "@nomiclabs/buidler/console.sol";
-
 /**
  * @title OVM_CanonicalTransactionChain
  */
@@ -267,7 +264,7 @@ contract OVM_CanonicalTransactionChain is iOVM_CanonicalTransactionChain, Lib_Ad
         }
 
         require(
-            shouldStartAtBatch == getTotalBatches(),
+            shouldStartAtBatch == getTotalElements(),
             "Actual batch start index does not match expected start index."
         );
 

packages/contracts/contracts/optimistic-ethereum/OVM/chain/OVM_StateCommitmentChain.sol

@@ -14,7 +14,6 @@ import { iOVM_StateCommitmentChain } from "../../iOVM/chain/iOVM_StateCommitment
 import { iOVM_CanonicalTransactionChain } from "../../iOVM/chain/iOVM_CanonicalTransactionChain.sol";
 import { iOVM_BondManager } from "../../iOVM/verification/iOVM_BondManager.sol";
 
-
 /**
  * @title OVM_StateCommitmentChain
  */
@@ -27,6 +26,7 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
      *************/
 
     uint256 constant public FRAUD_PROOF_WINDOW = 7 days;
+    uint256 constant public SEQUENCER_PUBLISH_WINDOW = 30 minutes;
 
 
     /*************
@@ -40,6 +40,7 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
     iOVM_FraudVerifier internal ovmFraudVerifier;
     iOVM_BondManager internal ovmBondManager;
 
+
     /***************
      * Constructor *
      ***************/
@@ -79,7 +80,8 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
             uint256 _totalElements
         )
     {
-        return uint256(uint216(batches.getExtraData()));
+        (uint40 totalElements, ) = _getBatchExtraData();
+        return uint256(totalElements);
     }
 
     /**
@@ -96,15 +98,38 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
         return uint256(batches.getLength());
     }
 
+    /**
+     * @inheritdoc iOVM_StateCommitmentChain
+     */
+    function getLastSequencerTimestamp()
+        override
+        public
+        view
+        returns (
+            uint256 _lastSequencerTimestamp
+        )
+    {
+        (, uint40 lastSequencerTimestamp) = _getBatchExtraData();
+        return uint256(lastSequencerTimestamp);
+    }
+
     /**
      * @inheritdoc iOVM_StateCommitmentChain
      */
     function appendStateBatch(
-        bytes32[] memory _batch
+        bytes32[] memory _batch,
+        uint256 _shouldStartAtElement
     )
         override
         public
     {
+        // Fail fast in to make sure our batch roots aren't accidentally made fraudulent by the
+        // publication of batches by some other user.
+        require(
+            _shouldStartAtElement == getTotalElements(),
+            "Actual batch start index does not match expected start index."
+        );
+
         // Proposers must have previously staked at the BondManager
         require(
             ovmBondManager.isCollateralized(msg.sender),
@@ -121,15 +146,10 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
             "Number of state roots cannot exceed the number of canonical transactions."
         );
 
-        bytes[] memory elements = new bytes[](_batch.length);
-        for (uint256 i = 0; i < _batch.length; i++) {
-            elements[i] = abi.encodePacked(_batch[i]);
-        }
-
         // Pass the block's timestamp and the publisher of the data
         // to be used in the fraud proofs
         _appendBatch(
-            elements,
+            _batch,
             abi.encode(block.timestamp, msg.sender)
         );
     }
@@ -149,7 +169,7 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
         );
 
         require(
-            Lib_OVMCodec.hashBatchHeader(_batchHeader) == batches.get(uint32(_batchHeader.batchIndex)),
+            _isValidBatchHeader(_batchHeader),
             "Invalid batch header."
         );
 
@@ -177,7 +197,7 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
         )
     {
         require(
-            Lib_OVMCodec.hashBatchHeader(_batchHeader) == batches.get(uint32(_batchHeader.batchIndex)),
+            _isValidBatchHeader(_batchHeader),
             "Invalid batch header."
         );
 
@@ -223,7 +243,7 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
     /**
      * @inheritdoc iOVM_StateCommitmentChain
      */
-    function setLastDeletableIndex(
+    function setLastOverwritableIndex(
         Lib_OVMCodec.ChainBatchHeader memory _stateBatchHeader,
         Lib_OVMCodec.Transaction memory _transaction,
         Lib_OVMCodec.TransactionChainElement memory _txChainElement,
@@ -234,18 +254,18 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
         public
     {
         require(
-            Lib_OVMCodec.hashBatchHeader(_stateBatchHeader) == batches.get(uint32(_stateBatchHeader.batchIndex)),
+            _isValidBatchHeader(_stateBatchHeader),
             "Invalid batch header."
         );
 
         require(
             insideFraudProofWindow(_stateBatchHeader) == false,
-            "Batch header must be outside of fraud proof window to be deletable."
+            "Batch header must be outside of fraud proof window to be overwritable."
         );
 
         require(
             _stateBatchHeader.batchIndex > lastDeletableIndex,
-            "Batch index must be greater than last deletable index."
+            "Batch index must be greater than last overwritable index."
         );
 
         require(
@@ -289,40 +309,107 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
      **********************/
 
     /**
-     * Appends a batch to the chain.
-     * @param _batchHeader Batch header to append.
+     * Parses the batch context from the extra data.
+     * @return Total number of elements submitted.
+     * @return Timestamp of the last batch submitted by the sequencer.
      */
-    function _appendBatch(
-        Lib_OVMCodec.ChainBatchHeader memory _batchHeader
-    )
+    function _getBatchExtraData()
         internal
+        view
+        returns (
+            uint40,
+            uint40
+        )
     {
-        batches.push(
-            Lib_OVMCodec.hashBatchHeader(_batchHeader),
-            bytes27(uint216(getTotalElements() + _batchHeader.batchSize))
+        bytes27 extraData = batches.getExtraData();
+
+        uint40 totalElements;
+        uint40 lastSequencerTimestamp;
+        assembly {
+            extraData              := shr(40, extraData)
+            totalElements          :=         and(extraData, 0x000000000000000000000000000000000000000000000000000000FFFFFFFFFF)
+            lastSequencerTimestamp := shr(40, and(extraData, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000))
+        }
+
+        return (
+            totalElements,
+            lastSequencerTimestamp
         );
     }
 
+    /**
+     * Encodes the batch context for the extra data.
+     * @param _totalElements Total number of elements submitted.
+     * @param _lastSequencerTimestamp Timestamp of the last batch submitted by the sequencer.
+     * @return Encoded batch context.
+     */
+    function _makeBatchExtraData(
+        uint40 _totalElements,
+        uint40 _lastSequencerTimestamp
+    )
+        internal
+        pure
+        returns (
+            bytes27
+        )
+    {
+        bytes27 extraData;
+        assembly {
+            extraData := _totalElements
+            extraData := or(extraData, shl(40, _lastSequencerTimestamp))
+            extraData := shl(40, extraData)
+        }
+
+        return extraData;
+    }
+
     /**
      * Appends a batch to the chain.
-     * @param _elements Elements within the batch.
+     * @param _batch Elements within the batch.
      * @param _extraData Any extra data to append to the batch.
      */
     function _appendBatch(
-        bytes[] memory _elements,
+        bytes32[] memory _batch,
         bytes memory _extraData
     )
         internal
     {
+        address sequencer = resolve("OVM_Sequencer");
+        (uint40 totalElements, uint40 lastSequencerTimestamp) = _getBatchExtraData();
+
+        if (msg.sender == sequencer) {
+            lastSequencerTimestamp = uint40(block.timestamp);
+        } else {
+            // We keep track of the last batch submitted by the sequencer so there's a window in
+            // which only the sequencer can publish state roots. A window like this just reduces
+            // the chance of "system breaking" state roots being published while we're still in
+            // testing mode. This window should be removed or significantly reduced in the future.
+            require(
+                lastSequencerTimestamp + SEQUENCER_PUBLISH_WINDOW < block.timestamp,
+                "Cannot publish state roots within the sequencer publication window."
+            );
+        }
+
+        bytes[] memory elements = new bytes[](_batch.length);
+        for (uint256 i = 0; i < _batch.length; i++) {
+            elements[i] = abi.encodePacked(_batch[i]);
+        }
+
         Lib_OVMCodec.ChainBatchHeader memory batchHeader = Lib_OVMCodec.ChainBatchHeader({
-            batchIndex: uint256(batches.getLength()),
-            batchRoot: Lib_MerkleUtils.getMerkleRoot(_elements),
-            batchSize: _elements.length,
-            prevTotalElements: getTotalElements(),
+            batchIndex: getTotalBatches(),
+            batchRoot: Lib_MerkleUtils.getMerkleRoot(elements),
+            batchSize: elements.length,
+            prevTotalElements: totalElements,
             extraData: _extraData
         });
 
-        _appendBatch(batchHeader);
+        batches.push(
+            Lib_OVMCodec.hashBatchHeader(batchHeader),
+            _makeBatchExtraData(
+                uint40(batchHeader.prevTotalElements + batchHeader.batchSize),
+                lastSequencerTimestamp
+            )
+        );
     }
 
     /**
@@ -340,13 +427,33 @@ contract OVM_StateCommitmentChain is iOVM_StateCommitmentChain, iRingBufferOverw
         );
 
         require(
-            Lib_OVMCodec.hashBatchHeader(_batchHeader) == batches.get(uint32(_batchHeader.batchIndex)),
+            _isValidBatchHeader(_batchHeader),
             "Invalid batch header."
         );
 
         batches.deleteElementsAfterInclusive(
             uint40(_batchHeader.batchIndex),
-            bytes27(uint216(_batchHeader.prevTotalElements))
+            _makeBatchExtraData(
+                uint40(_batchHeader.prevTotalElements),
+                0
+            )
         );
     }
+
+    /**
+     * Checks that a batch header matches the stored hash for the given index.
+     * @param _batchHeader Batch header to validate.
+     * @return Whether or not the header matches the stored one.
+     */
+    function _isValidBatchHeader(
+        Lib_OVMCodec.ChainBatchHeader memory _batchHeader
+    )
+        internal
+        view
+        returns (
+            bool
+        )
+    {
+        return Lib_OVMCodec.hashBatchHeader(_batchHeader) == batches.get(uint40(_batchHeader.batchIndex));
+    }
 }

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

@@ -15,9 +15,6 @@ import { iOVM_SafetyChecker } from "../../iOVM/execution/iOVM_SafetyChecker.sol"
 /* Contract Imports */
 import { OVM_ECDSAContractAccount } from "../accounts/OVM_ECDSAContractAccount.sol";
 
-/* Logging */
-import { console } from "@nomiclabs/buidler/console.sol";
-
 /**
  * @title OVM_ExecutionManager
  */

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

@@ -18,9 +18,6 @@ import { iOVM_StateManagerFactory } from "../../iOVM/execution/iOVM_StateManager
 /* Contract Imports */
 import { OVM_FraudContributor } from "./OVM_FraudContributor.sol";
 
-/* Logging Imports */
-import { console } from "@nomiclabs/buidler/console.sol";
-
 /**
  * @title OVM_StateTransitioner
  */

packages/contracts/contracts/optimistic-ethereum/iOVM/chain/iOVM_StateCommitmentChain.sol

@@ -36,12 +36,25 @@ interface iOVM_StateCommitmentChain {
             uint256 _totalBatches
         );
 
+    /**
+     * Retrieves the timestamp of the last batch submitted by the sequencer.
+     * @return _lastSequencerTimestamp Last sequencer batch timestamp.
+     */
+    function getLastSequencerTimestamp()
+        external
+        view
+        returns (
+            uint256 _lastSequencerTimestamp
+        );
+
     /**
      * Appends a batch of state roots to the chain.
      * @param _batch Batch of state roots.
+     * @param _shouldStartAtElement Index of the element at which this batch should start.
      */
     function appendStateBatch(
-        bytes32[] calldata _batch
+        bytes32[] calldata _batch,
+        uint256 _shouldStartAtElement
     )
         external;
 
@@ -93,7 +106,7 @@ interface iOVM_StateCommitmentChain {
      * @param _txBatchHeader Header of the batch the transaction was included in.
      * @param _txInclusionProof Inclusion proof for the provided transaction chain element.
      */
-    function setLastDeletableIndex(
+    function setLastOverwritableIndex(
         Lib_OVMCodec.ChainBatchHeader memory _stateBatchHeader,
         Lib_OVMCodec.Transaction memory _transaction,
         Lib_OVMCodec.TransactionChainElement memory _txChainElement,

packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_RingBuffer.sol

 pragma solidity ^0.7.0;
 
-/* Logging Imports */
-import { console } from "@nomiclabs/buidler/console.sol";
-
 interface iRingBufferOverwriter {
     function canOverwrite(bytes32 _id, uint256 _index) external returns (bool);
 }

packages/contracts/contracts/test-helpers/Helper_SimpleProxy.sol

 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.7.0;
 
-import { console } from "@nomiclabs/buidler/console.sol";
-
 contract Helper_SimpleProxy {
     address internal owner;
     address internal target;

packages/contracts/test/contracts/OVM/chain/OVM_StateCommitmentChain.spec.ts

@@ -14,13 +14,15 @@ import {
   toHexString32,
   getEthTime,
   NULL_BYTES32,
+  increaseEthTime,
 } from '../../../helpers'
 import { keccak256, defaultAbiCoder } from 'ethers/lib/utils'
 
 describe('OVM_StateCommitmentChain', () => {
-  let signer: Signer
+  let sequencer: Signer
+  let user: Signer
   before(async () => {
-    ;[signer] = await ethers.getSigners()
+    ;[sequencer, user] = await ethers.getSigners()
   })
 
   let AddressManager: Contract
@@ -52,6 +54,11 @@ describe('OVM_StateCommitmentChain', () => {
     )
 
     Mock__OVM_BondManager.smocked.isCollateralized.will.return.with(true)
+
+    await AddressManager.setAddress(
+      'OVM_Sequencer',
+      await sequencer.getAddress()
+    )
   })
 
   let Factory__OVM_StateCommitmentChain: ContractFactory
@@ -74,7 +81,7 @@ describe('OVM_StateCommitmentChain', () => {
 
       it('should revert', async () => {
         await expect(
-          OVM_StateCommitmentChain.appendStateBatch(batch)
+          OVM_StateCommitmentChain.appendStateBatch(batch, 0)
         ).to.be.revertedWith('Cannot submit an empty state batch.')
       })
     })
@@ -82,6 +89,16 @@ describe('OVM_StateCommitmentChain', () => {
     describe('when the provided batch is not empty', () => {
       const batch = [NON_NULL_BYTES32]
 
+      describe('when start index does not match total elements', () => {
+        it('should revert', async () => {
+          await expect(
+            OVM_StateCommitmentChain.appendStateBatch(batch, 1)
+          ).to.be.revertedWith(
+            'Actual batch start index does not match expected start index.'
+          )
+        })
+      })
+
       describe('when submitting more elements than present in the OVM_CanonicalTransactionChain', () => {
         before(() => {
           Mock__OVM_CanonicalTransactionChain.smocked.getTotalElements.will.return.with(
@@ -91,7 +108,7 @@ describe('OVM_StateCommitmentChain', () => {
 
         it('should revert', async () => {
           await expect(
-            OVM_StateCommitmentChain.appendStateBatch(batch)
+            OVM_StateCommitmentChain.appendStateBatch(batch, 0)
           ).to.be.revertedWith(
             'Number of state roots cannot exceed the number of canonical transactions.'
           )
@@ -106,8 +123,47 @@ describe('OVM_StateCommitmentChain', () => {
         })
 
         it('should append the state batch', async () => {
-          await expect(OVM_StateCommitmentChain.appendStateBatch(batch)).to.not
-            .be.reverted
+          await expect(OVM_StateCommitmentChain.appendStateBatch(batch, 0)).to
+            .not.be.reverted
+        })
+      })
+
+      describe('when a sequencer submits ', () => {
+        beforeEach(async () => {
+          Mock__OVM_CanonicalTransactionChain.smocked.getTotalElements.will.return.with(
+            batch.length * 2
+          )
+
+          await OVM_StateCommitmentChain.connect(sequencer).appendStateBatch(
+            batch,
+            0
+          )
+        })
+
+        describe('when inside sequencer publish window', () => {
+          it('should revert', async () => {
+            await expect(
+              OVM_StateCommitmentChain.connect(user).appendStateBatch(batch, 1)
+            ).to.be.revertedWith(
+              'Cannot publish state roots within the sequencer publication window.'
+            )
+          })
+        })
+
+        describe('when outside sequencer publish window', () => {
+          beforeEach(async () => {
+            const SEQUENCER_PUBLISH_WINDOW = await OVM_StateCommitmentChain.SEQUENCER_PUBLISH_WINDOW()
+            await increaseEthTime(
+              ethers.provider,
+              SEQUENCER_PUBLISH_WINDOW.toNumber() + 1
+            )
+          })
+
+          it('should succeed', async () => {
+            await expect(
+              OVM_StateCommitmentChain.connect(user).appendStateBatch(batch, 1)
+            ).to.not.be.reverted
+          })
         })
       })
     })
@@ -127,10 +183,10 @@ describe('OVM_StateCommitmentChain', () => {
       Mock__OVM_CanonicalTransactionChain.smocked.getTotalElements.will.return.with(
         batch.length
       )
-      await OVM_StateCommitmentChain.appendStateBatch(batch)
+      await OVM_StateCommitmentChain.appendStateBatch(batch, 0)
       batchHeader.extraData = defaultAbiCoder.encode(
         ['uint256', 'address'],
-        [await getEthTime(ethers.provider), await signer.getAddress()]
+        [await getEthTime(ethers.provider), await sequencer.getAddress()]
       )
     })
 
@@ -152,7 +208,7 @@ describe('OVM_StateCommitmentChain', () => {
       before(async () => {
         await AddressManager.setAddress(
           'OVM_FraudVerifier',
-          await signer.getAddress()
+          await sequencer.getAddress()
         )
       })
 
@@ -202,7 +258,7 @@ describe('OVM_StateCommitmentChain', () => {
         Mock__OVM_CanonicalTransactionChain.smocked.getTotalElements.will.return.with(
           batch.length
         )
-        await OVM_StateCommitmentChain.appendStateBatch(batch)
+        await OVM_StateCommitmentChain.appendStateBatch(batch, 0)
       })
 
       it('should return the number of inserted batch elements', async () => {
@@ -216,7 +272,7 @@ describe('OVM_StateCommitmentChain', () => {
         Mock__OVM_CanonicalTransactionChain.smocked.getTotalElements.will.return.with(
           batch.length
         )
-        await OVM_StateCommitmentChain.appendStateBatch(batch)
+        await OVM_StateCommitmentChain.appendStateBatch(batch, 0)
       })
 
       it('should return the number of inserted batch elements', async () => {
@@ -230,8 +286,8 @@ describe('OVM_StateCommitmentChain', () => {
         Mock__OVM_CanonicalTransactionChain.smocked.getTotalElements.will.return.with(
           batch.length * 2
         )
-        await OVM_StateCommitmentChain.appendStateBatch(batch)
-        await OVM_StateCommitmentChain.appendStateBatch(batch)
+        await OVM_StateCommitmentChain.appendStateBatch(batch, 0)
+        await OVM_StateCommitmentChain.appendStateBatch(batch, 32)
       })
 
       it('should return the number of inserted batch elements', async () => {
@@ -253,7 +309,7 @@ describe('OVM_StateCommitmentChain', () => {
         Mock__OVM_CanonicalTransactionChain.smocked.getTotalElements.will.return.with(
           batch.length
         )
-        await OVM_StateCommitmentChain.appendStateBatch(batch)
+        await OVM_StateCommitmentChain.appendStateBatch(batch, 0)
       })
 
       it('should return the number of inserted batch elements', async () => {
@@ -269,7 +325,7 @@ describe('OVM_StateCommitmentChain', () => {
         )
 
         for (let i = 0; i < 8; i++) {
-          await OVM_StateCommitmentChain.appendStateBatch(batch)
+          await OVM_StateCommitmentChain.appendStateBatch(batch, i)
         }
       })
 

packages/contracts/test/contracts/libraries/rlp/Lib_RLPReader.spec.ts

@@ -6,7 +6,6 @@ import { Lib_RLPReader_TEST_JSON } from '../../../data'
 import { runJsonTest, toHexString } from '../../../helpers'
 
 describe('Lib_RLPReader', () => {
-  //console.log(JSON.stringify(Lib_RLPReader_TEST_JSON2, null, 4))
   describe('JSON tests', () => {
     runJsonTest('TestLib_RLPReader', Lib_RLPReader_TEST_JSON)
   })