Skip to content

Commit

Permalink
Fix l2EP solhint (#10932)
Browse files Browse the repository at this point in the history
* fix imports & private funcs

* fix solhint
  • Loading branch information
RensR authored Oct 12, 2023
1 parent 33231a3 commit 0c50f5d
Show file tree
Hide file tree
Showing 15 changed files with 156 additions and 108 deletions.
2 changes: 0 additions & 2 deletions contracts/.solhintignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions contracts/foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion contracts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected] latest",
"solhint": "solhint --max-warnings 593 \"./src/v0.8/**/*.sol\""
"solhint": "solhint --max-warnings 502 \"./src/v0.8/**/*.sol\""
},
"files": [
"src/v0.8",
Expand Down
5 changes: 2 additions & 3 deletions contracts/src/v0.8/l2ep/dev/CrossDomainDelegateForwarder.sol
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/v0.8/l2ep/dev/CrossDomainForwarder.sol
Original file line number Diff line number Diff line change
@@ -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
Expand Down
7 changes: 5 additions & 2 deletions contracts/src/v0.8/l2ep/dev/CrossDomainOwnable.sol
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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");
_;
}
Expand All @@ -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");
_;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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");
_;
}
Expand All @@ -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");
_;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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");
_;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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;
Expand All @@ -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 {
Expand All @@ -181,15 +185,15 @@ 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);

s_rounds[roundId] = nextRound;
s_feedState = feedState;

emit NewRound(roundId, msg.sender, timestamp);
emit AnswerUpdated(getStatusAnswer(status), roundId, timestamp);
emit AnswerUpdated(_getStatusAnswer(status), roundId, timestamp);
}

/**
Expand All @@ -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();
}
Expand All @@ -214,46 +218,46 @@ 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;
}

/// @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;
}

Expand All @@ -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;
Expand All @@ -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;
Expand Down
Loading

0 comments on commit 0c50f5d

Please sign in to comment.