Skip to content

Commit

Permalink
feat: encode call
Browse files Browse the repository at this point in the history
  • Loading branch information
ashitakah committed Oct 28, 2024
1 parent d9ce30a commit 6ae19ce
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 97 deletions.
18 changes: 11 additions & 7 deletions solidity/contracts/AccessController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,17 @@ abstract contract AccessController is IAccessController {
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
})
&& IAccessControlModule(_accessControlModule).hasAccess(
abi.encode(
IAccessControlModule.AccessControlParameters({
sender: msg.sender,
user: _accessControl.user,
typehash: _typehash,
data: _accessControl.data,
params: _params
})
)
)
);
if (!_hasAccess) revert AccessControlData_NoAccess();
_;
Expand Down
26 changes: 7 additions & 19 deletions solidity/contracts/Oracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import {
_DISPUTE_TYPEHASH,
_ESCALATE_TYPEHASH,
_FINALIZE_TYPEHASH,
_PROPOSE_TYPEHASH,
_UPDATE_TYPEHASH
_PROPOSE_TYPEHASH
} from './utils/OracleTypehash.sol';

contract Oracle is IOracle, AccessController {
Expand Down Expand Up @@ -308,18 +307,8 @@ contract Oracle is IOracle, AccessController {
Request memory _request,
Response memory _response,
Dispute memory _dispute,
DisputeStatus _status,
AccessControl calldata _accessControl
)
external
isApproved(_accessControl.user, _request.accessControlModule)
hasAccess(
_request.accessControlModule,
_UPDATE_TYPEHASH,
abi.encode(_request, _response, _dispute, _status),
_accessControl
)
{
DisputeStatus _status
) external {
(bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);

if (disputeCreatedAt[_disputeId] == 0) {
Expand All @@ -336,11 +325,9 @@ contract Oracle is IOracle, AccessController {
Dispute memory _currentDispute = _dispute;
DisputeStatus _currentStatus = _status;

if (
_accessControl.user != address(_currentRequest.disputeModule)
&& _accessControl.user != address(_currentRequest.resolutionModule)
) {
revert Oracle_NotDisputeOrResolutionModule(_accessControl.user);
if (msg.sender != address(_currentRequest.disputeModule) && msg.sender != address(_currentRequest.resolutionModule))
{
revert Oracle_NotDisputeOrResolutionModule(msg.sender);
}
disputeStatus[_disputeId] = _currentStatus;
IDisputeModule(_currentRequest.disputeModule).onDisputeStatusChange(
Expand All @@ -357,6 +344,7 @@ contract Oracle is IOracle, AccessController {
AccessControl calldata _accessControl
)
external
isApproved(_accessControl.user, _request.accessControlModule)
hasAccess(_request.accessControlModule, _FINALIZE_TYPEHASH, abi.encode(_request, _response), _accessControl)
{
bytes32 _requestId;
Expand Down
3 changes: 0 additions & 3 deletions solidity/contracts/utils/OracleTypehash.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,4 @@ bytes32 constant _DISPUTE_TYPEHASH = keccak256('DisputeResponse(Request _request

bytes32 constant _ESCALATE_TYPEHASH = keccak256('EscalateDispute(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)');
4 changes: 1 addition & 3 deletions solidity/interfaces/IOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -481,14 +481,12 @@ interface IOracle is IAccessController {
* @param _response The disputed response
* @param _dispute The dispute that is being updated
* @param _status The new status of the dispute
* @param _accessControl The access control data
*/
function updateDisputeStatus(
Request calldata _request,
Response calldata _response,
Dispute calldata _dispute,
DisputeStatus _status,
AccessControl calldata _accessControl
DisputeStatus _status
) external;

/**
Expand Down
35 changes: 24 additions & 11 deletions solidity/interfaces/modules/accessControl/IAccessControlModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,33 @@ import {IModule} from '../../IModule.sol';
* @notice Common interface for all response modules
*/
interface IAccessControlModule is IModule {
/*///////////////////////////////////////////////////////////////
STRUCTS
//////////////////////////////////////////////////////////////*/

/**
* @notice Access control parameters
* @param sender The address of the sender
* @param user The address of the user
* @param typehash The typehash of the access control
* @param data The data for access control validation
* @param params The parameters for access control validation
*/
struct AccessControlParameters {
address sender;
address user;
bytes32 typehash;
bytes data;
bytes params;
}

/*///////////////////////////////////////////////////////////////
LOGIC
//////////////////////////////////////////////////////////////*/
/**
* @notice Checks if the caller has access to the user
* @param _caller The caller address
* @param _user The user address
* @param _typehash The typehash of the request
* @param _params The parameters of the request
* @param _data The data for access control validation
* @return _hasAccess True if the caller has access to the user
*/
function hasAccess(
address _caller,
address _user,
bytes32 _typehash,
bytes memory _params,
bytes calldata _data
) external returns (bool _hasAccess);
function hasAccess(bytes calldata _data) external returns (bool _hasAccess);
}
22 changes: 13 additions & 9 deletions solidity/test/mocks/contracts/MockAccessControlModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity ^0.8.19;
import {Module} from '../../../contracts/Module.sol';

import {IOracle} from '../../../interfaces/IOracle.sol';
import {IMockAccessControlModule} from '../interfaces/IMockAccessControlModule.sol';
import {IAccessControlModule, IMockAccessControlModule} from '../interfaces/IMockAccessControlModule.sol';

contract MockAccessControlModule is Module, IMockAccessControlModule {
mapping(address _caller => bool _hasAccess) public callerHasAccess;
Expand All @@ -19,13 +19,17 @@ contract MockAccessControlModule is Module, IMockAccessControlModule {

function moduleName() external view returns (string memory _moduleName) {}

function hasAccess(
address _caller,
address,
bytes32,
bytes memory,
bytes memory
) external view override returns (bool) {
return callerHasAccess[_caller];
function hasAccess(bytes memory _data) external view override returns (bool _hasAccess) {
IAccessControlModule.AccessControlParameters memory _accessControlData = decodeAccesControlData(_data);
_hasAccess = callerHasAccess[_accessControlData.sender];
}

function decodeAccesControlData(bytes memory _data)
public
pure
override
returns (IAccessControlModule.AccessControlParameters memory _accessControlData)
{
_accessControlData = abi.decode(_data, (IAccessControlModule.AccessControlParameters));
}
}
7 changes: 6 additions & 1 deletion solidity/test/mocks/interfaces/IMockAccessControlModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,9 @@ pragma solidity ^0.8.19;

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

interface IMockAccessControlModule is IAccessControlModule {}
interface IMockAccessControlModule is IAccessControlModule {
function decodeAccesControlData(bytes calldata _data)
external
view
returns (AccessControlParameters memory _accessControlData);
}
54 changes: 10 additions & 44 deletions solidity/test/unit/Oracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -750,9 +750,7 @@ contract Oracle_Unit_DisputeResponse is BaseTest {

contract Oracle_Unit_UpdateDisputeStatus is BaseTest {
modifier happyPath() {
mockAccessControl.user = address(disputeModule);
vm.startPrank(address(disputeModule));
oracle.mock_setAccessControlApproved(address(disputeModule), address(accessControlModule), true);
_;
}
/**
Expand Down Expand Up @@ -787,27 +785,14 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest {
emit DisputeStatusUpdated(_disputeId, mockDispute, IOracle.DisputeStatus(_newStatus));

// Test: change the status
oracle.updateDisputeStatus(
mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus(_newStatus), mockAccessControl
);
oracle.updateDisputeStatus(mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus(_newStatus));

// Check: correct status stored?
assertEq(_newStatus, uint256(oracle.disputeStatus(_disputeId)));
}
}
}

/**
* @notice Check that update dispute status with a non-approved access control module reverts
*/
function test_updateDisputeStatus_revertsIfNotApproved() public {
// Check: revert?
vm.expectRevert(IOracle.Oracle_AccessControlModuleNotApproved.selector);

// Test: try to create the request
oracle.updateDisputeStatus(mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus.Active, mockAccessControl);
}

/**
* @notice Providing a dispute that does not match the response should revert
*/
Expand All @@ -825,27 +810,7 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest {
vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_InvalidDisputeId.selector, _disputeId));

// Test: Try to update the dispute
oracle.updateDisputeStatus(
mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus(_newStatus), mockAccessControl
);
}

/**
* @notice Revert if the access control module returns false
*/
function test_updateDisputeStatus_revertsIfInvalidAccessControlData(address _caller) public {
vm.assume(_caller != address(disputeModule));

mockRequest.accessControlModule = address(0);
mockAccessControl.user = address(disputeModule);
oracle.mock_setAccessControlApproved(_caller, address(accessControlModule), true);

// Check: revert?
vm.expectRevert(IAccessController.AccessControlData_NoAccess.selector);

// Test: try to propose a response from a random address
vm.prank(_caller);
oracle.updateDisputeStatus(mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus.Active, mockAccessControl);
oracle.updateDisputeStatus(mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus(_newStatus));
}

/**
Expand All @@ -863,7 +828,6 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest {
oracle.mock_setDisputeCreatedAt(_disputeId, block.timestamp);
oracle.mock_setAccessControlApproved(proposer, address(accessControlModule), true);

mockAccessControl.user = proposer;
vm.mockCall(
address(accessControlModule), abi.encodeWithSelector(IAccessControlModule.hasAccess.selector), abi.encode(true)
);
Expand All @@ -873,9 +837,7 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest {

// Test: try to update the status from an EOA
vm.prank(proposer);
oracle.updateDisputeStatus(
mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus(_newStatus), mockAccessControl
);
oracle.updateDisputeStatus(mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus(_newStatus));
}

/**
Expand All @@ -884,8 +846,6 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest {
function test_updateDisputeStatus_revertsIfInvalidDispute() public {
bytes32 _disputeId = _getId(mockDispute);

mockAccessControl.user = address(resolutionModule);

oracle.mock_setDisputeCreatedAt(_disputeId, 0);
oracle.mock_setAccessControlApproved(address(resolutionModule), address(accessControlModule), true);

Expand All @@ -894,7 +854,7 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest {

// Test: try to update the status
vm.prank(address(resolutionModule));
oracle.updateDisputeStatus(mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus.Active, mockAccessControl);
oracle.updateDisputeStatus(mockRequest, mockResponse, mockDispute, IOracle.DisputeStatus.Active);
}
}

Expand Down Expand Up @@ -1077,6 +1037,7 @@ contract Oracle_Unit_Finalize is BaseTest {

modifier happyPath() {
mockAccessControl.user = address(requester);
oracle.mock_setAccessControlApproved(mockAccessControl.user, address(accessControlModule), true);
vm.startPrank(address(requester));
_;
}
Expand All @@ -1094,6 +1055,7 @@ contract Oracle_Unit_Finalize is BaseTest {
bytes32 _responseId = _getId(mockResponse);

mockAccessControl.user = _caller;
oracle.mock_setAccessControlApproved(mockAccessControl.user, address(accessControlModule), true);

oracle.mock_addResponseId(_requestId, _responseId);
oracle.mock_setRequestCreatedAt(_requestId, block.timestamp);
Expand Down Expand Up @@ -1236,6 +1198,7 @@ contract Oracle_Unit_Finalize is BaseTest {
oracle.mock_setRequestCreatedAt(_requestId, block.timestamp);
mockResponse.requestId = bytes32(0);
mockAccessControl.user = _caller;
oracle.mock_setAccessControlApproved(mockAccessControl.user, address(accessControlModule), true);

// Create mock request and store it
bytes memory _calldata = abi.encodeCall(IModule.finalizeRequest, (mockRequest, mockResponse, _caller));
Expand Down Expand Up @@ -1277,7 +1240,9 @@ contract Oracle_Unit_Finalize is BaseTest {

bytes32 _requestId = _getId(mockRequest);
oracle.mock_setRequestCreatedAt(_requestId, block.timestamp);

mockAccessControl.user = _caller;
oracle.mock_setAccessControlApproved(mockAccessControl.user, address(accessControlModule), true);

IOracle.DisputeStatus _disputeStatus = IOracle.DisputeStatus(_status);

Expand Down Expand Up @@ -1318,6 +1283,7 @@ contract Oracle_Unit_Finalize is BaseTest {
oracle.mock_setRequestCreatedAt(_getId(mockRequest), block.timestamp);

mockAccessControl.user = _caller;
oracle.mock_setAccessControlApproved(mockAccessControl.user, address(accessControlModule), true);

// Test: finalize a finalized request
vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AlreadyFinalized.selector, _requestId));
Expand Down

0 comments on commit 6ae19ce

Please sign in to comment.