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.

Sign up for GitHub

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

Jump to bottom

feat: access control #52

Merged
merged 20 commits into from
Nov 12, 2024
Merged

feat: access control #52

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b3222f2
feat: access control
ashitakah Oct 17, 2024
2a2a4f5
feat: access control
ashitakah Oct 17, 2024
bfebad1
fix: comments
ashitakah Oct 24, 2024
7fd9bbf
feat: tests
ashitakah Oct 24, 2024
d9ce30a
feat: modifier and tests
ashitakah Oct 24, 2024
6ae19ce
feat: encode call
ashitakah Oct 28, 2024
5852f90
fix: move to contract to export
ashitakah Oct 28, 2024
c4d3d48
feat: comments
ashitakah Oct 31, 2024
3afab79
feat: move logic and tests
ashitakah Nov 4, 2024
69e47b0
fix: comments
ashitakah Nov 5, 2024
2fc32d4
feat: eoa test
ashitakah Nov 6, 2024
69efe2b
fix: access module finalize
ashitakah Nov 6, 2024
2b68ec0
refactor: access module (#53)
xorsal Nov 7, 2024
0b8a5be
refactor: merge `hasAccess` and `isApproved` modifiers
gas1cent Nov 10, 2024
8bb062e
test: explicit mocking in Validator unit tests
gas1cent Nov 10, 2024
84fe5c0
feat: refactor has access
ashitakah Nov 11, 2024
47120e0
fix: typo
ashitakah Nov 11, 2024
d34d4ba
refactor: eliminate extra variables
gas1cent Nov 12, 2024
ba7cf26
fix: duplicated finalize call
gas1cent Nov 12, 2024
1d1e5a3
docs: natspec fixes
gas1cent Nov 12, 2024
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
33 changes: 33 additions & 0 deletions solidity/contracts/AccessController.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {IAccessController} from '../interfaces/IAccessController.sol';
import {IAccessControlModule} from '../interfaces/modules/accessControl/IAccessControlModule.sol';

abstract contract AccessController is IAccessController {
/**
* @notice Modifier to check if the caller has access to the user
* @param _accessControlModule The access control module
* @param _accessControl The access control struct
*/
modifier hasAccess(
address _accessControlModule,
bytes32 _typehash,
bytes memory _params,
AccessControl memory _accessControl
) {
bool _hasAccess = msg.sender == _accessControl.user
|| (
_accessControlModule != address(0)
&& IAccessControlModule(_accessControlModule).hasAccess({
_caller: msg.sender,
_user: _accessControl.user,
_typehash: _typehash,
_params: _params,
_data: _accessControl.data
})
);
if (!_hasAccess) revert AccessControlData_NoAccess();
_;
}
}
197 changes: 135 additions & 62 deletions solidity/contracts/Oracle.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,19 @@ 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 {ValidatorLib} from '../libraries/ValidatorLib.sol';

contract Oracle is IOracle {
import {AccessController} from './AccessController.sol';

import {
_CREATE_TYPEHASH,
_DISPUTE_TYPEHASH,
_ESCALATE_TYPEHASH,
_FINALIZE_TYPEHASH,
_PROPOSE_TYPEHASH,
_RESOLVE_TYPEHASH,
_UPDATE_TYPEHASH
} from './utils/OracleTypehash.sol';

contract Oracle is IOracle, AccessController {
using ValidatorLib for *;

/// @inheritdoc IOracle
Expand Down Expand Up @@ -44,31 +55,29 @@ contract Oracle is IOracle {
/// @inheritdoc IOracle
mapping(bytes32 _requestId => mapping(address _user => bool _isParticipant)) public isParticipant;

/// @inheritdoc IOracle
mapping(address _user => mapping(address _accessControlModule => bool _approved)) public isAccessControlApproved;
ashitakah marked this conversation as resolved.
Show resolved Hide resolved
0xJabberwock marked this conversation as resolved.
Show resolved Hide resolved

/// @inheritdoc IOracle
uint256 public totalRequestCount;

/**
* @notice The list of the response ids for each request
*/
mapping(bytes32 _requestId => bytes _responseIds) internal _responseIds;

/// @inheritdoc IOracle
uint256 public totalRequestCount;

/// @inheritdoc IOracle
function createRequest(Request calldata _request, bytes32 _ipfsHash) external returns (bytes32 _requestId) {
_requestId = _createRequest(_request, _ipfsHash);
}
function getResponseIds(bytes32 _requestId) public view returns (bytes32[] memory _ids) {
bytes memory _responses = _responseIds[_requestId];
uint256 _length = _responses.length / 32;

/// @inheritdoc IOracle
function createRequests(
Request[] calldata _requestsData,
bytes32[] calldata _ipfsHashes
) external returns (bytes32[] memory _batchRequestsIds) {
uint256 _requestsAmount = _requestsData.length;
_batchRequestsIds = new bytes32[](_requestsAmount);
assembly {
for { let _i := 0 } lt(_i, _length) { _i := add(_i, 1) } {
// Increase the size of the array
mstore(_ids, add(mload(_ids), 1))

for (uint256 _i = 0; _i < _requestsAmount;) {
_batchRequestsIds[_i] = _createRequest(_requestsData[_i], _ipfsHashes[_i]);
unchecked {
++_i;
// Store the response id in the array
mstore(add(_ids, add(32, mul(_i, 32))), mload(add(_responses, add(32, mul(_i, 32)))))
}
}
}
Expand Down Expand Up @@ -98,11 +107,42 @@ contract Oracle is IOracle {
}
}

/// @inheritdoc IOracle
function createRequest(
Request calldata _request,
bytes32 _ipfsHash,
AccessControl calldata _accessControl
) external returns (bytes32 _requestId) {
_requestId = _createRequest(_request, _ipfsHash, _accessControl);
}

/// @inheritdoc IOracle
function createRequests(
Request[] calldata _requestsData,
bytes32[] calldata _ipfsHashes,
AccessControl[] calldata _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], _accessControl[_i]);
unchecked {
++_i;
}
}
}

/// @inheritdoc IOracle
function proposeResponse(
ashitakah marked this conversation as resolved.
Show resolved Hide resolved
Request calldata _request,
Response calldata _response
) external returns (bytes32 _responseId) {
Response calldata _response,
AccessControl calldata _accessControl
)
external
hasAccess(_request.accessControlModule, _PROPOSE_TYPEHASH, abi.encode(_request, _response), _accessControl)
returns (bytes32 _responseId)
{
_responseId = ValidatorLib._validateResponse(_request, _response);

bytes32 _requestId = _response.requestId;
Expand All @@ -112,7 +152,7 @@ contract Oracle is IOracle {
}

// 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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I correctly understand the new intended behaviour:

  • the user, not the caller, must be the proposer,
  • even though the response may come from a dispute module (i.e., CircuitResolverModule or RootVerificationModule).

Copy link
Member

@0xJabberwock 0xJabberwock Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such dispute modules may set themselves as the access control users but not as the proposers; considering that that is the new expected behaviour:

  • The user, not the caller, must be the proposer,
  • unless the response is coming from a dispute module (i.e., CircuitResolverModule or RootVerificationModule).

The code comment should be updated accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

revert Oracle_InvalidProposer();
}

Expand All @@ -125,7 +165,7 @@ contract Oracle is IOracle {
revert Oracle_AlreadyFinalized(_requestId);
}
isParticipant[_requestId][_response.proposer] = true;
IResponseModule(_request.responseModule).propose(_request, _response, msg.sender);
IResponseModule(_request.responseModule).propose(_request, _response, _accessControl.user);
xorsal marked this conversation as resolved.
Show resolved Hide resolved
_responseIds[_requestId] = abi.encodePacked(_responseIds[_requestId], _responseId);
responseCreatedAt[_responseId] = block.timestamp;

Expand All @@ -136,8 +176,13 @@ contract Oracle is IOracle {
function disputeResponse(
Request calldata _request,
Response calldata _response,
Dispute calldata _dispute
) external returns (bytes32 _disputeId) {
Dispute calldata _dispute,
AccessControl calldata _accessControl
)
external
hasAccess(_request.accessControlModule, _DISPUTE_TYPEHASH, abi.encode(_request, _response, _dispute), _accessControl)
returns (bytes32 _disputeId)
{
bytes32 _responseId;
(_responseId, _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);

Expand All @@ -151,7 +196,7 @@ contract Oracle is IOracle {
revert Oracle_InvalidProposer();
}

if (_dispute.disputer != msg.sender) {
if (_dispute.disputer != _accessControl.user) {
revert Oracle_InvalidDisputer();
}

Expand All @@ -162,7 +207,7 @@ contract Oracle is IOracle {
if (disputeOf[_responseId] != bytes32(0)) {
revert Oracle_ResponseAlreadyDisputed(_responseId);
}
isParticipant[_requestId][msg.sender] = true;
isParticipant[_requestId][_accessControl.user] = true;
ashitakah marked this conversation as resolved.
Show resolved Hide resolved
disputeStatus[_disputeId] = DisputeStatus.Active;
disputeOf[_responseId] = _disputeId;
disputeCreatedAt[_disputeId] = block.timestamp;
Expand All @@ -173,7 +218,15 @@ contract Oracle is IOracle {
}

/// @inheritdoc IOracle
function escalateDispute(Request calldata _request, Response calldata _response, Dispute calldata _dispute) external {
function escalateDispute(
Request calldata _request,
Response calldata _response,
Dispute calldata _dispute,
AccessControl calldata _accessControl
0xJabberwock marked this conversation as resolved.
Show resolved Hide resolved
)
external
hasAccess(_request.accessControlModule, _ESCALATE_TYPEHASH, abi.encode(_request, _response, _dispute), _accessControl)
{
(bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);

if (disputeCreatedAt[_disputeId] == 0) {
Expand All @@ -194,7 +247,7 @@ contract Oracle is IOracle {
// Notify the dispute module about the escalation
IDisputeModule(_request.disputeModule).onDisputeStatusChange(_disputeId, _request, _response, _dispute);

emit DisputeEscalated(msg.sender, _disputeId, _dispute);
emit DisputeEscalated(_accessControl.user, _disputeId, _dispute);

if (address(_request.resolutionModule) != address(0)) {
// Initiate the resolution
Expand All @@ -203,7 +256,15 @@ contract Oracle is IOracle {
}

/// @inheritdoc IOracle
function resolveDispute(Request calldata _request, Response calldata _response, Dispute calldata _dispute) external {
function resolveDispute(
Request calldata _request,
Response calldata _response,
Dispute calldata _dispute,
AccessControl calldata _accessControl
)
external
hasAccess(_request.accessControlModule, _RESOLVE_TYPEHASH, abi.encode(_request, _response, _dispute), _accessControl)
ashitakah marked this conversation as resolved.
Show resolved Hide resolved
{
(bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);

if (disputeCreatedAt[_disputeId] == 0) {
Expand All @@ -226,16 +287,25 @@ contract Oracle is IOracle {

IResolutionModule(_request.resolutionModule).resolveDispute(_disputeId, _request, _response, _dispute);

emit DisputeResolved(_disputeId, _dispute, msg.sender);
emit DisputeResolved(_disputeId, _dispute);
}

/// @inheritdoc IOracle
function updateDisputeStatus(
ashitakah marked this conversation as resolved.
Show resolved Hide resolved
Request calldata _request,
Response calldata _response,
Dispute calldata _dispute,
DisputeStatus _status
) external {
DisputeStatus _status,
AccessControl calldata _accessControl
)
external
hasAccess(
_request.accessControlModule,
_UPDATE_TYPEHASH,
abi.encode(_request, _response, _dispute, _status),
_accessControl
)
{
(bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);

if (disputeCreatedAt[_disputeId] == 0) {
Expand All @@ -246,8 +316,11 @@ contract Oracle is IOracle {
revert Oracle_InvalidDisputeId(_disputeId);
}

if (msg.sender != address(_request.disputeModule) && msg.sender != address(_request.resolutionModule)) {
revert Oracle_NotDisputeOrResolutionModule(msg.sender);
if (
_accessControl.user != address(_request.disputeModule)
&& _accessControl.user != address(_request.resolutionModule)
) {
revert Oracle_NotDisputeOrResolutionModule(_accessControl.user);
ashitakah marked this conversation as resolved.
Show resolved Hide resolved
}
disputeStatus[_disputeId] = _status;
IDisputeModule(_request.disputeModule).onDisputeStatusChange(_disputeId, _request, _response, _dispute);
Expand All @@ -256,23 +329,14 @@ contract Oracle is IOracle {
}

/// @inheritdoc IOracle
function getResponseIds(bytes32 _requestId) public view returns (bytes32[] memory _ids) {
bytes memory _responses = _responseIds[_requestId];
uint256 _length = _responses.length / 32;

assembly {
for { let _i := 0 } lt(_i, _length) { _i := add(_i, 1) } {
// Increase the size of the array
mstore(_ids, add(mload(_ids), 1))

// Store the response id in the array
mstore(add(_ids, add(32, mul(_i, 32))), mload(add(_responses, add(32, mul(_i, 32)))))
}
}
}

/// @inheritdoc IOracle
function finalize(IOracle.Request calldata _request, IOracle.Response calldata _response) external {
function finalize(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the msg.senders in finalize? Is there a use for it? If not sure, can you ask in the prophet channel what the intended use is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented in the discord channel

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be meaningful to revise what each existing implementation of the Prophet modules expects as to the _finalizer parameter of finalizeRequest().

  • Apparently, all dispute and resolution modules don't utilize it.
  • Moreover, all request and finality modules only do it upon the emission of RequestFinalized event (except for EBORequestModule).
  • However, the unique response module does so with the aim of identifying whether the argument is an Oracle::allowedModule, apart from for the event emission.
  • Lastly, mind that CircuitResolverModule and RootVerificationModule (both dispute modules) and EBO's CouncilArbitrator (not a module) call Oracle::finalize.

IOracle.Request calldata _request,
IOracle.Response calldata _response,
AccessControl calldata _accessControl
0xJabberwock marked this conversation as resolved.
Show resolved Hide resolved
)
external
hasAccess(_request.accessControlModule, _FINALIZE_TYPEHASH, abi.encode(_request, _response), _accessControl)
{
bytes32 _requestId;
bytes32 _responseId;

Expand All @@ -290,18 +354,18 @@ contract Oracle is IOracle {
finalizedAt[_requestId] = block.timestamp;

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);
emit OracleRequestFinalized(_requestId, _responseId);
0xJabberwock marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -372,14 +436,23 @@ contract Oracle is IOracle {
*
* @param _request The request to be created
* @param _ipfsHash The hashed IPFS CID of the metadata json
* @param _accessControl The access control struct
* @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 calldata _accessControl
)
internal
hasAccess(_request.accessControlModule, _CREATE_TYPEHASH, abi.encode(_request), _accessControl)
ashitakah marked this conversation as resolved.
Show resolved Hide resolved
returns (bytes32 _requestId)
{
uint256 _requestNonce = totalRequestCount++;

if (_request.nonce == 0) _request.nonce = uint96(_requestNonce);

if (msg.sender != _request.requester || _requestNonce != _request.nonce) {
if (_accessControl.user != _request.requester || _requestNonce != _request.nonce) {
ashitakah marked this conversation as resolved.
Show resolved Hide resolved
revert Oracle_InvalidRequestBody();
}

Expand All @@ -393,9 +466,9 @@ contract Oracle is IOracle {
allowedModule[_requestId][_request.resolutionModule] = true;
allowedModule[_requestId][_request.finalityModule] = true;

isParticipant[_requestId][msg.sender] = true;
isParticipant[_requestId][_accessControl.user] = true;

IRequestModule(_request.requestModule).createRequest(_requestId, _request.requestModuleData, msg.sender);
IRequestModule(_request.requestModule).createRequest(_requestId, _request.requestModuleData, _accessControl.user);

emit RequestCreated(_requestId, _request, _ipfsHash);
}
Expand Down
18 changes: 18 additions & 0 deletions solidity/contracts/utils/OracleTypehash.sol
View file
Open in desktop
0xJabberwock marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

bytes32 constant _CREATE_TYPEHASH =
keccak256('CreateRequest(Request _request,bytes32 _ipfsHash,AccessControl _accessControl');

bytes32 constant _PROPOSE_TYPEHASH = keccak256('ProposeResponse(Request _request,Response _response)');

bytes32 constant _DISPUTE_TYPEHASH = keccak256('DisputeResponse(Request _request,Response _response,Dispute _dispute)');

bytes32 constant _ESCALATE_TYPEHASH = keccak256('EscalateDispute(Request _request,Response _response,Dispute _dispute)');

bytes32 constant _RESOLVE_TYPEHASH = keccak256('ResolveDispute(Request _request,Response _response,Dispute _dispute)');

bytes32 constant _UPDATE_TYPEHASH =
keccak256('UpdateDisputeStatus(Request _request,Response _response,Dispute _dispute,DisputeStatus _status)');

bytes32 constant _FINALIZE_TYPEHASH = keccak256('Finalize(Request _request,Response _response)');
Loading
Loading