Skip to content

Commit

Permalink
feat: standardized timestamp comparison (#72)
Browse files Browse the repository at this point in the history 
Co-authored-by: Gas One Cent <[email protected]>
  • Loading branch information
@0xShaito @gas1cent
0xShaito and gas1cent authored Nov 4, 2024
1 parent ed60e2e commit 7d7a76d
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 35 deletions.
10 changes: 5 additions & 5 deletions solidity/contracts/modules/dispute/BondEscalationModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract BondEscalationModule is Module, IBondEscalationModule {
RequestParameters memory _params = decodeRequestData(_request.disputeModuleData);
BondEscalation storage _escalation = _escalations[_dispute.requestId];

if (block.timestamp > ORACLE.responseCreatedAt(_dispute.responseId) + _params.disputeWindow) {
if (block.timestamp >= ORACLE.responseCreatedAt(_dispute.responseId) + _params.disputeWindow) {
revert BondEscalationModule_DisputeWindowOver();
}

Expand Down Expand Up @@ -82,7 +82,7 @@ contract BondEscalationModule is Module, IBondEscalationModule {
if (_disputeStatus == IOracle.DisputeStatus.Escalated) {
// The dispute has been escalated to the Resolution module
// Make sure the bond escalation deadline has passed and update the status
if (block.timestamp <= ORACLE.disputeCreatedAt(_disputeId) + _params.bondEscalationDeadline) {
if (block.timestamp < ORACLE.disputeCreatedAt(_disputeId) + _params.bondEscalationDeadline) {
revert BondEscalationModule_BondEscalationNotOver();
}

Expand Down Expand Up @@ -254,7 +254,7 @@ contract BondEscalationModule is Module, IBondEscalationModule {
BondEscalation storage _escalation = _escalations[_dispute.requestId];

uint256 _disputeCreatedAt = ORACLE.disputeCreatedAt(_disputeId);
if (block.timestamp <= _disputeCreatedAt + _params.bondEscalationDeadline + _params.tyingBuffer) {
if (block.timestamp < _disputeCreatedAt + _params.bondEscalationDeadline + _params.tyingBuffer) {
revert BondEscalationModule_BondEscalationNotOver();
}

Expand Down Expand Up @@ -302,7 +302,7 @@ contract BondEscalationModule is Module, IBondEscalationModule {
_params = decodeRequestData(_request.disputeModuleData);

uint256 _disputeCreatedAt = ORACLE.disputeCreatedAt(_disputeId);
if (block.timestamp > _disputeCreatedAt + _params.bondEscalationDeadline + _params.tyingBuffer) {
if (block.timestamp >= _disputeCreatedAt + _params.bondEscalationDeadline + _params.tyingBuffer) {
revert BondEscalationModule_BondEscalationOver();
}

Expand All @@ -322,7 +322,7 @@ contract BondEscalationModule is Module, IBondEscalationModule {
}

if (
block.timestamp > _disputeCreatedAt + _params.bondEscalationDeadline
block.timestamp >= _disputeCreatedAt + _params.bondEscalationDeadline
&& _numPledgersForDispute == _numPledgersAgainstDispute
) {
revert BondEscalationModule_CannotBreakTieDuringTyingBuffer();
Expand Down
6 changes: 3 additions & 3 deletions solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
_escalation.startTime + _params.committingTimeWindow,
_escalation.startTime + _params.committingTimeWindow + _params.revealingTimeWindow
);
if (block.timestamp <= _revealStartTime) revert PrivateERC20ResolutionModule_OnGoingCommittingPhase();
if (block.timestamp > _revealEndTime) revert PrivateERC20ResolutionModule_RevealingPhaseOver();
if (block.timestamp < _revealStartTime) revert PrivateERC20ResolutionModule_OnGoingCommittingPhase();
if (block.timestamp >= _revealEndTime) revert PrivateERC20ResolutionModule_RevealingPhaseOver();

VoterData storage _voterData = _votersData[_disputeId][msg.sender];

Expand Down Expand Up @@ -123,7 +123,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
if (block.timestamp < _escalation.startTime + _params.committingTimeWindow) {
revert PrivateERC20ResolutionModule_OnGoingCommittingPhase();
}
if (block.timestamp <= _escalation.startTime + _params.committingTimeWindow + _params.revealingTimeWindow) {
if (block.timestamp < _escalation.startTime + _params.committingTimeWindow + _params.revealingTimeWindow) {
revert PrivateERC20ResolutionModule_OnGoingRevealingPhase();
}

Expand Down
4 changes: 2 additions & 2 deletions solidity/test/integration/EscalateDispute.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ contract Integration_EscalateDispute is IntegrationBase {
maxNumberOfEscalations: 2,
bondEscalationDeadline: _expectedDeadline,
tyingBuffer: _tyingBuffer,
disputeWindow: 0
disputeWindow: _baseDisputeWindow
})
);

Expand Down Expand Up @@ -119,7 +119,7 @@ contract Integration_EscalateDispute is IntegrationBase {
vm.expectRevert(IBondEscalationModule.BondEscalationModule_CannotBreakTieDuringTyingBuffer.selector);
_bondEscalationModule.pledgeForDispute(mockRequest, mockDispute);

// Pledege revert if bond escalation is over
// Pledge revert if bond escalation is over
vm.warp(_disputeCreatedAt + _expectedDeadline + _tyingBuffer + 1);
vm.expectRevert(IBondEscalationModule.BondEscalationModule_BondEscalationOver.selector);
_bondEscalationModule.pledgeForDispute(mockRequest, mockDispute);
Expand Down
72 changes: 69 additions & 3 deletions solidity/test/unit/modules/dispute/BondEscalationModule.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ contract BondEscalationModule_Unit_EscalateDispute is BaseTest {
// dispute once before the bond escalation deadline is over, and that dispute goes through the escalation module.
bondEscalationModule.forTest_setEscalatedDispute(_requestId, _disputeId);

vm.warp(disputeCreatedAt + _params.bondEscalationDeadline - 1);

// Check: does it revert if the bond escalation is not over yet?
vm.expectRevert(IBondEscalationModule.BondEscalationModule_BondEscalationNotOver.selector);
vm.prank(address(oracle));
Expand Down Expand Up @@ -453,7 +455,7 @@ contract BondEscalationModule_Unit_DisputeResponse is BaseTest {
mockDispute.responseId = _responseId;

// Warp to a time after the disputeWindow is over.
vm.warp(block.timestamp + _disputeWindow + 1);
vm.warp(block.timestamp + _disputeWindow);

// Mock and expect IOracle.responseCreatedAt to be called
_mockAndExpect(address(oracle), abi.encodeCall(IOracle.responseCreatedAt, (_responseId)), abi.encode(1));
Expand Down Expand Up @@ -1045,13 +1047,44 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
_setBondEscalation(_requestId, _numForPledgers, _numAgainstPledgers);

// warp into the tyingBuffer
vm.warp(disputeCreatedAt + _params.bondEscalationDeadline + 1);
vm.warp(disputeCreatedAt + _params.bondEscalationDeadline);

// Check: does it revert if trying to tie outside of the tying buffer?
vm.expectRevert(IBondEscalationModule.BondEscalationModule_CannotBreakTieDuringTyingBuffer.selector);
bondEscalationModule.pledgeForDispute(mockRequest, _dispute);
}

/**
* @notice Tests that pledgeAgainstDispute reverts if someone tries to pledge for when the escalation is over
*/
function test_revertIfBondEscalationOver(IBondEscalationModule.RequestParameters memory _params)
public
assumeFuzzable(address(_params.accountingExtension))
{
_params.bondSize = 1000;
_params.maxNumberOfEscalations = 3;
_params.bondEscalationDeadline = bondEscalationDeadline;
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

(, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

// Mock and expect
bondEscalationModule.forTest_setEscalatedDispute(_requestId, _disputeId);

uint256 _numForPledgers = 2;
uint256 _numAgainstPledgers = _numForPledgers + 1;

_setBondEscalation(_requestId, _numForPledgers, _numAgainstPledgers);

vm.warp(disputeCreatedAt + _params.bondEscalationDeadline + _params.tyingBuffer + 1);

vm.expectRevert(IBondEscalationModule.BondEscalationModule_BondEscalationOver.selector);
bondEscalationModule.pledgeForDispute(mockRequest, _dispute);
}

/**
* @notice Tests that pledgeForDispute is called successfully
*/
Expand Down Expand Up @@ -1236,13 +1269,44 @@ contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest {

_setBondEscalation(_requestId, _numForPledgers, _numAgainstPledgers);

vm.warp(disputeCreatedAt + _params.bondEscalationDeadline + 1);
vm.warp(disputeCreatedAt + _params.bondEscalationDeadline);

// Check: does it revert if trying to tie outside of the tying buffer?
vm.expectRevert(IBondEscalationModule.BondEscalationModule_CannotBreakTieDuringTyingBuffer.selector);
bondEscalationModule.pledgeAgainstDispute(mockRequest, _dispute);
}

/**
* @notice Tests that pledgeAgainstDispute reverts if someone tries to pledge against when the escalation is over
*/
function test_revertIfBondEscalationOver(IBondEscalationModule.RequestParameters memory _params)
public
assumeFuzzable(address(_params.accountingExtension))
{
_params.bondSize = 1000;
_params.maxNumberOfEscalations = 3;
_params.bondEscalationDeadline = bondEscalationDeadline;
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

(, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

// Mock and expect
bondEscalationModule.forTest_setEscalatedDispute(_requestId, _disputeId);

uint256 _numForPledgers = 2;
uint256 _numAgainstPledgers = _numForPledgers + 1;

_setBondEscalation(_requestId, _numForPledgers, _numAgainstPledgers);

vm.warp(disputeCreatedAt + _params.bondEscalationDeadline + _params.tyingBuffer + 1);

vm.expectRevert(IBondEscalationModule.BondEscalationModule_BondEscalationOver.selector);
bondEscalationModule.pledgeAgainstDispute(mockRequest, _dispute);
}

/**
* @notice Tests that pledgeAgainstDispute is called successfully
*/
Expand Down Expand Up @@ -1331,6 +1395,8 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest {
address(oracle), abi.encodeCall(IOracle.disputeCreatedAt, (_getId(_dispute))), abi.encode(disputeCreatedAt)
);

vm.warp(disputeCreatedAt + bondEscalationDeadline - 1);

// Check: does it revert if the bond escalation is not over?
vm.expectRevert(IBondEscalationModule.BondEscalationModule_BondEscalationNotOver.selector);
bondEscalationModule.settleBondEscalation(mockRequest, _response, _dispute);
Expand Down
49 changes: 27 additions & 22 deletions solidity/test/unit/modules/resolution/PrivateERC20ResolutionModule.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ contract BaseTest is Test, Helpers {
// A mock token
IERC20 public token;

uint256 public constant REVEALING_TIME_WINDOW = 40_000;
uint256 public constant COMMITING_TIME_WINDOW = 40_000;
uint256 public constant START_TIME = 100_000;
uint256 public constant END_TIME = START_TIME + REVEALING_TIME_WINDOW;

// Events
event CommittingPhaseStarted(uint256 _startTime, bytes32 _disputeId);
event VoteCommitted(address _voter, bytes32 _disputeId, bytes32 _commitment);
Expand Down Expand Up @@ -95,7 +100,7 @@ contract BaseTest is Test, Helpers {
);
module.commitVote(_request, _dispute, _commitment);

vm.warp(140_001);
vm.warp(END_TIME + 1);

vm.mockCall(
address(token),
Expand Down Expand Up @@ -172,8 +177,8 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest {
accountingExtension: IAccountingExtension(makeAddr('AccountingExtension')),
votingToken: token,
minVotesForQuorum: 1,
committingTimeWindow: 40_000,
revealingTimeWindow: 40_000
committingTimeWindow: COMMITING_TIME_WINDOW,
revealingTimeWindow: REVEALING_TIME_WINDOW
})
);

Expand All @@ -183,7 +188,7 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest {
bytes32 _disputeId = _getId(mockDispute);

// Store mock escalation data with startTime 100_000
module.forTest_setStartTime(_disputeId, 100_000);
module.forTest_setStartTime(_disputeId, START_TIME);

// Set timestamp for valid committingTimeWindow
vm.warp(123_456);
Expand Down Expand Up @@ -365,16 +370,16 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest {
* @notice Test that `commitVote` reverts if called outside of the committing time window.
*/
function test_revertIfCommittingPhaseOver(uint256 _timestamp, bytes32 _commitment) public {
_timestamp = bound(_timestamp, 140_000, type(uint96).max);
_timestamp = bound(_timestamp, END_TIME, type(uint96).max);

// Set mock request data
mockRequest.resolutionModuleData = abi.encode(
IPrivateERC20ResolutionModule.RequestParameters({
accountingExtension: IAccountingExtension(makeAddr('AccountingExtension')),
votingToken: token,
minVotesForQuorum: 1,
committingTimeWindow: 40_000,
revealingTimeWindow: 40_000
committingTimeWindow: COMMITING_TIME_WINDOW,
revealingTimeWindow: REVEALING_TIME_WINDOW
})
);

Expand All @@ -384,7 +389,7 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest {
bytes32 _disputeId = _getId(mockDispute);

// Store mock escalation data with startTime 100_000
module.forTest_setStartTime(_disputeId, 100_000);
module.forTest_setStartTime(_disputeId, START_TIME);

// Warp to invalid timestamp for commitment
vm.warp(_timestamp);
Expand Down Expand Up @@ -413,8 +418,8 @@ contract PrivateERC20ResolutionModule_Unit_RevealVote is BaseTest {
accountingExtension: IAccountingExtension(makeAddr('AccountingExtension')),
votingToken: token,
minVotesForQuorum: 1,
committingTimeWindow: 40_000,
revealingTimeWindow: 40_000
committingTimeWindow: COMMITING_TIME_WINDOW,
revealingTimeWindow: REVEALING_TIME_WINDOW
})
);

Expand All @@ -427,7 +432,7 @@ contract PrivateERC20ResolutionModule_Unit_RevealVote is BaseTest {
_mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeCreatedAt, (_disputeId)), abi.encode(1));

// Store mock escalation data with startTime 100_000
module.forTest_setStartTime(_disputeId, 100_000);
module.forTest_setStartTime(_disputeId, START_TIME);

// Store commitment
vm.prank(_voter);
Expand Down Expand Up @@ -491,16 +496,16 @@ contract PrivateERC20ResolutionModule_Unit_RevealVote is BaseTest {
* @notice Test that `revealVote` reverts if called outside the revealing time window.
*/
function test_revertIfInvalidPhase(uint256 _numberOfVotes, bytes32 _salt, uint256 _timestamp) public {
vm.assume(_timestamp >= 100_000 && (_timestamp <= 140_000 || _timestamp > 180_000));
vm.assume(_timestamp >= 100_000 && (_timestamp < END_TIME || _timestamp > 180_000));

// Set mock request data
mockRequest.resolutionModuleData = abi.encode(
IPrivateERC20ResolutionModule.RequestParameters({
accountingExtension: IAccountingExtension(makeAddr('AccountingExtension')),
votingToken: token,
minVotesForQuorum: 1,
committingTimeWindow: 40_000,
revealingTimeWindow: 40_000
committingTimeWindow: COMMITING_TIME_WINDOW,
revealingTimeWindow: REVEALING_TIME_WINDOW
})
);

Expand All @@ -511,12 +516,12 @@ contract PrivateERC20ResolutionModule_Unit_RevealVote is BaseTest {

_mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeCreatedAt, _disputeId), abi.encode(1));

module.forTest_setStartTime(_disputeId, 100_000);
module.forTest_setStartTime(_disputeId, START_TIME);

// Jump to timestamp
vm.warp(_timestamp);

if (_timestamp <= 140_000) {
if (_timestamp < END_TIME) {
// Check: does it revert if trying to reveal during the committing phase?
vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_OnGoingCommittingPhase.selector);
module.revealVote(mockRequest, _dispute, _numberOfVotes, _salt);
Expand Down Expand Up @@ -549,8 +554,8 @@ contract PrivateERC20ResolutionModule_Unit_RevealVote is BaseTest {
accountingExtension: IAccountingExtension(makeAddr('AccountingExtension')),
votingToken: token,
minVotesForQuorum: 1,
committingTimeWindow: 40_000,
revealingTimeWindow: 40_000
committingTimeWindow: COMMITING_TIME_WINDOW,
revealingTimeWindow: REVEALING_TIME_WINDOW
})
);

Expand All @@ -562,7 +567,7 @@ contract PrivateERC20ResolutionModule_Unit_RevealVote is BaseTest {
// Mock and expect IOracle.disputeCreatedAt to be called
_mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeCreatedAt, (_disputeId)), abi.encode(1));

module.forTest_setStartTime(_disputeId, 100_000);
module.forTest_setStartTime(_disputeId, START_TIME);

vm.warp(150_000);

Expand Down Expand Up @@ -600,8 +605,8 @@ contract PrivateERC20ResolutionModule_Unit_ResolveDispute is BaseTest {
accountingExtension: IAccountingExtension(makeAddr('AccountingExtension')),
votingToken: token,
minVotesForQuorum: _minVotesForQuorum,
committingTimeWindow: 40_000,
revealingTimeWindow: 40_000
committingTimeWindow: COMMITING_TIME_WINDOW,
revealingTimeWindow: REVEALING_TIME_WINDOW
})
);

Expand All @@ -613,7 +618,7 @@ contract PrivateERC20ResolutionModule_Unit_ResolveDispute is BaseTest {
mockDispute.responseId = _responseId;
bytes32 _disputeId = _getId(mockDispute);

module.forTest_setStartTime(_disputeId, 100_000);
module.forTest_setStartTime(_disputeId, START_TIME);

// Store escalation data with startTime 100_000 and votes 0
uint256 _votersAmount = 5;
Expand Down

0 comments on commit 7d7a76d

Please sign in to comment.