Merge pull request OffchainLabs#137 from OffchainLabs/fix-lint-warnings

fix: format and lint warnings

.solhint.json

@@ -11,7 +11,7 @@
     "no-empty-blocks": "off",
     "reason-string": ["warn", { "maxLength": 128 }],
     "not-rely-on-time": "off",
-    "max-states-count": ["warn", 30],
+    "max-states-count": ["warn", 40],
     "no-inline-assembly": "off"
   },
   "plugins": ["prettier"]

README.md

@@ -22,14 +22,12 @@ yarn build
 
 Nitro is currently licensed under a [Business Source License](./LICENSE), similar to our friends at Uniswap and Aave, with an "Additional Use Grant" to ensure that everyone can have full comfort using and running nodes on all public Arbitrum chains.
 
-The Additional Use Grant also permits the deployment of the Nitro software, in a permissionless fashion and without cost, as a new blockchain provided that the chain settles to either Arbitrum One or Arbitrum Nova. 
+The Additional Use Grant also permits the deployment of the Nitro software, in a permissionless fashion and without cost, as a new blockchain provided that the chain settles to either Arbitrum One or Arbitrum Nova.
 
-For those that prefer to deploy the Nitro software either directly on Ethereum (i.e. an L2) or have it settle to another Layer-2 on top of Ethereum, the [Arbitrum Expansion Program (the "AEP")](https://docs.arbitrum.foundation/assets/files/Arbitrum%20Expansion%20Program%20Jan182024-4f08b0c2cb476a55dc153380fa3e64b0.pdf) was recently established. The AEP allows for the permissionless deployment in the aforementioned fashion provided that 10% of net revenue is contributed back to the Arbitrum community in accordance with the requirements of the AEP. 
+For those that prefer to deploy the Nitro software either directly on Ethereum (i.e. an L2) or have it settle to another Layer-2 on top of Ethereum, the [Arbitrum Expansion Program (the "AEP")](https://docs.arbitrum.foundation/assets/files/Arbitrum%20Expansion%20Program%20Jan182024-4f08b0c2cb476a55dc153380fa3e64b0.pdf) was recently established. The AEP allows for the permissionless deployment in the aforementioned fashion provided that 10% of net revenue is contributed back to the Arbitrum community in accordance with the requirements of the AEP.
 
 ## Contact
 
 Discord - [Arbitrum](https://discord.com/invite/5KE54JwyTs)
 
 Twitter: [Arbitrum](https://twitter.com/arbitrum)
-
-

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@arbitrum/nitro-contracts",
-  "version": "1.1.0",
+  "version": "1.2.0",
   "description": "Layer 2 precompiles and rollup for Arbitrum Nitro",
   "author": "Offchain Labs, Inc.",
   "license": "BUSL-1.1",
@@ -26,7 +26,7 @@
     "lint:test": "eslint ./test",
     "solhint": "solhint -f table src/**/*.sol",
     "prettier:solidity": "prettier --write src/**/*.sol",
-    "format": "prettier './**/*.{js,json,md,ts,yml,sol}' --write && yarn run lint:test --fix",
+    "format": "prettier './**/*.{js,json,ts,yml,sol}' --write && yarn run lint:test --fix",
     "build:0.6": "INTERFACE_TESTER_SOLC_VERSION=0.6.9 yarn run build",
     "build:0.7": "INTERFACE_TESTER_SOLC_VERSION=0.7.0 yarn run build",
     "test": "DISABLE_GAS_REPORTER=true hardhat --network hardhat test test/contract/*.spec.ts",

src/bridge/GasRefunder.sol

@@ -165,6 +165,7 @@ contract GasRefunder is IGasRefunder, Ownable {
 
     function withdraw(address payable destination, uint256 amount) external onlyOwner {
         // It's expected that destination is an EOA
+        // solhint-disable-next-line avoid-low-level-calls
         (bool success, ) = destination.call{value: amount}("");
         require(success, "WITHDRAW_FAILED");
         emit Withdrawn(msg.sender, destination, amount);
@@ -249,6 +250,7 @@ contract GasRefunder is IGasRefunder, Ownable {
         }
 
         // It's expected that refundee is an EOA
+        // solhint-disable-next-line avoid-low-level-calls
         (success, ) = refundee.call{value: refundAmount}("");
         emit RefundedGasCosts(refundee, msg.sender, success, gasUsed, estGasPrice, refundAmount);
     }

src/bridge/SequencerInbox.sol

@@ -52,11 +52,11 @@ import "../libraries/ArbitrumChecker.sol";
 import {IERC20Bridge} from "./IERC20Bridge.sol";
 
 /**
- * @title Accepts batches from the sequencer and adds them to the rollup inbox.
+ * @title  Accepts batches from the sequencer and adds them to the rollup inbox.
  * @notice Contains the inbox accumulator which is the ordering of all data and transactions to be processed by the rollup.
- * As part of submitting a batch the sequencer is also expected to include items enqueued
- * in the delayed inbox (Bridge.sol). If items in the delayed inbox are not included by a
- * sequencer within a time limit they can be force included into the rollup inbox by anyone.
+ *         As part of submitting a batch the sequencer is also expected to include items enqueued
+ *         in the delayed inbox (Bridge.sol). If items in the delayed inbox are not included by a
+ *         sequencer within a time limit they can be force included into the rollup inbox by anyone.
  */
 contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox {
     uint256 public totalDelayedMessagesRead;
@@ -92,6 +92,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
     mapping(address => bool) public isBatchPoster;
 
     // we previously stored the max time variation in a (uint,uint,uint,uint) struct here
+    // solhint-disable-next-line var-name-mixedcase
     uint256[4] private __LEGACY_MAX_TIME_VARIATION;
 
     mapping(bytes32 => DasKeySetInfo) public dasKeySetInfo;
@@ -323,9 +324,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         );
         uint256 __totalDelayedMessagesRead = _totalDelayedMessagesRead;
         uint256 prevSeqMsgCount = bridge.sequencerReportedSubMessageCount();
-        uint256 newSeqMsgCount = prevSeqMsgCount +
-            _totalDelayedMessagesRead -
-            totalDelayedMessagesRead;
+        uint256 newSeqMsgCount = prevSeqMsgCount; // force inclusion should not modify sequencer message count
         (
             uint256 seqMessageIndex,
             bytes32 beforeAcc,
@@ -608,8 +607,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         return (keccak256(bytes.concat(header, data)), timeBounds);
     }
 
-    /// @dev   Form a hash of the data being provided in 4844 data blobs
-    /// @param afterDelayedMessagesRead The delayed messages count read up to
+    /// @dev    Form a hash of the data being provided in 4844 data blobs
+    /// @param  afterDelayedMessagesRead The delayed messages count read up to
     /// @return The data hash
     /// @return The timebounds within which the message should be processed
     /// @return The normalized amount of gas used for blob posting

src/libraries/GasRefundEnabled.sol

@@ -9,7 +9,7 @@ import "./IReader4844.sol";
 import "./IGasRefunder.sol";
 
 abstract contract GasRefundEnabled {
-    uint256 immutable gasPerBlob = 2**17;
+    uint256 internal immutable gasPerBlob = 2**17;
 
     /// @dev this refunds the sender for execution costs of the tx
     /// calldata costs are only refunded if `msg.sender == tx.origin` to guarantee the value refunded relates to charging

src/libraries/IReader4844.sol

@@ -1,3 +1,7 @@
+// Copyright 2023-2024, Offchain Labs, Inc.
+// For license information, see https://github.com/OffchainLabs/nitro-contracts/blob/main/LICENSE
+// SPDX-License-Identifier: BUSL-1.1
+
 pragma solidity >=0.6.9 <0.9.0;
 
 interface IReader4844 {

src/mocks/InboxStub.sol

@@ -163,7 +163,7 @@ contract InboxStub is IInboxBase, IInbox {
         address,
         uint256,
         bytes calldata
-    ) external returns (uint256) {
+    ) external pure returns (uint256) {
         revert("NOT_IMPLEMENTED");
     }
 
@@ -173,7 +173,7 @@ contract InboxStub is IInboxBase, IInbox {
         uint256,
         uint256,
         address
-    ) external returns (uint256) {
+    ) external pure returns (uint256) {
         revert("NOT_IMPLEMENTED");
     }
 

src/mocks/Simple.sol

@@ -32,6 +32,7 @@ contract Simple {
     }
 
     function incrementRedeem() external {
+        // solhint-disable-next-line avoid-tx-origin
         require(msg.sender == tx.origin, "SENDER_NOT_ORIGIN");
         require(ArbSys(address(0x64)).wasMyCallersAddressAliased(), "NOT_ALIASED");
         counter++;
@@ -99,6 +100,7 @@ contract Simple {
             useTopLevel,
             delegateCase
         );
+        // solhint-disable-next-line avoid-low-level-calls
         (bool success, ) = address(this).delegatecall(data);
         require(success, "DELEGATE_CALL_FAILED");
 
@@ -119,6 +121,7 @@ contract Simple {
             useTopLevel,
             callCase
         );
+        // solhint-disable-next-line avoid-low-level-calls
         (success, ) = address(this).call(data);
         require(success, "CALL_FAILED");
     }
@@ -127,6 +130,7 @@ contract Simple {
         uint256 before = gasleft();
         // The inner call may revert, but we still want to return the amount of gas used,
         // so we ignore the result of this call.
+        // solhint-disable-next-line avoid-low-level-calls
         // solc-ignore-next-line unused-call-retval
         to.staticcall{gas: before - 10000}(input);
         return before - gasleft();

src/rollup/RollupCreator.sol

@@ -268,6 +268,7 @@ contract RollupCreator is Ownable {
             l2FactoriesDeployer.perform{value: cost}(_inbox, _nativeToken, _maxFeePerGas);
 
             // refund the caller
+            // solhint-disable-next-line avoid-low-level-calls
             (bool sent, ) = msg.sender.call{value: address(this).balance}("");
             require(sent, "Refund failed");
         } else {

test/contract/arbRollup.spec.ts

@@ -15,7 +15,7 @@
  */
 
 /* eslint-env node, mocha */
-import { ethers, network, run } from 'hardhat'
+import { ethers, network } from 'hardhat'
 import { Signer } from '@ethersproject/abstract-signer'
 import { BigNumberish, BigNumber } from '@ethersproject/bignumber'
 import { BytesLike } from '@ethersproject/bytes'
@@ -97,7 +97,7 @@ let admin: Signer
 let sequencer: Signer
 let challengeManager: ChallengeManager
 let upgradeExecutor: string
-let adminproxy: string
+// let adminproxy: string
 
 async function getDefaultConfig(
   _confirmPeriodBlocks = confirmationPeriodBlocks
@@ -542,7 +542,7 @@ describe('ArbRollup', () => {
     admin = adminI
     validators = validatorsI
     upgradeExecutor = upgradeExecutorAddress
-    adminproxy = adminproxyAddress
+    // adminproxy = adminproxyAddress
     rollup = new RollupContract(rollupUser.connect(validators[0]))
     batchPosterManager = batchPosterManagerI
   })

test/contract/sequencerInbox.spec.4844.ts

@@ -16,13 +16,9 @@
 
 /* eslint-env node, mocha */
 
-import { ethers, network } from 'hardhat'
+import { ethers } from 'hardhat'
 import { BigNumber } from '@ethersproject/bignumber'
-import {
-  Block,
-  JsonRpcProvider,
-  TransactionReceipt,
-} from '@ethersproject/providers'
+import { JsonRpcProvider, TransactionReceipt } from '@ethersproject/providers'
 import { expect } from 'chai'
 import {
   Bridge,
@@ -55,24 +51,6 @@ import { SequencerInbox } from '../../build/types/src/bridge/SequencerInbox'
 import { InboxMessageDeliveredEvent } from '../../build/types/src/bridge/AbsInbox'
 import { SequencerBatchDeliveredEvent } from '../../build/types/src/bridge/ISequencerInbox'
 
-const mineBlocks = async (
-  wallet: Wallet,
-  count: number,
-  timeDiffPerBlock = 14
-) => {
-  const block = (await network.provider.send('eth_getBlockByNumber', [
-    'latest',
-    false,
-  ])) as Block
-  let timestamp = BigNumber.from(block.timestamp).toNumber()
-  for (let i = 0; i < count; i++) {
-    timestamp = timestamp + timeDiffPerBlock
-    await (
-      await wallet.sendTransaction({ to: constants.AddressZero, value: 1 })
-    ).wait()
-  }
-}
-
 describe('SequencerInbox', async () => {
   const findMatchingLogs = <TInterface extends Interface, TEvent extends Event>(
     receipt: TransactionReceipt,
@@ -218,40 +196,6 @@ describe('SequencerInbox', async () => {
     return wallets
   }
 
-  const connectAddreses = (
-    user: Wallet,
-    deployer: Wallet,
-    batchPoster: Wallet,
-    addresses: {
-      user: string
-      bridge: string
-      inbox: string
-      sequencerInbox: string
-      messageTester: string
-      batchPoster: string
-      gasRefunder: string
-    }
-  ) => {
-    return {
-      user,
-      batchPoster,
-      bridge: Bridge__factory.connect(addresses.bridge, user),
-      inbox: Inbox__factory.connect(addresses.inbox, user),
-      sequencerInbox: SequencerInbox__factory.connect(
-        addresses.sequencerInbox,
-        user
-      ),
-      messageTester: MessageTester__factory.connect(
-        addresses.messageTester,
-        deployer
-      ),
-      gasRefunder: GasRefunder__factory.connect(
-        addresses.gasRefunder,
-        deployer
-      ),
-    }
-  }
-
   const setupSequencerInbox = async (
     fundingWallet: Wallet,
     maxDelayBlocks = 10,

test/contract/sequencerInboxForceInclude.spec.ts

@@ -222,10 +222,10 @@ describe('SequencerInboxForceInclude', async () => {
     const admin = accounts[0]
     const adminAddr = await admin.getAddress()
     const user = accounts[1]
-    const dummyRollup = accounts[2]
+    // const dummyRollup = accounts[2]
     const rollupOwner = accounts[3]
     const batchPoster = accounts[4]
-    const batchPosterManager = accounts[5]
+    // const batchPosterManager = accounts[5]
 
     const rollupMockFac = (await ethers.getContractFactory(
       'RollupMock'
@@ -391,6 +391,51 @@ describe('SequencerInboxForceInclude', async () => {
     )
   })
 
+  it('can force-include-with-max-seqReportedCount', async () => {
+    const { user, inbox, bridge, messageTester, batchPoster, sequencerInbox } =
+      await setupSequencerInbox()
+
+    await sequencerInbox
+      .connect(batchPoster)
+      [
+        'addSequencerL2BatchFromOrigin(uint256,bytes,uint256,address,uint256,uint256)'
+      ](
+        0,
+        '0x',
+        0,
+        ethers.constants.AddressZero,
+        0,
+        ethers.constants.MaxUint256
+      )
+
+    const delayedTx = await sendDelayedTx(
+      user,
+      inbox,
+      bridge,
+      messageTester,
+      1000000,
+      21000000000,
+      0,
+      await user.getAddress(),
+      BigNumber.from(10),
+      '0x1010'
+    )
+    const maxTimeVariation = await sequencerInbox.maxTimeVariation()
+
+    await mineBlocks(maxTimeVariation[0].toNumber())
+
+    await forceIncludeMessages(
+      sequencerInbox,
+      delayedTx.inboxAccountLength,
+      delayedTx.deliveredMessageEvent.kind,
+      delayedTx.l1BlockNumber,
+      delayedTx.l1BlockTimestamp,
+      delayedTx.baseFeeL1,
+      delayedTx.senderAddr,
+      delayedTx.deliveredMessageEvent.messageDataHash
+    )
+  })
+
   it('can force-include one after another', async () => {
     const { user, inbox, bridge, messageTester, sequencerInbox } =
       await setupSequencerInbox()

test/contract/toolkit4844.ts

@@ -1,12 +1,12 @@
 import { execSync } from 'child_process'
-import { ContractFactory, Signer, Wallet, ethers } from 'ethers'
+import { ContractFactory, Signer, ethers } from 'ethers'
 import * as http from 'http'
 import { IReader4844, IReader4844__factory } from '../../build/types'
 import { JsonRpcProvider } from '@ethersproject/providers'
 import { bytecode as Reader4844Bytecode } from '../../out/yul/Reader4844.yul/Reader4844.json'
 
 const wait = async (ms: number) =>
-  new Promise((res, rej) => {
+  new Promise(res => {
     setTimeout(res, ms)
   })