Skip to content

Commit

Permalink
Forbid addition of paused sealable withdrawal blockers
Browse files Browse the repository at this point in the history
  • Loading branch information
Psirex authored and bulbozaur committed Nov 6, 2024
1 parent a8b4ab9 commit 1d70fa5
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 37 deletions.
14 changes: 4 additions & 10 deletions contracts/libraries/SealableCalls.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,16 @@ pragma solidity 0.8.26;

import {ISealable} from "../interfaces/ISealable.sol";

/// @dev All calls to sealable addresses less than MIN_VALID_SEALABLE_ADDRESS are treated as unsuccessful
/// to prevent potential false positives for current or future precompiled addresses.
address constant MIN_VALID_SEALABLE_ADDRESS = address(1024);

library SealableCalls {
/// @notice Attempts to call `ISealable.getResumeSinceTimestamp()` method, returning whether the call succeeded
/// and the result of the `ISealable.getResumeSinceTimestamp()` call if it succeeded.
/// @dev Performs a static call to the `getResumeSinceTimestamp()` method on the `ISealable` interface.
/// Ensures that the function does not revert even if the `sealable` contract does not implement
/// the interface, has no code at the address, or returns unexpected data.
/// Calls to addresses less than `MIN_VALID_SEALABLE_ADDRESS` are treated as unsuccessful to prevent
/// potential false positives from current or future precompiled addresses.
///
/// IMPORTANT: `callGetResumeSinceTimestamp()` may yield false-positive results when called on certain
/// precompiled contract addresses like SHA-256 (address(0x02)) or RIPEMD-160 (address(0x03)). Such cases must
/// be managed outside this function.
/// @param sealable The address of the sealable contract to check.
/// @return success Indicates whether the call to `getResumeSinceTimestamp()` was successful.
/// @return resumeSinceTimestamp The timestamp when the contract is expected to become unpaused.
Expand All @@ -25,10 +23,6 @@ library SealableCalls {
view
returns (bool success, uint256 resumeSinceTimestamp)
{
if (sealable < MIN_VALID_SEALABLE_ADDRESS) {
return (false, 0);
}

// Low-level call to the `getResumeSinceTimestamp` function on the `sealable` contract
(bool isCallSucceed, bytes memory returndata) =
sealable.staticcall(abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector));
Expand Down
7 changes: 5 additions & 2 deletions contracts/libraries/Tiebreaker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,12 @@ library Tiebreaker {
if (sealableWithdrawalBlockersCount == maxSealableWithdrawalBlockersCount) {
revert SealableWithdrawalBlockersLimitReached();
}
(bool isCallSucceed, /* resumeSinceTimestamp */ ) =

(bool isCallSucceed, uint256 resumeSinceTimestamp) =
SealableCalls.callGetResumeSinceTimestamp(sealableWithdrawalBlocker);
if (!isCallSucceed) {

// Prevents addition of paused or misbehaving sealables
if (!isCallSucceed || resumeSinceTimestamp >= block.timestamp) {
revert InvalidSealable(sealableWithdrawalBlocker);
}

Expand Down
63 changes: 38 additions & 25 deletions test/unit/libraries/SealableCalls.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity 0.8.26;

import {stdError} from "forge-std/StdError.sol";
import {ISealable, SealableCalls, MIN_VALID_SEALABLE_ADDRESS} from "contracts/libraries/SealableCalls.sol";
import {ISealable, SealableCalls} from "contracts/libraries/SealableCalls.sol";

import {UnitTest} from "test/utils/unit-test.sol";
import {SealableMock} from "test/mocks/SealableMock.sol";
Expand Down Expand Up @@ -84,39 +84,52 @@ contract SealableCallsTest is UnitTest {
_assertGetResumeSinceTimestampCallResult({sealable: _SEALABLE, isCallSucceed: false, resumeSinceTimestamp: 0});
}

function test_callGetResumeSinceTimestamp_IsCallSucceedFalse_OnSealableLessThanMinValidSealableAddress() external {
// check addresses (0x00, 0x400) reserved for precompiles.
// Currently Ethereum has only precompiles for 0x01 - 0x09 but new precompiles may be added in the future
for (uint256 i = 0; i < uint160(MIN_VALID_SEALABLE_ADDRESS); ++i) {
address precompile = address(uint160(i));
_assertGetResumeSinceTimestampCallResult({
sealable: precompile,
isCallSucceed: false,
resumeSinceTimestamp: 0
});
}

address LAST_INVALID_SEALABLE_ADDRESS = address(uint160(MIN_VALID_SEALABLE_ADDRESS) - 1);
// assuming address MIN_VALID_SEALABLE_ADDRESS and LAST_INVALID_SEALABLE_ADDRESS has
// deployed bytecode which returns some value
_mockSealableResumeSinceTimestampResult(MIN_VALID_SEALABLE_ADDRESS, block.timestamp);
_mockSealableResumeSinceTimestampResult(LAST_INVALID_SEALABLE_ADDRESS, block.timestamp);
// ---
// Test False Positive Results On Precompiles
// ---

// call to the LAST_INVALID_SEALABLE_ADDRESS considered not succeed as made to precompile
function test_callGetResumeSinceTimestamp_IsCallSucceed_FalsePositiveResult_On_SHA256_Precompile() external {
_assertGetResumeSinceTimestampCallResult({
sealable: LAST_INVALID_SEALABLE_ADDRESS,
isCallSucceed: false,
resumeSinceTimestamp: 0
sealable: address(0x2),
isCallSucceed: true,
resumeSinceTimestamp: 0xc61a1ce4443e07760aea88e1ac096cb3006c1c4284ade7873025b96c2010e1c8
});
}

// but call to the MIN_VALID_SEALABLE_ADDRESS considered succeed
function test_callGetResumeSinceTimestamp_IsCallSucceed_FalsePositiveResult_On_RIPEMD160_Precompile() external {
_assertGetResumeSinceTimestampCallResult({
sealable: MIN_VALID_SEALABLE_ADDRESS,
sealable: address(0x3),
isCallSucceed: true,
resumeSinceTimestamp: block.timestamp
resumeSinceTimestamp: 0x00000000000000000000000075b4744a1c0e92713946840b9adc0cb967652b9c
});
}

// ---
// Other Precompiles (Split in 2 parts because of out of gas)
// ---

function test_callGetResumeSinceTimestamp_IsCallSucceed_CorrectResult_On_Other_Precompiles_Part1() external {
for (uint160 i = 1; i < 8; ++i) {
// Skip SHA-256 and RIPEMD-160 precompiles which lead to false positive results
if (i == 0x2 || i == 0x3) continue;
_assertGetResumeSinceTimestampCallResult({
sealable: address(i),
isCallSucceed: false,
resumeSinceTimestamp: 0
});
}
}

function test_callGetResumeSinceTimestamp_IsCallSucceed_CorrectResult_On_Other_Precompiles_Part2() external {
for (uint160 i = 8; i < 12; ++i) {
_assertGetResumeSinceTimestampCallResult({
sealable: address(i),
isCallSucceed: false,
resumeSinceTimestamp: 0
});
}
}

// ---
// Helper Test Methods
// ---
Expand Down
12 changes: 12 additions & 0 deletions test/unit/libraries/Tiebreaker.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ contract TiebreakerTest is UnitTest {
vm.expectRevert(abi.encodeWithSelector(Tiebreaker.InvalidSealable.selector, _SEALABLE));

this.external__addSealableWithdrawalBlocker(_SEALABLE);

// revert when sealable is paused for short period
_mockSealableResumeSinceTimestampResult(_SEALABLE, block.timestamp);
vm.expectRevert(abi.encodeWithSelector(Tiebreaker.InvalidSealable.selector, _SEALABLE));

this.external__addSealableWithdrawalBlocker(_SEALABLE);

// revert when sealable is paused for long period
_mockSealableResumeSinceTimestampResult(_SEALABLE, type(uint256).max);
vm.expectRevert(abi.encodeWithSelector(Tiebreaker.InvalidSealable.selector, _SEALABLE));

this.external__addSealableWithdrawalBlocker(_SEALABLE);
}

function test_removeSealableWithdrawalBlocker_HappyPath() external {
Expand Down

0 comments on commit 1d70fa5

Please sign in to comment.