Skip to content

Commit

Permalink
Renames tests to follow Solidity convetions
Browse files Browse the repository at this point in the history
  • Loading branch information
mohamed-mehany committed Dec 9, 2024
1 parent 54c0e5c commit d771edd
Show file tree
Hide file tree
Showing 16 changed files with 266 additions and 278 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/solidity-foundry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
{ "name": "functions", "setup": { "run-coverage": false, "min-coverage": 98.5, "run-gas-snapshot": true, "run-forge-fmt": false }},
{ "name": "keystone", "setup": { "run-coverage": true, "min-coverage": 72.8, "run-gas-snapshot": false, "run-forge-fmt": false }},
{ "name": "l2ep", "setup": { "run-coverage": true, "min-coverage": 65.0, "run-gas-snapshot": true, "run-forge-fmt": false }},
{ "name": "liquiditymanager", "setup": { "run-coverage": true, "min-coverage": 46.3, "run-gas-snapshot": true, "run-forge-fmt": false }},
{ "name": "liquiditymanager", "setup": { "run-coverage": true, "min-coverage": 44, "run-gas-snapshot": true, "run-forge-fmt": false }},
{ "name": "llo-feeds", "setup": { "run-coverage": true, "min-coverage": 49.3, "run-gas-snapshot": true, "run-forge-fmt": false }},
{ "name": "operatorforwarder", "setup": { "run-coverage": true, "min-coverage": 55.7, "run-gas-snapshot": true, "run-forge-fmt": false }},
{ "name": "shared", "setup": { "run-coverage": true, "extra-coverage-params": "--no-match-path='*CallWithExactGas*' --ir-minimum", "min-coverage": 32.6, "run-gas-snapshot": true, "run-forge-fmt": false }},
Expand Down
9 changes: 0 additions & 9 deletions contracts/.changeset/stale-cougars-approve.md

This file was deleted.

5 changes: 5 additions & 0 deletions contracts/.changeset/thirty-rules-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chainlink/contracts': patch
---

[L2EP] Refactor tests and fix file exclusion for coverage
245 changes: 113 additions & 132 deletions contracts/gas-snapshots/l2ep.gas-snapshot

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;

import {BaseSequencerUptimeFeed} from "../../dev/base/BaseSequencerUptimeFeed.sol";
import {BaseSequencerUptimeFeed} from "../../base/BaseSequencerUptimeFeed.sol";

contract MockBaseSequencerUptimeFeed is BaseSequencerUptimeFeed {
string public constant override typeAndVersion = "MockSequencerUptimeFeed 1.1.0-dev";
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/l2ep/test/mocks/MockBaseValidator.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;

import {BaseValidator} from "../../dev/base/BaseValidator.sol";
import {BaseValidator} from "../../base/BaseValidator.sol";

contract MockBaseValidator is BaseValidator {
string public constant override typeAndVersion = "MockValidator 1.1.0-dev";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,36 @@ pragma solidity 0.8.24;
import {MockOptimismL1CrossDomainMessenger} from "../../../../tests/MockOptimismL1CrossDomainMessenger.sol";
import {MockOptimismL2CrossDomainMessenger} from "../../../../tests/MockOptimismL2CrossDomainMessenger.sol";
import {OptimismSequencerUptimeFeed} from "../../../optimism/OptimismSequencerUptimeFeed.sol";
import {BaseSequencerUptimeFeed} from "../../../shared/BaseSequencerUptimeFeed.sol";
import {FeedConsumer} from "../../../../tests/FeedConsumer.sol";
import {BaseSequencerUptimeFeed} from "../../../base/BaseSequencerUptimeFeed.sol";
import {L2EPTest} from "../L2EPTest.t.sol";

contract OptimismSequencerUptimeFeedTestWrapper is OptimismSequencerUptimeFeed {
contract OptimismSequencerUptimeFeed_TestWrapper is OptimismSequencerUptimeFeed {
constructor(
address l1SenderAddress,
address l2CrossDomainMessengerAddr,
bool initialStatus
) OptimismSequencerUptimeFeed(l1SenderAddress, l2CrossDomainMessengerAddr, initialStatus) {}

/// @notice it exposes the internal _validateSender function for testing
/// @notice Exposes the internal `_validateSender` function for testing
function validateSenderTestWrapper(address l1Sender) external view {
super._validateSender(l1Sender);
}
}

contract OptimismSequencerUptimeFeedTest is L2EPTest {
contract OptimismSequencerUptimeFeed_Setup is L2EPTest {
event AnswerUpdated(int256 indexed current, uint256 indexed roundId, uint256 updatedAt);

/// L2EP contracts
MockOptimismL1CrossDomainMessenger internal s_mockOptimismL1CrossDomainMessenger;
MockOptimismL2CrossDomainMessenger internal s_mockOptimismL2CrossDomainMessenger;
OptimismSequencerUptimeFeedTestWrapper internal s_optimismSequencerUptimeFeed;
OptimismSequencerUptimeFeed_TestWrapper internal s_optimismSequencerUptimeFeed;

/// Setup
function setUp() public {
// Deploys contracts
// Deploy contracts
s_mockOptimismL1CrossDomainMessenger = new MockOptimismL1CrossDomainMessenger();
s_mockOptimismL2CrossDomainMessenger = new MockOptimismL2CrossDomainMessenger();
s_optimismSequencerUptimeFeed = new OptimismSequencerUptimeFeedTestWrapper(
s_optimismSequencerUptimeFeed = new OptimismSequencerUptimeFeed_TestWrapper(
s_l1OwnerAddr,
address(s_mockOptimismL2CrossDomainMessenger),
false
Expand All @@ -45,13 +44,13 @@ contract OptimismSequencerUptimeFeedTest is L2EPTest {
}
}

contract OptimismSequencerUptimeFeed_Constructor is OptimismSequencerUptimeFeedTest {
/// @notice it should have been deployed with the correct initial state
function test_InitialState() public {
contract OptimismSequencerUptimeFeed_Constructor is OptimismSequencerUptimeFeed_Setup {
/// @notice Tests the initial state of the contract
function test_Constructor_InitialState() public {
// Sets msg.sender and tx.origin to a valid address
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);

new OptimismSequencerUptimeFeedTestWrapper(s_l1OwnerAddr, address(s_mockOptimismL2CrossDomainMessenger), false);
new OptimismSequencerUptimeFeed_TestWrapper(s_l1OwnerAddr, address(s_mockOptimismL2CrossDomainMessenger), false);

// Checks L1 sender
address actualL1Addr = s_optimismSequencerUptimeFeed.l1Sender();
Expand All @@ -64,9 +63,9 @@ contract OptimismSequencerUptimeFeed_Constructor is OptimismSequencerUptimeFeedT
}
}

contract OptimismSequencerUptimeFeed_ValidateSender is OptimismSequencerUptimeFeedTest {
/// @notice it should revert if called by an address that is not the L2 Cross Domain Messenger
function test_RevertIfSenderIsNotL2CrossDomainMessengerAddr() public {
contract OptimismSequencerUptimeFeed_ValidateSender is OptimismSequencerUptimeFeed_Setup {
/// @notice Reverts if called by an address that is not the L2 Cross Domain Messenger
function test_ValidateSender_RevertWhen_SenderIsNotL2CrossDomainMessengerAddr() public {
address l2MessengerAddr = address(s_mockOptimismL2CrossDomainMessenger);
// Sets msg.sender to a different address
vm.startPrank(s_strangerAddr, l2MessengerAddr);
Expand All @@ -75,8 +74,8 @@ contract OptimismSequencerUptimeFeed_ValidateSender is OptimismSequencerUptimeFe
s_optimismSequencerUptimeFeed.validateSenderTestWrapper(s_l1OwnerAddr);
}

/// @notice it should revert if the L1 sender address is not the L1 Cross Domain Messenger Sender
function test_RevertIfL1CrossDomainMessengerAddrIsNotL1SenderAddr() public {
/// @notice Reverts if the L1 sender address is not the L1 Cross Domain Messenger Sender
function test_ValidateSender_RevertWhen_L1CrossDomainMessengerAddrIsNotL1SenderAddr() public {
// Sets msg.sender and tx.origin to an unauthorized address
address l2MessengerAddr = address(s_mockOptimismL2CrossDomainMessenger);
vm.startPrank(l2MessengerAddr, l2MessengerAddr);
Expand All @@ -85,8 +84,8 @@ contract OptimismSequencerUptimeFeed_ValidateSender is OptimismSequencerUptimeFe
s_optimismSequencerUptimeFeed.validateSenderTestWrapper(s_strangerAddr);
}

/// @notice it should update status when status has changed and incoming timestamp is the same as latest
function test_UpdateStatusWhenStatusChangeAndNoTimeChange() public {
/// @notice Updates status when status has changed and incoming timestamp is the same as the latest
function test_ValidateSender_UpdateStatusWhen_StatusChangeAndNoTimeChange() public {
// Sets msg.sender and tx.origin to a valid address
address l2MessengerAddr = address(s_mockOptimismL2CrossDomainMessenger);
vm.startPrank(l2MessengerAddr, l2MessengerAddr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {OptimismSequencerUptimeFeed} from "../../../optimism/OptimismSequencerUp
import {OptimismValidator} from "../../../optimism/OptimismValidator.sol";
import {L2EPTest} from "../L2EPTest.t.sol";

contract OptimismValidatorTest is L2EPTest {
contract OptimismValidator_Setup is L2EPTest {
/// Helper constants
address internal constant L2_SEQ_STATUS_RECORDER_ADDRESS = 0x491B1dDA0A8fa069bbC1125133A975BF4e85a91b;
uint32 internal constant INIT_GAS_LIMIT = 1900000;
Expand Down Expand Up @@ -42,16 +42,16 @@ contract OptimismValidatorTest is L2EPTest {
}
}

contract OptimismValidator_Validate is OptimismValidatorTest {
contract OptimismValidator_Validate is OptimismValidator_Setup {
/// @notice it reverts if called by account with no access
function test_RevertsIfCalledByAnAccountWithNoAccess() public {
function test_Validate_RevertWhen_CalledByAccountWithNoAccess() public {
vm.startPrank(s_strangerAddr);
vm.expectRevert("No access");
s_optimismValidator.validate(0, 0, 1, 1);
}

/// @notice it posts sequencer status when there is not status change
function test_PostSequencerStatusWhenThereIsNotStatusChange() public {
/// @notice it posts sequencer status when there is no status change
function test_Validate_PostSequencerStatus_NoStatusChange() public {
// Gives access to the s_eoaValidator
s_optimismValidator.addAccess(s_eoaValidator);

Expand All @@ -74,8 +74,8 @@ contract OptimismValidator_Validate is OptimismValidatorTest {
s_optimismValidator.validate(0, 0, 0, 0);
}

/// @notice it post sequencer offline
function test_PostSequencerOffline() public {
/// @notice it posts sequencer offline
function test_Validate_PostSequencerOffline() public {
// Gives access to the s_eoaValidator
s_optimismValidator.addAccess(s_eoaValidator);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ pragma solidity 0.8.24;
import {MockScrollL1CrossDomainMessenger} from "../../mocks/scroll/MockScrollL1CrossDomainMessenger.sol";
import {MockScrollL2CrossDomainMessenger} from "../../mocks/scroll/MockScrollL2CrossDomainMessenger.sol";
import {ScrollSequencerUptimeFeed} from "../../../scroll/ScrollSequencerUptimeFeed.sol";
import {BaseSequencerUptimeFeed} from "../../../shared/BaseSequencerUptimeFeed.sol";
import {FeedConsumer} from "../../../../tests/FeedConsumer.sol";
import {BaseSequencerUptimeFeed} from "../../../base/BaseSequencerUptimeFeed.sol";
import {L2EPTest} from "../L2EPTest.t.sol";

contract ScrollSequencerUptimeFeedTestWrapper is ScrollSequencerUptimeFeed {
Expand All @@ -15,13 +14,13 @@ contract ScrollSequencerUptimeFeedTestWrapper is ScrollSequencerUptimeFeed {
bool initialStatus
) ScrollSequencerUptimeFeed(l1SenderAddress, l2CrossDomainMessengerAddr, initialStatus) {}

/// @notice it exposes the internal _validateSender function for testing
/// @notice It exposes the internal _validateSender function for testing
function validateSenderTestWrapper(address l1Sender) external view {
super._validateSender(l1Sender);
}
}

contract ScrollSequencerUptimeFeedTest is L2EPTest {
contract ScrollSequencerUptimeFeed_Setup is L2EPTest {
/// Constants
uint256 internal constant GAS_USED_DEVIATION = 100;

Expand Down Expand Up @@ -51,9 +50,9 @@ contract ScrollSequencerUptimeFeedTest is L2EPTest {
}
}

contract ScrollSequencerUptimeFeed_Constructor is ScrollSequencerUptimeFeedTest {
/// @notice it should have been deployed with the correct initial state
function test_InitialStateWithInvalidL2XDomainManager() public {
contract ScrollSequencerUptimeFeed_Constructor is ScrollSequencerUptimeFeed_Setup {
/// @notice Reverts when L2 Cross Domain Messenger address is invalid
function test_Constructor_RevertWhen_InvalidL2XDomainMessenger() public {
// L2 cross domain messenger address must not be the zero address
vm.expectRevert(ScrollSequencerUptimeFeed.ZeroAddress.selector);
new ScrollSequencerUptimeFeed(s_l1OwnerAddr, address(0), false);
Expand All @@ -70,7 +69,8 @@ contract ScrollSequencerUptimeFeed_Constructor is ScrollSequencerUptimeFeedTest
assertEq(answer, 0);
}

function test_InitialStateWithValidL2XDomainManager() public {
/// @notice Tests initial state with valid L2 Cross Domain Messenger
function test_Constructor_InitialState_WhenValidL2XDomainMessenger() public {
vm.startPrank(s_l1OwnerAddr, s_l1OwnerAddr);
ScrollSequencerUptimeFeed scrollSequencerUptimeFeed = new ScrollSequencerUptimeFeed(
s_l1OwnerAddr,
Expand All @@ -89,26 +89,26 @@ contract ScrollSequencerUptimeFeed_Constructor is ScrollSequencerUptimeFeedTest
}
}

contract ScrollSequencerUptimeFeed_ValidateSender is ScrollSequencerUptimeFeedTest {
/// @notice it should revert if called by an address that is not the L2 Cross Domain Messenger
function test_RevertIfSenderIsNotL2CrossDomainMessengerAddr() public {
contract ScrollSequencerUptimeFeed_ValidateSender is ScrollSequencerUptimeFeed_Setup {
/// @notice Reverts when sender is not L2 Cross Domain Messenger address
function test_ValidateSender_RevertWhen_SenderIsNotL2CrossDomainMessengerAddr() public {
vm.startPrank(s_strangerAddr);

vm.expectRevert(BaseSequencerUptimeFeed.InvalidSender.selector);
s_scrollSequencerUptimeFeed.validateSenderTestWrapper(s_l1OwnerAddr);
}

/// @notice it should revert if the L1 sender address is not the L1 Cross Domain Messenger Sender
function test_RevertIfL1CrossDomainMessengerAddrIsNotL1SenderAddr() public {
/// @notice Reverts when L1 Cross Domain Messenger address is not L1 sender address
function test_ValidateSender_RevertWhen_L1CrossDomainMessengerAddrIsNotL1SenderAddr() public {
address l2MessengerAddr = address(s_mockScrollL2CrossDomainMessenger);
vm.startPrank(l2MessengerAddr);

vm.expectRevert(BaseSequencerUptimeFeed.InvalidSender.selector);
s_scrollSequencerUptimeFeed.validateSenderTestWrapper(s_strangerAddr);
}

/// @notice it should update status when status has changed and incoming timestamp is the same as latest
function test_UpdateStatusWhenStatusChangeAndNoTimeChange() public {
/// @notice Updates status when status changes and incoming timestamp is the same as latest
function test_ValidateSender_UpdateStatusWhen_StatusChangeAndNoTimeChange() public {
address l2MessengerAddr = address(s_mockScrollL2CrossDomainMessenger);
vm.startPrank(l2MessengerAddr);

Expand Down
22 changes: 11 additions & 11 deletions contracts/src/v0.8/l2ep/test/v1_0_0/scroll/ScrollValidator.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {ScrollSequencerUptimeFeed} from "../../../scroll/ScrollSequencerUptimeFe
import {ScrollValidator} from "../../../scroll/ScrollValidator.sol";
import {L2EPTest} from "../L2EPTest.t.sol";

contract ScrollValidatorTest is L2EPTest {
contract ScrollValidator_Setup is L2EPTest {
/// Helper constants
address internal immutable L2_SEQ_STATUS_RECORDER_ADDRESS = makeAddr("L2_SEQ_STATUS_RECORDER_ADDRESS");
uint32 internal constant INIT_GAS_LIMIT = 1900000;
Expand Down Expand Up @@ -53,9 +53,9 @@ contract ScrollValidatorTest is L2EPTest {
}
}

contract ScrollValidator_Constructor is ScrollValidatorTest {
/// @notice it should have been deployed with the correct initial state
function test_InitialState() public {
contract ScrollValidator_Constructor is ScrollValidator_Setup {
/// @notice Reverts when L1 message queue address is invalid
function test_Constructor_RevertWhen_InvalidL1MessageQueueAddress() public {
vm.startPrank(s_l1OwnerAddr);

vm.expectRevert("Invalid L1 message queue address");
Expand All @@ -68,16 +68,16 @@ contract ScrollValidator_Constructor is ScrollValidatorTest {
}
}

contract ScrollValidator_Validate is ScrollValidatorTest {
/// @notice it reverts if called by account with no access
function test_RevertsIfCalledByAnAccountWithNoAccess() public {
contract ScrollValidator_Validate is ScrollValidator_Setup {
/// @notice Reverts if called by an account with no access
function test_Validate_RevertWhen_CalledByAccountWithNoAccess() public {
vm.startPrank(s_strangerAddr);
vm.expectRevert("No access");
s_scrollValidator.validate(0, 0, 1, 1);
}

/// @notice it posts sequencer status when there is not status change
function test_PostSequencerStatusWhenThereIsNotStatusChange() public {
/// @notice Posts sequencer status when there is no status change
function test_Validate_PostSequencerStatus_NoStatusChange() public {
// Gives access to the s_eoaValidator
s_scrollValidator.addAccess(s_eoaValidator);

Expand All @@ -101,8 +101,8 @@ contract ScrollValidator_Validate is ScrollValidatorTest {
s_scrollValidator.validate(0, 0, 0, 0);
}

/// @notice it post sequencer offline
function test_PostSequencerOffline() public {
/// @notice Posts sequencer offline status
function test_Validate_PostSequencerOffline() public {
// Gives access to the s_eoaValidator
s_scrollValidator.addAccess(s_eoaValidator);

Expand Down
Loading

0 comments on commit d771edd

Please sign in to comment.