From 8d51e7615be0d0805b475864ef828ae1a943b864 Mon Sep 17 00:00:00 2001 From: shaito Date: Thu, 12 Sep 2024 15:40:56 -0300 Subject: [PATCH 1/6] feat: access control --- solidity/contracts/AccessController.sol | 33 +++++++++++++++ solidity/contracts/Oracle.sol | 18 +++++++-- solidity/interfaces/IOracle.sol | 40 ++++++++++++++----- .../accessControl/IAccessControlModule.sol | 12 ++++++ 4 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 solidity/contracts/AccessController.sol create mode 100644 solidity/interfaces/modules/accessControl/IAccessControlModule.sol diff --git a/solidity/contracts/AccessController.sol b/solidity/contracts/AccessController.sol new file mode 100644 index 0000000..03753bc --- /dev/null +++ b/solidity/contracts/AccessController.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import {IAccessControlModule} from '../interfaces/modules/accessControl/IAccessControlModule.sol'; + +abstract contract AccessController { + /** + * @notice The access control struct + * @param user The address of the user + * @param data The data for access control validation + */ + struct AccessControl { + address user; + bytes data; + } + + /** + * @notice Modifier to check if the caller has access to the user + * @param _caller The caller of the function + * @param _user The user to check access for + * @param _data The data to check access for + */ + modifier hasAccess(IAccessControlModule _accessControlModule, address _caller, AccessControl memory _accessControl) { + if ( + _caller == _accessControl.user + || ( + address(_accessControlModule) != address(0) + && _accessControlModule.hasAccess(_caller, _accessControl.user, _accessControl.data) + ) + ) revert AccessController_NoAccess(); + _; + } +} diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol index fe880af..c348fb3 100644 --- a/solidity/contracts/Oracle.sol +++ b/solidity/contracts/Oracle.sol @@ -11,7 +11,9 @@ import {IResolutionModule} from '../interfaces/modules/resolution/IResolutionMod import {IResponseModule} from '../interfaces/modules/response/IResponseModule.sol'; import {ValidatorLib} from '../libraries/ValidatorLib.sol'; -contract Oracle is IOracle { +import {AccessController} from './AccessController.sol'; + +contract Oracle is IOracle, AccessController { using ValidatorLib for *; /// @inheritdoc IOracle @@ -301,7 +303,9 @@ contract Oracle is IOracle { } /// @inheritdoc IOracle - function getResponseIds(bytes32 _requestId) public view returns (bytes32[] memory _ids) { + function getResponseIds( + bytes32 _requestId + ) public view returns (bytes32[] memory _ids) { bytes memory _responses = _responseIds[_requestId]; uint256 _length = _responses.length / 32; @@ -355,7 +359,9 @@ 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) { + function _finalizeWithoutResponse( + IOracle.Request calldata _request + ) internal view returns (bytes32 _requestId) { _requestId = ValidatorLib._getId(_request); if (requestCreatedAt[_requestId] == 0) { @@ -419,7 +425,11 @@ contract Oracle is IOracle { * @param _ipfsHash The hashed IPFS CID of the metadata json * @return _requestId The id of the created request */ - function _createRequest(Request memory _request, bytes32 _ipfsHash) internal returns (bytes32 _requestId) { + function _createRequest( + Request memory _request, + bytes32 _ipfsHash, + AccessControl memory _accessControl + ) internal hasAccess(_request.accessControlModule, msg.sender, _accessControl) returns (bytes32 _requestId) { uint256 _requestNonce = totalRequestCount++; if (_request.nonce == 0) _request.nonce = uint96(_requestNonce); diff --git a/solidity/interfaces/IOracle.sol b/solidity/interfaces/IOracle.sol index 81a1736..a53240b 100644 --- a/solidity/interfaces/IOracle.sol +++ b/solidity/interfaces/IOracle.sol @@ -193,6 +193,8 @@ interface IOracle { * @param disputeModule The address of the dispute module * @param resolutionModule The address of the resolution module * @param finalityModule The address of the finality module + * @param accessControlModule The address of the access control module + * // * @param accessControlModuleData The parameters for the access control module // TODO: Access control could be used to create a request so there is no way to have general data? * @param requestModuleData The parameters for the request module * @param responseModuleData The parameters for the response module * @param disputeModuleData The parameters for the dispute module @@ -209,6 +211,8 @@ interface IOracle { address disputeModule; address resolutionModule; address finalityModule; + address accessControlModule; + bytes accessControlModuleData; bytes requestModuleData; bytes responseModuleData; bytes disputeModuleData; @@ -252,7 +256,9 @@ interface IOracle { * @param _responseId The response id to get the dispute for * @return _disputeId The id of the dispute associated with the given response */ - function disputeOf(bytes32 _responseId) external view returns (bytes32 _disputeId); + function disputeOf( + bytes32 _responseId + ) external view returns (bytes32 _disputeId); /** * @notice Returns the total number of requests stored in the oracle @@ -267,7 +273,9 @@ interface IOracle { * @param _disputeId The id of the dispute * @return _status The status of the dispute */ - function disputeStatus(bytes32 _disputeId) external view returns (DisputeStatus _status); + function disputeStatus( + bytes32 _disputeId + ) external view returns (DisputeStatus _status); /** * @notice The id of each request in chronological order @@ -275,7 +283,9 @@ interface IOracle { * @param _nonce The nonce of the request * @return _requestId The id of the request */ - function nonceToRequestId(uint256 _nonce) external view returns (bytes32 _requestId); + function nonceToRequestId( + uint256 _nonce + ) external view returns (bytes32 _requestId); /** * @notice Returns the finalized response ID for a given request @@ -283,7 +293,9 @@ interface IOracle { * @param _requestId The id of the request * @return _finalizedResponseId The id of the finalized response */ - function finalizedResponseId(bytes32 _requestId) external view returns (bytes32 _finalizedResponseId); + function finalizedResponseId( + bytes32 _requestId + ) external view returns (bytes32 _finalizedResponseId); /** * @notice The number of the block at which a request was created @@ -291,7 +303,9 @@ interface IOracle { * @param _id The request id * @return _requestCreatedAt The block number */ - function requestCreatedAt(bytes32 _id) external view returns (uint128 _requestCreatedAt); + function requestCreatedAt( + bytes32 _id + ) external view returns (uint128 _requestCreatedAt); /** * @notice The number of the block at which a response was created @@ -299,7 +313,9 @@ interface IOracle { * @param _id The response id * @return _responseCreatedAt The block number */ - function responseCreatedAt(bytes32 _id) external view returns (uint128 _responseCreatedAt); + function responseCreatedAt( + bytes32 _id + ) external view returns (uint128 _responseCreatedAt); /** * @notice The number of the block at which a dispute was created @@ -307,7 +323,9 @@ interface IOracle { * @param _id The dispute id * @return _disputeCreatedAt The block number */ - function disputeCreatedAt(bytes32 _id) external view returns (uint128 _disputeCreatedAt); + function disputeCreatedAt( + bytes32 _id + ) external view returns (uint128 _disputeCreatedAt); /** * @notice The number of the block at which a request was finalized @@ -315,7 +333,9 @@ interface IOracle { * @param _requestId The request id * @return _finalizedAt The block number */ - function finalizedAt(bytes32 _requestId) external view returns (uint128 _finalizedAt); + function finalizedAt( + bytes32 _requestId + ) external view returns (uint128 _finalizedAt); /*/////////////////////////////////////////////////////////////// LOGIC @@ -435,7 +455,9 @@ interface IOracle { * @param _requestId The id of the request * @return _ids The ids of the responses */ - function getResponseIds(bytes32 _requestId) external view returns (bytes32[] memory _ids); + function getResponseIds( + bytes32 _requestId + ) external view returns (bytes32[] memory _ids); /** * @notice Finalizes the request and executes the post-request logic on the modules diff --git a/solidity/interfaces/modules/accessControl/IAccessControlModule.sol b/solidity/interfaces/modules/accessControl/IAccessControlModule.sol new file mode 100644 index 0000000..c2c2084 --- /dev/null +++ b/solidity/interfaces/modules/accessControl/IAccessControlModule.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import {IModule} from '../../IModule.sol'; + +/** + * @title ResponseModule + * @notice Common interface for all response modules + */ +interface IAccessControlModule is IModule { + function hasAccess(address _caller, address _user, bytes calldata _data) external view returns (bool _hasAccess); +} From 64001539ef7cf9db1fa6d8565d8be13229fc3f1c Mon Sep 17 00:00:00 2001 From: shaito Date: Thu, 12 Sep 2024 15:46:21 -0300 Subject: [PATCH 2/6] fix: linter --- solidity/contracts/Oracle.sol | 8 ++------ solidity/interfaces/IOracle.sol | 36 +++++++++------------------------ 2 files changed, 11 insertions(+), 33 deletions(-) diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol index c348fb3..a3debb3 100644 --- a/solidity/contracts/Oracle.sol +++ b/solidity/contracts/Oracle.sol @@ -303,9 +303,7 @@ contract Oracle is IOracle, AccessController { } /// @inheritdoc IOracle - function getResponseIds( - bytes32 _requestId - ) public view returns (bytes32[] memory _ids) { + function getResponseIds(bytes32 _requestId) public view returns (bytes32[] memory _ids) { bytes memory _responses = _responseIds[_requestId]; uint256 _length = _responses.length / 32; @@ -359,9 +357,7 @@ contract Oracle is IOracle, AccessController { * @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) { + function _finalizeWithoutResponse(IOracle.Request calldata _request) internal view returns (bytes32 _requestId) { _requestId = ValidatorLib._getId(_request); if (requestCreatedAt[_requestId] == 0) { diff --git a/solidity/interfaces/IOracle.sol b/solidity/interfaces/IOracle.sol index a53240b..05362b7 100644 --- a/solidity/interfaces/IOracle.sol +++ b/solidity/interfaces/IOracle.sol @@ -256,9 +256,7 @@ interface IOracle { * @param _responseId The response id to get the dispute for * @return _disputeId The id of the dispute associated with the given response */ - function disputeOf( - bytes32 _responseId - ) external view returns (bytes32 _disputeId); + function disputeOf(bytes32 _responseId) external view returns (bytes32 _disputeId); /** * @notice Returns the total number of requests stored in the oracle @@ -273,9 +271,7 @@ interface IOracle { * @param _disputeId The id of the dispute * @return _status The status of the dispute */ - function disputeStatus( - bytes32 _disputeId - ) external view returns (DisputeStatus _status); + function disputeStatus(bytes32 _disputeId) external view returns (DisputeStatus _status); /** * @notice The id of each request in chronological order @@ -283,9 +279,7 @@ interface IOracle { * @param _nonce The nonce of the request * @return _requestId The id of the request */ - function nonceToRequestId( - uint256 _nonce - ) external view returns (bytes32 _requestId); + function nonceToRequestId(uint256 _nonce) external view returns (bytes32 _requestId); /** * @notice Returns the finalized response ID for a given request @@ -293,9 +287,7 @@ interface IOracle { * @param _requestId The id of the request * @return _finalizedResponseId The id of the finalized response */ - function finalizedResponseId( - bytes32 _requestId - ) external view returns (bytes32 _finalizedResponseId); + function finalizedResponseId(bytes32 _requestId) external view returns (bytes32 _finalizedResponseId); /** * @notice The number of the block at which a request was created @@ -303,9 +295,7 @@ interface IOracle { * @param _id The request id * @return _requestCreatedAt The block number */ - function requestCreatedAt( - bytes32 _id - ) external view returns (uint128 _requestCreatedAt); + function requestCreatedAt(bytes32 _id) external view returns (uint128 _requestCreatedAt); /** * @notice The number of the block at which a response was created @@ -313,9 +303,7 @@ interface IOracle { * @param _id The response id * @return _responseCreatedAt The block number */ - function responseCreatedAt( - bytes32 _id - ) external view returns (uint128 _responseCreatedAt); + function responseCreatedAt(bytes32 _id) external view returns (uint128 _responseCreatedAt); /** * @notice The number of the block at which a dispute was created @@ -323,9 +311,7 @@ interface IOracle { * @param _id The dispute id * @return _disputeCreatedAt The block number */ - function disputeCreatedAt( - bytes32 _id - ) external view returns (uint128 _disputeCreatedAt); + function disputeCreatedAt(bytes32 _id) external view returns (uint128 _disputeCreatedAt); /** * @notice The number of the block at which a request was finalized @@ -333,9 +319,7 @@ interface IOracle { * @param _requestId The request id * @return _finalizedAt The block number */ - function finalizedAt( - bytes32 _requestId - ) external view returns (uint128 _finalizedAt); + function finalizedAt(bytes32 _requestId) external view returns (uint128 _finalizedAt); /*/////////////////////////////////////////////////////////////// LOGIC @@ -455,9 +439,7 @@ interface IOracle { * @param _requestId The id of the request * @return _ids The ids of the responses */ - function getResponseIds( - bytes32 _requestId - ) external view returns (bytes32[] memory _ids); + function getResponseIds(bytes32 _requestId) external view returns (bytes32[] memory _ids); /** * @notice Finalizes the request and executes the post-request logic on the modules From 8ac37b429e3b232fdec7dad333c110387d50d12e Mon Sep 17 00:00:00 2001 From: shaito Date: Fri, 13 Sep 2024 10:11:47 -0300 Subject: [PATCH 3/6] fix: check --- solidity/contracts/AccessController.sol | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/solidity/contracts/AccessController.sol b/solidity/contracts/AccessController.sol index 03753bc..8c762ab 100644 --- a/solidity/contracts/AccessController.sol +++ b/solidity/contracts/AccessController.sol @@ -16,18 +16,17 @@ abstract contract AccessController { /** * @notice Modifier to check if the caller has access to the user - * @param _caller The caller of the function - * @param _user The user to check access for - * @param _data The data to check access for + * @param _accessControlModule The access control module + * @param _caller The caller + * @param _accessControl The access control struct */ modifier hasAccess(IAccessControlModule _accessControlModule, address _caller, AccessControl memory _accessControl) { - if ( - _caller == _accessControl.user - || ( - address(_accessControlModule) != address(0) - && _accessControlModule.hasAccess(_caller, _accessControl.user, _accessControl.data) - ) - ) revert AccessController_NoAccess(); + bool _hasAccess = _caller == _accessControl.user + || ( + address(_accessControlModule) != address(0) + && _accessControlModule.hasAccess(_caller, _accessControl.user, _accessControl.data) + ); + if (!_hasAccess) revert AccessController_NoAccess(); _; } } From 467c577dc01cbb43ef24296f892493db53211ad1 Mon Sep 17 00:00:00 2001 From: shaito Date: Fri, 13 Sep 2024 10:22:24 -0300 Subject: [PATCH 4/6] fix: clean --- solidity/contracts/AccessController.sol | 6 ++-- solidity/contracts/Oracle.sol | 47 ++++++++++++++++--------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/solidity/contracts/AccessController.sol b/solidity/contracts/AccessController.sol index 8c762ab..09f3e4a 100644 --- a/solidity/contracts/AccessController.sol +++ b/solidity/contracts/AccessController.sol @@ -16,9 +16,9 @@ abstract contract AccessController { /** * @notice Modifier to check if the caller has access to the user - * @param _accessControlModule The access control module - * @param _caller The caller - * @param _accessControl The access control struct + * @param _accessControlModule The access control module + * @param _caller The caller + * @param _accessControl The access control struct */ modifier hasAccess(IAccessControlModule _accessControlModule, address _caller, AccessControl memory _accessControl) { bool _hasAccess = _caller == _accessControl.user diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol index a3debb3..a93e52d 100644 --- a/solidity/contracts/Oracle.sol +++ b/solidity/contracts/Oracle.sol @@ -59,20 +59,25 @@ contract Oracle is IOracle, AccessController { uint256 public totalRequestCount; /// @inheritdoc IOracle - function createRequest(Request calldata _request, bytes32 _ipfsHash) external returns (bytes32 _requestId) { - _requestId = _createRequest(_request, _ipfsHash); + function createRequest( + Request calldata _request, + AccessControl memory _accessControl, + bytes32 _ipfsHash + ) external returns (bytes32 _requestId) { + _requestId = _createRequest(_request, _ipfsHash, _accessControl); } /// @inheritdoc IOracle function createRequests( Request[] calldata _requestsData, - bytes32[] calldata _ipfsHashes + bytes32[] calldata _ipfsHashes, + AccessControl memory _accessControl ) external returns (bytes32[] memory _batchRequestsIds) { uint256 _requestsAmount = _requestsData.length; _batchRequestsIds = new bytes32[](_requestsAmount); for (uint256 _i = 0; _i < _requestsAmount;) { - _batchRequestsIds[_i] = _createRequest(_requestsData[_i], _ipfsHashes[_i]); + _batchRequestsIds[_i] = _createRequest(_requestsData[_i], _ipfsHashes[_i], _accessControl); unchecked { ++_i; } @@ -107,8 +112,9 @@ contract Oracle is IOracle, AccessController { /// @inheritdoc IOracle function proposeResponse( Request calldata _request, - Response calldata _response - ) external returns (bytes32 _responseId) { + Response calldata _response, + AccessControl memory _accessControl + ) external hasAccess(_request.accessControlModule, msg.sender, _accessControl) returns (bytes32 _responseId) { _responseId = ValidatorLib._validateResponse(_request, _response); bytes32 _requestId = _response.requestId; @@ -118,7 +124,7 @@ contract Oracle is IOracle, AccessController { } // The caller must be the proposer, unless the response is coming from a dispute module - if (msg.sender != _response.proposer && msg.sender != address(_request.disputeModule)) { + if (_accessControl.user != _response.proposer && _accessControl.user != address(_request.disputeModule)) { revert Oracle_InvalidResponseBody(); } @@ -132,7 +138,7 @@ contract Oracle is IOracle, AccessController { } _participants[_requestId] = abi.encodePacked(_participants[_requestId], _response.proposer); - IResponseModule(_request.responseModule).propose(_request, _response, msg.sender); + IResponseModule(_request.responseModule).propose(_request, _response, _accessControl.user); _responseIds[_requestId] = abi.encodePacked(_responseIds[_requestId], _responseId); responseCreatedAt[_responseId] = uint128(block.number); @@ -143,8 +149,9 @@ contract Oracle is IOracle, AccessController { function disputeResponse( Request calldata _request, Response calldata _response, - Dispute calldata _dispute - ) external returns (bytes32 _disputeId) { + Dispute calldata _dispute, + AccessControl memory _accessControl + ) external hasAccess(_request.accessControlModule, msg.sender, _accessControl) returns (bytes32 _disputeId) { bytes32 _responseId; (_responseId, _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute); @@ -158,7 +165,7 @@ contract Oracle is IOracle, AccessController { revert Oracle_InvalidDisputeBody(); } - if (_dispute.disputer != msg.sender) { + if (_dispute.disputer != _accessControl.user) { revert Oracle_InvalidDisputeBody(); } @@ -170,7 +177,7 @@ contract Oracle is IOracle, AccessController { revert Oracle_ResponseAlreadyDisputed(_responseId); } - _participants[_requestId] = abi.encodePacked(_participants[_requestId], msg.sender); + _participants[_requestId] = abi.encodePacked(_participants[_requestId], _accessControl.user); disputeStatus[_disputeId] = DisputeStatus.Active; disputeOf[_responseId] = _disputeId; disputeCreatedAt[_disputeId] = uint128(block.number); @@ -202,7 +209,8 @@ contract Oracle is IOracle, AccessController { // Notify the dispute module about the escalation IDisputeModule(_request.disputeModule).onDisputeStatusChange(_disputeId, _request, _response, _dispute); - emit DisputeEscalated(msg.sender, _disputeId, block.number); + // TODO: Let's remove the msg.sender from this event and we can remove the access control + emit DisputeEscalated(msg.sender, _disputeId, block.number); if (address(_request.resolutionModule) != address(0)) { // Initiate the resolution @@ -234,6 +242,7 @@ contract Oracle is IOracle, AccessController { IResolutionModule(_request.resolutionModule).resolveDispute(_disputeId, _request, _response, _dispute); + // TODO: Same, remove the sender from this event. We don't really need it emit DisputeResolved(_disputeId, _dispute, msg.sender, block.number); } @@ -319,7 +328,11 @@ contract Oracle is IOracle, AccessController { } /// @inheritdoc IOracle - function finalize(IOracle.Request calldata _request, IOracle.Response calldata _response) external { + function finalize( + IOracle.Request calldata _request, + IOracle.Response calldata _response, + AccessControl memory _accessControl + ) external hasAccess(_request.accessControlModule, msg.sender, _accessControl) { bytes32 _requestId; bytes32 _responseId; @@ -430,7 +443,7 @@ contract Oracle is IOracle, AccessController { if (_request.nonce == 0) _request.nonce = uint96(_requestNonce); - if (msg.sender != _request.requester || _requestNonce != _request.nonce) { + if (_accessControl.user != _request.requester || _requestNonce != _request.nonce) { revert Oracle_InvalidRequestBody(); } @@ -447,8 +460,8 @@ contract Oracle is IOracle, AccessController { _request.finalityModule ); - _participants[_requestId] = abi.encodePacked(_participants[_requestId], msg.sender); - IRequestModule(_request.requestModule).createRequest(_requestId, _request.requestModuleData, msg.sender); + _participants[_requestId] = abi.encodePacked(_participants[_requestId], _accessControl.user); + IRequestModule(_request.requestModule).createRequest(_requestId, _request.requestModuleData, _accessControl.user); emit RequestCreated(_requestId, _request, _ipfsHash, block.number); } From dd5658fdee0d0c4ed48df7cf729a1304f3a1b05d Mon Sep 17 00:00:00 2001 From: shaito Date: Fri, 13 Sep 2024 10:24:20 -0300 Subject: [PATCH 5/6] fix: finalize --- solidity/contracts/Oracle.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol index a93e52d..00b1d46 100644 --- a/solidity/contracts/Oracle.sol +++ b/solidity/contracts/Oracle.sol @@ -350,18 +350,18 @@ contract Oracle is IOracle, AccessController { finalizedAt[_requestId] = uint128(block.number); if (address(_request.finalityModule) != address(0)) { - IFinalityModule(_request.finalityModule).finalizeRequest(_request, _response, msg.sender); + IFinalityModule(_request.finalityModule).finalizeRequest(_request, _response, _accessControl.user); } if (address(_request.resolutionModule) != address(0)) { - IResolutionModule(_request.resolutionModule).finalizeRequest(_request, _response, msg.sender); + IResolutionModule(_request.resolutionModule).finalizeRequest(_request, _response, _accessControl.user); } - IDisputeModule(_request.disputeModule).finalizeRequest(_request, _response, msg.sender); - IResponseModule(_request.responseModule).finalizeRequest(_request, _response, msg.sender); - IRequestModule(_request.requestModule).finalizeRequest(_request, _response, msg.sender); + IDisputeModule(_request.disputeModule).finalizeRequest(_request, _response, _accessControl.user); + IResponseModule(_request.responseModule).finalizeRequest(_request, _response, _accessControl.user); + IRequestModule(_request.requestModule).finalizeRequest(_request, _response, _accessControl.user); - emit OracleRequestFinalized(_requestId, _responseId, msg.sender, block.number); + emit OracleRequestFinalized(_requestId, _responseId, _accessControl.user, block.number); } /** From 663edd2ba8b2f809dff2bb8283787d4c655bb651 Mon Sep 17 00:00:00 2001 From: shaito Date: Wed, 25 Sep 2024 11:35:25 +0200 Subject: [PATCH 6/6] feat: permit access control --- foundry.toml | 2 +- package.json | 4 +- solidity/contracts/AccessController.sol | 22 +++++-- solidity/contracts/Oracle.sol | 20 ++++-- solidity/contracts/OracleTypehash.sol | 8 +++ .../contracts/PermitAccessControlModule.sol | 62 +++++++++++++++++++ .../accessControl/IAccessControlModule.sol | 8 ++- yarn.lock | 5 ++ 8 files changed, 118 insertions(+), 13 deletions(-) create mode 100644 solidity/contracts/OracleTypehash.sol create mode 100644 solidity/contracts/PermitAccessControlModule.sol diff --git a/foundry.toml b/foundry.toml index 9277e53..cd2980e 100644 --- a/foundry.toml +++ b/foundry.toml @@ -9,7 +9,7 @@ multiline_func_header = 'params_first' sort_imports = true [profile.default] -solc_version = '0.8.19' +solc_version = '0.8.20' src = 'solidity' test = 'solidity/test' out = 'out' diff --git a/package.json b/package.json index 74cc75c..52d454f 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "devDependencies": { "@commitlint/cli": "17.0.3", "@commitlint/config-conventional": "17.0.3", + "@openzeppelin/contracts": "^5.0.2", "dotenv-cli": "7.2.1", "ds-test": "https://github.com/dapphub/ds-test.git#e282159d5170298eb2455a6c05280ab5a73a4ef0", "forge-std": "https://github.com/foundry-rs/forge-std.git#v1.7.3", @@ -48,5 +49,6 @@ "solhint-plugin-defi-wonderland": "1.1.2", "sort-package-json": "2.4.1", "standard-version": "9.5.0" - } + }, + "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e" } diff --git a/solidity/contracts/AccessController.sol b/solidity/contracts/AccessController.sol index 09f3e4a..69beae7 100644 --- a/solidity/contracts/AccessController.sol +++ b/solidity/contracts/AccessController.sol @@ -14,17 +14,29 @@ abstract contract AccessController { bytes data; } + error AccessController_NoAccess(); + /** * @notice Modifier to check if the caller has access to the user * @param _accessControlModule The access control module - * @param _caller The caller * @param _accessControl The access control struct */ - modifier hasAccess(IAccessControlModule _accessControlModule, address _caller, AccessControl memory _accessControl) { - bool _hasAccess = _caller == _accessControl.user + modifier hasAccess( + address _accessControlModule, + bytes32 _typehash, + bytes memory _params, + AccessControl memory _accessControl + ) { + bool _hasAccess = msg.sender == _accessControl.user || ( - address(_accessControlModule) != address(0) - && _accessControlModule.hasAccess(_caller, _accessControl.user, _accessControl.data) + _accessControlModule != address(0) + && IAccessControlModule(_accessControlModule).hasAccess({ + _caller: msg.sender, + _user: _accessControl.user, + _typehash: _typehash, + _params: _params, + _data: _accessControl.data + }) ); if (!_hasAccess) revert AccessController_NoAccess(); _; diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol index 00b1d46..42ddf63 100644 --- a/solidity/contracts/Oracle.sol +++ b/solidity/contracts/Oracle.sol @@ -13,7 +13,9 @@ import {ValidatorLib} from '../libraries/ValidatorLib.sol'; import {AccessController} from './AccessController.sol'; -contract Oracle is IOracle, AccessController { +import {OracleTypehash} from './OracleTypehash.sol'; + +contract Oracle is IOracle, AccessController, OracleTypehash { using ValidatorLib for *; /// @inheritdoc IOracle @@ -114,7 +116,11 @@ contract Oracle is IOracle, AccessController { Request calldata _request, Response calldata _response, AccessControl memory _accessControl - ) external hasAccess(_request.accessControlModule, msg.sender, _accessControl) returns (bytes32 _responseId) { + ) + external + hasAccess(_request.accessControlModule, PROPOSE_TYPEHASH, abi.encode(_request, _response), _accessControl) + returns (bytes32 _responseId) + { _responseId = ValidatorLib._validateResponse(_request, _response); bytes32 _requestId = _response.requestId; @@ -210,7 +216,7 @@ contract Oracle is IOracle, AccessController { IDisputeModule(_request.disputeModule).onDisputeStatusChange(_disputeId, _request, _response, _dispute); // TODO: Let's remove the msg.sender from this event and we can remove the access control - emit DisputeEscalated(msg.sender, _disputeId, block.number); + emit DisputeEscalated(msg.sender, _disputeId, block.number); if (address(_request.resolutionModule) != address(0)) { // Initiate the resolution @@ -312,7 +318,9 @@ contract Oracle is IOracle, AccessController { } /// @inheritdoc IOracle - function getResponseIds(bytes32 _requestId) public view returns (bytes32[] memory _ids) { + function getResponseIds( + bytes32 _requestId + ) public view returns (bytes32[] memory _ids) { bytes memory _responses = _responseIds[_requestId]; uint256 _length = _responses.length / 32; @@ -370,7 +378,9 @@ contract Oracle is IOracle, AccessController { * @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) { + function _finalizeWithoutResponse( + IOracle.Request calldata _request + ) internal view returns (bytes32 _requestId) { _requestId = ValidatorLib._getId(_request); if (requestCreatedAt[_requestId] == 0) { diff --git a/solidity/contracts/OracleTypehash.sol b/solidity/contracts/OracleTypehash.sol new file mode 100644 index 0000000..38838b5 --- /dev/null +++ b/solidity/contracts/OracleTypehash.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +contract OracleTypehash { + bytes32 internal constant PROPOSE_TYPEHASH = keccak256('ProposeResponse(Request _request, Response _response)'); + bytes32 internal constant DISPUTE_TYPEHASH = + keccak256('DisputeResponse(Request _request, Response _response, Dispute _dispute,)'); +} diff --git a/solidity/contracts/PermitAccessControlModule.sol b/solidity/contracts/PermitAccessControlModule.sol new file mode 100644 index 0000000..2ce66c2 --- /dev/null +++ b/solidity/contracts/PermitAccessControlModule.sol @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IAccessControlModule} from '../interfaces/modules/accessControl/IAccessControlModule.sol'; +import {Nonces} from '@openzeppelin/contracts/utils/Nonces.sol'; + +import {ECDSA} from '@openzeppelin/contracts/utils/cryptography/ECDSA.sol'; +import {EIP712} from '@openzeppelin/contracts/utils/cryptography/EIP712.sol'; + +contract PermitAccessControl is Nonces, EIP712 { + /** + * @notice The access control struct + * @param user The address of the user + * @param data The data for access control validation + */ + struct AccessControlData { + uint256 nonce; + uint256 deadline; + uint8 v; + bytes32 r; + bytes32 s; + } + + error ERC2612ExpiredSignature(uint256 deadline); + error ERC2612InvalidSigner(address signer, address owner); + + /** + * @dev Initializes the {EIP712} domain separator using the `name` parameter, and setting `version` to `"1"`. + * + * It's a good idea to use the same `name` that is defined as the ERC-20 token name. + */ + constructor( + string memory name + ) EIP712(name, '1') {} + + function hasAccess( + address, + address _user, + bytes32 _signature, + bytes memory _params, + bytes calldata _data + ) external returns (bool _hasAccess) { + AccessControlData memory _permit = abi.decode(_data, (AccessControlData)); + // I don't think you care about validating the _caller + // You do care for repeatability, so a nonce is 100% necessary + // You care about expiration, so a deadline is 100% necessary + // You care abxout the function and the parameters that were approved + if (block.timestamp > _permit.deadline) { + revert ERC2612ExpiredSignature(_permit.deadline); + } + // signature, params (removing the last parameter for the access control), nonce, deadline + bytes32 structHash = keccak256(abi.encode(_signature, _params, _useNonce(_user), _permit.deadline)); + + bytes32 hash = _hashTypedDataV4(structHash); + + address signer = ECDSA.recover(hash, _permit.v, _permit.r, _permit.s); + + if (signer != _user) { + revert ERC2612InvalidSigner(signer, _user); + } + } +} diff --git a/solidity/interfaces/modules/accessControl/IAccessControlModule.sol b/solidity/interfaces/modules/accessControl/IAccessControlModule.sol index c2c2084..16dd503 100644 --- a/solidity/interfaces/modules/accessControl/IAccessControlModule.sol +++ b/solidity/interfaces/modules/accessControl/IAccessControlModule.sol @@ -8,5 +8,11 @@ import {IModule} from '../../IModule.sol'; * @notice Common interface for all response modules */ interface IAccessControlModule is IModule { - function hasAccess(address _caller, address _user, bytes calldata _data) external view returns (bool _hasAccess); + function hasAccess( + address _caller, + address _user, + bytes32 _typehash, + bytes memory _params, + bytes calldata _data + ) external returns (bool _hasAccess); } diff --git a/yarn.lock b/yarn.lock index 2c7a91b..fc66992 100644 --- a/yarn.lock +++ b/yarn.lock @@ -227,6 +227,11 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" +"@openzeppelin/contracts@^5.0.2": + version "5.0.2" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.2.tgz#b1d03075e49290d06570b2fd42154d76c2a5d210" + integrity sha512-ytPc6eLGcHHnapAZ9S+5qsdomhjo6QBHTDRRBFfTxXIpsicMhVPouPgmUPebZZZGX7vt9USA+Z+0M0dSVtSUEA== + "@solidity-parser/parser@^0.14.1": version "0.14.5" resolved "https://registry.yarnpkg.com/@solidity-parser/parser/-/parser-0.14.5.tgz#87bc3cc7b068e08195c219c91cd8ddff5ef1a804"