Skip to content

Commit

Permalink
fix: remove repeated checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xShaito
0xShaito committed Aug 5, 2024
1 parent fc33382 commit e1ccfe4
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 105 deletions.
4 changes: 0 additions & 4 deletions solidity/contracts/extensions/BondEscalationAccounting.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
bytes32 _requestId = _getId(_request);
bytes32 _disputeId = _validateDispute(_request, _dispute);

if (ORACLE.disputeCreatedAt(_disputeId) == 0) revert BondEscalationAccounting_InvalidDispute();

if (!ORACLE.allowedModule(_requestId, msg.sender)) revert AccountingExtension_UnauthorizedModule();

if (pledges[_disputeId][_token] < _amountPerPledger * _winningPledgersLength) {
Expand Down Expand Up @@ -133,8 +131,6 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
bytes32 _requestId = _getId(_request);
bytes32 _disputeId = _validateDispute(_request, _dispute);

if (ORACLE.disputeCreatedAt(_disputeId) == 0) revert BondEscalationAccounting_InvalidDispute();

if (!ORACLE.allowedModule(_requestId, msg.sender)) revert AccountingExtension_UnauthorizedModule();

if (pledges[_disputeId][_token] < _amount) revert BondEscalationAccounting_InsufficientFunds();
Expand Down
1 change: 0 additions & 1 deletion solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
IOracle.Response calldata _response,
IOracle.Dispute calldata _dispute
) external onlyOracle {
if (ORACLE.disputeCreatedAt(_disputeId) == 0) revert PrivateERC20ResolutionModule_NonExistentDispute();
if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated) {
revert PrivateERC20ResolutionModule_AlreadyResolved();
}
Expand Down
5 changes: 0 additions & 5 deletions solidity/interfaces/extensions/IBondEscalationAccounting.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ interface IBondEscalationAccounting is IAccountingExtension {
*/
error BondEscalationAccounting_AlreadySettled();

/**
* @notice Thrown when the dispute is invalid
*/
error BondEscalationAccounting_InvalidDispute();

/*///////////////////////////////////////////////////////////////
STRUCTS
//////////////////////////////////////////////////////////////*/
Expand Down
5 changes: 0 additions & 5 deletions solidity/interfaces/modules/resolution/IPrivateERC20ResolutionModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,6 @@ interface IPrivateERC20ResolutionModule is IResolutionModule {
*/
error PrivateERC20ResolutionModule_OnGoingRevealingPhase();

/**
* @notice Thrown when trying to resolve a dispute that does not exist
*/
error PrivateERC20ResolutionModule_NonExistentDispute();

/**
* @notice Thrown when trying to commit an empty commitment
*/
Expand Down
33 changes: 10 additions & 23 deletions solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,24 +107,11 @@ contract BaseTest is Test, Helpers {

bondEscalationAccounting = new ForTest_BondEscalationAccounting(oracle);
}

function _getRequestAndDispute()
internal
returns (IOracle.Response memory _response, IOracle.Dispute memory _dispute)
{
// Compute proper IDs
_response = _getResponse(mockRequest, proposer);
_dispute = _getDispute(mockRequest, _response);
bytes32 _disputeId = _getId(_dispute);

// Mock and expect IOracle.disputeCreatedAt to be called
_mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeCreatedAt, (_disputeId)), abi.encode(1));
}
}

contract BondEscalationAccounting_Unit_Pledge is BaseTest {
function test_revertIfDisallowedModule(address _pledger, uint256 _amount) public {
(, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

// Mock and expect the call to oracle checking if the module is allowed
_mockAndExpect(
Expand All @@ -146,7 +133,7 @@ contract BondEscalationAccounting_Unit_Pledge is BaseTest {
function test_revertIfNotEnoughDeposited(address _pledger, uint256 _amount) public {
vm.assume(_amount > 0);

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

_mockAndExpect(
Expand All @@ -166,7 +153,7 @@ contract BondEscalationAccounting_Unit_Pledge is BaseTest {
}

function test_successfulCall(address _pledger, uint256 _amount) public {
(, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand Down Expand Up @@ -204,7 +191,7 @@ contract BondEscalationAccounting_Unit_Pledge is BaseTest {

contract BondEscalationAccounting_Unit_OnSettleBondEscalation is BaseTest {
function test_revertIfDisallowedModule(uint256 _numOfWinningPledgers, uint256 _amountPerPledger) public {
(, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);
bytes32 _requestId = _getId(mockRequest);

// Mock and expect the call to oracle checking if the module is allowed
Expand Down Expand Up @@ -259,7 +246,7 @@ contract BondEscalationAccounting_Unit_OnSettleBondEscalation is BaseTest {
}

function test_revertIfInsufficientFunds(uint256 _amountPerPledger, uint256 _numOfWinningPledgers) public {
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 249 in solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand Down Expand Up @@ -292,7 +279,7 @@ contract BondEscalationAccounting_Unit_OnSettleBondEscalation is BaseTest {
}

function test_successfulCall(uint256 _numOfWinningPledgers, uint256 _amountPerPledger) public {
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 282 in solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand Down Expand Up @@ -339,7 +326,7 @@ contract BondEscalationAccounting_Unit_OnSettleBondEscalation is BaseTest {

contract BondEscalationAccounting_Unit_ReleasePledge is BaseTest {
function test_revertIfDisallowedModule(address _pledger, uint256 _amount) public {
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 329 in solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Check warning on line 331 in solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_disputeId" is unused

Expand All @@ -363,7 +350,7 @@ contract BondEscalationAccounting_Unit_ReleasePledge is BaseTest {
function test_revertIfInsufficientFunds(uint256 _amount, address _pledger) public {
vm.assume(_amount < type(uint256).max);

(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 353 in solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand All @@ -388,7 +375,7 @@ contract BondEscalationAccounting_Unit_ReleasePledge is BaseTest {
}

function test_successfulCall(uint256 _amount, address _pledger) public {
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 378 in solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand All @@ -412,7 +399,7 @@ contract BondEscalationAccounting_Unit_ReleasePledge is BaseTest {
}

function test_emitsEvent(uint256 _amount, address _pledger) public {
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 402 in solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand Down
49 changes: 18 additions & 31 deletions solidity/test/unit/modules/dispute/BondEscalationModule.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,6 @@ contract BaseTest is Test, Helpers {
bondEscalationModule = new ForTest_BondEscalationModule(oracle);
}

function _getRequestAndDispute()
internal
returns (IOracle.Response memory _response, IOracle.Dispute memory _dispute)
{
// Compute proper IDs
_response = _getResponse(mockRequest, proposer);
_dispute = _getDispute(mockRequest, _response);
bytes32 _disputeId = _getId(_dispute);

// Mock and expect IOracle.disputeCreatedAt to be called
_mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeCreatedAt, (_disputeId)), abi.encode(1));
}

function _getRandomDispute(bytes32 _requestId) internal view returns (IOracle.Dispute memory _dispute) {
_dispute =
IOracle.Dispute({disputer: disputer, responseId: bytes32('response'), proposer: proposer, requestId: _requestId});
Expand Down Expand Up @@ -957,7 +944,7 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
* @notice Tests that pledgeForDispute reverts if the dispute is not going through the bond escalation mechanism.
*/
function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess() public {
(, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

// Check: does it revert if the dispute is not escalated yet?
vm.expectRevert(IBondEscalationModule.BondEscalationModule_InvalidDispute.selector);
Expand All @@ -977,7 +964,7 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand All @@ -1003,7 +990,7 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand Down Expand Up @@ -1032,7 +1019,7 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
_params.bondEscalationDeadline = block.timestamp + 1;
mockRequest.disputeModuleData = abi.encode(_params);

(, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 1022 in solidity/test/unit/modules/dispute/BondEscalationModule.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);
bondEscalationModule.forTest_setEscalatedDispute(_requestId, _disputeId);
Expand Down Expand Up @@ -1061,7 +1048,7 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 1051 in solidity/test/unit/modules/dispute/BondEscalationModule.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand Down Expand Up @@ -1090,7 +1077,7 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);

Check warning on line 1080 in solidity/test/unit/modules/dispute/BondEscalationModule.t.sol

View workflow job for this annotation

GitHub Actions / Run Linters (16.x) 

Variable "_response" is unused
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand Down Expand Up @@ -1141,7 +1128,7 @@ contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest {
* @notice Tests that pledgeAgainstDispute reverts if the dispute is not going through the bond escalation mechanism.
*/
function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess() public {
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getRequestAndDispute();
(IOracle.Response memory _response, IOracle.Dispute memory _dispute) = _getResponseAndDispute(oracle);
bytes32 _requestId = _getId(mockRequest);
bytes32 _disputeId = _getId(_dispute);

Expand All @@ -1163,7 +1150,7 @@ contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand All @@ -1190,7 +1177,7 @@ contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand Down Expand Up @@ -1221,7 +1208,7 @@ contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest {
_params.bondEscalationDeadline = block.timestamp + 1;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand Down Expand Up @@ -1253,7 +1240,7 @@ contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand Down Expand Up @@ -1282,7 +1269,7 @@ contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand Down Expand Up @@ -1343,15 +1330,15 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest {
* @notice Tests that settleBondEscalation reverts if someone tries to settle the escalation before the tying buffer
* has elapsed.
*/
function test_revertIfTimestampLessThanEndOfTryingBuffer(IBondEscalationModule.RequestParameters memory _params)
function test_revertIfTimestampLessThanEndOfTyingBuffer(IBondEscalationModule.RequestParameters memory _params)
public
assumeFuzzable(address(_params.accountingExtension))
{
_params.tyingBuffer = bound(_params.tyingBuffer, 0, type(uint128).max);
_params.bondEscalationDeadline = block.timestamp;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand All @@ -1372,7 +1359,7 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand All @@ -1397,7 +1384,7 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand Down Expand Up @@ -1428,7 +1415,7 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand Down Expand Up @@ -1472,7 +1459,7 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest {
_params.tyingBuffer = 1000;
mockRequest.disputeModuleData = abi.encode(_params);

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

Expand Down
Loading

0 comments on commit e1ccfe4

Please sign in to comment.