Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test Coverage #43

Merged
merged 10 commits into from
May 28, 2024
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ out/
/broadcast/*/31337/
/broadcast/**/dry-run/

# Ignores coverage
/coverage

# Dotenv file
.env

Expand Down
1 change: 0 additions & 1 deletion script/Laminator.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
pragma solidity ^0.8.19;

import {Script} from "forge-std/Script.sol";

import {BaseDeployer} from "./BaseDeployer.s.sol";
import {Laminator} from "../src/lamination/Laminator.sol";

Expand Down
3 changes: 2 additions & 1 deletion src/lamination/LaminatedProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ contract LaminatedProxy is LaminatedStorage, ReentrancyGuard {
/// @notice Cancels all pending calls
/// @dev Sets the executed flag to true for all pending calls
function cancelAllPending() external onlyOwner {
for (uint256 i = 0; i < executingSequenceNumber(); i++) {
uint256 _count = nextSequenceNumber();
for (uint256 i = executingSequenceNumber(); i < _count; i++) {
if (_deferredCalls[i].executed == false) {
_deferredCalls[i].executed = true;
}
Expand Down
28 changes: 0 additions & 28 deletions src/lamination/LaminatedStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,6 @@ abstract contract LaminatedStorage {
/// @notice The map from sequence number to calls held in the mempool.
mapping(uint256 => CallObjectHolderStorage) internal _deferredCalls;

/// @dev Indicates that the owner or laminator has already been set and cannot be set again.
/// @dev Selector 0xef34ca5c
error AlreadyInit();

/// @dev Cannot set Laminator to null address
/// @dev Selector 0x9c89a95b
error NullLaminator();

/// @dev Cannot set Owner to null address
/// @dev Selector 0xc77a0100
error NullOwner();

function deferredCalls(uint256 index) public view returns (CallObjectHolder memory holder) {
holder = _deferredCalls[index].load();
}
Expand Down Expand Up @@ -134,15 +122,7 @@ abstract contract LaminatedStorage {
/// @notice Set the owner address
/// @dev This function is called once during initialization.
/// @param _owner The address of the contract's owner.
/// @custom:reverts AlreadyInit() when owner has already been initialized.
/// @custom:reverts NullOwner() when owner is the null address.
function _setOwner(address _owner) internal {
if (owner() != address(0)) {
revert AlreadyInit();
}
if (_owner == address(0)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we removing these checks?

Copy link
Contributor Author

@TokenTitan TokenTitan May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, so there are two reasons here:

  1. This condition will not occur since we are initialising this through our own Laminator contract
  2. Even if we do manage to break this, it is initialized using create2 code block which does not revert and emits a ProxyCreated event anyway

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, that's fair, thanks.

revert NullOwner();
}
uint256 slot = uint256(OWNER_SLOT);
assembly ("memory-safe") {
sstore(slot, _owner)
Expand All @@ -152,15 +132,7 @@ abstract contract LaminatedStorage {
/// @notice Set the laminator address
/// @dev This function is called once during initialization.
/// @param _laminator The address of the contract's laminator.
/// @custom:reverts AlreadyInit() when laminator has already been initialized.
/// @custom:reverts NullOwner() when laminator is the null address.
function _setLaminator(address _laminator) internal {
if (address(laminator()) != address(0)) {
revert AlreadyInit();
}
if (_laminator == address(0)) {
revert NullLaminator();
}
uint256 slot = uint256(LAMINATOR_SLOT);
assembly ("memory-safe") {
sstore(slot, _laminator)
Expand Down
16 changes: 2 additions & 14 deletions src/timetravel/CallBreaker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,6 @@ contract CallBreaker is CallBreakerStorage {
payable(tipAddr).transfer(msg.value);
}

/// @dev Modifier to make a function callable only when the portal is open.
/// Reverts if the portal is closed. Portal is opened by `verify`.
modifier ensureTurnerOpen() {
if (!isPortalOpen()) {
revert PortalClosed();
}
_;
}

/// @notice Verifies that the given calls, when executed, gives the correct return values
/// @dev SECURITY NOTICE: This function is only callable when the portal is closed. It requires the caller to be an EOA.
/// @param callsBytes The bytes representing the calls to be verified
Expand Down Expand Up @@ -207,11 +198,8 @@ contract CallBreaker is CallBreakerStorage {
/// @notice Fetches the currently executing call index
/// @dev This function reverts if the portal is closed
/// @return The currently executing call index
function getCurrentlyExecuting() public view returns (uint256) {
if (!isPortalOpen()) {
revert PortalClosed();
}
return executingCallIndex();
function getCurrentlyExecuting() public view onlyPortalOpen returns (uint256) {
return _executingCallIndex();
}

function _populateCallIndices() internal {
Expand Down
2 changes: 1 addition & 1 deletion src/timetravel/CallBreakerStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ abstract contract CallBreakerStorage {

/// @notice Returns the index number of the currently executing call.
/// @return _execCallIndex The sequence number of the currently executing call.
function executingCallIndex() public view returns (uint256 _execCallIndex) {
function _executingCallIndex() internal view returns (uint256 _execCallIndex) {
uint256 slot = uint256(EXECUTING_CALL_INDEX_SLOT);
assembly ("memory-safe") {
_execCallIndex := sload(slot)
Expand Down
23 changes: 15 additions & 8 deletions src/timetravel/SmarterContract.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import "../timetravel/CallBreaker.sol";
contract SmarterContract {
CallBreaker public callbreaker;

/// @notice The address passed was a zero address
error AddressZero();

/// @dev Selector 0xab63c583
error FutureCallExpected();

Expand All @@ -32,14 +35,8 @@ contract SmarterContract {
/// @dev Selector 0xc19f17a9
error NotApproved();

/// @dev Constructs a new SmarterContract instance
/// @param _callbreaker The address of the CallBreaker contract
constructor(address _callbreaker) {
callbreaker = CallBreaker(payable(_callbreaker));
}

/// @notice Modifier to ensure that the Turner is open
modifier ensureTurnerOpen() {
/// @notice Modifier to ensure that the Portal is open
modifier onlyPortalOpen() {
if (!callbreaker.isPortalOpen()) {
revert PortalClosed();
}
Expand All @@ -59,6 +56,16 @@ contract SmarterContract {
_;
}

/// @dev Constructs a new SmarterContract instance
/// @param _callbreaker The address of the CallBreaker contract
constructor(address _callbreaker) {
if (_callbreaker == address(0)) {
revert AddressZero();
}

callbreaker = CallBreaker(payable(_callbreaker));
}

/// @notice Returns the call index, callobj, and returnobj of the currently executing call
/// @dev This function allows for time travel by returning the returnobj of the currently executing call
/// @return A pair consisting of the CallObject and ReturnObject of the currently executing call
Expand Down
62 changes: 2 additions & 60 deletions test/CallBreaker.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,67 +2,9 @@
pragma solidity >=0.6.2 <0.9.0;

import "forge-std/Test.sol";
//import {VmSafe} from "forge-std/Vm.sol";

import "../src/timetravel/CallBreaker.sol";

contract CallBreakerHarness is CallBreaker {
function populateCallIndicesHarness() public {
_populateCallIndices();
}

function resetTraceStoresWithHarness(CallObject[] memory calls, ReturnObject[] memory returnValues) public {
_resetTraceStoresWith(calls, returnValues);
}

function _executeAndVerifyCallHarness(uint256 index) public {
_executeAndVerifyCall(index);
}

function cleanUpStorageHarness() public {
_cleanUpStorage();
}

function populateAssociatedDataStoreHarness(bytes memory encodedData) public {
_populateAssociatedDataStore(encodedData);
}

function populateHintdicesHarness(bytes memory encodedData) public {
_populateHintdices(encodedData);
}

function insertIntoHintdicesHarness(bytes32 key, uint256 value) public {
_insertIntoHintdices(key, value);
}

function insertIntoAssociatedDataStore(bytes32 key, bytes memory value) public {
_insertIntoAssociatedDataStore(key, value);
}

function expectCallAtHarness(CallObject memory callObj, uint256 index) public view {
_expectCallAt(callObj, index);
}

function getCallStoreLengthHarness() public view returns (uint256) {
return callStore.length;
}

function getReturnStoreLengthHarness() public view returns (uint256) {
return returnStore.length;
}

function getCallListLengthHarness() public view returns (uint256) {
return callList.length;
}

function getAssociatedDataKeyListLengthHarness() public view returns (uint256) {
return associatedDataKeyList.length;
}

function getHintdicesStoreKeyListLengthHarness() public view returns (uint256) {
return hintdicesStoreKeyList.length;
}
}
import {CallBreaker, CallObject, ReturnObject} from "src/timetravel/CallBreaker.sol";
import {CallBreakerHarness} from "test/contracts/CallBreakerHarness.sol";

contract CallBreakerTest is Test {
CallBreakerHarness public callbreaker;
Expand Down
Loading