From 2f5b441b8d082d447a25eee74b24f2be9e29c5a8 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 08:59:12 +0100
Subject: [PATCH 01/16] refactor: avoid calldata to memory copy in sequencer
 inbox when not a DAS batch

---
 contracts/src/bridge/SequencerInbox.sol | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 6e029a4459..7a895b8f27 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -199,15 +199,16 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         emit SequencerBatchData(sequenceNumber, data);
     }
 
-    function dasKeysetHashFromBatchData(bytes memory data) internal pure returns (bytes32) {
-        if (data.length < 33 || data[0] & 0x80 == 0) {
-            return bytes32(0);
-        }
-        bytes32 temp;
+    function isDasBatch(bytes calldata data) internal pure returns (bool) {
+        return data.length >= 33 && data[0] & 0x80 != 0;
+    }
+
+    /// @dev assumes data has already been validated to be a DAS batch
+    function dasKeysetHashFromBatchData(bytes memory data) internal pure returns (bytes32 keysetHash) {
         assembly {
-            temp := mload(add(data, 33))
+            keysetHash := mload(add(data, 33))
         }
-        return temp;
+        return keysetHash;
     }
 
     function packHeader(uint256 afterDelayedMessagesRead)
@@ -233,8 +234,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         view
         returns (bytes32, TimeBounds memory)
     {
-        bytes32 dasKeysetHash = dasKeysetHashFromBatchData(data);
-        if (dasKeysetHash != bytes32(0)) {
+        if (isDasBatch(data)) {
+            bytes32 dasKeysetHash = dasKeysetHashFromBatchData(data);
             if (!isValidKeysetHash[dasKeysetHash]) revert NoSuchKeyset(dasKeysetHash);
         }
         uint256 fullDataLen = HEADER_LENGTH + data.length;

From 5dfc6f684c1827b40426b30287e12da24255cb36 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 09:14:23 +0100
Subject: [PATCH 02/16] refactor: use modifier to validate batch correctness
 for das

---
 contracts/src/bridge/SequencerInbox.sol | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 7a895b8f27..5d4b8b1a31 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -199,16 +199,18 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         emit SequencerBatchData(sequenceNumber, data);
     }
 
-    function isDasBatch(bytes calldata data) internal pure returns (bool) {
-        return data.length >= 33 && data[0] & 0x80 != 0;
-    }
-
-    /// @dev assumes data has already been validated to be a DAS batch
-    function dasKeysetHashFromBatchData(bytes memory data) internal pure returns (bytes32 keysetHash) {
-        assembly {
-            keysetHash := mload(add(data, 33))
+    modifier validateBatchData(bytes calldata data) {
+        if (data.length < 33 || data[0] & 0x80 == 0) {
+            // not a DAS batch, so we don't need keyset validation
+            _;
+        } else {
+            bytes32 dasKeysetHash;
+            assembly {
+                dasKeysetHash := calldataload(add(data.offset, 33))
+            }
+            if (!isValidKeysetHash[dasKeysetHash]) revert NoSuchKeyset(dasKeysetHash);
+            _;
         }
-        return keysetHash;
     }
 
     function packHeader(uint256 afterDelayedMessagesRead)
@@ -232,12 +234,9 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     function formDataHash(bytes calldata data, uint256 afterDelayedMessagesRead)
         internal
         view
+        validateBatchData(data)
         returns (bytes32, TimeBounds memory)
     {
-        if (isDasBatch(data)) {
-            bytes32 dasKeysetHash = dasKeysetHashFromBatchData(data);
-            if (!isValidKeysetHash[dasKeysetHash]) revert NoSuchKeyset(dasKeysetHash);
-        }
         uint256 fullDataLen = HEADER_LENGTH + data.length;
         if (fullDataLen < HEADER_LENGTH) revert DataLengthOverflow();
         if (fullDataLen > MAX_DATA_SIZE) revert DataTooLarge(fullDataLen, MAX_DATA_SIZE);

From 99a13711287f22ad8d43db45bd012a253f8ef6c9 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 09:38:12 +0100
Subject: [PATCH 03/16] refactor: track das keysets in struct

---
 contracts/src/bridge/SequencerInbox.sol | 31 ++++++++++++++++---------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 5d4b8b1a31..d16595275e 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -37,8 +37,11 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     mapping(address => bool) public isBatchPoster;
     ISequencerInbox.MaxTimeVariation public maxTimeVariation;
 
-    mapping(bytes32 => bool) public isValidKeysetHash;
-    mapping(bytes32 => uint256) public keysetHashCreationBlock;
+    struct DasKeySetInfo {
+        bool isValidKeyset;
+        uint64 creationBlock;
+    }
+    mapping(bytes32 => DasKeySetInfo) public dasKeySetInfo;
 
     modifier onlyRollupOwner() {
         if (msg.sender != IRollupUserAbs(rollup).owner()) revert NotOwner(msg.sender, rollup);
@@ -208,7 +211,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
             assembly {
                 dasKeysetHash := calldataload(add(data.offset, 33))
             }
-            if (!isValidKeysetHash[dasKeysetHash]) revert NoSuchKeyset(dasKeysetHash);
+            if (!dasKeySetInfo[dasKeysetHash].isValidKeyset) revert NoSuchKeyset(dasKeysetHash);
             _;
         }
     }
@@ -344,9 +347,11 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      */
     function setValidKeyset(bytes calldata keysetBytes) external onlyRollupOwner {
         bytes32 ksHash = keccak256(keysetBytes);
-        if (isValidKeysetHash[ksHash]) revert AlreadyValidDASKeyset(ksHash);
-        isValidKeysetHash[ksHash] = true;
-        keysetHashCreationBlock[ksHash] = block.number;
+        if (dasKeySetInfo[ksHash].isValidKeyset) revert AlreadyValidDASKeyset(ksHash);
+        dasKeySetInfo[ksHash] = DasKeySetInfo({
+            isValidKeyset: true,
+            creationBlock: uint64(block.number)
+        });
         emit SetValidKeyset(ksHash, keysetBytes);
         emit OwnerFunctionCalled(2);
     }
@@ -356,15 +361,19 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      * @param ksHash hash of the keyset
      */
     function invalidateKeysetHash(bytes32 ksHash) external onlyRollupOwner {
-        if (!isValidKeysetHash[ksHash]) revert NoSuchKeyset(ksHash);
-        isValidKeysetHash[ksHash] = false;
+        if (!dasKeySetInfo[ksHash].isValidKeyset) revert NoSuchKeyset(ksHash);
+        dasKeySetInfo[ksHash].isValidKeyset = false;
         emit InvalidateKeyset(ksHash);
         emit OwnerFunctionCalled(3);
     }
 
+    function isValidKeysetHash(bytes32 ksHash) external view returns (bool) {
+        return dasKeySetInfo[ksHash].isValidKeyset;
+    }
+
     function getKeysetCreationBlock(bytes32 ksHash) external view returns (uint256) {
-        uint256 bnum = keysetHashCreationBlock[ksHash];
-        if (bnum == 0) revert NoSuchKeyset(ksHash);
-        return bnum;
+        DasKeySetInfo memory ksInfo = dasKeySetInfo[ksHash];
+        if (ksInfo.creationBlock == 0 || !ksInfo.isValidKeyset) revert NoSuchKeyset(ksHash);
+        return uint256(ksInfo.creationBlock);
     }
 }

From be47f36d841326707be5e79d96da6a7272e1e443 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 09:58:56 +0100
Subject: [PATCH 04/16] fix das byte check and add other batch validation logic
 to modifier

---
 contracts/src/bridge/SequencerInbox.sol | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index d16595275e..668bfbc754 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -203,14 +203,19 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     }
 
     modifier validateBatchData(bytes calldata data) {
-        if (data.length < 33 || data[0] & 0x80 == 0) {
+        uint256 fullDataLen = HEADER_LENGTH + data.length;
+        if (fullDataLen < HEADER_LENGTH) revert DataLengthOverflow();
+        if (fullDataLen > MAX_DATA_SIZE) revert DataTooLarge(fullDataLen, MAX_DATA_SIZE);
+        if (data.length > 0 && (data[0] & DATA_AUTHENTICATED_FLAG) == DATA_AUTHENTICATED_FLAG) {
+            revert DataNotAuthenticated();
+        }
+        // the first byte is used to identify the type of batch data
+        if (data[0] & 0x80 == 0) {
             // not a DAS batch, so we don't need keyset validation
             _;
         } else {
-            bytes32 dasKeysetHash;
-            assembly {
-                dasKeysetHash := calldataload(add(data.offset, 33))
-            }
+            // the data length should always be > 33 here due to the HEADER_LENGTH check above
+            bytes32 dasKeysetHash = bytes32(data[1:33]);
             if (!dasKeySetInfo[dasKeysetHash].isValidKeyset) revert NoSuchKeyset(dasKeysetHash);
             _;
         }
@@ -241,17 +246,13 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         returns (bytes32, TimeBounds memory)
     {
         uint256 fullDataLen = HEADER_LENGTH + data.length;
-        if (fullDataLen < HEADER_LENGTH) revert DataLengthOverflow();
-        if (fullDataLen > MAX_DATA_SIZE) revert DataTooLarge(fullDataLen, MAX_DATA_SIZE);
         bytes memory fullData = new bytes(fullDataLen);
         (bytes memory header, TimeBounds memory timeBounds) = packHeader(afterDelayedMessagesRead);
 
         for (uint256 i = 0; i < HEADER_LENGTH; i++) {
             fullData[i] = header[i];
         }
-        if (data.length > 0 && (data[0] & DATA_AUTHENTICATED_FLAG) == DATA_AUTHENTICATED_FLAG) {
-            revert DataNotAuthenticated();
-        }
+
         // copy data into fullData at offset of HEADER_LENGTH (the extra 32 offset is because solidity puts the array len first)
         assembly {
             calldatacopy(add(fullData, add(HEADER_LENGTH, 32)), data.offset, data.length)

From 9e9a31322ff214d6649a5dd2e5d901d309ba9019 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 10:01:56 +0100
Subject: [PATCH 05/16] add data length check

---
 contracts/src/bridge/SequencerInbox.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 668bfbc754..44ffdfdaad 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -214,7 +214,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
             // not a DAS batch, so we don't need keyset validation
             _;
         } else {
-            // the data length should always be > 33 here due to the HEADER_LENGTH check above
+            if(data.length < 33) revert DataLengthOverflow();
             bytes32 dasKeysetHash = bytes32(data[1:33]);
             if (!dasKeySetInfo[dasKeysetHash].isValidKeyset) revert NoSuchKeyset(dasKeysetHash);
             _;

From aa4f18e1f4a00066e12513a94285ad368b03c505 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 10:04:53 +0100
Subject: [PATCH 06/16] remove overflow check not needed in solc >=0.8

---
 contracts/src/bridge/ISequencerInbox.sol | 2 +-
 contracts/src/bridge/SequencerInbox.sol  | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/contracts/src/bridge/ISequencerInbox.sol b/contracts/src/bridge/ISequencerInbox.sol
index 55da250c8d..c9bbd72349 100644
--- a/contracts/src/bridge/ISequencerInbox.sol
+++ b/contracts/src/bridge/ISequencerInbox.sol
@@ -56,7 +56,7 @@ interface ISequencerInbox is IDelayedMessageProvider {
     /// @dev Thrown when someone attempts to read more messages than exist
     error DelayedTooFar();
 
-    /// @dev Thrown if the length of the header plus the length of the batch overflows
+    /// @dev Thrown if the length of the batch isn't long enough
     error DataLengthOverflow();
 
     /// @dev Force include can only read messages more blocks old than the delay period
diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 44ffdfdaad..710fbe9f1a 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -204,7 +204,6 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
 
     modifier validateBatchData(bytes calldata data) {
         uint256 fullDataLen = HEADER_LENGTH + data.length;
-        if (fullDataLen < HEADER_LENGTH) revert DataLengthOverflow();
         if (fullDataLen > MAX_DATA_SIZE) revert DataTooLarge(fullDataLen, MAX_DATA_SIZE);
         if (data.length > 0 && (data[0] & DATA_AUTHENTICATED_FLAG) == DATA_AUTHENTICATED_FLAG) {
             revert DataNotAuthenticated();

From d57e4a42a87e0eddaed15c200d398167e833a06c Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 10:06:18 +0100
Subject: [PATCH 07/16] fix error name

---
 contracts/src/bridge/ISequencerInbox.sol | 4 ++--
 contracts/src/bridge/SequencerInbox.sol  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contracts/src/bridge/ISequencerInbox.sol b/contracts/src/bridge/ISequencerInbox.sol
index c9bbd72349..17778e1bdf 100644
--- a/contracts/src/bridge/ISequencerInbox.sol
+++ b/contracts/src/bridge/ISequencerInbox.sol
@@ -56,8 +56,8 @@ interface ISequencerInbox is IDelayedMessageProvider {
     /// @dev Thrown when someone attempts to read more messages than exist
     error DelayedTooFar();
 
-    /// @dev Thrown if the length of the batch isn't long enough
-    error DataLengthOverflow();
+    /// @dev Thrown if the length of the batch isn't long enough to be valid
+    error DataLengthUnderflow();
 
     /// @dev Force include can only read messages more blocks old than the delay period
     error ForceIncludeBlockTooSoon();
diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 710fbe9f1a..970cd679fb 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -213,7 +213,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
             // not a DAS batch, so we don't need keyset validation
             _;
         } else {
-            if(data.length < 33) revert DataLengthOverflow();
+            if(data.length < 33) revert DataLengthUnderflow();
             bytes32 dasKeysetHash = bytes32(data[1:33]);
             if (!dasKeySetInfo[dasKeysetHash].isValidKeyset) revert NoSuchKeyset(dasKeysetHash);
             _;

From 960ea5c6067023c3d71ca1b15da41a36214cdaa7 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 10:10:01 +0100
Subject: [PATCH 08/16] synchronize deletion of creation block

---
 contracts/src/bridge/SequencerInbox.sol | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 970cd679fb..612da62de0 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -213,7 +213,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
             // not a DAS batch, so we don't need keyset validation
             _;
         } else {
-            if(data.length < 33) revert DataLengthUnderflow();
+            if (data.length < 33) revert DataLengthUnderflow();
             bytes32 dasKeysetHash = bytes32(data[1:33]);
             if (!dasKeySetInfo[dasKeysetHash].isValidKeyset) revert NoSuchKeyset(dasKeysetHash);
             _;
@@ -362,7 +362,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      */
     function invalidateKeysetHash(bytes32 ksHash) external onlyRollupOwner {
         if (!dasKeySetInfo[ksHash].isValidKeyset) revert NoSuchKeyset(ksHash);
-        dasKeySetInfo[ksHash].isValidKeyset = false;
+        dasKeySetInfo[ksHash] = DasKeySetInfo({isValidKeyset: false, creationBlock: uint64(0)});
         emit InvalidateKeyset(ksHash);
         emit OwnerFunctionCalled(3);
     }
@@ -372,8 +372,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     }
 
     function getKeysetCreationBlock(bytes32 ksHash) external view returns (uint256) {
-        DasKeySetInfo memory ksInfo = dasKeySetInfo[ksHash];
-        if (ksInfo.creationBlock == 0 || !ksInfo.isValidKeyset) revert NoSuchKeyset(ksHash);
-        return uint256(ksInfo.creationBlock);
+        uint64 bnum = dasKeySetInfo[ksHash].creationBlock;
+        if (bnum == 0) revert NoSuchKeyset(ksHash);
+        return uint256(bnum);
     }
 }

From 07339b9154ebeda80b1f57db3ec89be91bbf4296 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 10:35:16 +0100
Subject: [PATCH 09/16] fix length check

---
 contracts/src/bridge/ISequencerInbox.sol | 3 ---
 contracts/src/bridge/SequencerInbox.sol  | 3 +--
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/contracts/src/bridge/ISequencerInbox.sol b/contracts/src/bridge/ISequencerInbox.sol
index 17778e1bdf..ad1395fc74 100644
--- a/contracts/src/bridge/ISequencerInbox.sol
+++ b/contracts/src/bridge/ISequencerInbox.sol
@@ -56,9 +56,6 @@ interface ISequencerInbox is IDelayedMessageProvider {
     /// @dev Thrown when someone attempts to read more messages than exist
     error DelayedTooFar();
 
-    /// @dev Thrown if the length of the batch isn't long enough to be valid
-    error DataLengthUnderflow();
-
     /// @dev Force include can only read messages more blocks old than the delay period
     error ForceIncludeBlockTooSoon();
 
diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 612da62de0..620a1eb360 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -209,11 +209,10 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
             revert DataNotAuthenticated();
         }
         // the first byte is used to identify the type of batch data
-        if (data[0] & 0x80 == 0) {
+        if (data.length < 33 || data[0] & 0x80 == 0) {
             // not a DAS batch, so we don't need keyset validation
             _;
         } else {
-            if (data.length < 33) revert DataLengthUnderflow();
             bytes32 dasKeysetHash = bytes32(data[1:33]);
             if (!dasKeySetInfo[dasKeysetHash].isValidKeyset) revert NoSuchKeyset(dasKeysetHash);
             _;

From a1284d19645e6a9412aff2c96c9ca717773c9c13 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 11:44:07 +0100
Subject: [PATCH 10/16] refactor das block creation to list every block a
 keyset was updated

---
 contracts/src/bridge/SequencerInbox.sol | 40 ++++++++++++++++++-------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 620a1eb360..e85acd5398 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -39,7 +39,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
 
     struct DasKeySetInfo {
         bool isValidKeyset;
-        uint64 creationBlock;
+        uint64[] blocksValueUpdated;
     }
     mapping(bytes32 => DasKeySetInfo) public dasKeySetInfo;
 
@@ -346,11 +346,12 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      */
     function setValidKeyset(bytes calldata keysetBytes) external onlyRollupOwner {
         bytes32 ksHash = keccak256(keysetBytes);
-        if (dasKeySetInfo[ksHash].isValidKeyset) revert AlreadyValidDASKeyset(ksHash);
-        dasKeySetInfo[ksHash] = DasKeySetInfo({
-            isValidKeyset: true,
-            creationBlock: uint64(block.number)
-        });
+        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
+        if (dkInfo.isValidKeyset) revert AlreadyValidDASKeyset(ksHash);
+
+        dkInfo.isValidKeyset = true;
+        dkInfo.blocksValueUpdated.push(uint64(block.number));
+
         emit SetValidKeyset(ksHash, keysetBytes);
         emit OwnerFunctionCalled(2);
     }
@@ -360,8 +361,12 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      * @param ksHash hash of the keyset
      */
     function invalidateKeysetHash(bytes32 ksHash) external onlyRollupOwner {
-        if (!dasKeySetInfo[ksHash].isValidKeyset) revert NoSuchKeyset(ksHash);
-        dasKeySetInfo[ksHash] = DasKeySetInfo({isValidKeyset: false, creationBlock: uint64(0)});
+        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
+        if (!dkInfo.isValidKeyset) revert NoSuchKeyset(ksHash);
+
+        dkInfo.isValidKeyset = false;
+        dkInfo.blocksValueUpdated.push(uint64(block.number));
+
         emit InvalidateKeyset(ksHash);
         emit OwnerFunctionCalled(3);
     }
@@ -371,8 +376,23 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     }
 
     function getKeysetCreationBlock(bytes32 ksHash) external view returns (uint256) {
-        uint64 bnum = dasKeySetInfo[ksHash].creationBlock;
-        if (bnum == 0) revert NoSuchKeyset(ksHash);
+        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
+        uint256 length = dkInfo.blocksValueUpdated.length;
+        if (length == 0) revert NoSuchKeyset(ksHash);
+        uint64 bnum = dkInfo.blocksValueUpdated[0];
         return uint256(bnum);
     }
+
+    function getKeysetLatestBlock(bytes32 ksHash) external view returns (uint256) {
+        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
+        uint256 length = dkInfo.blocksValueUpdated.length;
+        if (length == 0) revert NoSuchKeyset(ksHash);
+        uint64 bnum = dkInfo.blocksValueUpdated[length - 1];
+        return uint256(bnum);
+    }
+
+    function getKeysetBlocksLength(bytes32 ksHash) external view returns (uint256) {
+        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
+        return dkInfo.blocksValueUpdated.length;
+    }
 }

From c03848c7e70e2ffc53165197771f0e1e08a11a36 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 11:58:49 +0100
Subject: [PATCH 11/16] docs: comment around 33 magic value

---
 contracts/src/bridge/SequencerInbox.sol | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index e85acd5398..d9b5633c6d 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -209,10 +209,12 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
             revert DataNotAuthenticated();
         }
         // the first byte is used to identify the type of batch data
+        // das batches expect to have the type byte set, followed by the keyset (so they should have at least 33 bytes)
         if (data.length < 33 || data[0] & 0x80 == 0) {
             // not a DAS batch, so we don't need keyset validation
             _;
         } else {
+            // we skip the first byte, then read the next 32 bytes for the keyset
             bytes32 dasKeysetHash = bytes32(data[1:33]);
             if (!dasKeySetInfo[dasKeysetHash].isValidKeyset) revert NoSuchKeyset(dasKeysetHash);
             _;

From a32123c29a4119dce0867417912f210b82026ab3 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 12:01:27 +0100
Subject: [PATCH 12/16] Revert "refactor das block creation to list every block
 a keyset was updated"

This reverts commit a1284d19645e6a9412aff2c96c9ca717773c9c13.
---
 contracts/src/bridge/SequencerInbox.sol | 40 +++++++------------------
 1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index d9b5633c6d..313f0ede20 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -39,7 +39,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
 
     struct DasKeySetInfo {
         bool isValidKeyset;
-        uint64[] blocksValueUpdated;
+        uint64 creationBlock;
     }
     mapping(bytes32 => DasKeySetInfo) public dasKeySetInfo;
 
@@ -348,12 +348,11 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      */
     function setValidKeyset(bytes calldata keysetBytes) external onlyRollupOwner {
         bytes32 ksHash = keccak256(keysetBytes);
-        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
-        if (dkInfo.isValidKeyset) revert AlreadyValidDASKeyset(ksHash);
-
-        dkInfo.isValidKeyset = true;
-        dkInfo.blocksValueUpdated.push(uint64(block.number));
-
+        if (dasKeySetInfo[ksHash].isValidKeyset) revert AlreadyValidDASKeyset(ksHash);
+        dasKeySetInfo[ksHash] = DasKeySetInfo({
+            isValidKeyset: true,
+            creationBlock: uint64(block.number)
+        });
         emit SetValidKeyset(ksHash, keysetBytes);
         emit OwnerFunctionCalled(2);
     }
@@ -363,12 +362,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      * @param ksHash hash of the keyset
      */
     function invalidateKeysetHash(bytes32 ksHash) external onlyRollupOwner {
-        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
-        if (!dkInfo.isValidKeyset) revert NoSuchKeyset(ksHash);
-
-        dkInfo.isValidKeyset = false;
-        dkInfo.blocksValueUpdated.push(uint64(block.number));
-
+        if (!dasKeySetInfo[ksHash].isValidKeyset) revert NoSuchKeyset(ksHash);
+        dasKeySetInfo[ksHash] = DasKeySetInfo({isValidKeyset: false, creationBlock: uint64(0)});
         emit InvalidateKeyset(ksHash);
         emit OwnerFunctionCalled(3);
     }
@@ -378,23 +373,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     }
 
     function getKeysetCreationBlock(bytes32 ksHash) external view returns (uint256) {
-        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
-        uint256 length = dkInfo.blocksValueUpdated.length;
-        if (length == 0) revert NoSuchKeyset(ksHash);
-        uint64 bnum = dkInfo.blocksValueUpdated[0];
+        uint64 bnum = dasKeySetInfo[ksHash].creationBlock;
+        if (bnum == 0) revert NoSuchKeyset(ksHash);
         return uint256(bnum);
     }
-
-    function getKeysetLatestBlock(bytes32 ksHash) external view returns (uint256) {
-        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
-        uint256 length = dkInfo.blocksValueUpdated.length;
-        if (length == 0) revert NoSuchKeyset(ksHash);
-        uint64 bnum = dkInfo.blocksValueUpdated[length - 1];
-        return uint256(bnum);
-    }
-
-    function getKeysetBlocksLength(bytes32 ksHash) external view returns (uint256) {
-        DasKeySetInfo storage dkInfo = dasKeySetInfo[ksHash];
-        return dkInfo.blocksValueUpdated.length;
-    }
 }

From 6c84471178672dc3068de9e4f1eba7ef6ca717f1 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 12:03:19 +0100
Subject: [PATCH 13/16] Revert "synchronize deletion of creation block"

This reverts commit 960ea5c6067023c3d71ca1b15da41a36214cdaa7.
---
 contracts/src/bridge/SequencerInbox.sol | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 313f0ede20..27f98e4d72 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -363,7 +363,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      */
     function invalidateKeysetHash(bytes32 ksHash) external onlyRollupOwner {
         if (!dasKeySetInfo[ksHash].isValidKeyset) revert NoSuchKeyset(ksHash);
-        dasKeySetInfo[ksHash] = DasKeySetInfo({isValidKeyset: false, creationBlock: uint64(0)});
+        dasKeySetInfo[ksHash].isValidKeyset = false;
         emit InvalidateKeyset(ksHash);
         emit OwnerFunctionCalled(3);
     }
@@ -373,8 +373,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     }
 
     function getKeysetCreationBlock(bytes32 ksHash) external view returns (uint256) {
-        uint64 bnum = dasKeySetInfo[ksHash].creationBlock;
-        if (bnum == 0) revert NoSuchKeyset(ksHash);
-        return uint256(bnum);
+        DasKeySetInfo memory ksInfo = dasKeySetInfo[ksHash];
+        if (ksInfo.creationBlock == 0 || !ksInfo.isValidKeyset) revert NoSuchKeyset(ksHash);
+        return uint256(ksInfo.creationBlock);
     }
 }

From 23e45306cb45bb267ec4fb6781b4a0c6e1fb4c75 Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 12:05:47 +0100
Subject: [PATCH 14/16] add comments explaining code expectations

---
 contracts/src/bridge/SequencerInbox.sol | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 27f98e4d72..bf2a0eb77a 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -363,6 +363,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      */
     function invalidateKeysetHash(bytes32 ksHash) external onlyRollupOwner {
         if (!dasKeySetInfo[ksHash].isValidKeyset) revert NoSuchKeyset(ksHash);
+        // this is used to fetch the hash preimage in the SetValidKeyset event  
+        // which is emitted when the key is initially created.
         dasKeySetInfo[ksHash].isValidKeyset = false;
         emit InvalidateKeyset(ksHash);
         emit OwnerFunctionCalled(3);
@@ -372,9 +374,10 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         return dasKeySetInfo[ksHash].isValidKeyset;
     }
 
+    /// @notice the creation block is intended to still be available after a keyset is deleted
     function getKeysetCreationBlock(bytes32 ksHash) external view returns (uint256) {
         DasKeySetInfo memory ksInfo = dasKeySetInfo[ksHash];
-        if (ksInfo.creationBlock == 0 || !ksInfo.isValidKeyset) revert NoSuchKeyset(ksHash);
+        if (ksInfo.creationBlock == 0) revert NoSuchKeyset(ksHash);
         return uint256(ksInfo.creationBlock);
     }
 }

From 9d956c093e86cb4cbe8bb5a480ff919c34b1a13c Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 12:14:06 +0100
Subject: [PATCH 15/16] use high level solidity for concat of bytes

---
 contracts/src/bridge/SequencerInbox.sol | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index bf2a0eb77a..4924b94b52 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -245,19 +245,9 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         validateBatchData(data)
         returns (bytes32, TimeBounds memory)
     {
-        uint256 fullDataLen = HEADER_LENGTH + data.length;
-        bytes memory fullData = new bytes(fullDataLen);
         (bytes memory header, TimeBounds memory timeBounds) = packHeader(afterDelayedMessagesRead);
-
-        for (uint256 i = 0; i < HEADER_LENGTH; i++) {
-            fullData[i] = header[i];
-        }
-
-        // copy data into fullData at offset of HEADER_LENGTH (the extra 32 offset is because solidity puts the array len first)
-        assembly {
-            calldatacopy(add(fullData, add(HEADER_LENGTH, 32)), data.offset, data.length)
-        }
-        return (keccak256(fullData), timeBounds);
+        bytes32 dataHash = keccak256(bytes.concat(header, data));
+        return (dataHash, timeBounds);
     }
 
     function formEmptyDataHash(uint256 afterDelayedMessagesRead)
@@ -363,7 +353,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      */
     function invalidateKeysetHash(bytes32 ksHash) external onlyRollupOwner {
         if (!dasKeySetInfo[ksHash].isValidKeyset) revert NoSuchKeyset(ksHash);
-        // this is used to fetch the hash preimage in the SetValidKeyset event  
+        // this is used to fetch the hash preimage in the SetValidKeyset event
         // which is emitted when the key is initially created.
         dasKeySetInfo[ksHash].isValidKeyset = false;
         emit InvalidateKeyset(ksHash);

From 77923a0e073a29707a26bdecc51a5bdcd4031a3c Mon Sep 17 00:00:00 2001
From: fredlacs <32464905+fredlacs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 13:23:18 +0100
Subject: [PATCH 16/16] clarify docs

---
 contracts/src/bridge/SequencerInbox.sol | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/contracts/src/bridge/SequencerInbox.sol b/contracts/src/bridge/SequencerInbox.sol
index 4924b94b52..751c141af5 100644
--- a/contracts/src/bridge/SequencerInbox.sol
+++ b/contracts/src/bridge/SequencerInbox.sol
@@ -353,8 +353,9 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
      */
     function invalidateKeysetHash(bytes32 ksHash) external onlyRollupOwner {
         if (!dasKeySetInfo[ksHash].isValidKeyset) revert NoSuchKeyset(ksHash);
-        // this is used to fetch the hash preimage in the SetValidKeyset event
-        // which is emitted when the key is initially created.
+        // we don't delete the block creation value since its used to fetch the SetValidKeyset
+        // event efficiently. The event provides the hash preimage of the key.
+        // this is still needed when syncing the chain after a keyset is invalidated.
         dasKeySetInfo[ksHash].isValidKeyset = false;
         emit InvalidateKeyset(ksHash);
         emit OwnerFunctionCalled(3);