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

fix: restrict duplicate calls to resolveDispute for the same dispute #68

Open
wants to merge 1 commit into
base: fix/oz-audit
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions solidity/contracts/Oracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ contract Oracle is IOracle, OracleAccessController {
/// @inheritdoc IOracle
mapping(bytes32 _disputeId => DisputeStatus _status) public disputeStatus;

/// @inheritdoc IOracle
mapping(bytes32 _disputeId => bool _resolved) public disputeResolved;

/// @inheritdoc IOracle
mapping(uint256 _requestNumber => bytes32 _id) public nonceToRequestId;

Expand Down Expand Up @@ -278,6 +281,10 @@ contract Oracle is IOracle, OracleAccessController {
revert Oracle_InvalidDisputeId(_disputeId);
}

if (disputeResolved[_disputeId]) {
revert Oracle_DisputeAlreadyResolved();
}

// Revert if the dispute is not active nor escalated
DisputeStatus _currentStatus = disputeStatus[_disputeId];
if (_currentStatus != DisputeStatus.Active && _currentStatus != DisputeStatus.Escalated) {
Expand All @@ -288,6 +295,7 @@ contract Oracle is IOracle, OracleAccessController {
revert Oracle_NoResolutionModule(_disputeId);
}

disputeResolved[_disputeId] = true;
IResolutionModule(_request.resolutionModule).resolveDispute(_disputeId, _request, _response, _dispute);

emit DisputeResolved(_disputeId, _dispute);
Expand Down
13 changes: 13 additions & 0 deletions solidity/interfaces/IOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ interface IOracle is IOracleAccessController {
*/
error Oracle_InvalidDisputer();

/**
* @notice Thrown when the dispute is already resolved
*/
error Oracle_DisputeAlreadyResolved();

/*///////////////////////////////////////////////////////////////
ENUMS
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -299,6 +304,14 @@ interface IOracle is IOracleAccessController {
*/
function disputeStatus(bytes32 _disputeId) external view returns (DisputeStatus _status);

/**
* @notice Returns whether the dispute has been resolved
*
* @param _disputeId The id of the dispute
* @return _resolved True if the dispute has been resolved, false otherwise
*/
function disputeResolved(bytes32 _disputeId) external view returns (bool _resolved);

/**
* @notice The id of each request in chronological order
*
Expand Down
21 changes: 21 additions & 0 deletions solidity/test/unit/Oracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ contract MockOracle is Oracle {
disputeStatus[_disputeId] = _status;
}

function mock_setDisputeResolved(bytes32 _disputeId, bool _resolved) external {
disputeResolved[_disputeId] = _resolved;
}

function mock_setRequestId(uint256 _nonce, bytes32 _requestId) external {
nonceToRequestId[_nonce] = _requestId;
}
Expand Down Expand Up @@ -877,6 +881,23 @@ contract Oracle_Unit_ResolveDispute is BaseTest {
oracle.resolveDispute(mockRequest, mockResponse, mockDispute, mockAccessControl);
}

function test_resolveDispute_revertsWhenAlreadyResolved() public happyPath {
// Mock the dispute
bytes32 _disputeId = _getId(mockDispute);
oracle.mock_setRequestCreatedAt(_getId(mockRequest), block.timestamp);
oracle.mock_setResponseCreatedAt(_getId(mockResponse), block.timestamp);
oracle.mock_setDisputeCreatedAt(_disputeId, block.timestamp);

oracle.mock_setDisputeOf(_getId(mockResponse), _disputeId);
oracle.mock_setDisputeResolved(_disputeId, true);

// Check: reverts?
vm.expectRevert(IOracle.Oracle_DisputeAlreadyResolved.selector);

// Test: resolve the dispute
oracle.resolveDispute(mockRequest, mockResponse, mockDispute, mockAccessControl);
}

/**
* @notice Test the revert when the function is called with an non-existent dispute id
*/
Expand Down
Loading