From ab237525d5c2dc1f0182f297f0eceb044f351c16 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Thu, 30 Nov 2023 19:47:58 +0000 Subject: [PATCH 01/16] Adds scroll L2EP contracts and tests --- contracts/package.json | 1 + contracts/pnpm-lock.yaml | 17 +- .../ScrollSequencerUptimeFeedInterface.sol | 6 + .../dev/scroll/ScrollCrossDomainForwarder.sol | 87 ++++ .../dev/scroll/ScrollCrossDomainGovernor.sol | 71 +++ .../dev/scroll/ScrollSequencerUptimeFeed.sol | 270 +++++++++++ .../v0.8/l2ep/dev/scroll/ScrollValidator.sol | 99 ++++ .../MockScrollL1CrossDomainMessenger.sol | 61 +++ .../MockScrollL2CrossDomainMessenger.sol | 55 +++ .../vendor/MockScrollCrossDomainMessenger.sol | 119 +++++ .../dev/ScrollCrossDomainForwarder.test.ts | 259 ++++++++++ .../dev/ScrollCrossDomainGovernor.test.ts | 459 ++++++++++++++++++ .../dev/ScrollSequencerUptimeFeed.test.ts | 426 ++++++++++++++++ .../test/v0.8/dev/ScrollValidator.test.ts | 118 +++++ 14 files changed, 2046 insertions(+), 2 deletions(-) create mode 100644 contracts/src/v0.8/l2ep/dev/interfaces/ScrollSequencerUptimeFeedInterface.sol create mode 100644 contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol create mode 100644 contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol create mode 100644 contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol create mode 100644 contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol create mode 100644 contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol create mode 100644 contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol create mode 100644 contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol create mode 100644 contracts/test/v0.8/dev/ScrollCrossDomainForwarder.test.ts create mode 100644 contracts/test/v0.8/dev/ScrollCrossDomainGovernor.test.ts create mode 100644 contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts create mode 100644 contracts/test/v0.8/dev/ScrollValidator.test.ts diff --git a/contracts/package.json b/contracts/package.json index 1503c822ea4..5932a638979 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -85,6 +85,7 @@ }, "dependencies": { "@eth-optimism/contracts": "0.5.37", + "@scroll-tech/contracts": "0.1.0", "@openzeppelin/contracts": "4.9.3", "@openzeppelin/contracts-upgradeable": "4.9.3" } diff --git a/contracts/pnpm-lock.yaml b/contracts/pnpm-lock.yaml index fbae71fb3d4..a3036c8c8fd 100644 --- a/contracts/pnpm-lock.yaml +++ b/contracts/pnpm-lock.yaml @@ -17,6 +17,9 @@ dependencies: '@openzeppelin/contracts-upgradeable': specifier: 4.9.3 version: 4.9.3 + '@scroll-tech/contracts': + specifier: 0.1.0 + version: 0.1.0 devDependencies: '@ethereum-waffle/mock-contract': @@ -1387,6 +1390,10 @@ packages: - supports-color dev: true + /@scroll-tech/contracts@0.1.0: + resolution: {integrity: sha512-aBbDOc3WB/WveZdpJYcrfvMYMz7ZTEiW8M9XMJLba8p9FAR5KGYB/cV+8+EUsq3MKt7C1BfR+WnXoTVdvwIY6w==} + dev: false + /@scure/base@1.1.1: resolution: {integrity: sha512-ZxOhsSyxYwLJj3pLZCefNitxsj093tb2vq90mp2txoYeBqbcjDjqFhyM8eUjq/uFm6zJ+mUuqxlS2FkuSY1MTA==} dev: true @@ -5861,7 +5868,7 @@ packages: heap: 0.2.6 level-sublevel: 6.6.4 levelup: 3.1.1 - lodash: 4.17.21 + lodash: 4.17.20 lru-cache: 5.1.1 merkle-patricia-tree: 3.0.0 patch-package: 6.2.2 @@ -7919,6 +7926,12 @@ packages: /minimalistic-crypto-utils@1.0.1: resolution: {integrity: sha512-JIYlbt6g8i5jKfJ3xz7rF0LXmv2TkDxBLUkiBeZ7bAx4GnnNMr8xFpGnOxn6GhTEHx3SjRrZEoU+j04prX1ktg==} + /minimatch@3.0.4: + resolution: {integrity: sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==} + dependencies: + brace-expansion: 1.1.11 + dev: true + /minimatch@3.1.2: resolution: {integrity: sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==} dependencies: @@ -9232,7 +9245,7 @@ packages: resolution: {integrity: sha512-nRCcW9Sj7NuZwa2XvH9co8NPeXUBhZP7CRKJtU+cS6PW9FpCIFoI5ib0NT1ZrbNuPoRy0ylyCaUL8Gih4LSyFg==} engines: {node: '>=0.10.0'} dependencies: - minimatch: 3.1.2 + minimatch: 3.0.4 dev: true /reduce-flatten@2.0.0: diff --git a/contracts/src/v0.8/l2ep/dev/interfaces/ScrollSequencerUptimeFeedInterface.sol b/contracts/src/v0.8/l2ep/dev/interfaces/ScrollSequencerUptimeFeedInterface.sol new file mode 100644 index 00000000000..5b66553c949 --- /dev/null +++ b/contracts/src/v0.8/l2ep/dev/interfaces/ScrollSequencerUptimeFeedInterface.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +interface ScrollSequencerUptimeFeedInterface { + function updateStatus(bool status, uint64 timestamp) external; +} diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol new file mode 100644 index 00000000000..5f26ae0d363 --- /dev/null +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +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} from "../CrossDomainForwarder.sol"; +import {CrossDomainOwnable} from "../CrossDomainOwnable.sol"; + +import {IScrollMessenger} from "@scroll-tech/contracts/libraries/IScrollMessenger.sol"; +import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; + +/** + * @title ScrollCrossDomainForwarder - L1 xDomain account representation + * @notice L2 Contract which receives messages from a specific L1 address and transparently forwards them to the destination. + * @dev Any other L2 contract which uses this contract's address as a privileged position, + * can be considered to be owned by the `l1Owner` + */ +contract ScrollCrossDomainForwarder is TypeAndVersionInterface, CrossDomainForwarder { + // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i + IScrollMessenger private immutable SCROLL_CROSS_DOMAIN_MESSENGER; + + /** + * @notice creates a new Scroll xDomain Forwarder contract + * @param crossDomainMessengerAddr the xDomain bridge messenger (Scroll bridge L2) contract address + * @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn + */ + constructor(IScrollMessenger crossDomainMessengerAddr, address l1OwnerAddr) CrossDomainOwnable(l1OwnerAddr) { + // solhint-disable-next-line custom-errors + require(address(crossDomainMessengerAddr) != address(0), "Invalid xDomain Messenger address"); + SCROLL_CROSS_DOMAIN_MESSENGER = crossDomainMessengerAddr; + } + + /** + * @notice versions: + * + * - ScrollCrossDomainForwarder 1.0.0: initial release + * + * @inheritdoc TypeAndVersionInterface + */ + function typeAndVersion() external pure virtual override returns (string memory) { + return "ScrollCrossDomainForwarder 1.0.0"; + } + + /** + * @dev forwarded only if L2 Messenger calls with `xDomainMessageSender` being the L1 owner address + * @inheritdoc ForwarderInterface + */ + function forward(address target, bytes memory data) external virtual override onlyL1Owner { + Address.functionCall(target, data, "Forwarder call reverted"); + } + + /** + * @notice This is always the address of the Scroll Cross Domain Messenger contract + */ + function crossDomainMessenger() public view returns (address) { + return address(SCROLL_CROSS_DOMAIN_MESSENGER); + } + + /** + * @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( + IScrollMessenger(crossDomainMessenger()).xDomainMessageSender() == l1Owner(), + "xDomain sender is not the L1 owner" + ); + _; + } + + /** + * @notice The call MUST come from the proposed L1 owner (via cross-chain message.) Reverts otherwise. + */ + 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(IScrollMessenger(messenger).xDomainMessageSender() == s_l1PendingOwner, "Must be proposed L1 owner"); + _; + } +} diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol new file mode 100644 index 00000000000..9124bcbcc44 --- /dev/null +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {DelegateForwarderInterface} from "../interfaces/DelegateForwarderInterface.sol"; +// solhint-disable-next-line no-unused-import +import {ForwarderInterface} from "../interfaces/ForwarderInterface.sol"; + +import {ScrollCrossDomainForwarder} from "./ScrollCrossDomainForwarder.sol"; + +import {IScrollMessenger} from "@scroll-tech/contracts/libraries/IScrollMessenger.sol"; +import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; + +/** + * @title ScrollCrossDomainGovernor - L1 xDomain account representation (with delegatecall support) for Scroll + * @notice L2 Contract which receives messages from a specific L1 address and transparently forwards them to the destination. + * @dev Any other L2 contract which uses this contract's address as a privileged position, + * can be considered to be simultaneously owned by the `l1Owner` and L2 `owner` + */ +contract ScrollCrossDomainGovernor is DelegateForwarderInterface, ScrollCrossDomainForwarder { + /** + * @notice creates a new Scroll xDomain Forwarder contract + * @param crossDomainMessengerAddr the xDomain bridge messenger (Scroll bridge L2) contract address + * @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn + * @dev Empty constructor required due to inheriting from abstract contract CrossDomainForwarder + */ + constructor( + IScrollMessenger crossDomainMessengerAddr, + address l1OwnerAddr + ) ScrollCrossDomainForwarder(crossDomainMessengerAddr, l1OwnerAddr) {} + + /** + * @notice versions: + * + * - ScrollCrossDomainGovernor 1.0.0: initial release + */ + function typeAndVersion() external pure virtual override returns (string memory) { + return "ScrollCrossDomainGovernor 1.0.0"; + } + + /** + * @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner + * @inheritdoc ForwarderInterface + */ + function forward(address target, bytes memory data) external override onlyLocalOrCrossDomainOwner { + Address.functionCall(target, data, "Governor call reverted"); + } + + /** + * @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner + * @inheritdoc DelegateForwarderInterface + */ + function forwardDelegate(address target, bytes memory data) external override onlyLocalOrCrossDomainOwner { + Address.functionDelegateCall(target, data, "Governor delegatecall reverted"); + } + + /** + * @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise. + */ + 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(IScrollMessenger(messenger).xDomainMessageSender() == l1Owner(), "xDomain sender is not the L1 owner"); + } + _; + } +} diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol new file mode 100644 index 00000000000..11235eb690b --- /dev/null +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol @@ -0,0 +1,270 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.4; + +import {AggregatorInterface} from "../../../shared/interfaces/AggregatorInterface.sol"; +import {AggregatorV3Interface} from "../../../shared/interfaces/AggregatorV3Interface.sol"; +import {AggregatorV2V3Interface} from "../../../shared/interfaces/AggregatorV2V3Interface.sol"; +import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; +import {ScrollSequencerUptimeFeedInterface} from "../interfaces/ScrollSequencerUptimeFeedInterface.sol"; +import {SimpleReadAccessController} from "../../../shared/access/SimpleReadAccessController.sol"; +import {IL2ScrollMessenger} from "@scroll-tech/contracts/L2/IL2ScrollMessenger.sol"; + +/** + * @title ScrollSequencerUptimeFeed - L2 sequencer uptime status aggregator + * @notice L2 contract that receives status updates, + * and records a new answer if the status changed + */ +contract ScrollSequencerUptimeFeed is + AggregatorV2V3Interface, + ScrollSequencerUptimeFeedInterface, + TypeAndVersionInterface, + SimpleReadAccessController +{ + /// @dev Round info (for uptime history) + struct Round { + bool status; + uint64 startedAt; + uint64 updatedAt; + } + + /// @dev Packed state struct to save sloads + struct FeedState { + uint80 latestRoundId; + bool latestStatus; + uint64 startedAt; + uint64 updatedAt; + } + + /// @notice Sender is not the L2 messenger + error InvalidSender(); + /// @notice Replacement for AggregatorV3Interface "No data present" + error NoDataPresent(); + + event L1SenderTransferred(address indexed from, address indexed to); + /// @dev Emitted when an `updateStatus` call is ignored due to unchanged status or stale timestamp + event UpdateIgnored(bool latestStatus, uint64 latestTimestamp, bool incomingStatus, uint64 incomingTimestamp); + /// @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 + address private s_l1Sender; + /// @dev s_latestRoundId == 0 means this contract is uninitialized. + 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 + IL2ScrollMessenger private immutable s_l2CrossDomainMessenger; + + /** + * @param l1SenderAddress Address of the L1 contract that is permissioned to call this contract + * @param l2CrossDomainMessengerAddr Address of the L2CrossDomainMessenger contract + * @param initialStatus The initial status of the feed + */ + constructor(address l1SenderAddress, address l2CrossDomainMessengerAddr, bool initialStatus) { + _setL1Sender(l1SenderAddress); + s_l2CrossDomainMessenger = IL2ScrollMessenger(l2CrossDomainMessengerAddr); + uint64 timestamp = uint64(block.timestamp); + + // Initialise roundId == 1 as the first round + _recordRound(1, initialStatus, timestamp); + } + + /** + * @notice Check if a roundId is valid in this current contract state + * @dev Mainly used for AggregatorV2V3Interface functions + * @param roundId Round ID to check + */ + function _isValidRound(uint256 roundId) private view returns (bool) { + return roundId > 0 && roundId <= type(uint80).max && s_feedState.latestRoundId >= roundId; + } + + /** + * @notice versions: + * + * - ScrollSequencerUptimeFeed 1.0.0: initial release + * + * @inheritdoc TypeAndVersionInterface + */ + function typeAndVersion() external pure virtual override returns (string memory) { + return "ScrollSequencerUptimeFeed 1.0.0"; + } + + /// @return L1 sender address + function l1Sender() public view virtual returns (address) { + return s_l1Sender; + } + + /** + * @notice Set the allowed L1 sender for this contract to a new L1 sender + * @dev Can be disabled by setting the L1 sender as `address(0)`. Accessible only by owner. + * @param to new L1 sender that will be allowed to call `updateStatus` on this contract + */ + function transferL1Sender(address to) external virtual onlyOwner { + _setL1Sender(to); + } + + /// @notice internal method that stores the L1 sender + function _setL1Sender(address to) private { + address from = s_l1Sender; + if (from != to) { + s_l1Sender = to; + emit L1SenderTransferred(from, to); + } + } + + /** + * @dev Returns an AggregatorV2V3Interface compatible answer from status flag + * + * @param status The status flag to convert to an aggregator-compatible answer + */ + function _getStatusAnswer(bool status) private pure returns (int256) { + return status ? int256(1) : int256(0); + } + + /** + * @notice Helper function to record a round and set the latest feed state. + * + * @param roundId The round ID to record + * @param status Sequencer status + * @param timestamp The L1 block timestamp of status update + */ + 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); + + s_rounds[roundId] = nextRound; + s_feedState = feedState; + + emit NewRound(roundId, msg.sender, timestamp); + emit AnswerUpdated(_getStatusAnswer(status), roundId, timestamp); + } + + /** + * @notice Helper function to update when a round was last updated + * + * @param roundId The round ID to update + * @param status Sequencer status + */ + 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); + } + + /** + * @notice Record a new status and timestamp if it has changed since the last round. + * @dev This function will revert if not called from `l1Sender` via the L1->L2 messenger. + * + * @param status Sequencer status + * @param timestamp Block timestamp of status update + */ + function updateStatus(bool status, uint64 timestamp) external override { + FeedState memory feedState = s_feedState; + if ( + msg.sender != address(s_l2CrossDomainMessenger) || s_l2CrossDomainMessenger.xDomainMessageSender() != s_l1Sender + ) { + revert InvalidSender(); + } + + // Ignore if latest recorded timestamp is newer + if (feedState.startedAt > timestamp) { + emit UpdateIgnored(feedState.latestStatus, feedState.startedAt, status, timestamp); + return; + } + + if (feedState.latestStatus == status) { + _updateRound(feedState.latestRoundId, status); + } else { + feedState.latestRoundId += 1; + _recordRound(feedState.latestRoundId, status, timestamp); + } + } + + /// @inheritdoc AggregatorInterface + function latestAnswer() external view override checkAccess returns (int256) { + FeedState memory feedState = s_feedState; + return _getStatusAnswer(feedState.latestStatus); + } + + /// @inheritdoc AggregatorInterface + function latestTimestamp() external view override checkAccess returns (uint256) { + FeedState memory feedState = s_feedState; + return feedState.startedAt; + } + + /// @inheritdoc AggregatorInterface + function latestRound() external view override checkAccess returns (uint256) { + FeedState memory feedState = s_feedState; + return feedState.latestRoundId; + } + + /// @inheritdoc AggregatorInterface + function getAnswer(uint256 roundId) external view override checkAccess returns (int256) { + if (_isValidRound(roundId)) { + return _getStatusAnswer(s_rounds[uint80(roundId)].status); + } + + revert NoDataPresent(); + } + + /// @inheritdoc AggregatorInterface + function getTimestamp(uint256 roundId) external view override checkAccess returns (uint256) { + if (_isValidRound(roundId)) { + return s_rounds[uint80(roundId)].startedAt; + } + + revert NoDataPresent(); + } + + /// @inheritdoc AggregatorV3Interface + function getRoundData( + uint80 _roundId + ) + public + view + override + checkAccess + returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) + { + if (!_isValidRound(_roundId)) { + revert NoDataPresent(); + } + + Round memory round = s_rounds[_roundId]; + answer = _getStatusAnswer(round.status); + startedAt = uint256(round.startedAt); + roundId = _roundId; + updatedAt = uint256(round.updatedAt); + answeredInRound = roundId; + + return (roundId, answer, startedAt, updatedAt, answeredInRound); + } + + /// @inheritdoc AggregatorV3Interface + function latestRoundData() + external + view + override + checkAccess + returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) + { + FeedState memory feedState = s_feedState; + + roundId = feedState.latestRoundId; + answer = _getStatusAnswer(feedState.latestStatus); + startedAt = feedState.startedAt; + updatedAt = feedState.updatedAt; + answeredInRound = roundId; + + return (roundId, answer, startedAt, updatedAt, answeredInRound); + } +} diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol new file mode 100644 index 00000000000..6fce08000fe --- /dev/null +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {AggregatorValidatorInterface} from "../../../shared/interfaces/AggregatorValidatorInterface.sol"; +import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; +import {ScrollSequencerUptimeFeedInterface} from "../interfaces/ScrollSequencerUptimeFeedInterface.sol"; + +import {SimpleWriteAccessController} from "../../../shared/access/SimpleWriteAccessController.sol"; + +import {IL1ScrollMessenger} from "@scroll-tech/contracts/L1/IL1ScrollMessenger.sol"; + +/** + * @title ScrollValidator - makes cross chain call to update the Sequencer Uptime Feed on L2 + */ +contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterface, SimpleWriteAccessController { + 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; + + /** + * @notice emitted when gas cost to spend on L2 is updated + * @param gasLimit updated gas cost + */ + event GasLimitUpdated(uint32 gasLimit); + + /** + * @param l1CrossDomainMessengerAddress address the L1CrossDomainMessenger contract address + * @param l2UptimeFeedAddr the address of the ScrollSequencerUptimeFeed contract address + * @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 ScrollSequencerUptimeFeed contract address"); + L1_CROSS_DOMAIN_MESSENGER_ADDRESS = l1CrossDomainMessengerAddress; + L2_UPTIME_FEED_ADDR = l2UptimeFeedAddr; + s_gasLimit = gasLimit; + } + + /** + * @notice versions: + * + * - ScrollValidator 1.0.0: initial release + * + * @inheritdoc TypeAndVersionInterface + */ + function typeAndVersion() external pure virtual override returns (string memory) { + return "ScrollValidator 1.0.0"; + } + + /** + * @notice sets the new gas cost to spend when sending cross chain message + * @param gasLimit the updated gas cost + */ + function setGasLimit(uint32 gasLimit) external onlyOwner { + s_gasLimit = gasLimit; + emit GasLimitUpdated(gasLimit); + } + + /** + * @notice fetches the gas cost of sending a cross chain message + */ + function getGasLimit() external view returns (uint32) { + return s_gasLimit; + } + + /** + * @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 currentAnswer new aggregator answer - value of 1 considers the sequencer offline. + */ + function validate( + uint256 /* previousRoundId */, + int256 /* previousAnswer */, + uint256 /* currentRoundId */, + int256 currentAnswer + ) external override checkAccess returns (bool) { + // Encode the ScrollSequencerUptimeFeed call + bytes4 selector = ScrollSequencerUptimeFeedInterface.updateStatus.selector; + bool status = currentAnswer == ANSWER_SEQ_OFFLINE; + uint64 timestamp = uint64(block.timestamp); + // Encode `status` and `timestamp` + bytes memory message = abi.encodeWithSelector(selector, status, timestamp); + // Make the xDomain call + IL1ScrollMessenger(L1_CROSS_DOMAIN_MESSENGER_ADDRESS).sendMessage( + L2_UPTIME_FEED_ADDR, // Target (the address of the account that receives the message) + 0, // The amount of ether passed when calling the target contract. + message, // The content of the message. This is the arbitrary calldata to be executed. + s_gasLimit // Gas limit required to complete the message relay on the corresponding chain. + ); + // return success + return true; + } +} diff --git a/contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol b/contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol new file mode 100644 index 00000000000..aeb2408c348 --- /dev/null +++ b/contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +/* Interface Imports */ +import {IL1ScrollMessenger} from "@scroll-tech/contracts/L1/IL1ScrollMessenger.sol"; + +contract MockScrollL1CrossDomainMessenger is IL1ScrollMessenger { + uint256 private s_nonce; + + function xDomainMessageSender() public pure returns (address) { + return address(0); + } + + function sendMessage( + address _target, + uint256 _value, + bytes calldata _message, + uint256 _gasLimit + ) external payable override { + emit SentMessage(msg.sender, _target, _value, s_nonce, _gasLimit, _message); + s_nonce++; + } + + function sendMessage( + address _target, + uint256 _value, + bytes calldata _message, + uint256 _gasLimit, + address _refundAddress + ) external payable override { + emit SentMessage(msg.sender, _target, _value, s_nonce, _gasLimit, _message); + s_nonce++; + } + + function relayMessageWithProof( + address from, + address to, + uint256 value, + uint256 nonce, + bytes memory message, + L2MessageProof memory proof + ) external override {} + + function replayMessage( + address from, + address to, + uint256 value, + uint256 messageNonce, + bytes memory message, + uint32 newGasLimit, + address refundAddress + ) external payable override {} + + function dropMessage( + address from, + address to, + uint256 value, + uint256 messageNonce, + bytes memory message + ) external override {} +} diff --git a/contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol b/contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol new file mode 100644 index 00000000000..4d1541c6fb2 --- /dev/null +++ b/contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +/* Interface Imports */ +import {IL2ScrollMessenger} from "@scroll-tech/contracts/L2/IL2ScrollMessenger.sol"; + +contract MockScrollL2CrossDomainMessenger is IL2ScrollMessenger { + uint256 private s_nonce; + address private s_sender; + + function xDomainMessageSender() public view returns (address) { + return s_sender; + } + + function sendMessage( + address _target, + uint256 _value, + bytes calldata _message, + uint256 _gasLimit + ) external payable override { + emit SentMessage(msg.sender, _target, _value, s_nonce, _gasLimit, _message); + s_nonce++; + } + + function sendMessage( + address _target, + uint256 _value, + bytes calldata _message, + uint256 _gasLimit, + address _refundAddress + ) external payable override { + emit SentMessage(msg.sender, _target, _value, s_nonce, _gasLimit, _message); + s_nonce++; + } + + function relayMessage( + address from, + address to, + uint256 value, + uint256 nonce, + bytes calldata message + ) external override {} + + /* + * Needed for testing + */ + function setSender(address newSender) external { + s_sender = newSender; + } + + /* + * Needed for testing + */ + receive() external payable {} +} diff --git a/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol b/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol new file mode 100644 index 00000000000..e571823caa6 --- /dev/null +++ b/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol @@ -0,0 +1,119 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.16; + +import "./openzeppelin-solidity/v4.8.3/contracts/utils/Address.sol"; + +/* + * sourced from: https://github.com/scroll-tech/scroll/blob/develop/contracts/src/libraries/IScrollMessenger.sol + */ +interface IScrollMessenger { + /********** + * Events * + **********/ + + /// @notice Emitted when a cross domain message is sent. + /// @param sender The address of the sender who initiates the message. + /// @param target The address of target contract to call. + /// @param value The amount of value passed to the target contract. + /// @param messageNonce The nonce of the message. + /// @param gasLimit The optional gas limit passed to L1 or L2. + /// @param message The calldata passed to the target contract. + event SentMessage( + address indexed sender, + address indexed target, + uint256 value, + uint256 messageNonce, + uint256 gasLimit, + bytes message + ); + + /// @notice Emitted when a cross domain message is relayed successfully. + /// @param messageHash The hash of the message. + event RelayedMessage(bytes32 indexed messageHash); + + /// @notice Emitted when a cross domain message is failed to relay. + /// @param messageHash The hash of the message. + event FailedRelayedMessage(bytes32 indexed messageHash); + + /************************* + * Public View Functions * + *************************/ + + /// @notice Return the sender of a cross domain message. + function xDomainMessageSender() external view returns (address); + + /***************************** + * Public Mutating Functions * + *****************************/ + + /// @notice Send cross chain message from L1 to L2 or L2 to L1. + /// @param target The address of account who receive the message. + /// @param value The amount of ether passed when call target contract. + /// @param message The content of the message. + /// @param gasLimit Gas limit required to complete the message relay on corresponding chain. + function sendMessage(address target, uint256 value, bytes calldata message, uint256 gasLimit) external payable; + + /// @notice Send cross chain message from L1 to L2 or L2 to L1. + /// @param target The address of account who receive the message. + /// @param value The amount of ether passed when call target contract. + /// @param message The content of the message. + /// @param gasLimit Gas limit required to complete the message relay on corresponding chain. + /// @param refundAddress The address of account who will receive the refunded fee. + function sendMessage( + address target, + uint256 value, + bytes calldata message, + uint256 gasLimit, + address refundAddress + ) external payable; +} + +contract MockScrollCrossDomainMessenger is IScrollMessenger { + address internal mockMessageSender; + + constructor(address sender) { + mockMessageSender = sender; + } + + function xDomainMessageSender() external view override returns (address) { + return mockMessageSender; + } + + function _setMockMessageSender(address sender) external { + mockMessageSender = sender; + } + + /***************************** + * Public Mutating Functions * + *****************************/ + + /// @notice Send cross chain message from L1 to L2 or L2 to L1. + /// @param _target The address of account who receive the message. + /// @param _value The amount of ether passed when call target contract. + /// @param _message The content of the message. + /// @param _gasLimit Gas limit required to complete the message relay on corresponding chain. + function sendMessage( + address _target, + uint256 _value, + bytes calldata _message, + uint256 _gasLimit + ) external payable override { + Address.functionCall(_target, _message, "sendMessage reverted"); + } + + /// @notice Send cross chain message from L1 to L2 or L2 to L1. + /// @param _target The address of account who receive the message. + /// @param _value The amount of ether passed when call target contract. + /// @param _message The content of the message. + /// @param _gasLimit Gas limit required to complete the message relay on corresponding chain. + /// @param _refundAddress The address of account who will receive the refunded fee. + function sendMessage( + address _target, + uint256 _value, + bytes calldata _message, + uint256 _gasLimit, + address _refundAddress + ) external payable override { + Address.functionCall(_target, _message, "sendMessage reverted"); + } +} diff --git a/contracts/test/v0.8/dev/ScrollCrossDomainForwarder.test.ts b/contracts/test/v0.8/dev/ScrollCrossDomainForwarder.test.ts new file mode 100644 index 00000000000..923d41326ae --- /dev/null +++ b/contracts/test/v0.8/dev/ScrollCrossDomainForwarder.test.ts @@ -0,0 +1,259 @@ +import { ethers } from 'hardhat' +import { assert, expect } from 'chai' +import { Contract, ContractFactory } from 'ethers' +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' +import { publicAbi } from '../../test-helpers/helpers' + +let owner: SignerWithAddress +let stranger: SignerWithAddress +let l1OwnerAddress: string +let newL1OwnerAddress: string +let forwarderFactory: ContractFactory +let greeterFactory: ContractFactory +let crossDomainMessengerFactory: ContractFactory +let crossDomainMessenger: Contract +let forwarder: Contract +let greeter: Contract + +before(async () => { + const accounts = await ethers.getSigners() + owner = accounts[0] + stranger = accounts[1] + + // forwarder config + l1OwnerAddress = owner.address + newL1OwnerAddress = stranger.address + + // Contract factories + forwarderFactory = await ethers.getContractFactory( + 'src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol:ScrollCrossDomainForwarder', + owner, + ) + greeterFactory = await ethers.getContractFactory( + 'src/v0.8/tests/Greeter.sol:Greeter', + owner, + ) + crossDomainMessengerFactory = await ethers.getContractFactory( + 'src/v0.8/vendor/MockScrollCrossDomainMessenger.sol:MockScrollCrossDomainMessenger', + ) +}) + +describe('ScrollCrossDomainForwarder', () => { + beforeEach(async () => { + crossDomainMessenger = + await crossDomainMessengerFactory.deploy(l1OwnerAddress) + forwarder = await forwarderFactory.deploy( + crossDomainMessenger.address, + l1OwnerAddress, + ) + greeter = await greeterFactory.deploy(forwarder.address) + }) + + it('has a limited public interface [ @skip-coverage ]', async () => { + publicAbi(forwarder, [ + 'typeAndVersion', + 'crossDomainMessenger', + 'forward', + 'l1Owner', + 'transferL1Ownership', + 'acceptL1Ownership', + // ConfirmedOwner methods: + 'owner', + 'transferOwnership', + 'acceptOwnership', + ]) + }) + + describe('#constructor', () => { + it('should set the owner correctly', async () => { + const response = await forwarder.owner() + assert.equal(response, owner.address) + }) + + it('should set the l1Owner correctly', async () => { + const response = await forwarder.l1Owner() + assert.equal(response, l1OwnerAddress) + }) + + it('should set the crossdomain messenger correctly', async () => { + const response = await forwarder.crossDomainMessenger() + assert.equal(response, crossDomainMessenger.address) + }) + + it('should set the typeAndVersion correctly', async () => { + const response = await forwarder.typeAndVersion() + assert.equal(response, 'ScrollCrossDomainForwarder 1.0.0') + }) + }) + + describe('#forward', () => { + it('should not be callable by unknown address', async () => { + await expect( + forwarder.connect(stranger).forward(greeter.address, '0x'), + ).to.be.revertedWith('Sender is not the L2 messenger') + }) + + it('should be callable by crossdomain messenger address / L1 owner', async () => { + const newGreeting = 'hello' + const setGreetingData = greeterFactory.interface.encodeFunctionData( + 'setGreeting', + [newGreeting], + ) + const forwardData = forwarderFactory.interface.encodeFunctionData( + 'forward', + [greeter.address, setGreetingData], + ) + await crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + forwarder.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ) + + const updatedGreeting = await greeter.greeting() + assert.equal(updatedGreeting, newGreeting) + }) + + it('should revert when contract call reverts', async () => { + const setGreetingData = greeterFactory.interface.encodeFunctionData( + 'setGreeting', + [''], + ) + const forwardData = forwarderFactory.interface.encodeFunctionData( + 'forward', + [greeter.address, setGreetingData], + ) + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + forwarder.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ).to.be.revertedWith('Invalid greeting length') + }) + }) + + describe('#transferL1Ownership', () => { + it('should not be callable by non-owners', async () => { + await expect( + forwarder.connect(stranger).transferL1Ownership(stranger.address), + ).to.be.revertedWith('Sender is not the L2 messenger') + }) + + it('should not be callable by L2 owner', async () => { + const forwarderOwner = await forwarder.owner() + assert.equal(forwarderOwner, owner.address) + + await expect( + forwarder.connect(owner).transferL1Ownership(stranger.address), + ).to.be.revertedWith('Sender is not the L2 messenger') + }) + + it('should be callable by current L1 owner', async () => { + const currentL1Owner = await forwarder.l1Owner() + const forwardData = forwarderFactory.interface.encodeFunctionData( + 'transferL1Ownership', + [newL1OwnerAddress], + ) + + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + forwarder.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ) + .to.emit(forwarder, 'L1OwnershipTransferRequested') + .withArgs(currentL1Owner, newL1OwnerAddress) + }) + + it('should be callable by current L1 owner to zero address', async () => { + const currentL1Owner = await forwarder.l1Owner() + const forwardData = forwarderFactory.interface.encodeFunctionData( + 'transferL1Ownership', + [ethers.constants.AddressZero], + ) + + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + forwarder.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ) + .to.emit(forwarder, 'L1OwnershipTransferRequested') + .withArgs(currentL1Owner, ethers.constants.AddressZero) + }) + }) + + describe('#acceptL1Ownership', () => { + it('should not be callable by non pending-owners', async () => { + const forwardData = forwarderFactory.interface.encodeFunctionData( + 'acceptL1Ownership', + [], + ) + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + forwarder.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ).to.be.revertedWith('Must be proposed L1 owner') + }) + + it('should be callable by pending L1 owner', async () => { + const currentL1Owner = await forwarder.l1Owner() + + // Transfer ownership + const forwardTransferData = forwarderFactory.interface.encodeFunctionData( + 'transferL1Ownership', + [newL1OwnerAddress], + ) + await crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + forwarder.address, // target + 0, // value + forwardTransferData, // message + 0, // gasLimit + ) + + const forwardAcceptData = forwarderFactory.interface.encodeFunctionData( + 'acceptL1Ownership', + [], + ) + // Simulate cross-chain message from another sender + await crossDomainMessenger._setMockMessageSender(newL1OwnerAddress) + + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + forwarder.address, // target + 0, // value + forwardAcceptData, // message + 0, // gasLimit + ), + ) + .to.emit(forwarder, 'L1OwnershipTransferred') + .withArgs(currentL1Owner, newL1OwnerAddress) + + const updatedL1Owner = await forwarder.l1Owner() + assert.equal(updatedL1Owner, newL1OwnerAddress) + }) + }) +}) diff --git a/contracts/test/v0.8/dev/ScrollCrossDomainGovernor.test.ts b/contracts/test/v0.8/dev/ScrollCrossDomainGovernor.test.ts new file mode 100644 index 00000000000..adb78c26248 --- /dev/null +++ b/contracts/test/v0.8/dev/ScrollCrossDomainGovernor.test.ts @@ -0,0 +1,459 @@ +import { ethers } from 'hardhat' +import { assert, expect } from 'chai' +import etherslib, { Contract, ContractFactory } from 'ethers' +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' +import { publicAbi, stripHexPrefix } from '../../test-helpers/helpers' + +let owner: SignerWithAddress +let stranger: SignerWithAddress +let l1OwnerAddress: string +let newL1OwnerAddress: string +let governorFactory: ContractFactory +let greeterFactory: ContractFactory +let multisendFactory: ContractFactory +let crossDomainMessengerFactory: ContractFactory +let crossDomainMessenger: Contract +let governor: Contract +let greeter: Contract +let multisend: Contract + +before(async () => { + const accounts = await ethers.getSigners() + owner = accounts[0] + stranger = accounts[1] + + // governor config + l1OwnerAddress = owner.address + newL1OwnerAddress = stranger.address + + // Contract factories + governorFactory = await ethers.getContractFactory( + 'src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol:ScrollCrossDomainGovernor', + owner, + ) + greeterFactory = await ethers.getContractFactory( + 'src/v0.8/tests/Greeter.sol:Greeter', + owner, + ) + multisendFactory = await ethers.getContractFactory( + 'src/v0.8/vendor/MultiSend.sol:MultiSend', + owner, + ) + crossDomainMessengerFactory = await ethers.getContractFactory( + 'src/v0.8/vendor/MockScrollCrossDomainMessenger.sol:MockScrollCrossDomainMessenger', + ) +}) + +describe('ScrollCrossDomainGovernor', () => { + beforeEach(async () => { + crossDomainMessenger = + await crossDomainMessengerFactory.deploy(l1OwnerAddress) + governor = await governorFactory.deploy( + crossDomainMessenger.address, + l1OwnerAddress, + ) + greeter = await greeterFactory.deploy(governor.address) + multisend = await multisendFactory.deploy() + }) + + it('has a limited public interface [ @skip-coverage ]', async () => { + publicAbi(governor, [ + 'typeAndVersion', + 'crossDomainMessenger', + 'forward', + 'forwardDelegate', + 'l1Owner', + 'transferL1Ownership', + 'acceptL1Ownership', + // ConfirmedOwner methods: + 'owner', + 'transferOwnership', + 'acceptOwnership', + ]) + }) + + describe('#constructor', () => { + it('should set the owner correctly', async () => { + const response = await governor.owner() + assert.equal(response, owner.address) + }) + + it('should set the l1Owner correctly', async () => { + const response = await governor.l1Owner() + assert.equal(response, l1OwnerAddress) + }) + + it('should set the crossdomain messenger correctly', async () => { + const response = await governor.crossDomainMessenger() + assert.equal(response, crossDomainMessenger.address) + }) + + it('should set the typeAndVersion correctly', async () => { + const response = await governor.typeAndVersion() + assert.equal(response, 'ScrollCrossDomainGovernor 1.0.0') + }) + }) + + describe('#forward', () => { + it('should not be callable by unknown address', async () => { + await expect( + governor.connect(stranger).forward(greeter.address, '0x'), + ).to.be.revertedWith('Sender is not the L2 messenger') + }) + + it('should be callable by crossdomain messenger address / L1 owner', async () => { + const newGreeting = 'hello' + const setGreetingData = greeterFactory.interface.encodeFunctionData( + 'setGreeting', + [newGreeting], + ) + const forwardData = governorFactory.interface.encodeFunctionData( + 'forward', + [greeter.address, setGreetingData], + ) + await crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ) + + const updatedGreeting = await greeter.greeting() + assert.equal(updatedGreeting, newGreeting) + }) + + it('should be callable by L2 owner', async () => { + const newGreeting = 'hello' + const setGreetingData = greeterFactory.interface.encodeFunctionData( + 'setGreeting', + [newGreeting], + ) + await governor.connect(owner).forward(greeter.address, setGreetingData) + + const updatedGreeting = await greeter.greeting() + assert.equal(updatedGreeting, newGreeting) + }) + + it('should revert when contract call reverts', async () => { + const setGreetingData = greeterFactory.interface.encodeFunctionData( + 'setGreeting', + [''], + ) + const forwardData = governorFactory.interface.encodeFunctionData( + 'forward', + [greeter.address, setGreetingData], + ) + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ).to.be.revertedWith('Invalid greeting length') + }) + }) + + describe('#forwardDelegate', () => { + it('should not be callable by unknown address', async () => { + await expect( + governor.connect(stranger).forwardDelegate(multisend.address, '0x'), + ).to.be.revertedWith('Sender is not the L2 messenger') + }) + + it('should be callable by crossdomain messenger address / L1 owner', async () => { + const calls = [ + { + to: greeter.address, + data: greeterFactory.interface.encodeFunctionData('setGreeting', [ + 'foo', + ]), + value: 0, + }, + { + to: greeter.address, + data: greeterFactory.interface.encodeFunctionData('setGreeting', [ + 'bar', + ]), + value: 0, + }, + ] + const multisendData = encodeMultisendData(multisend.interface, calls) + const forwardData = governorFactory.interface.encodeFunctionData( + 'forwardDelegate', + [multisend.address, multisendData], + ) + + await crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ) + + const updatedGreeting = await greeter.greeting() + assert.equal(updatedGreeting, 'bar') + }) + + it('should be callable by L2 owner', async () => { + const calls = [ + { + to: greeter.address, + data: greeterFactory.interface.encodeFunctionData('setGreeting', [ + 'foo', + ]), + value: 0, + }, + { + to: greeter.address, + data: greeterFactory.interface.encodeFunctionData('setGreeting', [ + 'bar', + ]), + value: 0, + }, + ] + const multisendData = encodeMultisendData(multisend.interface, calls) + await governor + .connect(owner) + .forwardDelegate(multisend.address, multisendData) + + const updatedGreeting = await greeter.greeting() + assert.equal(updatedGreeting, 'bar') + }) + + it('should revert batch when one call fails', async () => { + const calls = [ + { + to: greeter.address, + data: greeterFactory.interface.encodeFunctionData('setGreeting', [ + 'foo', + ]), + value: 0, + }, + { + to: greeter.address, + data: greeterFactory.interface.encodeFunctionData('setGreeting', [ + '', // should revert + ]), + value: 0, + }, + ] + const multisendData = encodeMultisendData(multisend.interface, calls) + const forwardData = governorFactory.interface.encodeFunctionData( + 'forwardDelegate', + [multisend.address, multisendData], + ) + + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ).to.be.revertedWith('Governor delegatecall reverted') + + const greeting = await greeter.greeting() + assert.equal(greeting, '') // Unchanged + }) + + it('should bubble up revert when contract call reverts', async () => { + const triggerRevertData = + greeterFactory.interface.encodeFunctionData('triggerRevert') + const forwardData = governorFactory.interface.encodeFunctionData( + 'forwardDelegate', + [greeter.address, triggerRevertData], + ) + + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ).to.be.revertedWith('Greeter: revert triggered') + }) + }) + + describe('#transferL1Ownership', () => { + it('should not be callable by non-owners', async () => { + await expect( + governor.connect(stranger).transferL1Ownership(stranger.address), + ).to.be.revertedWith('Sender is not the L2 messenger') + }) + + it('should not be callable by L2 owner', async () => { + const governorOwner = await governor.owner() + assert.equal(governorOwner, owner.address) + + await expect( + governor.connect(owner).transferL1Ownership(stranger.address), + ).to.be.revertedWith('Sender is not the L2 messenger') + }) + + it('should be callable by current L1 owner', async () => { + const currentL1Owner = await governor.l1Owner() + const forwardData = governorFactory.interface.encodeFunctionData( + 'transferL1Ownership', + [newL1OwnerAddress], + ) + + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ) + .to.emit(governor, 'L1OwnershipTransferRequested') + .withArgs(currentL1Owner, newL1OwnerAddress) + }) + + it('should be callable by current L1 owner to zero address', async () => { + const currentL1Owner = await governor.l1Owner() + const forwardData = governorFactory.interface.encodeFunctionData( + 'transferL1Ownership', + [ethers.constants.AddressZero], + ) + + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ) + .to.emit(governor, 'L1OwnershipTransferRequested') + .withArgs(currentL1Owner, ethers.constants.AddressZero) + }) + }) + + describe('#acceptL1Ownership', () => { + it('should not be callable by non pending-owners', async () => { + const forwardData = governorFactory.interface.encodeFunctionData( + 'acceptL1Ownership', + [], + ) + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardData, // message + 0, // gasLimit + ), + ).to.be.revertedWith('Must be proposed L1 owner') + }) + + it('should be callable by pending L1 owner', async () => { + const currentL1Owner = await governor.l1Owner() + + // Transfer ownership + const forwardTransferData = governorFactory.interface.encodeFunctionData( + 'transferL1Ownership', + [newL1OwnerAddress], + ) + await crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardTransferData, // message + 0, // gasLimit + ) + + const forwardAcceptData = governorFactory.interface.encodeFunctionData( + 'acceptL1Ownership', + [], + ) + // Simulate cross-chain message from another sender + await crossDomainMessenger._setMockMessageSender(newL1OwnerAddress) + + await expect( + crossDomainMessenger // Simulate cross-chain message + .connect(stranger) + ['sendMessage(address,uint256,bytes,uint256)']( + governor.address, // target + 0, // value + forwardAcceptData, // message + 0, // gasLimit + ), + ) + .to.emit(governor, 'L1OwnershipTransferred') + .withArgs(currentL1Owner, newL1OwnerAddress) + + const updatedL1Owner = await governor.l1Owner() + assert.equal(updatedL1Owner, newL1OwnerAddress) + }) + }) +}) + +// Multisend contract helpers + +/** + * Encodes an underlying transaction for the Multisend contract + * + * @param operation 0 for CALL, 1 for DELEGATECALL + * @param to tx target address + * @param value tx value + * @param data tx data + */ +export function encodeTxData( + operation: number, + to: string, + value: number, + data: string, +): string { + const dataBuffer = Buffer.from(stripHexPrefix(data), 'hex') + const types = ['uint8', 'address', 'uint256', 'uint256', 'bytes'] + const values = [operation, to, value, dataBuffer.length, dataBuffer] + const encoded = ethers.utils.solidityPack(types, values) + return stripHexPrefix(encoded) +} + +/** + * Encodes a Multisend call + * + * @param MultisendInterface Ethers Interface object of the Multisend contract + * @param transactions one or more transactions to include in the Multisend call + * @param to tx target address + * @param value tx value + * @param data tx data + */ +export function encodeMultisendData( + MultisendInterface: etherslib.utils.Interface, + transactions: { to: string; value: number; data: string }[], +): string { + let nestedTransactionData = '0x' + for (const transaction of transactions) { + nestedTransactionData += encodeTxData( + 0, + transaction.to, + transaction.value, + transaction.data, + ) + } + const encodedMultisendFnData = MultisendInterface.encodeFunctionData( + 'multiSend', + [nestedTransactionData], + ) + return encodedMultisendFnData +} diff --git a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts new file mode 100644 index 00000000000..44aa321b4c1 --- /dev/null +++ b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts @@ -0,0 +1,426 @@ +import { ethers, network } from 'hardhat' +import { BigNumber, Contract } from 'ethers' +import { expect } from 'chai' +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' + +describe('ScrollSequencerUptimeFeed', () => { + let l2CrossDomainMessenger: Contract + let scrollUptimeFeed: Contract + let uptimeFeedConsumer: Contract + let deployer: SignerWithAddress + let l1Owner: SignerWithAddress + let l2Messenger: SignerWithAddress + let dummy: SignerWithAddress + const gasUsedDeviation = 100 + const initialStatus = 0 + + before(async () => { + const accounts = await ethers.getSigners() + deployer = accounts[0] + l1Owner = accounts[1] + dummy = accounts[3] + + const l2CrossDomainMessengerFactory = await ethers.getContractFactory( + 'src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol:MockScrollL2CrossDomainMessenger', + deployer, + ) + + l2CrossDomainMessenger = await l2CrossDomainMessengerFactory.deploy() + + // Pretend we're on L2 + await network.provider.request({ + method: 'hardhat_impersonateAccount', + params: [l2CrossDomainMessenger.address], + }) + l2Messenger = await ethers.getSigner(l2CrossDomainMessenger.address) + // Credit the L2 messenger with some ETH + await dummy.sendTransaction({ + to: l2Messenger.address, + value: ethers.utils.parseEther('10'), + }) + }) + + beforeEach(async () => { + const scrollSequencerStatusRecorderFactory = + await ethers.getContractFactory( + 'src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol:ScrollSequencerUptimeFeed', + deployer, + ) + scrollUptimeFeed = await scrollSequencerStatusRecorderFactory.deploy( + l1Owner.address, + l2CrossDomainMessenger.address, + initialStatus, + ) + + // Set mock sender in mock L2 messenger contract + await l2CrossDomainMessenger.setSender(l1Owner.address) + + // Mock consumer + const statusFeedConsumerFactory = await ethers.getContractFactory( + 'src/v0.8/tests/FeedConsumer.sol:FeedConsumer', + deployer, + ) + uptimeFeedConsumer = await statusFeedConsumerFactory.deploy( + scrollUptimeFeed.address, + ) + }) + + describe('constructor', () => { + it('should have been deployed with the correct initial state', async () => { + const l1Sender = await scrollUptimeFeed.l1Sender() + expect(l1Sender).to.equal(l1Owner.address) + const { roundId, answer } = await scrollUptimeFeed.latestRoundData() + expect(roundId).to.equal(1) + expect(answer).to.equal(initialStatus) + }) + }) + + describe('#updateStatus', () => { + it('should revert if called by an address that is not the L2 Cross Domain Messenger', async () => { + const timestamp = await scrollUptimeFeed.latestTimestamp() + expect( + scrollUptimeFeed.connect(dummy).updateStatus(true, timestamp), + ).to.be.revertedWith('InvalidSender') + }) + + it('should revert if called by an address that is not the L2 Cross Domain Messenger and is not the L1 sender', async () => { + const timestamp = await scrollUptimeFeed.latestTimestamp() + await l2CrossDomainMessenger.setSender(dummy.address) + expect( + scrollUptimeFeed.connect(dummy).updateStatus(true, timestamp), + ).to.be.revertedWith('InvalidSender') + }) + + it(`should update status when status has not changed and incoming timestamp is the same as latest`, async () => { + const timestamp = await scrollUptimeFeed.latestTimestamp() + let tx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(true, timestamp) + await expect(tx) + .to.emit(scrollUptimeFeed, 'AnswerUpdated') + .withArgs(1, 2 /** roundId */, timestamp) + expect(await scrollUptimeFeed.latestAnswer()).to.equal(1) + + const latestRoundBeforeUpdate = await scrollUptimeFeed.latestRoundData() + + tx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(true, timestamp.add(200)) + + // Submit another status update with the same status + const latestBlock = await ethers.provider.getBlock('latest') + + await expect(tx) + .to.emit(scrollUptimeFeed, 'RoundUpdated') + .withArgs(1, latestBlock.timestamp) + expect(await scrollUptimeFeed.latestAnswer()).to.equal(1) + expect(await scrollUptimeFeed.latestTimestamp()).to.equal(timestamp) + + // Verify that latest round has been properly updated + const latestRoundDataAfterUpdate = + await scrollUptimeFeed.latestRoundData() + expect(latestRoundDataAfterUpdate.roundId).to.equal( + latestRoundBeforeUpdate.roundId, + ) + expect(latestRoundDataAfterUpdate.answer).to.equal( + latestRoundBeforeUpdate.answer, + ) + expect(latestRoundDataAfterUpdate.startedAt).to.equal( + latestRoundBeforeUpdate.startedAt, + ) + expect(latestRoundDataAfterUpdate.answeredInRound).to.equal( + latestRoundBeforeUpdate.answeredInRound, + ) + expect(latestRoundDataAfterUpdate.updatedAt).to.equal( + latestBlock.timestamp, + ) + }) + + it(`should update status when status has changed and incoming timestamp is newer than the latest`, async () => { + let timestamp = await scrollUptimeFeed.latestTimestamp() + let tx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(true, timestamp) + await expect(tx) + .to.emit(scrollUptimeFeed, 'AnswerUpdated') + .withArgs(1, 2 /** roundId */, timestamp) + expect(await scrollUptimeFeed.latestAnswer()).to.equal(1) + + // Submit another status update, different status, newer timestamp should update + timestamp = timestamp.add(2000) + tx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(false, timestamp) + await expect(tx) + .to.emit(scrollUptimeFeed, 'AnswerUpdated') + .withArgs(0, 3 /** roundId */, timestamp) + expect(await scrollUptimeFeed.latestAnswer()).to.equal(0) + expect(await scrollUptimeFeed.latestTimestamp()).to.equal(timestamp) + }) + + it(`should update status when status has changed and incoming timestamp is the same as latest`, async () => { + const timestamp = await scrollUptimeFeed.latestTimestamp() + let tx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(true, timestamp) + await expect(tx) + .to.emit(scrollUptimeFeed, 'AnswerUpdated') + .withArgs(1, 2 /** roundId */, timestamp) + expect(await scrollUptimeFeed.latestAnswer()).to.equal(1) + + // Submit another status update, different status, same timestamp should update + tx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(false, timestamp) + await expect(tx) + .to.emit(scrollUptimeFeed, 'AnswerUpdated') + .withArgs(0, 3 /** roundId */, timestamp) + expect(await scrollUptimeFeed.latestAnswer()).to.equal(0) + expect(await scrollUptimeFeed.latestTimestamp()).to.equal(timestamp) + }) + + it('should ignore out-of-order updates', async () => { + const timestamp = (await scrollUptimeFeed.latestTimestamp()).add(10_000) + // Update status + let tx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(true, timestamp) + await expect(tx) + .to.emit(scrollUptimeFeed, 'AnswerUpdated') + .withArgs(1, 2 /** roundId */, timestamp) + expect(await scrollUptimeFeed.latestAnswer()).to.equal(1) + + // Update with different status, but stale timestamp, should be ignored + const staleTimestamp = timestamp.sub(1000) + tx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(false, staleTimestamp) + await expect(tx) + .to.not.emit(scrollUptimeFeed, 'AnswerUpdated') + .withArgs(1, 2 /** roundId */, timestamp) + await expect(tx).to.emit(scrollUptimeFeed, 'UpdateIgnored') + }) + }) + + describe('AggregatorV3Interface', () => { + it('should return valid answer from getRoundData and latestRoundData', async () => { + let [roundId, answer, startedAt, updatedAt, answeredInRound] = + await scrollUptimeFeed.latestRoundData() + expect(roundId).to.equal(1) + expect(answer).to.equal(0) + expect(answeredInRound).to.equal(roundId) + expect(startedAt).to.equal(updatedAt) + + // Submit status update with different status and newer timestamp, should update + const timestamp = (startedAt as BigNumber).add(1000) + await scrollUptimeFeed.connect(l2Messenger).updateStatus(true, timestamp) + ;[roundId, answer, startedAt, updatedAt, answeredInRound] = + await scrollUptimeFeed.getRoundData(2) + expect(roundId).to.equal(2) + expect(answer).to.equal(1) + expect(answeredInRound).to.equal(roundId) + expect(startedAt).to.equal(timestamp) + expect(updatedAt.lte(startedAt)).to.be.true + + // Check that last round is still returning the correct data + ;[roundId, answer, startedAt, updatedAt, answeredInRound] = + await scrollUptimeFeed.getRoundData(1) + expect(roundId).to.equal(1) + expect(answer).to.equal(0) + expect(answeredInRound).to.equal(roundId) + expect(startedAt).to.equal(updatedAt) + + // Assert latestRoundData corresponds to latest round id + expect(await scrollUptimeFeed.getRoundData(2)).to.deep.equal( + await scrollUptimeFeed.latestRoundData(), + ) + }) + + it('should revert from #getRoundData when round does not yet exist (future roundId)', async () => { + expect(scrollUptimeFeed.getRoundData(2)).to.be.revertedWith( + 'NoDataPresent()', + ) + }) + + it('should revert from #getAnswer when round does not yet exist (future roundId)', async () => { + expect(scrollUptimeFeed.getAnswer(2)).to.be.revertedWith( + 'NoDataPresent()', + ) + }) + + it('should revert from #getTimestamp when round does not yet exist (future roundId)', async () => { + expect(scrollUptimeFeed.getTimestamp(2)).to.be.revertedWith( + 'NoDataPresent()', + ) + }) + }) + + describe('Protect reads on AggregatorV2V3Interface functions', () => { + it('should disallow reads on AggregatorV2V3Interface functions when consuming contract is not whitelisted', async () => { + // Sanity - consumer is not whitelisted + expect(await scrollUptimeFeed.checkEnabled()).to.be.true + expect( + await scrollUptimeFeed.hasAccess(uptimeFeedConsumer.address, '0x00'), + ).to.be.false + + // Assert reads are not possible from consuming contract + await expect(uptimeFeedConsumer.latestAnswer()).to.be.revertedWith( + 'No access', + ) + await expect(uptimeFeedConsumer.latestRoundData()).to.be.revertedWith( + 'No access', + ) + }) + + it('should allow reads on AggregatorV2V3Interface functions when consuming contract is whitelisted', async () => { + // Whitelist consumer + await scrollUptimeFeed.addAccess(uptimeFeedConsumer.address) + // Sanity - consumer is whitelisted + expect(await scrollUptimeFeed.checkEnabled()).to.be.true + expect( + await scrollUptimeFeed.hasAccess(uptimeFeedConsumer.address, '0x00'), + ).to.be.true + + // Assert reads are possible from consuming contract + expect(await uptimeFeedConsumer.latestAnswer()).to.be.equal('0') + const [roundId, answer] = await uptimeFeedConsumer.latestRoundData() + expect(roundId).to.equal(1) + expect(answer).to.equal(0) + }) + }) + + describe('Gas costs', () => { + it('should consume a known amount of gas for updates @skip-coverage', async () => { + // Sanity - start at flag = 0 (`false`) + expect(await scrollUptimeFeed.latestAnswer()).to.equal(0) + let timestamp = await scrollUptimeFeed.latestTimestamp() + + // Gas for no update + timestamp = timestamp.add(1000) + const _noUpdateTx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(false, timestamp) + const noUpdateTx = await _noUpdateTx.wait(1) + // Assert no update + expect(await scrollUptimeFeed.latestAnswer()).to.equal(0) + expect(noUpdateTx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 38594, + gasUsedDeviation, + ) + + // Gas for update + timestamp = timestamp.add(1000) + const _updateTx = await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(true, timestamp) + const updateTx = await _updateTx.wait(1) + // Assert update + expect(await scrollUptimeFeed.latestAnswer()).to.equal(1) + expect(updateTx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 60170, + gasUsedDeviation, + ) + }) + + describe('Aggregator interface', () => { + beforeEach(async () => { + const timestamp = (await scrollUptimeFeed.latestTimestamp()).add(1000) + // Initialise a round + await scrollUptimeFeed + .connect(l2Messenger) + .updateStatus(true, timestamp) + }) + + it('should consume a known amount of gas for getRoundData(uint80) @skip-coverage', async () => { + const _tx = await l2Messenger.sendTransaction( + await scrollUptimeFeed + .connect(l2Messenger) + .populateTransaction.getRoundData(1), + ) + const tx = await _tx.wait(1) + expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 30952, + gasUsedDeviation, + ) + }) + + it('should consume a known amount of gas for latestRoundData() @skip-coverage', async () => { + const _tx = await l2Messenger.sendTransaction( + await scrollUptimeFeed + .connect(l2Messenger) + .populateTransaction.latestRoundData(), + ) + const tx = await _tx.wait(1) + expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 28523, + gasUsedDeviation, + ) + }) + + it('should consume a known amount of gas for latestAnswer() @skip-coverage', async () => { + const _tx = await l2Messenger.sendTransaction( + await scrollUptimeFeed + .connect(l2Messenger) + .populateTransaction.latestAnswer(), + ) + const tx = await _tx.wait(1) + expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 28329, + gasUsedDeviation, + ) + }) + + it('should consume a known amount of gas for latestTimestamp() @skip-coverage', async () => { + const _tx = await l2Messenger.sendTransaction( + await scrollUptimeFeed + .connect(l2Messenger) + .populateTransaction.latestTimestamp(), + ) + const tx = await _tx.wait(1) + expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 28229, + gasUsedDeviation, + ) + }) + + it('should consume a known amount of gas for latestRound() @skip-coverage', async () => { + const _tx = await l2Messenger.sendTransaction( + await scrollUptimeFeed + .connect(l2Messenger) + .populateTransaction.latestRound(), + ) + const tx = await _tx.wait(1) + expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 28245, + gasUsedDeviation, + ) + }) + + it('should consume a known amount of gas for getAnswer(roundId) @skip-coverage', async () => { + const _tx = await l2Messenger.sendTransaction( + await scrollUptimeFeed + .connect(l2Messenger) + .populateTransaction.getAnswer(1), + ) + const tx = await _tx.wait(1) + expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 30682, + gasUsedDeviation, + ) + }) + + it('should consume a known amount of gas for getTimestamp(roundId) @skip-coverage', async () => { + const _tx = await l2Messenger.sendTransaction( + await scrollUptimeFeed + .connect(l2Messenger) + .populateTransaction.getTimestamp(1), + ) + const tx = await _tx.wait(1) + expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( + 30570, + gasUsedDeviation, + ) + }) + }) + }) +}) diff --git a/contracts/test/v0.8/dev/ScrollValidator.test.ts b/contracts/test/v0.8/dev/ScrollValidator.test.ts new file mode 100644 index 00000000000..df6c0a30db0 --- /dev/null +++ b/contracts/test/v0.8/dev/ScrollValidator.test.ts @@ -0,0 +1,118 @@ +import { ethers } from 'hardhat' +import { BigNumber, Contract, ContractFactory } from 'ethers' +import { expect } from 'chai' +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' + +describe('ScrollValidator', () => { + const GAS_LIMIT = BigNumber.from(1_900_000) + /** Fake L2 target */ + const L2_SEQ_STATUS_RECORDER_ADDRESS = + '0x491B1dDA0A8fa069bbC1125133A975BF4e85a91b' + let scrollValidator: Contract + let scrollUptimeFeedFactory: ContractFactory + let mockScrollL1CrossDomainMessenger: Contract + let deployer: SignerWithAddress + let eoaValidator: SignerWithAddress + + before(async () => { + const accounts = await ethers.getSigners() + deployer = accounts[0] + eoaValidator = accounts[1] + }) + + beforeEach(async () => { + // Required for building the calldata + scrollUptimeFeedFactory = await ethers.getContractFactory( + 'src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol:ScrollSequencerUptimeFeed', + deployer, + ) + + // Scroll Messenger contract on L1 + const mockScrollL1CrossDomainMessengerFactory = + await ethers.getContractFactory( + 'src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol:MockScrollL1CrossDomainMessenger', + ) + mockScrollL1CrossDomainMessenger = + await mockScrollL1CrossDomainMessengerFactory.deploy() + + // Contract under test + const scrollValidatorFactory = await ethers.getContractFactory( + 'src/v0.8/l2ep/dev/scroll/ScrollValidator.sol:ScrollValidator', + deployer, + ) + + scrollValidator = await scrollValidatorFactory.deploy( + mockScrollL1CrossDomainMessenger.address, + L2_SEQ_STATUS_RECORDER_ADDRESS, + GAS_LIMIT, + ) + }) + + describe('#setGasLimit', () => { + it('correctly updates the gas limit', async () => { + const newGasLimit = BigNumber.from(2_000_000) + const tx = await scrollValidator.setGasLimit(newGasLimit) + await tx.wait() + const currentGasLimit = await scrollValidator.getGasLimit() + expect(currentGasLimit).to.equal(newGasLimit) + }) + }) + + describe('#validate', () => { + it('reverts if called by account with no access', async () => { + await expect( + scrollValidator.connect(eoaValidator).validate(0, 0, 1, 1), + ).to.be.revertedWith('No access') + }) + + it('posts sequencer status when there is not status change', async () => { + await scrollValidator.addAccess(eoaValidator.address) + + const currentBlock = await ethers.provider.getBlock('latest') + const futureTimestamp = currentBlock.timestamp + 5000 + + await ethers.provider.send('evm_setNextBlockTimestamp', [futureTimestamp]) + const sequencerStatusRecorderCallData = + scrollUptimeFeedFactory.interface.encodeFunctionData('updateStatus', [ + false, + futureTimestamp, + ]) + + await expect(scrollValidator.connect(eoaValidator).validate(0, 0, 0, 0)) + .to.emit(mockScrollL1CrossDomainMessenger, 'SentMessage') + .withArgs( + scrollValidator.address, // sender + L2_SEQ_STATUS_RECORDER_ADDRESS, // target + 0, // value + 0, // nonce + GAS_LIMIT, // gas limit + sequencerStatusRecorderCallData, // message + ) + }) + + it('post sequencer offline', async () => { + await scrollValidator.addAccess(eoaValidator.address) + + const currentBlock = await ethers.provider.getBlock('latest') + const futureTimestamp = currentBlock.timestamp + 10000 + + await ethers.provider.send('evm_setNextBlockTimestamp', [futureTimestamp]) + const sequencerStatusRecorderCallData = + scrollUptimeFeedFactory.interface.encodeFunctionData('updateStatus', [ + true, + futureTimestamp, + ]) + + await expect(scrollValidator.connect(eoaValidator).validate(0, 0, 1, 1)) + .to.emit(mockScrollL1CrossDomainMessenger, 'SentMessage') + .withArgs( + scrollValidator.address, // sender + L2_SEQ_STATUS_RECORDER_ADDRESS, // target + 0, // value + 0, // nonce + GAS_LIMIT, // gas limit + sequencerStatusRecorderCallData, // message + ) + }) + }) +}) From 74d7a91ebbfe5ff88dc64b8076396931bcf03b43 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Fri, 1 Dec 2023 17:57:13 +0000 Subject: [PATCH 02/16] fixes comments, adds fixed solidity compiler version, and adds scroll tech contracts as a dev dependency --- contracts/package.json | 2 +- contracts/pnpm-lock.yaml | 8 +- .../ScrollSequencerUptimeFeedInterface.sol | 2 +- .../dev/scroll/ScrollCrossDomainForwarder.sol | 66 +++++------ .../dev/scroll/ScrollCrossDomainGovernor.sol | 58 +++++----- .../dev/scroll/ScrollSequencerUptimeFeed.sol | 105 +++++++++--------- .../v0.8/l2ep/dev/scroll/ScrollValidator.sol | 64 +++++------ .../MockScrollL1CrossDomainMessenger.sol | 1 - .../MockScrollL2CrossDomainMessenger.sol | 1 - .../vendor/MockScrollCrossDomainMessenger.sol | 30 ++--- 10 files changed, 167 insertions(+), 170 deletions(-) diff --git a/contracts/package.json b/contracts/package.json index 5932a638979..0e2fe4503fb 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -43,6 +43,7 @@ "@nomiclabs/hardhat-etherscan": "^3.1.7", "@nomiclabs/hardhat-waffle": "2.0.6", "@openzeppelin/hardhat-upgrades": "1.28.0", + "@scroll-tech/contracts": "0.1.0", "@openzeppelin/test-helpers": "^0.5.16", "@typechain/ethers-v5": "^7.2.0", "@typechain/hardhat": "^7.0.0", @@ -85,7 +86,6 @@ }, "dependencies": { "@eth-optimism/contracts": "0.5.37", - "@scroll-tech/contracts": "0.1.0", "@openzeppelin/contracts": "4.9.3", "@openzeppelin/contracts-upgradeable": "4.9.3" } diff --git a/contracts/pnpm-lock.yaml b/contracts/pnpm-lock.yaml index a3036c8c8fd..dffcb0a7c7c 100644 --- a/contracts/pnpm-lock.yaml +++ b/contracts/pnpm-lock.yaml @@ -17,9 +17,6 @@ dependencies: '@openzeppelin/contracts-upgradeable': specifier: 4.9.3 version: 4.9.3 - '@scroll-tech/contracts': - specifier: 0.1.0 - version: 0.1.0 devDependencies: '@ethereum-waffle/mock-contract': @@ -58,6 +55,9 @@ devDependencies: '@openzeppelin/test-helpers': specifier: ^0.5.16 version: 0.5.16(bn.js@4.12.0) + '@scroll-tech/contracts': + specifier: 0.1.0 + version: 0.1.0 '@typechain/ethers-v5': specifier: ^7.2.0 version: 7.2.0(@ethersproject/abi@5.7.0)(@ethersproject/bytes@5.7.0)(@ethersproject/providers@5.7.2)(ethers@5.7.2)(typechain@8.3.2)(typescript@5.2.2) @@ -1392,7 +1392,7 @@ packages: /@scroll-tech/contracts@0.1.0: resolution: {integrity: sha512-aBbDOc3WB/WveZdpJYcrfvMYMz7ZTEiW8M9XMJLba8p9FAR5KGYB/cV+8+EUsq3MKt7C1BfR+WnXoTVdvwIY6w==} - dev: false + dev: true /@scure/base@1.1.1: resolution: {integrity: sha512-ZxOhsSyxYwLJj3pLZCefNitxsj093tb2vq90mp2txoYeBqbcjDjqFhyM8eUjq/uFm6zJ+mUuqxlS2FkuSY1MTA==} diff --git a/contracts/src/v0.8/l2ep/dev/interfaces/ScrollSequencerUptimeFeedInterface.sol b/contracts/src/v0.8/l2ep/dev/interfaces/ScrollSequencerUptimeFeedInterface.sol index 5b66553c949..f0f716d6f02 100644 --- a/contracts/src/v0.8/l2ep/dev/interfaces/ScrollSequencerUptimeFeedInterface.sol +++ b/contracts/src/v0.8/l2ep/dev/interfaces/ScrollSequencerUptimeFeedInterface.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.19; interface ScrollSequencerUptimeFeedInterface { function updateStatus(bool status, uint64 timestamp) external; diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol index 5f26ae0d363..8e623aea4bc 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol @@ -1,67 +1,67 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.19; 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 */ +// ./dev dependencies - to be moved from ./dev after audit import {CrossDomainForwarder} from "../CrossDomainForwarder.sol"; import {CrossDomainOwnable} from "../CrossDomainOwnable.sol"; import {IScrollMessenger} from "@scroll-tech/contracts/libraries/IScrollMessenger.sol"; import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; -/** - * @title ScrollCrossDomainForwarder - L1 xDomain account representation - * @notice L2 Contract which receives messages from a specific L1 address and transparently forwards them to the destination. - * @dev Any other L2 contract which uses this contract's address as a privileged position, - * can be considered to be owned by the `l1Owner` - */ +/// +/// @title ScrollCrossDomainForwarder - L1 xDomain account representation +/// @notice L2 Contract which receives messages from a specific L1 address and transparently forwards them to the destination. +/// @dev Any other L2 contract which uses this contract's address as a privileged position, +/// can be considered to be owned by the `l1Owner` +/// contract ScrollCrossDomainForwarder is TypeAndVersionInterface, CrossDomainForwarder { // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i IScrollMessenger private immutable SCROLL_CROSS_DOMAIN_MESSENGER; - /** - * @notice creates a new Scroll xDomain Forwarder contract - * @param crossDomainMessengerAddr the xDomain bridge messenger (Scroll bridge L2) contract address - * @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn - */ + /// + /// @notice creates a new Scroll xDomain Forwarder contract + /// @param crossDomainMessengerAddr the xDomain bridge messenger (Scroll bridge L2) contract address + /// @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn + /// constructor(IScrollMessenger crossDomainMessengerAddr, address l1OwnerAddr) CrossDomainOwnable(l1OwnerAddr) { // solhint-disable-next-line custom-errors require(address(crossDomainMessengerAddr) != address(0), "Invalid xDomain Messenger address"); SCROLL_CROSS_DOMAIN_MESSENGER = crossDomainMessengerAddr; } - /** - * @notice versions: - * - * - ScrollCrossDomainForwarder 1.0.0: initial release - * - * @inheritdoc TypeAndVersionInterface - */ + /// + /// @notice versions: + /// + /// - ScrollCrossDomainForwarder 1.0.0: initial release + /// + /// @inheritdoc TypeAndVersionInterface + /// function typeAndVersion() external pure virtual override returns (string memory) { return "ScrollCrossDomainForwarder 1.0.0"; } - /** - * @dev forwarded only if L2 Messenger calls with `xDomainMessageSender` being the L1 owner address - * @inheritdoc ForwarderInterface - */ + /// + /// @dev forwarded only if L2 Messenger calls with `xDomainMessageSender` being the L1 owner address + /// @inheritdoc ForwarderInterface + /// function forward(address target, bytes memory data) external virtual override onlyL1Owner { Address.functionCall(target, data, "Forwarder call reverted"); } - /** - * @notice This is always the address of the Scroll Cross Domain Messenger contract - */ + /// + /// @notice This is always the address of the Scroll Cross Domain Messenger contract + /// function crossDomainMessenger() public view returns (address) { return address(SCROLL_CROSS_DOMAIN_MESSENGER); } - /** - * @notice The call MUST come from the L1 owner (via cross-chain message.) Reverts otherwise. - */ + /// + /// @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"); @@ -73,9 +73,9 @@ contract ScrollCrossDomainForwarder is TypeAndVersionInterface, CrossDomainForwa _; } - /** - * @notice The call MUST come from the proposed L1 owner (via cross-chain message.) Reverts otherwise. - */ + /// + /// @notice The call MUST come from the proposed L1 owner (via cross-chain message.) Reverts otherwise. + /// modifier onlyProposedL1Owner() override { address messenger = crossDomainMessenger(); // solhint-disable-next-line custom-errors diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol index 9124bcbcc44..777dd3ad748 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.19; import {DelegateForwarderInterface} from "../interfaces/DelegateForwarderInterface.sol"; // solhint-disable-next-line no-unused-import @@ -10,52 +10,52 @@ import {ScrollCrossDomainForwarder} from "./ScrollCrossDomainForwarder.sol"; import {IScrollMessenger} from "@scroll-tech/contracts/libraries/IScrollMessenger.sol"; import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; -/** - * @title ScrollCrossDomainGovernor - L1 xDomain account representation (with delegatecall support) for Scroll - * @notice L2 Contract which receives messages from a specific L1 address and transparently forwards them to the destination. - * @dev Any other L2 contract which uses this contract's address as a privileged position, - * can be considered to be simultaneously owned by the `l1Owner` and L2 `owner` - */ +/// +/// @title ScrollCrossDomainGovernor - L1 xDomain account representation (with delegatecall support) for Scroll +/// @notice L2 Contract which receives messages from a specific L1 address and transparently forwards them to the destination. +/// @dev Any other L2 contract which uses this contract's address as a privileged position, +/// can be considered to be simultaneously owned by the `l1Owner` and L2 `owner` +/// contract ScrollCrossDomainGovernor is DelegateForwarderInterface, ScrollCrossDomainForwarder { - /** - * @notice creates a new Scroll xDomain Forwarder contract - * @param crossDomainMessengerAddr the xDomain bridge messenger (Scroll bridge L2) contract address - * @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn - * @dev Empty constructor required due to inheriting from abstract contract CrossDomainForwarder - */ + /// + /// @notice creates a new Scroll xDomain Forwarder contract + /// @param crossDomainMessengerAddr the xDomain bridge messenger (Scroll bridge L2) contract address + /// @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn + /// @dev Empty constructor required due to inheriting from abstract contract CrossDomainForwarder + /// constructor( IScrollMessenger crossDomainMessengerAddr, address l1OwnerAddr ) ScrollCrossDomainForwarder(crossDomainMessengerAddr, l1OwnerAddr) {} - /** - * @notice versions: - * - * - ScrollCrossDomainGovernor 1.0.0: initial release - */ + /// + /// @notice versions: + /// + /// - ScrollCrossDomainGovernor 1.0.0: initial release + /// function typeAndVersion() external pure virtual override returns (string memory) { return "ScrollCrossDomainGovernor 1.0.0"; } - /** - * @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner - * @inheritdoc ForwarderInterface - */ + /// + /// @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner + /// @inheritdoc ForwarderInterface + /// function forward(address target, bytes memory data) external override onlyLocalOrCrossDomainOwner { Address.functionCall(target, data, "Governor call reverted"); } - /** - * @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner - * @inheritdoc DelegateForwarderInterface - */ + /// + /// @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner + /// @inheritdoc DelegateForwarderInterface + /// function forwardDelegate(address target, bytes memory data) external override onlyLocalOrCrossDomainOwner { Address.functionDelegateCall(target, data, "Governor delegatecall reverted"); } - /** - * @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise. - */ + /// + /// @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise. + /// modifier onlyLocalOrCrossDomainOwner() { address messenger = crossDomainMessenger(); // 1. The delegatecall MUST come from either the L1 owner (via cross-chain message) or the L2 owner diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol index 11235eb690b..1452e27aadc 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.4; +pragma solidity 0.8.19; import {AggregatorInterface} from "../../../shared/interfaces/AggregatorInterface.sol"; import {AggregatorV3Interface} from "../../../shared/interfaces/AggregatorV3Interface.sol"; @@ -9,11 +9,10 @@ import {ScrollSequencerUptimeFeedInterface} from "../interfaces/ScrollSequencerU import {SimpleReadAccessController} from "../../../shared/access/SimpleReadAccessController.sol"; import {IL2ScrollMessenger} from "@scroll-tech/contracts/L2/IL2ScrollMessenger.sol"; -/** - * @title ScrollSequencerUptimeFeed - L2 sequencer uptime status aggregator - * @notice L2 contract that receives status updates, - * and records a new answer if the status changed - */ +/// +/// @title ScrollSequencerUptimeFeed - L2 sequencer uptime status aggregator +/// @notice L2 contract that receives status updates, and records a new answer if the status changed +/// contract ScrollSequencerUptimeFeed is AggregatorV2V3Interface, ScrollSequencerUptimeFeedInterface, @@ -62,11 +61,11 @@ contract ScrollSequencerUptimeFeed is // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i IL2ScrollMessenger private immutable s_l2CrossDomainMessenger; - /** - * @param l1SenderAddress Address of the L1 contract that is permissioned to call this contract - * @param l2CrossDomainMessengerAddr Address of the L2CrossDomainMessenger contract - * @param initialStatus The initial status of the feed - */ + /// + /// @param l1SenderAddress Address of the L1 contract that is permissioned to call this contract + /// @param l2CrossDomainMessengerAddr Address of the L2CrossDomainMessenger contract + /// @param initialStatus The initial status of the feed + /// constructor(address l1SenderAddress, address l2CrossDomainMessengerAddr, bool initialStatus) { _setL1Sender(l1SenderAddress); s_l2CrossDomainMessenger = IL2ScrollMessenger(l2CrossDomainMessengerAddr); @@ -76,22 +75,22 @@ contract ScrollSequencerUptimeFeed is _recordRound(1, initialStatus, timestamp); } - /** - * @notice Check if a roundId is valid in this current contract state - * @dev Mainly used for AggregatorV2V3Interface functions - * @param roundId Round ID to check - */ + /// + /// @notice Check if a roundId is valid in this current contract state + /// @dev Mainly used for AggregatorV2V3Interface functions + /// @param roundId Round ID to check + /// function _isValidRound(uint256 roundId) private view returns (bool) { return roundId > 0 && roundId <= type(uint80).max && s_feedState.latestRoundId >= roundId; } - /** - * @notice versions: - * - * - ScrollSequencerUptimeFeed 1.0.0: initial release - * - * @inheritdoc TypeAndVersionInterface - */ + /// + /// @notice versions: + /// + /// - ScrollSequencerUptimeFeed 1.0.0: initial release + /// + /// @inheritdoc TypeAndVersionInterface + /// function typeAndVersion() external pure virtual override returns (string memory) { return "ScrollSequencerUptimeFeed 1.0.0"; } @@ -101,11 +100,11 @@ contract ScrollSequencerUptimeFeed is return s_l1Sender; } - /** - * @notice Set the allowed L1 sender for this contract to a new L1 sender - * @dev Can be disabled by setting the L1 sender as `address(0)`. Accessible only by owner. - * @param to new L1 sender that will be allowed to call `updateStatus` on this contract - */ + /// + /// @notice Set the allowed L1 sender for this contract to a new L1 sender + /// @dev Can be disabled by setting the L1 sender as `address(0)`. Accessible only by owner. + /// @param to new L1 sender that will be allowed to call `updateStatus` on this contract + /// function transferL1Sender(address to) external virtual onlyOwner { _setL1Sender(to); } @@ -119,22 +118,22 @@ contract ScrollSequencerUptimeFeed is } } - /** - * @dev Returns an AggregatorV2V3Interface compatible answer from status flag - * - * @param status The status flag to convert to an aggregator-compatible answer - */ + /// + /// @dev Returns an AggregatorV2V3Interface compatible answer from status flag + /// + /// @param status The status flag to convert to an aggregator-compatible answer + /// function _getStatusAnswer(bool status) private pure returns (int256) { return status ? int256(1) : int256(0); } - /** - * @notice Helper function to record a round and set the latest feed state. - * - * @param roundId The round ID to record - * @param status Sequencer status - * @param timestamp The L1 block timestamp of status update - */ + /// + /// @notice Helper function to record a round and set the latest feed state. + /// + /// @param roundId The round ID to record + /// @param status Sequencer status + /// @param timestamp The L1 block timestamp of status update + /// function _recordRound(uint80 roundId, bool status, uint64 timestamp) private { uint64 updatedAt = uint64(block.timestamp); Round memory nextRound = Round(status, timestamp, updatedAt); @@ -147,12 +146,12 @@ contract ScrollSequencerUptimeFeed is emit AnswerUpdated(_getStatusAnswer(status), roundId, timestamp); } - /** - * @notice Helper function to update when a round was last updated - * - * @param roundId The round ID to update - * @param status Sequencer status - */ + /// + /// @notice Helper function to update when a round was last updated + /// + /// @param roundId The round ID to update + /// @param status Sequencer status + /// function _updateRound(uint80 roundId, bool status) private { uint64 updatedAt = uint64(block.timestamp); s_rounds[roundId].updatedAt = updatedAt; @@ -160,13 +159,13 @@ contract ScrollSequencerUptimeFeed is emit RoundUpdated(_getStatusAnswer(status), updatedAt); } - /** - * @notice Record a new status and timestamp if it has changed since the last round. - * @dev This function will revert if not called from `l1Sender` via the L1->L2 messenger. - * - * @param status Sequencer status - * @param timestamp Block timestamp of status update - */ + /// + /// @notice Record a new status and timestamp if it has changed since the last round. + /// @dev This function will revert if not called from `l1Sender` via the L1->L2 messenger. + /// + /// @param status Sequencer status + /// @param timestamp Block timestamp of status update + /// function updateStatus(bool status, uint64 timestamp) external override { FeedState memory feedState = s_feedState; if ( diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol index 6fce08000fe..ea691072d31 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.19; import {AggregatorValidatorInterface} from "../../../shared/interfaces/AggregatorValidatorInterface.sol"; import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; @@ -9,9 +9,9 @@ import {SimpleWriteAccessController} from "../../../shared/access/SimpleWriteAcc import {IL1ScrollMessenger} from "@scroll-tech/contracts/L1/IL1ScrollMessenger.sol"; -/** - * @title ScrollValidator - makes cross chain call to update the Sequencer Uptime Feed on L2 - */ +/// +/// @title ScrollValidator - makes cross chain call to update the Sequencer Uptime Feed on L2 +/// contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterface, SimpleWriteAccessController { int256 private constant ANSWER_SEQ_OFFLINE = 1; uint32 private s_gasLimit; @@ -21,17 +21,17 @@ contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterfac // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i address public immutable L2_UPTIME_FEED_ADDR; - /** - * @notice emitted when gas cost to spend on L2 is updated - * @param gasLimit updated gas cost - */ + /// + /// @notice emitted when gas cost to spend on L2 is updated + /// @param gasLimit updated gas cost + /// event GasLimitUpdated(uint32 gasLimit); - /** - * @param l1CrossDomainMessengerAddress address the L1CrossDomainMessenger contract address - * @param l2UptimeFeedAddr the address of the ScrollSequencerUptimeFeed contract address - * @param gasLimit the gasLimit to use for sending a message from L1 to L2 - */ + /// + /// @param l1CrossDomainMessengerAddress address the L1CrossDomainMessenger contract address + /// @param l2UptimeFeedAddr the address of the ScrollSequencerUptimeFeed contract address + /// @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"); @@ -42,38 +42,38 @@ contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterfac s_gasLimit = gasLimit; } - /** - * @notice versions: - * - * - ScrollValidator 1.0.0: initial release - * - * @inheritdoc TypeAndVersionInterface - */ + /// + /// @notice versions: + /// + /// - ScrollValidator 1.0.0: initial release + /// + /// @inheritdoc TypeAndVersionInterface + /// function typeAndVersion() external pure virtual override returns (string memory) { return "ScrollValidator 1.0.0"; } - /** - * @notice sets the new gas cost to spend when sending cross chain message - * @param gasLimit the updated gas cost - */ + /// + /// @notice sets the new gas cost to spend when sending cross chain message + /// @param gasLimit the updated gas cost + /// function setGasLimit(uint32 gasLimit) external onlyOwner { s_gasLimit = gasLimit; emit GasLimitUpdated(gasLimit); } - /** - * @notice fetches the gas cost of sending a cross chain message - */ + /// + /// @notice fetches the gas cost of sending a cross chain message + /// function getGasLimit() external view returns (uint32) { return s_gasLimit; } - /** - * @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 currentAnswer new aggregator answer - value of 1 considers the sequencer offline. - */ + /// + /// @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 currentAnswer new aggregator answer - value of 1 considers the sequencer offline. + /// function validate( uint256 /* previousRoundId */, int256 /* previousAnswer */, diff --git a/contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol b/contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol index aeb2408c348..e2137dc3071 100644 --- a/contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol +++ b/contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.9; -/* Interface Imports */ import {IL1ScrollMessenger} from "@scroll-tech/contracts/L1/IL1ScrollMessenger.sol"; contract MockScrollL1CrossDomainMessenger is IL1ScrollMessenger { diff --git a/contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol b/contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol index 4d1541c6fb2..77fb299b97d 100644 --- a/contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol +++ b/contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.9; -/* Interface Imports */ import {IL2ScrollMessenger} from "@scroll-tech/contracts/L2/IL2ScrollMessenger.sol"; contract MockScrollL2CrossDomainMessenger is IL2ScrollMessenger { diff --git a/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol b/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol index e571823caa6..d335300e536 100644 --- a/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol +++ b/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol @@ -3,13 +3,13 @@ pragma solidity ^0.8.16; import "./openzeppelin-solidity/v4.8.3/contracts/utils/Address.sol"; -/* - * sourced from: https://github.com/scroll-tech/scroll/blob/develop/contracts/src/libraries/IScrollMessenger.sol - */ +/// +/// sourced from: https://github.com/scroll-tech/scroll/blob/develop/contracts/src/libraries/IScrollMessenger.sol +/// interface IScrollMessenger { - /********** - * Events * - **********/ + /// ********** + /// * Events * + /// ********** /// @notice Emitted when a cross domain message is sent. /// @param sender The address of the sender who initiates the message. @@ -35,16 +35,16 @@ interface IScrollMessenger { /// @param messageHash The hash of the message. event FailedRelayedMessage(bytes32 indexed messageHash); - /************************* - * Public View Functions * - *************************/ + /// ************************* + /// * Public View Functions * + /// ************************* /// @notice Return the sender of a cross domain message. function xDomainMessageSender() external view returns (address); - /***************************** - * Public Mutating Functions * - *****************************/ + /// ***************************** + /// * Public Mutating Functions * + /// ***************************** /// @notice Send cross chain message from L1 to L2 or L2 to L1. /// @param target The address of account who receive the message. @@ -83,9 +83,9 @@ contract MockScrollCrossDomainMessenger is IScrollMessenger { mockMessageSender = sender; } - /***************************** - * Public Mutating Functions * - *****************************/ + /// ***************************** + /// * Public Mutating Functions * + /// ***************************** /// @notice Send cross chain message from L1 to L2 or L2 to L1. /// @param _target The address of account who receive the message. From 2443b2d96033cc8b212b2e57cde97b6f0f937a18 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Fri, 1 Dec 2023 18:13:15 +0000 Subject: [PATCH 03/16] renames SCROLL_CROSS_DOMAIN_MESSENGER to i_SCROLL_CROSS_DOMAIN_MESSENGER for solhint --- .../v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol index 8e623aea4bc..207ce76def1 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol @@ -19,8 +19,7 @@ import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/ut /// can be considered to be owned by the `l1Owner` /// contract ScrollCrossDomainForwarder is TypeAndVersionInterface, CrossDomainForwarder { - // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i - IScrollMessenger private immutable SCROLL_CROSS_DOMAIN_MESSENGER; + IScrollMessenger private immutable i_SCROLL_CROSS_DOMAIN_MESSENGER; /// /// @notice creates a new Scroll xDomain Forwarder contract @@ -30,7 +29,7 @@ contract ScrollCrossDomainForwarder is TypeAndVersionInterface, CrossDomainForwa constructor(IScrollMessenger crossDomainMessengerAddr, address l1OwnerAddr) CrossDomainOwnable(l1OwnerAddr) { // solhint-disable-next-line custom-errors require(address(crossDomainMessengerAddr) != address(0), "Invalid xDomain Messenger address"); - SCROLL_CROSS_DOMAIN_MESSENGER = crossDomainMessengerAddr; + i_SCROLL_CROSS_DOMAIN_MESSENGER = crossDomainMessengerAddr; } /// @@ -56,7 +55,7 @@ contract ScrollCrossDomainForwarder is TypeAndVersionInterface, CrossDomainForwa /// @notice This is always the address of the Scroll Cross Domain Messenger contract /// function crossDomainMessenger() public view returns (address) { - return address(SCROLL_CROSS_DOMAIN_MESSENGER); + return address(i_SCROLL_CROSS_DOMAIN_MESSENGER); } /// From 6ec4f8112adbd13ea703de99704b579efe24858a Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Fri, 1 Dec 2023 18:15:50 +0000 Subject: [PATCH 04/16] removes unnecessary solhint disable rule --- .../src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol index 207ce76def1..6cb60124aea 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol @@ -2,7 +2,6 @@ pragma solidity 0.8.19; 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 From a6e29b7bb6b44b14b3609cab2732e204070b464c Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Fri, 1 Dec 2023 18:26:54 +0000 Subject: [PATCH 05/16] moves scroll mocks from tests folder to l2ep folder --- .../test/mocks}/MockScrollL1CrossDomainMessenger.sol | 0 .../test/mocks}/MockScrollL2CrossDomainMessenger.sol | 0 contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts | 2 +- contracts/test/v0.8/dev/ScrollValidator.test.ts | 2 +- 4 files changed, 2 insertions(+), 2 deletions(-) rename contracts/src/v0.8/{tests => l2ep/test/mocks}/MockScrollL1CrossDomainMessenger.sol (100%) rename contracts/src/v0.8/{tests => l2ep/test/mocks}/MockScrollL2CrossDomainMessenger.sol (100%) diff --git a/contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol similarity index 100% rename from contracts/src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol rename to contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol diff --git a/contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol similarity index 100% rename from contracts/src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol rename to contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol diff --git a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts index 44aa321b4c1..87c852247f7 100644 --- a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts +++ b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts @@ -21,7 +21,7 @@ describe('ScrollSequencerUptimeFeed', () => { dummy = accounts[3] const l2CrossDomainMessengerFactory = await ethers.getContractFactory( - 'src/v0.8/tests/MockScrollL2CrossDomainMessenger.sol:MockScrollL2CrossDomainMessenger', + 'src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol:MockScrollL2CrossDomainMessenger', deployer, ) diff --git a/contracts/test/v0.8/dev/ScrollValidator.test.ts b/contracts/test/v0.8/dev/ScrollValidator.test.ts index df6c0a30db0..866d52b202f 100644 --- a/contracts/test/v0.8/dev/ScrollValidator.test.ts +++ b/contracts/test/v0.8/dev/ScrollValidator.test.ts @@ -30,7 +30,7 @@ describe('ScrollValidator', () => { // Scroll Messenger contract on L1 const mockScrollL1CrossDomainMessengerFactory = await ethers.getContractFactory( - 'src/v0.8/tests/MockScrollL1CrossDomainMessenger.sol:MockScrollL1CrossDomainMessenger', + 'src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol:MockScrollL1CrossDomainMessenger', ) mockScrollL1CrossDomainMessenger = await mockScrollL1CrossDomainMessengerFactory.deploy() From c26330f11a719241d8fffedd3854e82737edb0d3 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Fri, 1 Dec 2023 23:25:51 +0000 Subject: [PATCH 06/16] resolve solhint errors for scroll --- .../v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol | 1 + .../v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol | 1 + 2 files changed, 2 insertions(+) diff --git a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol index e2137dc3071..c474acedc8d 100644 --- a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol +++ b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol @@ -25,6 +25,7 @@ contract MockScrollL1CrossDomainMessenger is IL1ScrollMessenger { uint256 _value, bytes calldata _message, uint256 _gasLimit, + // solhint-disable-next-line no-unused-vars address _refundAddress ) external payable override { emit SentMessage(msg.sender, _target, _value, s_nonce, _gasLimit, _message); diff --git a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol index 77fb299b97d..711207dd3c3 100644 --- a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol +++ b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol @@ -26,6 +26,7 @@ contract MockScrollL2CrossDomainMessenger is IL2ScrollMessenger { uint256 _value, bytes calldata _message, uint256 _gasLimit, + // solhint-disable-next-line no-unused-vars address _refundAddress ) external payable override { emit SentMessage(msg.sender, _target, _value, s_nonce, _gasLimit, _message); From 7a849cc78226e7368dcde4cf0111357c2b3a0b38 Mon Sep 17 00:00:00 2001 From: Rens Rooimans Date: Mon, 4 Dec 2023 12:47:08 +0100 Subject: [PATCH 07/16] proposed restructure to reduce inheritance --- contracts/foundry.toml | 2 +- .../dev/scroll/ScrollCrossDomainForwarder.sol | 51 ++++--------- .../dev/scroll/ScrollCrossDomainGovernor.sol | 75 +++++++++++-------- .../MockScrollL1CrossDomainMessenger.sol | 3 +- 4 files changed, 60 insertions(+), 71 deletions(-) diff --git a/contracts/foundry.toml b/contracts/foundry.toml index cf27c0f2a8b..003c836b1f3 100644 --- a/contracts/foundry.toml +++ b/contracts/foundry.toml @@ -37,7 +37,7 @@ test = 'src/v0.8/automation/test' optimizer_runs = 1000000 src = 'src/v0.8/l2ep' test = 'src/v0.8/l2ep/test' - +solc_version = '0.8.19' [profile.llo-feeds] optimizer_runs = 1000000 diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol index 6cb60124aea..c37dabf815e 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol @@ -11,75 +11,56 @@ import {CrossDomainOwnable} from "../CrossDomainOwnable.sol"; import {IScrollMessenger} from "@scroll-tech/contracts/libraries/IScrollMessenger.sol"; import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; -/// /// @title ScrollCrossDomainForwarder - L1 xDomain account representation /// @notice L2 Contract which receives messages from a specific L1 address and transparently forwards them to the destination. /// @dev Any other L2 contract which uses this contract's address as a privileged position, -/// can be considered to be owned by the `l1Owner` -/// +/// can be considered to be owned by the `l1Owner` contract ScrollCrossDomainForwarder is TypeAndVersionInterface, CrossDomainForwarder { - IScrollMessenger private immutable i_SCROLL_CROSS_DOMAIN_MESSENGER; + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables + string public constant override typeAndVersion = "ScrollCrossDomainForwarder 1.0.0"; + + address internal immutable i_scrollCrossDomainMessenger; - /// - /// @notice creates a new Scroll xDomain Forwarder contract /// @param crossDomainMessengerAddr the xDomain bridge messenger (Scroll bridge L2) contract address /// @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn - /// constructor(IScrollMessenger crossDomainMessengerAddr, address l1OwnerAddr) CrossDomainOwnable(l1OwnerAddr) { // solhint-disable-next-line custom-errors require(address(crossDomainMessengerAddr) != address(0), "Invalid xDomain Messenger address"); - i_SCROLL_CROSS_DOMAIN_MESSENGER = crossDomainMessengerAddr; - } - - /// - /// @notice versions: - /// - /// - ScrollCrossDomainForwarder 1.0.0: initial release - /// - /// @inheritdoc TypeAndVersionInterface - /// - function typeAndVersion() external pure virtual override returns (string memory) { - return "ScrollCrossDomainForwarder 1.0.0"; + i_scrollCrossDomainMessenger = address(crossDomainMessengerAddr); } - /// /// @dev forwarded only if L2 Messenger calls with `xDomainMessageSender` being the L1 owner address /// @inheritdoc ForwarderInterface - /// - function forward(address target, bytes memory data) external virtual override onlyL1Owner { + function forward(address target, bytes memory data) external override onlyL1Owner { Address.functionCall(target, data, "Forwarder call reverted"); } - /// /// @notice This is always the address of the Scroll Cross Domain Messenger contract - /// - function crossDomainMessenger() public view returns (address) { - return address(i_SCROLL_CROSS_DOMAIN_MESSENGER); + function crossDomainMessenger() external view returns (address) { + return address(i_scrollCrossDomainMessenger); } - /// /// @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"); + require(msg.sender == i_scrollCrossDomainMessenger, "Sender is not the L2 messenger"); // solhint-disable-next-line custom-errors require( - IScrollMessenger(crossDomainMessenger()).xDomainMessageSender() == l1Owner(), + IScrollMessenger(i_scrollCrossDomainMessenger).xDomainMessageSender() == l1Owner(), "xDomain sender is not the L1 owner" ); _; } - /// /// @notice The call MUST come from the proposed L1 owner (via cross-chain message.) Reverts otherwise. - /// modifier onlyProposedL1Owner() override { - address messenger = crossDomainMessenger(); // solhint-disable-next-line custom-errors - require(msg.sender == messenger, "Sender is not the L2 messenger"); + require(msg.sender == i_scrollCrossDomainMessenger, "Sender is not the L2 messenger"); // solhint-disable-next-line custom-errors - require(IScrollMessenger(messenger).xDomainMessageSender() == s_l1PendingOwner, "Must be proposed L1 owner"); + require( + IScrollMessenger(i_scrollCrossDomainMessenger).xDomainMessageSender() == s_l1PendingOwner, + "Must be proposed L1 owner" + ); _; } } diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol index 777dd3ad748..030a98ddd52 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol @@ -1,71 +1,80 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.19; +import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; import {DelegateForwarderInterface} from "../interfaces/DelegateForwarderInterface.sol"; // solhint-disable-next-line no-unused-import import {ForwarderInterface} from "../interfaces/ForwarderInterface.sol"; -import {ScrollCrossDomainForwarder} from "./ScrollCrossDomainForwarder.sol"; +import {CrossDomainForwarder} from "../CrossDomainForwarder.sol"; +import {CrossDomainOwnable} from "../CrossDomainOwnable.sol"; import {IScrollMessenger} from "@scroll-tech/contracts/libraries/IScrollMessenger.sol"; import {Address} from "../../../vendor/openzeppelin-solidity/v4.7.3/contracts/utils/Address.sol"; -/// /// @title ScrollCrossDomainGovernor - L1 xDomain account representation (with delegatecall support) for Scroll /// @notice L2 Contract which receives messages from a specific L1 address and transparently forwards them to the destination. /// @dev Any other L2 contract which uses this contract's address as a privileged position, -/// can be considered to be simultaneously owned by the `l1Owner` and L2 `owner` -/// -contract ScrollCrossDomainGovernor is DelegateForwarderInterface, ScrollCrossDomainForwarder { - /// - /// @notice creates a new Scroll xDomain Forwarder contract +/// can be considered to be simultaneously owned by the `l1Owner` and L2 `owner` +contract ScrollCrossDomainGovernor is DelegateForwarderInterface, TypeAndVersionInterface, CrossDomainForwarder { + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables + string public constant override typeAndVersion = "ScrollCrossDomainGovernor 1.0.0"; + + address internal immutable i_scrollCrossDomainMessenger; + /// @param crossDomainMessengerAddr the xDomain bridge messenger (Scroll bridge L2) contract address /// @param l1OwnerAddr the L1 owner address that will be allowed to call the forward fn - /// @dev Empty constructor required due to inheriting from abstract contract CrossDomainForwarder - /// - constructor( - IScrollMessenger crossDomainMessengerAddr, - address l1OwnerAddr - ) ScrollCrossDomainForwarder(crossDomainMessengerAddr, l1OwnerAddr) {} - - /// - /// @notice versions: - /// - /// - ScrollCrossDomainGovernor 1.0.0: initial release - /// - function typeAndVersion() external pure virtual override returns (string memory) { - return "ScrollCrossDomainGovernor 1.0.0"; + constructor(IScrollMessenger crossDomainMessengerAddr, address l1OwnerAddr) CrossDomainOwnable(l1OwnerAddr) { + // solhint-disable-next-line custom-errors + require(address(crossDomainMessengerAddr) != address(0), "Invalid xDomain Messenger address"); + i_scrollCrossDomainMessenger = address(crossDomainMessengerAddr); } - /// - /// @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner /// @inheritdoc ForwarderInterface - /// + /// @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner function forward(address target, bytes memory data) external override onlyLocalOrCrossDomainOwner { Address.functionCall(target, data, "Governor call reverted"); } - /// - /// @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner /// @inheritdoc DelegateForwarderInterface - /// + /// @dev forwarded only if L2 Messenger calls with `msg.sender` being the L1 owner address, or called by the L2 owner function forwardDelegate(address target, bytes memory data) external override onlyLocalOrCrossDomainOwner { Address.functionDelegateCall(target, data, "Governor delegatecall reverted"); } - /// + /// @notice The address of the Scroll Cross Domain Messenger contract + function crossDomainMessenger() external view returns (address) { + return address(i_scrollCrossDomainMessenger); + } + /// @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise. - /// 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"); + require( + msg.sender == i_scrollCrossDomainMessenger || 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) { + if (msg.sender == i_scrollCrossDomainMessenger) { // solhint-disable-next-line custom-errors - require(IScrollMessenger(messenger).xDomainMessageSender() == l1Owner(), "xDomain sender is not the L1 owner"); + require( + IScrollMessenger(i_scrollCrossDomainMessenger).xDomainMessageSender() == l1Owner(), + "xDomain sender is not the L1 owner" + ); } _; } + + /// @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 == i_scrollCrossDomainMessenger, "Sender is not the L2 messenger"); + // solhint-disable-next-line custom-errors + require( + IScrollMessenger(i_scrollCrossDomainMessenger).xDomainMessageSender() == s_l1PendingOwner, + "Must be proposed L1 owner" + ); + _; + } } diff --git a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol index c474acedc8d..e63847d6557 100644 --- a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol +++ b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL1CrossDomainMessenger.sol @@ -25,8 +25,7 @@ contract MockScrollL1CrossDomainMessenger is IL1ScrollMessenger { uint256 _value, bytes calldata _message, uint256 _gasLimit, - // solhint-disable-next-line no-unused-vars - address _refundAddress + address ) external payable override { emit SentMessage(msg.sender, _target, _value, s_nonce, _gasLimit, _message); s_nonce++; From 1405c41e44ea37e891f23d65fc4b462ff193dc4f Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Mon, 4 Dec 2023 20:08:54 +0000 Subject: [PATCH 08/16] removes extraneous comments from scroll contracts and refactors typeAndVersion --- .../dev/scroll/ScrollSequencerUptimeFeed.sol | 18 +++---------- .../v0.8/l2ep/dev/scroll/ScrollValidator.sol | 25 ++----------------- .../MockScrollL2CrossDomainMessenger.sol | 3 +-- 3 files changed, 6 insertions(+), 40 deletions(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol index 1452e27aadc..7a5548a6045 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol @@ -9,16 +9,17 @@ import {ScrollSequencerUptimeFeedInterface} from "../interfaces/ScrollSequencerU import {SimpleReadAccessController} from "../../../shared/access/SimpleReadAccessController.sol"; import {IL2ScrollMessenger} from "@scroll-tech/contracts/L2/IL2ScrollMessenger.sol"; -/// /// @title ScrollSequencerUptimeFeed - L2 sequencer uptime status aggregator /// @notice L2 contract that receives status updates, and records a new answer if the status changed -/// contract ScrollSequencerUptimeFeed is AggregatorV2V3Interface, ScrollSequencerUptimeFeedInterface, TypeAndVersionInterface, SimpleReadAccessController { + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables + string public constant override typeAndVersion = "ScrollSequencerUptimeFeed 1.0.0"; + /// @dev Round info (for uptime history) struct Round { bool status; @@ -61,11 +62,9 @@ contract ScrollSequencerUptimeFeed is // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i IL2ScrollMessenger private immutable s_l2CrossDomainMessenger; - /// /// @param l1SenderAddress Address of the L1 contract that is permissioned to call this contract /// @param l2CrossDomainMessengerAddr Address of the L2CrossDomainMessenger contract /// @param initialStatus The initial status of the feed - /// constructor(address l1SenderAddress, address l2CrossDomainMessengerAddr, bool initialStatus) { _setL1Sender(l1SenderAddress); s_l2CrossDomainMessenger = IL2ScrollMessenger(l2CrossDomainMessengerAddr); @@ -84,17 +83,6 @@ contract ScrollSequencerUptimeFeed is return roundId > 0 && roundId <= type(uint80).max && s_feedState.latestRoundId >= roundId; } - /// - /// @notice versions: - /// - /// - ScrollSequencerUptimeFeed 1.0.0: initial release - /// - /// @inheritdoc TypeAndVersionInterface - /// - function typeAndVersion() external pure virtual override returns (string memory) { - return "ScrollSequencerUptimeFeed 1.0.0"; - } - /// @return L1 sender address function l1Sender() public view virtual returns (address) { return s_l1Sender; diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol index ea691072d31..e6f82fa2e38 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol @@ -9,10 +9,10 @@ import {SimpleWriteAccessController} from "../../../shared/access/SimpleWriteAcc import {IL1ScrollMessenger} from "@scroll-tech/contracts/L1/IL1ScrollMessenger.sol"; -/// /// @title ScrollValidator - makes cross chain call to update the Sequencer Uptime Feed on L2 -/// contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterface, SimpleWriteAccessController { + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables + string public constant override typeAndVersion = "ScrollValidator 1.0.0"; int256 private constant ANSWER_SEQ_OFFLINE = 1; uint32 private s_gasLimit; @@ -21,17 +21,13 @@ contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterfac // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i address public immutable L2_UPTIME_FEED_ADDR; - /// /// @notice emitted when gas cost to spend on L2 is updated /// @param gasLimit updated gas cost - /// event GasLimitUpdated(uint32 gasLimit); - /// /// @param l1CrossDomainMessengerAddress address the L1CrossDomainMessenger contract address /// @param l2UptimeFeedAddr the address of the ScrollSequencerUptimeFeed contract address /// @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"); @@ -42,38 +38,21 @@ contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterfac s_gasLimit = gasLimit; } - /// - /// @notice versions: - /// - /// - ScrollValidator 1.0.0: initial release - /// - /// @inheritdoc TypeAndVersionInterface - /// - function typeAndVersion() external pure virtual override returns (string memory) { - return "ScrollValidator 1.0.0"; - } - - /// /// @notice sets the new gas cost to spend when sending cross chain message /// @param gasLimit the updated gas cost - /// function setGasLimit(uint32 gasLimit) external onlyOwner { s_gasLimit = gasLimit; emit GasLimitUpdated(gasLimit); } - /// /// @notice fetches the gas cost of sending a cross chain message - /// function getGasLimit() external view returns (uint32) { return s_gasLimit; } - /// /// @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 currentAnswer new aggregator answer - value of 1 considers the sequencer offline. - /// function validate( uint256 /* previousRoundId */, int256 /* previousAnswer */, diff --git a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol index 711207dd3c3..2f507da608a 100644 --- a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol +++ b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol @@ -26,8 +26,7 @@ contract MockScrollL2CrossDomainMessenger is IL2ScrollMessenger { uint256 _value, bytes calldata _message, uint256 _gasLimit, - // solhint-disable-next-line no-unused-vars - address _refundAddress + address ) external payable override { emit SentMessage(msg.sender, _target, _value, s_nonce, _gasLimit, _message); s_nonce++; From 67861738903ddfd7982c8bb9067ee1088874f713 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Mon, 4 Dec 2023 20:31:12 +0000 Subject: [PATCH 09/16] use named parameters in mappings for scroll contracts --- .../src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol index 7a5548a6045..1a6797e4c26 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol @@ -57,7 +57,7 @@ contract ScrollSequencerUptimeFeed is address private s_l1Sender; /// @dev s_latestRoundId == 0 means this contract is uninitialized. FeedState private s_feedState = FeedState({latestRoundId: 0, latestStatus: false, startedAt: 0, updatedAt: 0}); - mapping(uint80 => Round) private s_rounds; + mapping(uint80 roundId => Round round) private s_rounds; // solhint-disable-next-line chainlink-solidity/prefix-immutable-variables-with-i IL2ScrollMessenger private immutable s_l2CrossDomainMessenger; From b6df5fe2462bd2aca296c22cf395b231e9ee91ed Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Mon, 4 Dec 2023 20:36:28 +0000 Subject: [PATCH 10/16] removes extraneous empty comments from scroll contracts (again) --- .../l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol index 1a6797e4c26..569f578caad 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol @@ -74,11 +74,9 @@ contract ScrollSequencerUptimeFeed is _recordRound(1, initialStatus, timestamp); } - /// /// @notice Check if a roundId is valid in this current contract state /// @dev Mainly used for AggregatorV2V3Interface functions /// @param roundId Round ID to check - /// function _isValidRound(uint256 roundId) private view returns (bool) { return roundId > 0 && roundId <= type(uint80).max && s_feedState.latestRoundId >= roundId; } @@ -88,11 +86,9 @@ contract ScrollSequencerUptimeFeed is return s_l1Sender; } - /// /// @notice Set the allowed L1 sender for this contract to a new L1 sender /// @dev Can be disabled by setting the L1 sender as `address(0)`. Accessible only by owner. /// @param to new L1 sender that will be allowed to call `updateStatus` on this contract - /// function transferL1Sender(address to) external virtual onlyOwner { _setL1Sender(to); } @@ -106,22 +102,16 @@ contract ScrollSequencerUptimeFeed is } } - /// /// @dev Returns an AggregatorV2V3Interface compatible answer from status flag - /// /// @param status The status flag to convert to an aggregator-compatible answer - /// function _getStatusAnswer(bool status) private pure returns (int256) { return status ? int256(1) : int256(0); } - /// /// @notice Helper function to record a round and set the latest feed state. - /// /// @param roundId The round ID to record /// @param status Sequencer status /// @param timestamp The L1 block timestamp of status update - /// function _recordRound(uint80 roundId, bool status, uint64 timestamp) private { uint64 updatedAt = uint64(block.timestamp); Round memory nextRound = Round(status, timestamp, updatedAt); @@ -134,12 +124,9 @@ contract ScrollSequencerUptimeFeed is emit AnswerUpdated(_getStatusAnswer(status), roundId, timestamp); } - /// /// @notice Helper function to update when a round was last updated - /// /// @param roundId The round ID to update /// @param status Sequencer status - /// function _updateRound(uint80 roundId, bool status) private { uint64 updatedAt = uint64(block.timestamp); s_rounds[roundId].updatedAt = updatedAt; @@ -147,13 +134,11 @@ contract ScrollSequencerUptimeFeed is emit RoundUpdated(_getStatusAnswer(status), updatedAt); } - /// /// @notice Record a new status and timestamp if it has changed since the last round. /// @dev This function will revert if not called from `l1Sender` via the L1->L2 messenger. /// /// @param status Sequencer status /// @param timestamp Block timestamp of status update - /// function updateStatus(bool status, uint64 timestamp) external override { FeedState memory feedState = s_feedState; if ( From a006199de6bdfb37a4ccd87013221a4b80c6f5d7 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Mon, 4 Dec 2023 20:45:55 +0000 Subject: [PATCH 11/16] removes unnecessary comment from scroll cross domain governor --- .../src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol index c37dabf815e..f18f7c3270b 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainForwarder.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.19; import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; import {ForwarderInterface} from "../interfaces/ForwarderInterface.sol"; -// ./dev dependencies - to be moved from ./dev after audit import {CrossDomainForwarder} from "../CrossDomainForwarder.sol"; import {CrossDomainOwnable} from "../CrossDomainOwnable.sol"; From 3c38e37126ed3be7703b7a8ebca300f5be61d7a1 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Mon, 4 Dec 2023 20:52:34 +0000 Subject: [PATCH 12/16] adds minor formatting updates to scroll mocks --- .../MockScrollL2CrossDomainMessenger.sol | 8 ++----- .../vendor/MockScrollCrossDomainMessenger.sol | 22 ++----------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol index 2f507da608a..f63faa35179 100644 --- a/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol +++ b/contracts/src/v0.8/l2ep/test/mocks/MockScrollL2CrossDomainMessenger.sol @@ -40,15 +40,11 @@ contract MockScrollL2CrossDomainMessenger is IL2ScrollMessenger { bytes calldata message ) external override {} - /* - * Needed for testing - */ + /// Needed for testing function setSender(address newSender) external { s_sender = newSender; } - /* - * Needed for testing - */ + /// Needed for testing receive() external payable {} } diff --git a/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol b/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol index d335300e536..bb5390b945d 100644 --- a/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol +++ b/contracts/src/v0.8/vendor/MockScrollCrossDomainMessenger.sol @@ -3,9 +3,7 @@ pragma solidity ^0.8.16; import "./openzeppelin-solidity/v4.8.3/contracts/utils/Address.sol"; -/// /// sourced from: https://github.com/scroll-tech/scroll/blob/develop/contracts/src/libraries/IScrollMessenger.sol -/// interface IScrollMessenger { /// ********** /// * Events * @@ -89,31 +87,15 @@ contract MockScrollCrossDomainMessenger is IScrollMessenger { /// @notice Send cross chain message from L1 to L2 or L2 to L1. /// @param _target The address of account who receive the message. - /// @param _value The amount of ether passed when call target contract. /// @param _message The content of the message. - /// @param _gasLimit Gas limit required to complete the message relay on corresponding chain. - function sendMessage( - address _target, - uint256 _value, - bytes calldata _message, - uint256 _gasLimit - ) external payable override { + function sendMessage(address _target, uint256, bytes calldata _message, uint256) external payable override { Address.functionCall(_target, _message, "sendMessage reverted"); } /// @notice Send cross chain message from L1 to L2 or L2 to L1. /// @param _target The address of account who receive the message. - /// @param _value The amount of ether passed when call target contract. /// @param _message The content of the message. - /// @param _gasLimit Gas limit required to complete the message relay on corresponding chain. - /// @param _refundAddress The address of account who will receive the refunded fee. - function sendMessage( - address _target, - uint256 _value, - bytes calldata _message, - uint256 _gasLimit, - address _refundAddress - ) external payable override { + function sendMessage(address _target, uint256, bytes calldata _message, uint256, address) external payable override { Address.functionCall(_target, _message, "sendMessage reverted"); } } From ced25dfee58c1d772ba4a861ec5314baa15ee4f2 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Mon, 4 Dec 2023 22:54:24 +0000 Subject: [PATCH 13/16] adds onlyL1Owner modifier back to ScrollCrossDomainGovernor --- .../l2ep/dev/scroll/ScrollCrossDomainGovernor.sol | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol index 030a98ddd52..00ef9219b26 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollCrossDomainGovernor.sol @@ -47,6 +47,18 @@ contract ScrollCrossDomainGovernor is DelegateForwarderInterface, TypeAndVersion return address(i_scrollCrossDomainMessenger); } + /// @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 == i_scrollCrossDomainMessenger, "Sender is not the L2 messenger"); + // solhint-disable-next-line custom-errors + require( + IScrollMessenger(i_scrollCrossDomainMessenger).xDomainMessageSender() == l1Owner(), + "xDomain sender is not the L1 owner" + ); + _; + } + /// @notice The call MUST come from either the L1 owner (via cross-chain message) or the L2 owner. Reverts otherwise. modifier onlyLocalOrCrossDomainOwner() { // 1. The delegatecall MUST come from either the L1 owner (via cross-chain message) or the L2 owner From 5f58f3956b6b9195b44a58e1c2cc8ef636ca272a Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Wed, 6 Dec 2023 01:02:27 +0000 Subject: [PATCH 14/16] adds style and formatting improvements --- .../dev/scroll/ScrollSequencerUptimeFeed.sol | 46 ++++++++----------- .../v0.8/l2ep/dev/scroll/ScrollValidator.sol | 29 ++++++------ .../dev/ScrollSequencerUptimeFeed.test.ts | 6 +-- 3 files changed, 37 insertions(+), 44 deletions(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol index 569f578caad..cbedfd8d088 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol @@ -1,12 +1,14 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.19; +import {ScrollSequencerUptimeFeedInterface} from "../interfaces/ScrollSequencerUptimeFeedInterface.sol"; import {AggregatorInterface} from "../../../shared/interfaces/AggregatorInterface.sol"; import {AggregatorV3Interface} from "../../../shared/interfaces/AggregatorV3Interface.sol"; import {AggregatorV2V3Interface} from "../../../shared/interfaces/AggregatorV2V3Interface.sol"; import {TypeAndVersionInterface} from "../../../interfaces/TypeAndVersionInterface.sol"; -import {ScrollSequencerUptimeFeedInterface} from "../interfaces/ScrollSequencerUptimeFeedInterface.sol"; + import {SimpleReadAccessController} from "../../../shared/access/SimpleReadAccessController.sol"; + import {IL2ScrollMessenger} from "@scroll-tech/contracts/L2/IL2ScrollMessenger.sol"; /// @title ScrollSequencerUptimeFeed - L2 sequencer uptime status aggregator @@ -163,38 +165,35 @@ contract ScrollSequencerUptimeFeed is /// @inheritdoc AggregatorInterface function latestAnswer() external view override checkAccess returns (int256) { - FeedState memory feedState = s_feedState; - return _getStatusAnswer(feedState.latestStatus); + return _getStatusAnswer(s_feedState.latestStatus); } /// @inheritdoc AggregatorInterface function latestTimestamp() external view override checkAccess returns (uint256) { - FeedState memory feedState = s_feedState; - return feedState.startedAt; + return s_feedState.startedAt; } /// @inheritdoc AggregatorInterface function latestRound() external view override checkAccess returns (uint256) { - FeedState memory feedState = s_feedState; - return feedState.latestRoundId; + return s_feedState.latestRoundId; } /// @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)) { + revert NoDataPresent(); } - revert NoDataPresent(); + return _getStatusAnswer(s_rounds[uint80(roundId)].status); } /// @inheritdoc AggregatorInterface function getTimestamp(uint256 roundId) external view override checkAccess returns (uint256) { - if (_isValidRound(roundId)) { - return s_rounds[uint80(roundId)].startedAt; + if (!_isValidRound(roundId)) { + revert NoDataPresent(); } - revert NoDataPresent(); + return s_rounds[uint80(roundId)].startedAt; } /// @inheritdoc AggregatorV3Interface @@ -212,13 +211,8 @@ contract ScrollSequencerUptimeFeed is } Round memory round = s_rounds[_roundId]; - answer = _getStatusAnswer(round.status); - startedAt = uint256(round.startedAt); - roundId = _roundId; - updatedAt = uint256(round.updatedAt); - answeredInRound = roundId; - return (roundId, answer, startedAt, updatedAt, answeredInRound); + return (_roundId, _getStatusAnswer(round.status), uint256(round.startedAt), uint256(round.updatedAt), _roundId); } /// @inheritdoc AggregatorV3Interface @@ -231,12 +225,12 @@ contract ScrollSequencerUptimeFeed is { FeedState memory feedState = s_feedState; - roundId = feedState.latestRoundId; - answer = _getStatusAnswer(feedState.latestStatus); - startedAt = feedState.startedAt; - updatedAt = feedState.updatedAt; - answeredInRound = roundId; - - return (roundId, answer, startedAt, updatedAt, answeredInRound); + return ( + feedState.latestRoundId, + _getStatusAnswer(feedState.latestStatus), + feedState.startedAt, + feedState.updatedAt, + feedState.latestRoundId + ); } } diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol index e6f82fa2e38..31a5f0764ef 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollValidator.sol @@ -11,16 +11,16 @@ import {IL1ScrollMessenger} from "@scroll-tech/contracts/L1/IL1ScrollMessenger.s /// @title ScrollValidator - makes cross chain call to update the Sequencer Uptime Feed on L2 contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterface, SimpleWriteAccessController { - // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables - string public constant override typeAndVersion = "ScrollValidator 1.0.0"; - 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; + // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables + string public constant override typeAndVersion = "ScrollValidator 1.0.0"; + int256 private constant ANSWER_SEQ_OFFLINE = 1; + uint32 private s_gasLimit; + /// @notice emitted when gas cost to spend on L2 is updated /// @param gasLimit updated gas cost event GasLimitUpdated(uint32 gasLimit); @@ -59,19 +59,18 @@ contract ScrollValidator is TypeAndVersionInterface, AggregatorValidatorInterfac uint256 /* currentRoundId */, int256 currentAnswer ) external override checkAccess returns (bool) { - // Encode the ScrollSequencerUptimeFeed call - bytes4 selector = ScrollSequencerUptimeFeedInterface.updateStatus.selector; - bool status = currentAnswer == ANSWER_SEQ_OFFLINE; - uint64 timestamp = uint64(block.timestamp); - // Encode `status` and `timestamp` - bytes memory message = abi.encodeWithSelector(selector, status, timestamp); // Make the xDomain call IL1ScrollMessenger(L1_CROSS_DOMAIN_MESSENGER_ADDRESS).sendMessage( - L2_UPTIME_FEED_ADDR, // Target (the address of the account that receives the message) - 0, // The amount of ether passed when calling the target contract. - message, // The content of the message. This is the arbitrary calldata to be executed. - s_gasLimit // Gas limit required to complete the message relay on the corresponding chain. + L2_UPTIME_FEED_ADDR, + 0, + abi.encodeWithSelector( + ScrollSequencerUptimeFeedInterface.updateStatus.selector, + currentAnswer == ANSWER_SEQ_OFFLINE, + uint64(block.timestamp) + ), + s_gasLimit ); + // return success return true; } diff --git a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts index 87c852247f7..50fd3838ff2 100644 --- a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts +++ b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts @@ -365,7 +365,7 @@ describe('ScrollSequencerUptimeFeed', () => { ) const tx = await _tx.wait(1) expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( - 28329, + 28229, gasUsedDeviation, ) }) @@ -378,7 +378,7 @@ describe('ScrollSequencerUptimeFeed', () => { ) const tx = await _tx.wait(1) expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( - 28229, + 28129, gasUsedDeviation, ) }) @@ -391,7 +391,7 @@ describe('ScrollSequencerUptimeFeed', () => { ) const tx = await _tx.wait(1) expect(tx.cumulativeGasUsed.toNumber()).to.be.closeTo( - 28245, + 28145, gasUsedDeviation, ) }) From f281a4b91d4d9725c43a48ad480bdcfb5db88cc0 Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Wed, 6 Dec 2023 20:18:28 +0000 Subject: [PATCH 15/16] adds formatting updates --- .../dev/scroll/ScrollSequencerUptimeFeed.sol | 43 ++++++++----------- .../dev/ScrollSequencerUptimeFeed.test.ts | 4 +- 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol index cbedfd8d088..9a8333b88e6 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol @@ -70,10 +70,9 @@ contract ScrollSequencerUptimeFeed is constructor(address l1SenderAddress, address l2CrossDomainMessengerAddr, bool initialStatus) { _setL1Sender(l1SenderAddress); s_l2CrossDomainMessenger = IL2ScrollMessenger(l2CrossDomainMessengerAddr); - uint64 timestamp = uint64(block.timestamp); // Initialise roundId == 1 as the first round - _recordRound(1, initialStatus, timestamp); + _recordRound(1, initialStatus, uint64(block.timestamp)); } /// @notice Check if a roundId is valid in this current contract state @@ -115,12 +114,8 @@ contract ScrollSequencerUptimeFeed is /// @param status Sequencer status /// @param timestamp The L1 block timestamp of status update 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); - - s_rounds[roundId] = nextRound; - s_feedState = feedState; + s_feedState = FeedState(roundId, status, timestamp, uint64(block.timestamp)); + s_rounds[roundId] = Round(status, timestamp, s_feedState.updatedAt); emit NewRound(roundId, msg.sender, timestamp); emit AnswerUpdated(_getStatusAnswer(status), roundId, timestamp); @@ -130,10 +125,9 @@ contract ScrollSequencerUptimeFeed is /// @param roundId The round ID to update /// @param status Sequencer status 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); + s_feedState.updatedAt = uint64(block.timestamp); + s_rounds[roundId].updatedAt = s_feedState.updatedAt; + emit RoundUpdated(_getStatusAnswer(status), s_feedState.updatedAt); } /// @notice Record a new status and timestamp if it has changed since the last round. @@ -142,7 +136,6 @@ contract ScrollSequencerUptimeFeed is /// @param status Sequencer status /// @param timestamp Block timestamp of status update function updateStatus(bool status, uint64 timestamp) external override { - FeedState memory feedState = s_feedState; if ( msg.sender != address(s_l2CrossDomainMessenger) || s_l2CrossDomainMessenger.xDomainMessageSender() != s_l1Sender ) { @@ -150,16 +143,16 @@ contract ScrollSequencerUptimeFeed is } // Ignore if latest recorded timestamp is newer - if (feedState.startedAt > timestamp) { - emit UpdateIgnored(feedState.latestStatus, feedState.startedAt, status, timestamp); + if (s_feedState.startedAt > timestamp) { + emit UpdateIgnored(s_feedState.latestStatus, s_feedState.startedAt, status, timestamp); return; } - if (feedState.latestStatus == status) { - _updateRound(feedState.latestRoundId, status); + if (s_feedState.latestStatus == status) { + _updateRound(s_feedState.latestRoundId, status); } else { - feedState.latestRoundId += 1; - _recordRound(feedState.latestRoundId, status, timestamp); + s_feedState.latestRoundId += 1; + _recordRound(s_feedState.latestRoundId, status, timestamp); } } @@ -223,14 +216,12 @@ contract ScrollSequencerUptimeFeed is checkAccess returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) { - FeedState memory feedState = s_feedState; - return ( - feedState.latestRoundId, - _getStatusAnswer(feedState.latestStatus), - feedState.startedAt, - feedState.updatedAt, - feedState.latestRoundId + s_feedState.latestRoundId, + _getStatusAnswer(s_feedState.latestStatus), + s_feedState.startedAt, + s_feedState.updatedAt, + s_feedState.latestRoundId ); } } diff --git a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts index 50fd3838ff2..283ecc36d18 100644 --- a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts +++ b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts @@ -304,7 +304,7 @@ describe('ScrollSequencerUptimeFeed', () => { // Assert no update expect(await scrollUptimeFeed.latestAnswer()).to.equal(0) expect(noUpdateTx.cumulativeGasUsed.toNumber()).to.be.closeTo( - 38594, + 38694, gasUsedDeviation, ) @@ -317,7 +317,7 @@ describe('ScrollSequencerUptimeFeed', () => { // Assert update expect(await scrollUptimeFeed.latestAnswer()).to.equal(1) expect(updateTx.cumulativeGasUsed.toNumber()).to.be.closeTo( - 60170, + 58995, gasUsedDeviation, ) }) From ebbb79173cafc5d05f3adddd8e4226a581e2289a Mon Sep 17 00:00:00 2001 From: Chris De Leon Date: Sun, 10 Dec 2023 01:05:39 -0800 Subject: [PATCH 16/16] refactors scroll sequencer uptime feed to reduce gas and updates test suites --- .../dev/scroll/ScrollSequencerUptimeFeed.sol | 32 +++++++++++-------- .../dev/ScrollSequencerUptimeFeed.test.ts | 4 +-- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol index 9a8333b88e6..22b5ed1e2b6 100644 --- a/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol +++ b/contracts/src/v0.8/l2ep/dev/scroll/ScrollSequencerUptimeFeed.sol @@ -115,7 +115,7 @@ contract ScrollSequencerUptimeFeed is /// @param timestamp The L1 block timestamp of status update function _recordRound(uint80 roundId, bool status, uint64 timestamp) private { s_feedState = FeedState(roundId, status, timestamp, uint64(block.timestamp)); - s_rounds[roundId] = Round(status, timestamp, s_feedState.updatedAt); + s_rounds[roundId] = Round(status, timestamp, uint64(block.timestamp)); emit NewRound(roundId, msg.sender, timestamp); emit AnswerUpdated(_getStatusAnswer(status), roundId, timestamp); @@ -126,8 +126,8 @@ contract ScrollSequencerUptimeFeed is /// @param status Sequencer status function _updateRound(uint80 roundId, bool status) private { s_feedState.updatedAt = uint64(block.timestamp); - s_rounds[roundId].updatedAt = s_feedState.updatedAt; - emit RoundUpdated(_getStatusAnswer(status), s_feedState.updatedAt); + s_rounds[roundId].updatedAt = uint64(block.timestamp); + emit RoundUpdated(_getStatusAnswer(status), uint64(block.timestamp)); } /// @notice Record a new status and timestamp if it has changed since the last round. @@ -136,6 +136,8 @@ contract ScrollSequencerUptimeFeed is /// @param status Sequencer status /// @param timestamp Block timestamp of status update function updateStatus(bool status, uint64 timestamp) external override { + FeedState memory feedState = s_feedState; + if ( msg.sender != address(s_l2CrossDomainMessenger) || s_l2CrossDomainMessenger.xDomainMessageSender() != s_l1Sender ) { @@ -143,16 +145,16 @@ contract ScrollSequencerUptimeFeed is } // Ignore if latest recorded timestamp is newer - if (s_feedState.startedAt > timestamp) { - emit UpdateIgnored(s_feedState.latestStatus, s_feedState.startedAt, status, timestamp); + if (feedState.startedAt > timestamp) { + emit UpdateIgnored(feedState.latestStatus, feedState.startedAt, status, timestamp); return; } - if (s_feedState.latestStatus == status) { - _updateRound(s_feedState.latestRoundId, status); + if (feedState.latestStatus == status) { + _updateRound(feedState.latestRoundId, status); } else { - s_feedState.latestRoundId += 1; - _recordRound(s_feedState.latestRoundId, status, timestamp); + feedState.latestRoundId += 1; + _recordRound(feedState.latestRoundId, status, timestamp); } } @@ -216,12 +218,14 @@ contract ScrollSequencerUptimeFeed is checkAccess returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) { + FeedState memory feedState = s_feedState; + return ( - s_feedState.latestRoundId, - _getStatusAnswer(s_feedState.latestStatus), - s_feedState.startedAt, - s_feedState.updatedAt, - s_feedState.latestRoundId + feedState.latestRoundId, + _getStatusAnswer(feedState.latestStatus), + feedState.startedAt, + feedState.updatedAt, + feedState.latestRoundId ); } } diff --git a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts index 283ecc36d18..b294032e73d 100644 --- a/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts +++ b/contracts/test/v0.8/dev/ScrollSequencerUptimeFeed.test.ts @@ -304,7 +304,7 @@ describe('ScrollSequencerUptimeFeed', () => { // Assert no update expect(await scrollUptimeFeed.latestAnswer()).to.equal(0) expect(noUpdateTx.cumulativeGasUsed.toNumber()).to.be.closeTo( - 38694, + 38594, gasUsedDeviation, ) @@ -317,7 +317,7 @@ describe('ScrollSequencerUptimeFeed', () => { // Assert update expect(await scrollUptimeFeed.latestAnswer()).to.equal(1) expect(updateTx.cumulativeGasUsed.toNumber()).to.be.closeTo( - 58995, + 58458, gasUsedDeviation, ) })