Skip to content

Commit

Permalink
fix: id collision
Browse files Browse the repository at this point in the history
  • Loading branch information
0xJabberwock committed Jul 16, 2024
1 parent 77e9130 commit c15c1af
Show file tree
Hide file tree
Showing 10 changed files with 214 additions and 147 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"postinstall": "husky install",
"lint:check": "yarn lint:sol-tests && yarn lint:sol-logic && forge fmt --check",
"lint:fix": "sort-package-json && forge fmt && yarn lint:sol-tests --fix && yarn lint:sol-logic --fix",
"lint:sol-logic": "solhint 'solidity/contracts/**/*.sol' 'solidity/interfaces/**/*.sol'",
"lint:sol-logic": "solhint 'solidity/contracts/**/*.sol' 'solidity/interfaces/**/*.sol' 'solidity/libraries/**/*.sol'",
"lint:sol-tests": "solhint -c .solhint.tests.json 'solidity/test/**/*.sol'",
"prepare": "husky install",
"prepublishOnly": "pinst --disable",
Expand All @@ -32,7 +32,7 @@
},
"lint-staged": {
"*.{js,css,md,ts,sol}": "forge fmt",
"*.sol": "solhint --fix 'solidity/contracts/**/*.sol' 'solidity/interfaces/**/*.sol' && solhint --fix -c .solhint.tests.json 'solidity/test/**/*.sol'",
"*.sol": "solhint --fix 'solidity/contracts/**/*.sol' 'solidity/interfaces/**/*.sol' 'solidity/libraries/**/*.sol' && solhint --fix -c .solhint.tests.json 'solidity/test/**/*.sol'",
"package.json": "sort-package-json"
},
"devDependencies": {
Expand Down
46 changes: 11 additions & 35 deletions solidity/contracts/Module.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@ pragma solidity ^0.8.19;
import {IModule} from '../interfaces/IModule.sol';
import {IOracle} from '../interfaces/IOracle.sol';

import {IDEncoder} from '../libraries/IDEncoder.sol';

abstract contract Module is IModule {
using IDEncoder for IOracle.Request;
using IDEncoder for IOracle.Response;
using IDEncoder for IOracle.Dispute;

/// @inheritdoc IModule
IOracle public immutable ORACLE;

Expand All @@ -30,36 +36,6 @@ abstract contract Module is IModule {
/// @inheritdoc IModule
function validateParameters(bytes calldata _encodedParameters) external pure virtual returns (bool _valid) {}

/**
* @notice Computes the id a given request
*
* @param _request The request to compute the id for
* @return _id The id the request
*/
function _getId(IOracle.Request calldata _request) internal pure returns (bytes32 _id) {
_id = keccak256(abi.encode(_request));
}

/**
* @notice Computes the id a given response
*
* @param _response The response to compute the id for
* @return _id The id the response
*/
function _getId(IOracle.Response calldata _response) internal pure returns (bytes32 _id) {
_id = keccak256(abi.encode(_response));
}

/**
* @notice Computes the id a given dispute
*
* @param _dispute The dispute to compute the id for
* @return _id The id the dispute
*/
function _getId(IOracle.Dispute calldata _dispute) internal pure returns (bytes32 _id) {
_id = keccak256(abi.encode(_dispute));
}

/**
* @notice Validates the correctness of a request-response pair
*
Expand All @@ -71,8 +47,8 @@ abstract contract Module is IModule {
IOracle.Request calldata _request,
IOracle.Response calldata _response
) internal pure returns (bytes32 _responseId) {
bytes32 _requestId = _getId(_request);
_responseId = _getId(_response);
bytes32 _requestId = _request.getId();
_responseId = _response.getId();
if (_response.requestId != _requestId) revert Module_InvalidResponseBody();
}

Expand All @@ -89,9 +65,9 @@ abstract contract Module is IModule {
IOracle.Response calldata _response,
IOracle.Dispute calldata _dispute
) internal pure returns (bytes32 _disputeId) {
bytes32 _requestId = _getId(_request);
bytes32 _responseId = _getId(_response);
_disputeId = _getId(_dispute);
bytes32 _requestId = _request.getId();
bytes32 _responseId = _response.getId();
_disputeId = _dispute.getId();

if (_dispute.requestId != _requestId || _dispute.responseId != _responseId) revert Module_InvalidDisputeBody();
if (_response.requestId != _requestId) revert Module_InvalidResponseBody();
Expand Down
28 changes: 17 additions & 11 deletions solidity/contracts/Oracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ import {IRequestModule} from '../interfaces/modules/request/IRequestModule.sol';
import {IResolutionModule} from '../interfaces/modules/resolution/IResolutionModule.sol';
import {IResponseModule} from '../interfaces/modules/response/IResponseModule.sol';

import {IDEncoder} from '../libraries/IDEncoder.sol';

contract Oracle is IOracle {
using IDEncoder for Request;
using IDEncoder for Response;
using IDEncoder for Dispute;

/// @inheritdoc IOracle
mapping(bytes32 _requestId => uint128 _finalizedAt) public finalizedAt;

Expand Down Expand Up @@ -283,7 +289,7 @@ contract Oracle is IOracle {
}

/// @inheritdoc IOracle
function finalize(IOracle.Request calldata _request, IOracle.Response calldata _response) external {
function finalize(Request calldata _request, Response calldata _response) external {
bytes32 _requestId;
bytes32 _responseId;

Expand Down Expand Up @@ -321,8 +327,8 @@ contract Oracle is IOracle {
* @param _request The request to be finalized
* @return _requestId The id of the finalized request
*/
function _finalizeWithoutResponse(IOracle.Request calldata _request) internal view returns (bytes32 _requestId) {
_requestId = keccak256(abi.encode(_request));
function _finalizeWithoutResponse(Request calldata _request) internal view returns (bytes32 _requestId) {
_requestId = _request.getId();
bytes32[] memory _responses = getResponseIds(_requestId);
uint256 _responsesAmount = _responses.length;

Expand Down Expand Up @@ -353,8 +359,8 @@ contract Oracle is IOracle {
* @return _responseId The id of the final response
*/
function _finalizeWithResponse(
IOracle.Request calldata _request,
IOracle.Response calldata _response
Request calldata _request,
Response calldata _response
) internal returns (bytes32 _requestId, bytes32 _responseId) {
_responseId = _validateResponse(_request, _response);
_requestId = _response.requestId;
Expand Down Expand Up @@ -387,7 +393,7 @@ contract Oracle is IOracle {
revert Oracle_InvalidRequestBody();
}

_requestId = keccak256(abi.encode(_request));
_requestId = _request.getId();
nonceToRequestId[_requestNonce] = _requestId;
createdAt[_requestId] = uint128(block.number);

Expand Down Expand Up @@ -417,8 +423,8 @@ contract Oracle is IOracle {
Request calldata _request,
Response calldata _response
) internal pure returns (bytes32 _responseId) {
bytes32 _requestId = keccak256(abi.encode(_request));
_responseId = keccak256(abi.encode(_response));
bytes32 _requestId = _request.getId();
_responseId = _response.getId();
if (_response.requestId != _requestId) revert Oracle_InvalidResponseBody();
}

Expand All @@ -435,9 +441,9 @@ contract Oracle is IOracle {
Response calldata _response,
Dispute calldata _dispute
) internal pure returns (bytes32 _disputeId) {
bytes32 _requestId = keccak256(abi.encode(_request));
bytes32 _responseId = keccak256(abi.encode(_response));
_disputeId = keccak256(abi.encode(_dispute));
bytes32 _requestId = _request.getId();
bytes32 _responseId = _response.getId();
_disputeId = _dispute.getId();

if (_dispute.requestId != _requestId || _dispute.responseId != _responseId) revert Oracle_InvalidDisputeBody();
if (_response.requestId != _requestId) revert Oracle_InvalidResponseBody();
Expand Down
50 changes: 50 additions & 0 deletions solidity/libraries/IDEncoder.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {IOracle} from '../interfaces/IOracle.sol';

/**
* @title IDEncoder
* @notice Library for encoding IDs of requests, responses and disputes
*/
library IDEncoder {
bytes32 private constant _REQUEST_TYPEHASH = keccak256(
'IOracle.Request(uint96 nonce,address requester,address requestModule,address responseModule,address disputeModule,address resolutionModule,address finalityModule,bytes requestModuleData,bytes responseModuleData,bytes disputeModuleData,bytes resolutionModuleData,bytes finalityModuleData)'
);

bytes32 private constant _RESPONSE_TYPEHASH =
keccak256('IOracle.Response(address proposer,bytes32 requestId,bytes response)');

bytes32 private constant _DISPUTE_TYPEHASH =
keccak256('IOracle.Dispute(address disputer,address proposer,bytes32 responseId,bytes32 requestId)');

/**
* @notice Computes the ID of a given request
*
* @param _request The request to compute the ID for
* @return _id The ID the request
*/
function getId(IOracle.Request memory _request) internal pure returns (bytes32 _id) {
_id = keccak256(abi.encode(_REQUEST_TYPEHASH, _request));
}

/**
* @notice Computes the ID of a given response
*
* @param _response The response to compute the ID for
* @return _id The ID the response
*/
function getId(IOracle.Response memory _response) internal pure returns (bytes32 _id) {
_id = keccak256(abi.encode(_RESPONSE_TYPEHASH, _response));
}

/**
* @notice Computes the ID of a given dispute
*
* @param _dispute The dispute to compute the ID for
* @return _id The ID the dispute
*/
function getId(IOracle.Dispute memory _dispute) internal pure returns (bytes32 _id) {
_id = keccak256(abi.encode(_DISPUTE_TYPEHASH, _dispute));
}
}
6 changes: 5 additions & 1 deletion solidity/test/integration/EscalateDispute.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ pragma solidity ^0.8.19;
import './IntegrationBase.sol';

contract Integration_EscalateDispute is IntegrationBase {
using IDEncoder for IOracle.Request;
using IDEncoder for IOracle.Response;
using IDEncoder for IOracle.Dispute;

function test_escalateDispute() public {
// Create the request
vm.prank(requester);
Expand All @@ -21,7 +25,7 @@ contract Integration_EscalateDispute is IntegrationBase {
oracle.escalateDispute(mockRequest, mockResponse, mockDispute);

// We check that the dispute was escalated
bytes32 _disputeId = _getId(mockDispute);
bytes32 _disputeId = mockDispute.getId();
assertTrue(oracle.disputeStatus(_disputeId) == IOracle.DisputeStatus.Escalated);

// Escalate dispute reverts if dispute is not active
Expand Down
12 changes: 8 additions & 4 deletions solidity/test/integration/Finalization.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ pragma solidity ^0.8.19;
import './IntegrationBase.sol';

contract Integration_Finalization is IntegrationBase {
using IDEncoder for IOracle.Request;
using IDEncoder for IOracle.Response;
using IDEncoder for IOracle.Dispute;

address internal _finalizer = makeAddr('finalizer');
address internal _callbackTarget = makeAddr('target');

Expand Down Expand Up @@ -54,7 +58,7 @@ contract Integration_Finalization is IntegrationBase {

bytes32 _responseId = oracle.finalizedResponseId(_requestId);
// Check: is request finalized?
assertEq(_responseId, _getId(mockResponse));
assertEq(_responseId, mockResponse.getId());
}

/**
Expand All @@ -78,9 +82,9 @@ contract Integration_Finalization is IntegrationBase {
mockRequest.finalityModuleData =
abi.encode(IMockFinalityModule.RequestParameters({target: _callbackTarget, data: bytes('')}));

mockResponse.requestId = _getId(mockRequest);
mockResponse.requestId = mockRequest.getId();
mockDispute.requestId = mockResponse.requestId;
mockDispute.responseId = _getId(mockResponse);
mockDispute.responseId = mockResponse.getId();

vm.prank(requester);
oracle.createRequest(mockRequest, _ipfsHash);
Expand Down Expand Up @@ -129,7 +133,7 @@ contract Integration_Finalization is IntegrationBase {
* @notice Internal helper function to setup the finalization stage of a request.
*/
function _jumpToFinalization() internal returns (bytes32 _responseId) {
mockResponse.requestId = _getId(mockRequest);
mockResponse.requestId = mockRequest.getId();

vm.prank(proposer);
_responseId = oracle.proposeResponse(mockRequest, mockResponse);
Expand Down
12 changes: 9 additions & 3 deletions solidity/test/integration/IntegrationBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {IResponseModule} from '../../interfaces/modules/response/IResponseModule

import {IOracle, Oracle} from '../../contracts/Oracle.sol';

import {IDEncoder} from '../../libraries/IDEncoder.sol';

import {IMockAccounting, MockAccounting} from '../mocks/contracts/MockAccounting.sol';
import {MockCallback} from '../mocks/contracts/MockCallback.sol';

Expand All @@ -32,6 +34,10 @@ import {IWETH9} from '../utils/external/IWETH9.sol';
// solhint-enable no-unused-import

contract IntegrationBase is TestConstants, Helpers {
using IDEncoder for IOracle.Request;
using IDEncoder for IOracle.Response;
using IDEncoder for IOracle.Dispute;

uint256 public constant FORK_BLOCK = 122_612_760;

uint256 internal _initialBalance = 100_000 ether;
Expand Down Expand Up @@ -134,11 +140,11 @@ contract IntegrationBase is TestConstants, Helpers {
mockRequest.requester = requester;

// Configure the mock response
mockResponse.requestId = _getId(mockRequest);
mockResponse.requestId = mockRequest.getId();

// Configure the mock dispute
mockDispute.requestId = _getId(mockRequest);
mockDispute.responseId = _getId(mockResponse);
mockDispute.requestId = mockRequest.getId();
mockDispute.responseId = mockResponse.getId();
}

function _mineBlock() internal {
Expand Down
6 changes: 5 additions & 1 deletion solidity/test/integration/ResponseProposal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ pragma solidity ^0.8.19;
import './IntegrationBase.sol';

contract Integration_ResponseProposal is IntegrationBase {
using IDEncoder for IOracle.Request;
using IDEncoder for IOracle.Response;
using IDEncoder for IOracle.Dispute;

bytes32 internal _requestId;

function setUp() public override {
Expand All @@ -25,7 +29,7 @@ contract Integration_ResponseProposal is IntegrationBase {

// Check: response data is correctly stored?
assertEq(_responseIds.length, 1);
assertEq(_responseIds[0], _getId(mockResponse));
assertEq(_responseIds[0], mockResponse.getId());
}

function test_proposeResponse_finalizedRequest(uint256 _timestamp) public {
Expand Down
Loading

0 comments on commit c15c1af

Please sign in to comment.