From 0c50f5d028e2bb74da69980dceb2595e58f4ad2b Mon Sep 17 00:00:00 2001 From: Rens Rooimans Date: Thu, 12 Oct 2023 16:37:47 +0200 Subject: [PATCH] Fix l2EP solhint (#10932) * fix imports & private funcs * fix solhint --- contracts/.solhintignore | 2 - contracts/foundry.toml | 9 ++- contracts/package.json | 2 +- .../l2ep/dev/CrossDomainDelegateForwarder.sol | 5 +- .../v0.8/l2ep/dev/CrossDomainForwarder.sol | 4 +- .../src/v0.8/l2ep/dev/CrossDomainOwnable.sol | 7 ++- .../arbitrum/ArbitrumCrossDomainForwarder.sol | 15 +++-- .../arbitrum/ArbitrumCrossDomainGovernor.sol | 14 +++-- .../arbitrum/ArbitrumSequencerUptimeFeed.sol | 58 ++++++++++--------- .../l2ep/dev/arbitrum/ArbitrumValidator.sol | 49 +++++++++------- .../dev/interfaces/IArbitrumDelayedInbox.sol | 2 +- .../optimism/OptimismCrossDomainForwarder.sol | 18 ++++-- .../optimism/OptimismCrossDomainGovernor.sol | 16 +++-- .../optimism/OptimismSequencerUptimeFeed.sol | 42 ++++++++------ .../l2ep/dev/optimism/OptimismValidator.sol | 21 +++---- 15 files changed, 156 insertions(+), 108 deletions(-) diff --git a/contracts/.solhintignore b/contracts/.solhintignore index 9176571e570..d36b4a27261 100644 --- a/contracts/.solhintignore +++ b/contracts/.solhintignore @@ -4,8 +4,6 @@ #./src/v0.8/dev # 91 warnings #./src/v0.8/functions -# 91 warnings -#./src/v0.8/l2ep/dev # Ignore tests, this should not be the long term plan but is OK in the short term diff --git a/contracts/foundry.toml b/contracts/foundry.toml index d1bec52c391..1228ce3ec03 100644 --- a/contracts/foundry.toml +++ b/contracts/foundry.toml @@ -33,13 +33,18 @@ optimizer_runs = 10000 src = 'src/v0.8/automation' test = 'src/v0.8/automation/test' +[profile.l2ep] +optimizer_runs = 1000000 +src = 'src/v0.8/l2ep' +test = 'src/v0.8/l2ep/test' + + [profile.llo-feeds] optimizer_runs = 1000000 src = 'src/v0.8/llo-feeds' test = 'src/v0.8/llo-feeds/test' solc_version = '0.8.16' -# We cannot turn on deny_warnings = true as that will -# hide any CI failure +# We cannot turn on deny_warnings = true as that will hide any CI failure [profile.shared] optimizer_runs = 1000000 diff --git a/contracts/package.json b/contracts/package.json index b20b653c305..6b5b57df8d9 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -18,7 +18,7 @@ "prepublishOnly": "pnpm compile && ./scripts/prepublish_generate_abi_folder", "publish-beta": "pnpm publish --tag beta", "publish-prod": "npm dist-tag add @chainlink/contracts@0.8.0 latest", - "solhint": "solhint --max-warnings 593 \"./src/v0.8/**/*.sol\"" + "solhint": "solhint --max-warnings 502 \"./src/v0.8/**/*.sol\"" }, "files": [ "src/v0.8", diff --git a/contracts/src/v0.8/l2ep/dev/CrossDomainDelegateForwarder.sol b/contracts/src/v0.8/l2ep/dev/CrossDomainDelegateForwarder.sol index a4c912b8ef0..1eb6cba932d 100644 --- a/contracts/src/v0.8/l2ep/dev/CrossDomainDelegateForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/CrossDomainDelegateForwarder.sol @@ -1,9 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "./CrossDomainForwarder.sol"; -import "./interfaces/ForwarderInterface.sol"; -import "./interfaces/DelegateForwarderInterface.sol"; +import {CrossDomainOwnable} from "./CrossDomainOwnable.sol"; +import {DelegateForwarderInterface} from "./interfaces/DelegateForwarderInterface.sol"; /** * @title CrossDomainDelegateForwarder - L1 xDomain account representation (with delegatecall support) diff --git a/contracts/src/v0.8/l2ep/dev/CrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/CrossDomainForwarder.sol index 4377c2cc0d7..1f9066c5054 100644 --- a/contracts/src/v0.8/l2ep/dev/CrossDomainForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/CrossDomainForwarder.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "./CrossDomainOwnable.sol"; -import "./interfaces/ForwarderInterface.sol"; +import {CrossDomainOwnable} from "./CrossDomainOwnable.sol"; +import {ForwarderInterface} from "./interfaces/ForwarderInterface.sol"; /** * @title CrossDomainForwarder - L1 xDomain account representation diff --git a/contracts/src/v0.8/l2ep/dev/CrossDomainOwnable.sol b/contracts/src/v0.8/l2ep/dev/CrossDomainOwnable.sol index 966a66372fe..b9a435a7e21 100644 --- a/contracts/src/v0.8/l2ep/dev/CrossDomainOwnable.sol +++ b/contracts/src/v0.8/l2ep/dev/CrossDomainOwnable.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "../../shared/access/ConfirmedOwner.sol"; -import "./interfaces/CrossDomainOwnableInterface.sol"; +import {ConfirmedOwner} from "../../shared/access/ConfirmedOwner.sol"; +import {CrossDomainOwnableInterface} from "./interfaces/CrossDomainOwnableInterface.sol"; /** * @title The CrossDomainOwnable contract @@ -42,6 +42,7 @@ contract CrossDomainOwnable is CrossDomainOwnableInterface, ConfirmedOwner { * @notice validate, transfer ownership, and emit relevant events */ function _transferL1Ownership(address to) internal { + // solhint-disable-next-line custom-errors require(to != msg.sender, "Cannot transfer to self"); s_l1PendingOwner = to; @@ -64,6 +65,7 @@ contract CrossDomainOwnable is CrossDomainOwnableInterface, ConfirmedOwner { * @notice Reverts if called by anyone other than the L1 owner. */ modifier onlyL1Owner() virtual { + // solhint-disable-next-line custom-errors require(msg.sender == s_l1Owner, "Only callable by L1 owner"); _; } @@ -72,6 +74,7 @@ contract CrossDomainOwnable is CrossDomainOwnableInterface, ConfirmedOwner { * @notice Reverts if called by anyone other than the L1 owner. */ modifier onlyProposedL1Owner() virtual { + // solhint-disable-next-line custom-errors require(msg.sender == s_l1PendingOwner, "Only callable by proposed L1 owner"); _; } diff --git a/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumCrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumCrossDomainForwarder.sol index 4f4a197d809..cdab6d49612 100644 --- a/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumCrossDomainForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumCrossDomainForwarder.sol @@ -1,10 +1,15 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "../../../interfaces/TypeAndVersionInterface.sol"; -import "../../../vendor/arb-bridge-eth/v0.8.0-custom/contracts/libraries/AddressAliasHelper.sol"; -import "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; -import "../CrossDomainForwarder.sol"; +import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; +// solhint-disable-next-line no-unused-import +import {ForwarderInterface} from "../interfaces/ForwarderInterface.sol"; + +import {CrossDomainForwarder} from "../CrossDomainForwarder.sol"; +import {CrossDomainOwnable} from "../CrossDomainOwnable.sol"; + +import {AddressAliasHelper} from "../../../vendor/arb-bridge-eth/v0.8.0-custom/contracts/libraries/AddressAliasHelper.sol"; +import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; /** * @title ArbitrumCrossDomainForwarder - L1 xDomain account representation @@ -51,6 +56,7 @@ contract ArbitrumCrossDomainForwarder is TypeAndVersionInterface, CrossDomainFor * @notice The call MUST come from the L1 owner (via cross-chain message.) Reverts otherwise. */ modifier onlyL1Owner() override { + // solhint-disable-next-line custom-errors require(msg.sender == crossDomainMessenger(), "Sender is not the L2 messenger"); _; } @@ -59,6 +65,7 @@ contract ArbitrumCrossDomainForwarder is TypeAndVersionInterface, CrossDomainFor * @notice The call MUST come from the proposed L1 owner (via cross-chain message.) Reverts otherwise. */ modifier onlyProposedL1Owner() override { + // solhint-disable-next-line custom-errors require(msg.sender == AddressAliasHelper.applyL1ToL2Alias(s_l1PendingOwner), "Must be proposed L1 owner"); _; } diff --git a/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumCrossDomainGovernor.sol b/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumCrossDomainGovernor.sol index 8035d07ad2f..2f1d775e489 100644 --- a/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumCrossDomainGovernor.sol +++ b/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumCrossDomainGovernor.sol @@ -1,10 +1,15 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "../interfaces/DelegateForwarderInterface.sol"; -import "../../../vendor/arb-bridge-eth/v0.8.0-custom/contracts/libraries/AddressAliasHelper.sol"; -import "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; -import "./ArbitrumCrossDomainForwarder.sol"; +// solhint-disable-next-line no-unused-import +import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; +// solhint-disable-next-line no-unused-import +import {ForwarderInterface} from "../interfaces/ForwarderInterface.sol"; +import {DelegateForwarderInterface} from "../interfaces/DelegateForwarderInterface.sol"; + +import {ArbitrumCrossDomainForwarder} from "./ArbitrumCrossDomainForwarder.sol"; + +import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; /** * @title ArbitrumCrossDomainGovernor - L1 xDomain account representation (with delegatecall support) for Arbitrum @@ -51,6 +56,7 @@ contract ArbitrumCrossDomainGovernor is DelegateForwarderInterface, ArbitrumCros * @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise. */ modifier onlyLocalOrCrossDomainOwner() { + // solhint-disable-next-line custom-errors require(msg.sender == crossDomainMessenger() || msg.sender == owner(), "Sender is not the L2 messenger or owner"); _; } diff --git a/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumSequencerUptimeFeed.sol index 5c9b3268c7f..8af16ad7b3e 100644 --- a/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumSequencerUptimeFeed.sol @@ -53,11 +53,15 @@ contract ArbitrumSequencerUptimeFeed is address public constant FLAG_L2_SEQ_OFFLINE = address(bytes20(bytes32(uint256(keccak256("chainlink.flags.arbitrum-seq-offline")) - 1))); + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables uint8 public constant override decimals = 0; + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables string public constant override description = "L2 Sequencer Uptime Status Feed"; + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables uint256 public constant override version = 1; /// @dev Flags contract to raise/lower flags on, during status transitions + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i FlagsInterface public immutable FLAGS; /// @dev L1 address address private s_l1Sender; @@ -70,7 +74,7 @@ contract ArbitrumSequencerUptimeFeed is * @param l1SenderAddress Address of the L1 contract that is permissioned to call this contract */ constructor(address flagsAddress, address l1SenderAddress) { - setL1Sender(l1SenderAddress); + _setL1Sender(l1SenderAddress); FLAGS = FlagsInterface(flagsAddress); } @@ -80,12 +84,12 @@ contract ArbitrumSequencerUptimeFeed is * @dev Mainly used for AggregatorV2V3Interface functions * @param roundId Round ID to check */ - function isValidRound(uint256 roundId) private view returns (bool) { + function _isValidRound(uint256 roundId) private view returns (bool) { return roundId > 0 && roundId <= type(uint80).max && s_feedState.latestRoundId >= roundId; } /// @notice Check that this contract is initialised, otherwise throw - function requireInitialized(uint80 latestRoundId) private pure { + function _requireInitialized(uint80 latestRoundId) private pure { if (latestRoundId == 0) { revert Uninitialized(); } @@ -106,7 +110,7 @@ contract ArbitrumSequencerUptimeFeed is bool currentStatus = FLAGS.getFlag(FLAG_L2_SEQ_OFFLINE); // Initialise roundId == 1 as the first round - recordRound(1, currentStatus, timestamp); + _recordRound(1, currentStatus, timestamp); emit Initialized(); } @@ -133,11 +137,11 @@ contract ArbitrumSequencerUptimeFeed is * @param to new L1 sender that will be allowed to call `updateStatus` on this contract */ function transferL1Sender(address to) external virtual onlyOwner { - setL1Sender(to); + _setL1Sender(to); } /// @notice internal method that stores the L1 sender - function setL1Sender(address to) private { + function _setL1Sender(address to) private { address from = s_l1Sender; if (from != to) { s_l1Sender = to; @@ -159,14 +163,14 @@ contract ArbitrumSequencerUptimeFeed is * * @param status The status flag to convert to an aggregator-compatible answer */ - function getStatusAnswer(bool status) private pure returns (int256) { + function _getStatusAnswer(bool status) private pure returns (int256) { return status ? int256(1) : int256(0); } /** * @notice Raise or lower the flag on the stored Flags contract. */ - function forwardStatusToFlags(bool status) private { + function _forwardStatusToFlags(bool status) private { if (status) { FLAGS.raiseFlag(FLAG_L2_SEQ_OFFLINE); } else { @@ -181,7 +185,7 @@ contract ArbitrumSequencerUptimeFeed is * @param status Sequencer status * @param timestamp Block timestamp of status update */ - function recordRound(uint80 roundId, bool status, uint64 timestamp) private { + function _recordRound(uint80 roundId, bool status, uint64 timestamp) private { Round memory nextRound = Round(status, timestamp); FeedState memory feedState = FeedState(roundId, status, timestamp); @@ -189,7 +193,7 @@ contract ArbitrumSequencerUptimeFeed is s_feedState = feedState; emit NewRound(roundId, msg.sender, timestamp); - emit AnswerUpdated(getStatusAnswer(status), roundId, timestamp); + emit AnswerUpdated(_getStatusAnswer(status), roundId, timestamp); } /** @@ -201,7 +205,7 @@ contract ArbitrumSequencerUptimeFeed is */ function updateStatus(bool status, uint64 timestamp) external override { FeedState memory feedState = s_feedState; - requireInitialized(feedState.latestRoundId); + _requireInitialized(feedState.latestRoundId); if (msg.sender != aliasedL1MessageSender()) { revert InvalidSender(); } @@ -214,37 +218,37 @@ contract ArbitrumSequencerUptimeFeed is // Prepare a new round with updated status feedState.latestRoundId += 1; - recordRound(feedState.latestRoundId, status, timestamp); + _recordRound(feedState.latestRoundId, status, timestamp); - forwardStatusToFlags(status); + _forwardStatusToFlags(status); } /// @inheritdoc AggregatorInterface function latestAnswer() external view override checkAccess returns (int256) { FeedState memory feedState = s_feedState; - requireInitialized(feedState.latestRoundId); - return getStatusAnswer(feedState.latestStatus); + _requireInitialized(feedState.latestRoundId); + return _getStatusAnswer(feedState.latestStatus); } /// @inheritdoc AggregatorInterface function latestTimestamp() external view override checkAccess returns (uint256) { FeedState memory feedState = s_feedState; - requireInitialized(feedState.latestRoundId); + _requireInitialized(feedState.latestRoundId); return feedState.latestTimestamp; } /// @inheritdoc AggregatorInterface function latestRound() external view override checkAccess returns (uint256) { FeedState memory feedState = s_feedState; - requireInitialized(feedState.latestRoundId); + _requireInitialized(feedState.latestRoundId); return feedState.latestRoundId; } /// @inheritdoc AggregatorInterface function getAnswer(uint256 roundId) external view override checkAccess returns (int256) { - requireInitialized(s_feedState.latestRoundId); - if (isValidRound(roundId)) { - return getStatusAnswer(s_rounds[uint80(roundId)].status); + _requireInitialized(s_feedState.latestRoundId); + if (_isValidRound(roundId)) { + return _getStatusAnswer(s_rounds[uint80(roundId)].status); } return 0; @@ -252,8 +256,8 @@ contract ArbitrumSequencerUptimeFeed is /// @inheritdoc AggregatorInterface function getTimestamp(uint256 roundId) external view override checkAccess returns (uint256) { - requireInitialized(s_feedState.latestRoundId); - if (isValidRound(roundId)) { + _requireInitialized(s_feedState.latestRoundId); + if (_isValidRound(roundId)) { return s_rounds[uint80(roundId)].timestamp; } @@ -270,11 +274,11 @@ contract ArbitrumSequencerUptimeFeed is checkAccess returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) { - requireInitialized(s_feedState.latestRoundId); + _requireInitialized(s_feedState.latestRoundId); - if (isValidRound(_roundId)) { + if (_isValidRound(_roundId)) { Round memory round = s_rounds[_roundId]; - answer = getStatusAnswer(round.status); + answer = _getStatusAnswer(round.status); startedAt = uint256(round.timestamp); } else { answer = 0; @@ -294,10 +298,10 @@ contract ArbitrumSequencerUptimeFeed is returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) { FeedState memory feedState = s_feedState; - requireInitialized(feedState.latestRoundId); + _requireInitialized(feedState.latestRoundId); roundId = feedState.latestRoundId; - answer = getStatusAnswer(feedState.latestStatus); + answer = _getStatusAnswer(feedState.latestStatus); startedAt = feedState.latestTimestamp; updatedAt = startedAt; answeredInRound = roundId; diff --git a/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumValidator.sol b/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumValidator.sol index c882a09254a..3b5fd277e56 100644 --- a/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumValidator.sol +++ b/contracts/src/v0.8/l2ep/dev/arbitrum/ArbitrumValidator.sol @@ -1,19 +1,17 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "../../../interfaces/AggregatorValidatorInterface.sol"; -import "../../../interfaces/TypeAndVersionInterface.sol"; -import "../../../shared/interfaces/AccessControllerInterface.sol"; -import "../../../interfaces/AggregatorV3Interface.sol"; -import "../../../shared/access/SimpleWriteAccessController.sol"; +import {AggregatorValidatorInterface} from "../../../interfaces/AggregatorValidatorInterface.sol"; +import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; +import {AccessControllerInterface} from "../../../shared/interfaces/AccessControllerInterface.sol"; +import {SimpleWriteAccessController} from "../../../shared/access/SimpleWriteAccessController.sol"; /* ./dev dependencies - to be moved from ./dev after audit */ -import "../interfaces/ArbitrumSequencerUptimeFeedInterface.sol"; -import "../../../dev/interfaces/FlagsInterface.sol"; -import "../interfaces/IArbitrumDelayedInbox.sol"; -import "../../../vendor/arb-bridge-eth/v0.8.0-custom/contracts/libraries/AddressAliasHelper.sol"; -import "../../../vendor/@arbitrum/nitro-contracts/src/precompiles/ArbSys.sol"; -import "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; +import {ArbitrumSequencerUptimeFeedInterface} from "../interfaces/ArbitrumSequencerUptimeFeedInterface.sol"; +import {IArbitrumDelayedInbox} from "../interfaces/IArbitrumDelayedInbox.sol"; +import {AddressAliasHelper} from "../../../vendor/arb-bridge-eth/v0.8.0-custom/contracts/libraries/AddressAliasHelper.sol"; +import {ArbSys} from "../../../vendor/@arbitrum/nitro-contracts/src/precompiles/ArbSys.sol"; +import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; /** * @title ArbitrumValidator - makes xDomain L2 Flags contract call (using L2 xDomain Forwarder contract) @@ -36,14 +34,17 @@ contract ArbitrumValidator is TypeAndVersionInterface, AggregatorValidatorInterf } /// @dev Precompiled contract that exists in every Arbitrum chain at address(100). Exposes a variety of system-level functionality. - address constant ARBSYS_ADDR = address(0x0000000000000000000000000000000000000064); + address internal constant ARBSYS_ADDR = address(0x0000000000000000000000000000000000000064); int256 private constant ANSWER_SEQ_OFFLINE = 1; /// @notice The address of Arbitrum's DelayedInbox + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i address public immutable CROSS_DOMAIN_MESSENGER; + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i address public immutable L2_SEQ_STATUS_RECORDER; // L2 xDomain alias address of this contract + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i address public immutable L2_ALIAS = AddressAliasHelper.applyL1ToL2Alias(address(this)); PaymentStrategy private s_paymentStrategy; @@ -85,7 +86,7 @@ contract ArbitrumValidator is TypeAndVersionInterface, AggregatorValidatorInterf * @param maxGas gas limit for immediate L2 execution attempt. A value around 1M should be sufficient * @param gasPriceBid maximum L2 gas price to pay * @param gasPriceL1FeedAddr address of the L1 gas price feed (used to approximate Arbitrum retryable ticket submission cost) - * @param paymentStrategy strategy describing how the contract pays for xDomain calls + * @param _paymentStrategy strategy describing how the contract pays for xDomain calls */ constructor( address crossDomainMessengerAddr, @@ -95,16 +96,18 @@ contract ArbitrumValidator is TypeAndVersionInterface, AggregatorValidatorInterf uint256 gasPriceBid, uint256 baseFee, address gasPriceL1FeedAddr, - PaymentStrategy paymentStrategy + PaymentStrategy _paymentStrategy ) { + // solhint-disable-next-line custom-errors require(crossDomainMessengerAddr != address(0), "Invalid xDomain Messenger address"); + // solhint-disable-next-line custom-errors require(l2ArbitrumSequencerUptimeFeedAddr != address(0), "Invalid ArbitrumSequencerUptimeFeed contract address"); CROSS_DOMAIN_MESSENGER = crossDomainMessengerAddr; L2_SEQ_STATUS_RECORDER = l2ArbitrumSequencerUptimeFeedAddr; // Additional L2 payment configuration _setConfigAC(configACAddr); _setGasConfig(maxGas, gasPriceBid, baseFee, gasPriceL1FeedAddr); - _setPaymentStrategy(paymentStrategy); + _setPaymentStrategy(_paymentStrategy); } /** @@ -231,10 +234,10 @@ contract ArbitrumValidator is TypeAndVersionInterface, AggregatorValidatorInterf /** * @notice sets the payment strategy * @dev access control provided by `configAC` - * @param paymentStrategy strategy describing how the contract pays for xDomain calls + * @param _paymentStrategy strategy describing how the contract pays for xDomain calls */ - function setPaymentStrategy(PaymentStrategy paymentStrategy) external onlyOwnerOrConfigAccess { - _setPaymentStrategy(paymentStrategy); + function setPaymentStrategy(PaymentStrategy _paymentStrategy) external onlyOwnerOrConfigAccess { + _setPaymentStrategy(_paymentStrategy); } /** @@ -289,15 +292,18 @@ contract ArbitrumValidator is TypeAndVersionInterface, AggregatorValidatorInterf } /// @notice internal method that stores the payment strategy - function _setPaymentStrategy(PaymentStrategy paymentStrategy) internal { - s_paymentStrategy = paymentStrategy; - emit PaymentStrategySet(paymentStrategy); + function _setPaymentStrategy(PaymentStrategy _paymentStrategy) internal { + s_paymentStrategy = _paymentStrategy; + emit PaymentStrategySet(_paymentStrategy); } /// @notice internal method that stores the gas configuration function _setGasConfig(uint256 maxGas, uint256 gasPriceBid, uint256 baseFee, address gasPriceL1FeedAddr) internal { + // solhint-disable-next-line custom-errors require(maxGas > 0, "Max gas is zero"); + // solhint-disable-next-line custom-errors require(gasPriceBid > 0, "Gas price bid is zero"); + // solhint-disable-next-line custom-errors require(gasPriceL1FeedAddr != address(0), "Gas price Aggregator is zero address"); s_gasConfig = GasConfig(maxGas, gasPriceBid, baseFee, gasPriceL1FeedAddr); emit GasConfigSet(maxGas, gasPriceBid, gasPriceL1FeedAddr); @@ -337,6 +343,7 @@ contract ArbitrumValidator is TypeAndVersionInterface, AggregatorValidatorInterf /// @dev reverts if the caller does not have access to change the configuration modifier onlyOwnerOrConfigAccess() { + // solhint-disable-next-line custom-errors require( msg.sender == owner() || (address(s_configAC) != address(0) && s_configAC.hasAccess(msg.sender, msg.data)), "No access" diff --git a/contracts/src/v0.8/l2ep/dev/interfaces/IArbitrumDelayedInbox.sol b/contracts/src/v0.8/l2ep/dev/interfaces/IArbitrumDelayedInbox.sol index 65e437189a7..e18efd65ad2 100644 --- a/contracts/src/v0.8/l2ep/dev/interfaces/IArbitrumDelayedInbox.sol +++ b/contracts/src/v0.8/l2ep/dev/interfaces/IArbitrumDelayedInbox.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "../../../vendor/arb-bridge-eth/v0.8.0-custom/contracts/bridge/interfaces/IInbox.sol"; +import {IInbox} from "../../../vendor/arb-bridge-eth/v0.8.0-custom/contracts/bridge/interfaces/IInbox.sol"; /** * @notice This interface extends Arbitrum's IInbox interface to include diff --git a/contracts/src/v0.8/l2ep/dev/optimism/OptimismCrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/optimism/OptimismCrossDomainForwarder.sol index 1d7c211bbac..672720156db 100644 --- a/contracts/src/v0.8/l2ep/dev/optimism/OptimismCrossDomainForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/optimism/OptimismCrossDomainForwarder.sol @@ -1,12 +1,16 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "../../../interfaces/TypeAndVersionInterface.sol"; +import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; +// solhint-disable-next-line no-unused-import +import {ForwarderInterface} from "../interfaces/ForwarderInterface.sol"; /* ./dev dependencies - to be moved from ./dev after audit */ -import "../CrossDomainForwarder.sol"; -import "../../../vendor/@eth-optimism/contracts/v0.4.7/contracts/optimistic-ethereum/iOVM/bridge/messaging/iOVM_CrossDomainMessenger.sol"; -import "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; +import {CrossDomainForwarder} from "../CrossDomainForwarder.sol"; +import {CrossDomainOwnable} from "../CrossDomainOwnable.sol"; + +import {iOVM_CrossDomainMessenger} from "../../../vendor/@eth-optimism/contracts/v0.4.7/contracts/optimistic-ethereum/iOVM/bridge/messaging/iOVM_CrossDomainMessenger.sol"; +import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; /** * @title OptimismCrossDomainForwarder - L1 xDomain account representation @@ -16,6 +20,7 @@ import "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol */ contract OptimismCrossDomainForwarder is TypeAndVersionInterface, CrossDomainForwarder { // OVM_L2CrossDomainMessenger is a precompile usually deployed to 0x4200000000000000000000000000000000000007 + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i iOVM_CrossDomainMessenger private immutable OVM_CROSS_DOMAIN_MESSENGER; /** @@ -24,6 +29,7 @@ contract OptimismCrossDomainForwarder is TypeAndVersionInterface, CrossDomainFor * @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn */ constructor(iOVM_CrossDomainMessenger crossDomainMessengerAddr, address l1OwnerAddr) CrossDomainOwnable(l1OwnerAddr) { + // solhint-disable-next-line custom-errors require(address(crossDomainMessengerAddr) != address(0), "Invalid xDomain Messenger address"); OVM_CROSS_DOMAIN_MESSENGER = crossDomainMessengerAddr; } @@ -59,7 +65,9 @@ contract OptimismCrossDomainForwarder is TypeAndVersionInterface, CrossDomainFor * @notice The call MUST come from the L1 owner (via cross-chain message.) Reverts otherwise. */ modifier onlyL1Owner() override { + // solhint-disable-next-line custom-errors require(msg.sender == crossDomainMessenger(), "Sender is not the L2 messenger"); + // solhint-disable-next-line custom-errors require( iOVM_CrossDomainMessenger(crossDomainMessenger()).xDomainMessageSender() == l1Owner(), "xDomain sender is not the L1 owner" @@ -72,7 +80,9 @@ contract OptimismCrossDomainForwarder is TypeAndVersionInterface, CrossDomainFor */ modifier onlyProposedL1Owner() override { address messenger = crossDomainMessenger(); + // solhint-disable-next-line custom-errors require(msg.sender == messenger, "Sender is not the L2 messenger"); + // solhint-disable-next-line custom-errors require( iOVM_CrossDomainMessenger(messenger).xDomainMessageSender() == s_l1PendingOwner, "Must be proposed L1 owner" diff --git a/contracts/src/v0.8/l2ep/dev/optimism/OptimismCrossDomainGovernor.sol b/contracts/src/v0.8/l2ep/dev/optimism/OptimismCrossDomainGovernor.sol index bd1b0b9e885..1f630a3fbde 100644 --- a/contracts/src/v0.8/l2ep/dev/optimism/OptimismCrossDomainGovernor.sol +++ b/contracts/src/v0.8/l2ep/dev/optimism/OptimismCrossDomainGovernor.sol @@ -1,10 +1,14 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "../interfaces/DelegateForwarderInterface.sol"; -import "../../../vendor/@eth-optimism/contracts/v0.4.7/contracts/optimistic-ethereum/iOVM/bridge/messaging/iOVM_CrossDomainMessenger.sol"; -import "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; -import "./OptimismCrossDomainForwarder.sol"; +import {DelegateForwarderInterface} from "../interfaces/DelegateForwarderInterface.sol"; +// solhint-disable-next-line no-unused-import +import {ForwarderInterface} from "../interfaces/ForwarderInterface.sol"; + +import {OptimismCrossDomainForwarder} from "./OptimismCrossDomainForwarder.sol"; + +import {iOVM_CrossDomainMessenger} from "../../../vendor/@eth-optimism/contracts/v0.4.7/contracts/optimistic-ethereum/iOVM/bridge/messaging/iOVM_CrossDomainMessenger.sol"; +import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; /** * @title OptimismCrossDomainGovernor - L1 xDomain account representation (with delegatecall support) for Optimism @@ -28,8 +32,6 @@ contract OptimismCrossDomainGovernor is DelegateForwarderInterface, OptimismCros * @notice versions: * * - OptimismCrossDomainForwarder 1.0.0: initial release - * - * @inheritdoc TypeAndVersionInterface */ function typeAndVersion() external pure virtual override returns (string memory) { return "OptimismCrossDomainGovernor 1.0.0"; @@ -57,9 +59,11 @@ contract OptimismCrossDomainGovernor is DelegateForwarderInterface, OptimismCros modifier onlyLocalOrCrossDomainOwner() { address messenger = crossDomainMessenger(); // 1. The delegatecall MUST come from either the L1 owner (via cross-chain message) or the L2 owner + // solhint-disable-next-line custom-errors require(msg.sender == messenger || msg.sender == owner(), "Sender is not the L2 messenger or owner"); // 2. The L2 Messenger's caller MUST be the L1 Owner if (msg.sender == messenger) { + // solhint-disable-next-line custom-errors require( iOVM_CrossDomainMessenger(messenger).xDomainMessageSender() == l1Owner(), "xDomain sender is not the L1 owner" diff --git a/contracts/src/v0.8/l2ep/dev/optimism/OptimismSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/optimism/OptimismSequencerUptimeFeed.sol index af7ccc90574..b522a600bab 100644 --- a/contracts/src/v0.8/l2ep/dev/optimism/OptimismSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/optimism/OptimismSequencerUptimeFeed.sol @@ -46,8 +46,11 @@ contract OptimismSequencerUptimeFeed is /// @dev Emitted when a updateStatus is called without the status changing event RoundUpdated(int256 status, uint64 updatedAt); + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables uint8 public constant override decimals = 0; + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables string public constant override description = "L2 Sequencer Uptime Status Feed"; + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables uint256 public constant override version = 1; /// @dev L1 address @@ -56,6 +59,7 @@ contract OptimismSequencerUptimeFeed is FeedState private s_feedState = FeedState({latestRoundId: 0, latestStatus: false, startedAt: 0, updatedAt: 0}); mapping(uint80 => Round) private s_rounds; + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i IL2CrossDomainMessenger private immutable s_l2CrossDomainMessenger; /** @@ -64,12 +68,12 @@ contract OptimismSequencerUptimeFeed is * @param initialStatus The initial status of the feed */ constructor(address l1SenderAddress, address l2CrossDomainMessengerAddr, bool initialStatus) { - setL1Sender(l1SenderAddress); + _setL1Sender(l1SenderAddress); s_l2CrossDomainMessenger = IL2CrossDomainMessenger(l2CrossDomainMessengerAddr); uint64 timestamp = uint64(block.timestamp); // Initialise roundId == 1 as the first round - recordRound(1, initialStatus, timestamp); + _recordRound(1, initialStatus, timestamp); } /** @@ -77,7 +81,7 @@ contract OptimismSequencerUptimeFeed is * @dev Mainly used for AggregatorV2V3Interface functions * @param roundId Round ID to check */ - function isValidRound(uint256 roundId) private view returns (bool) { + function _isValidRound(uint256 roundId) private view returns (bool) { return roundId > 0 && roundId <= type(uint80).max && s_feedState.latestRoundId >= roundId; } @@ -103,11 +107,11 @@ contract OptimismSequencerUptimeFeed is * @param to new L1 sender that will be allowed to call `updateStatus` on this contract */ function transferL1Sender(address to) external virtual onlyOwner { - setL1Sender(to); + _setL1Sender(to); } /// @notice internal method that stores the L1 sender - function setL1Sender(address to) private { + function _setL1Sender(address to) private { address from = s_l1Sender; if (from != to) { s_l1Sender = to; @@ -120,7 +124,7 @@ contract OptimismSequencerUptimeFeed is * * @param status The status flag to convert to an aggregator-compatible answer */ - function getStatusAnswer(bool status) private pure returns (int256) { + function _getStatusAnswer(bool status) private pure returns (int256) { return status ? int256(1) : int256(0); } @@ -131,7 +135,7 @@ contract OptimismSequencerUptimeFeed is * @param status Sequencer status * @param timestamp The L1 block timestamp of status update */ - function recordRound(uint80 roundId, bool status, uint64 timestamp) private { + function _recordRound(uint80 roundId, bool status, uint64 timestamp) private { uint64 updatedAt = uint64(block.timestamp); Round memory nextRound = Round(status, timestamp, updatedAt); FeedState memory feedState = FeedState(roundId, status, timestamp, updatedAt); @@ -140,7 +144,7 @@ contract OptimismSequencerUptimeFeed is s_feedState = feedState; emit NewRound(roundId, msg.sender, timestamp); - emit AnswerUpdated(getStatusAnswer(status), roundId, timestamp); + emit AnswerUpdated(_getStatusAnswer(status), roundId, timestamp); } /** @@ -149,11 +153,11 @@ contract OptimismSequencerUptimeFeed is * @param roundId The round ID to update * @param status Sequencer status */ - function updateRound(uint80 roundId, bool status) private { + function _updateRound(uint80 roundId, bool status) private { uint64 updatedAt = uint64(block.timestamp); s_rounds[roundId].updatedAt = updatedAt; s_feedState.updatedAt = updatedAt; - emit RoundUpdated(getStatusAnswer(status), updatedAt); + emit RoundUpdated(_getStatusAnswer(status), updatedAt); } /** @@ -178,17 +182,17 @@ contract OptimismSequencerUptimeFeed is } if (feedState.latestStatus == status) { - updateRound(feedState.latestRoundId, status); + _updateRound(feedState.latestRoundId, status); } else { feedState.latestRoundId += 1; - recordRound(feedState.latestRoundId, status, timestamp); + _recordRound(feedState.latestRoundId, status, timestamp); } } /// @inheritdoc AggregatorInterface function latestAnswer() external view override checkAccess returns (int256) { FeedState memory feedState = s_feedState; - return getStatusAnswer(feedState.latestStatus); + return _getStatusAnswer(feedState.latestStatus); } /// @inheritdoc AggregatorInterface @@ -205,8 +209,8 @@ contract OptimismSequencerUptimeFeed is /// @inheritdoc AggregatorInterface function getAnswer(uint256 roundId) external view override checkAccess returns (int256) { - if (isValidRound(roundId)) { - return getStatusAnswer(s_rounds[uint80(roundId)].status); + if (_isValidRound(roundId)) { + return _getStatusAnswer(s_rounds[uint80(roundId)].status); } revert NoDataPresent(); @@ -214,7 +218,7 @@ contract OptimismSequencerUptimeFeed is /// @inheritdoc AggregatorInterface function getTimestamp(uint256 roundId) external view override checkAccess returns (uint256) { - if (isValidRound(roundId)) { + if (_isValidRound(roundId)) { return s_rounds[uint80(roundId)].startedAt; } @@ -231,9 +235,9 @@ contract OptimismSequencerUptimeFeed is checkAccess returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) { - if (isValidRound(_roundId)) { + if (_isValidRound(_roundId)) { Round memory round = s_rounds[_roundId]; - answer = getStatusAnswer(round.status); + answer = _getStatusAnswer(round.status); startedAt = uint256(round.startedAt); roundId = _roundId; updatedAt = uint256(round.updatedAt); @@ -254,7 +258,7 @@ contract OptimismSequencerUptimeFeed is FeedState memory feedState = s_feedState; roundId = feedState.latestRoundId; - answer = getStatusAnswer(feedState.latestStatus); + answer = _getStatusAnswer(feedState.latestStatus); startedAt = feedState.startedAt; updatedAt = feedState.updatedAt; answeredInRound = roundId; diff --git a/contracts/src/v0.8/l2ep/dev/optimism/OptimismValidator.sol b/contracts/src/v0.8/l2ep/dev/optimism/OptimismValidator.sol index c25b481166f..a955b7e92ca 100644 --- a/contracts/src/v0.8/l2ep/dev/optimism/OptimismValidator.sol +++ b/contracts/src/v0.8/l2ep/dev/optimism/OptimismValidator.sol @@ -1,15 +1,13 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import "../../../interfaces/AggregatorValidatorInterface.sol"; -import "../../../interfaces/TypeAndVersionInterface.sol"; -import "../../../shared/interfaces/AccessControllerInterface.sol"; -import "../../../interfaces/AggregatorV3Interface.sol"; -import "../../../shared/access/SimpleWriteAccessController.sol"; +import {AggregatorValidatorInterface} from "../../../interfaces/AggregatorValidatorInterface.sol"; +import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; +import {OptimismSequencerUptimeFeedInterface} from "./../interfaces/OptimismSequencerUptimeFeedInterface.sol"; -import "./../interfaces/OptimismSequencerUptimeFeedInterface.sol"; -import "@eth-optimism/contracts/L1/messaging/IL1CrossDomainMessenger.sol"; -import "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; +import {SimpleWriteAccessController} from "../../../shared/access/SimpleWriteAccessController.sol"; + +import {IL1CrossDomainMessenger} from "@eth-optimism/contracts/L1/messaging/IL1CrossDomainMessenger.sol"; /** * @title OptimismValidator - makes cross chain call to update the Sequencer Uptime Feed on L2 @@ -18,7 +16,9 @@ contract OptimismValidator is TypeAndVersionInterface, AggregatorValidatorInterf int256 private constant ANSWER_SEQ_OFFLINE = 1; uint32 private s_gasLimit; + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i address public immutable L1_CROSS_DOMAIN_MESSENGER_ADDRESS; + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i address public immutable L2_UPTIME_FEED_ADDR; /** @@ -33,7 +33,9 @@ contract OptimismValidator is TypeAndVersionInterface, AggregatorValidatorInterf * @param gasLimit the gasLimit to use for sending a message from L1 to L2 */ constructor(address l1CrossDomainMessengerAddress, address l2UptimeFeedAddr, uint32 gasLimit) { + // solhint-disable-next-line custom-errors require(l1CrossDomainMessengerAddress != address(0), "Invalid xDomain Messenger address"); + // solhint-disable-next-line custom-errors require(l2UptimeFeedAddr != address(0), "Invalid OptimismSequencerUptimeFeed contract address"); L1_CROSS_DOMAIN_MESSENGER_ADDRESS = l1CrossDomainMessengerAddress; L2_UPTIME_FEED_ADDR = l2UptimeFeedAddr; @@ -73,12 +75,11 @@ contract OptimismValidator is TypeAndVersionInterface, AggregatorValidatorInterf /** * @notice validate method sends an xDomain L2 tx to update Uptime Feed contract on L2. * @dev A message is sent using the L1CrossDomainMessenger. This method is accessed controlled. - * @param previousAnswer previous aggregator answer * @param currentAnswer new aggregator answer - value of 1 considers the sequencer offline. */ function validate( uint256 /* previousRoundId */, - int256 previousAnswer, + int256 /* previousAnswer */, uint256 /* currentRoundId */, int256 currentAnswer ) external override checkAccess returns (bool) {