Skip to content

Commit

Permalink
Add SignatureValidatorManager contract (#109)
Browse files Browse the repository at this point in the history
* [#108] Create SignatureValidatorManager contract

* [#108] SignatureValidatorManager to be used as a function handler

* wip: tests

* [#108] Implement signature validation flow

* [#108] Add validator hooks

* [#108] Add test for signature validation with hooks

* [#108] Add tests for signature validator flow

* [#108] Add tests for signature validator, minor fix in Registry

* [#108] Rename ISafeProtocol712SignatureValidator to ISafeProtocolSignatureValidator, Update natspec doc

* [#108] Define signature selector as const, reorg signature selector tests

* [#108] Document layout of signatures

* [#108] Validate messageHash

* [#108] Add mocks and tests for signature validators with hooks

* [#108] Add tests for default validation flow, minor test updates

* [#108] Fix lint issues

* [#108] Update natspec for events

* [#108] Remove ISafeAccount interface

* [#108] Add domain separator in default validation flow

* [#108] Fix typo

* [#108] Use number as type for constants

* [#108] Use if-else in deciding validator routing

* [#108] use hash in defaultValidator instead of hash of hash

* [#108] Use Signature Validator Manager's domain separator and type hash for default valdiation

* Update contracts/SignatureValidatorManager.sol

Co-authored-by: Nicholas Rodrigues Lordello <[email protected]>

* [#108] Fix lint issue

* [#108] Define preise error reason, fix tests

* [#108] Create function that checks if contract supports interface in Registry

* [#108] SignatureMalidatorManager handle(...) function is view

---------

Co-authored-by: Nicholas Rodrigues Lordello <[email protected]>
  • Loading branch information
akshay-ap and nlordell authored Oct 30, 2023
1 parent a603214 commit 0f3ee32
Show file tree
Hide file tree
Showing 14 changed files with 963 additions and 46 deletions.
61 changes: 37 additions & 24 deletions contracts/SafeProtocolRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {ISafeProtocolRegistry} from "./interfaces/Registry.sol";
import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol";
import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
import {Enum} from "./common/Enum.sol";
import {ISafeProtocolFunctionHandler, ISafeProtocolHooks, ISafeProtocolPlugin} from "./interfaces/Modules.sol";
import {MODULE_TYPE_PLUGIN, MODULE_TYPE_HOOKS, MODULE_TYPE_FUNCTION_HANDLER} from "./common/Constants.sol";
import {ISafeProtocolFunctionHandler, ISafeProtocolHooks, ISafeProtocolPlugin, ISafeProtocolSignatureValidator, ISafeProtocolSignatureValidatorHooks} from "./interfaces/Modules.sol";
import {MODULE_TYPE_PLUGIN, MODULE_TYPE_HOOKS, MODULE_TYPE_FUNCTION_HANDLER, MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS, MODULE_TYPE_SIGNATURE_VALIDATOR} from "./common/Constants.sol";

contract SafeProtocolRegistry is ISafeProtocolRegistry, Ownable2Step {
mapping(address => ModuleInfo) public listedModules;
Expand All @@ -17,7 +17,8 @@ contract SafeProtocolRegistry is ISafeProtocolRegistry, Ownable2Step {
}

error CannotFlagModule(address module);
error CannotAddModule(address module, uint8 moduleTypes);
error ModuleAlreadyListed(address module);
error InvalidModuleType(address module, uint8 givenModuleType);
error ModuleDoesNotSupportExpectedInterfaceId(address module, bytes4 expectedInterfaceId);

event ModuleAdded(address indexed module);
Expand Down Expand Up @@ -59,36 +60,48 @@ contract SafeProtocolRegistry is ISafeProtocolRegistry, Ownable2Step {
ModuleInfo memory moduleInfo = listedModules[module];

// Check if module is already listed or if moduleTypes is greater than 8.
// Maximum allowed value of moduleTypes is 7. i.e. 2^0 (Plugin) + 2^1 (Function Handler) + 2^2 (Hooks)
if (moduleInfo.listedAt != 0 || moduleTypes > 7) {
revert CannotAddModule(module, moduleTypes);
if (moduleInfo.listedAt != 0) {
revert ModuleAlreadyListed(module);
}

// Check if module supports expected interface
if (
moduleTypes & MODULE_TYPE_HOOKS == MODULE_TYPE_HOOKS && !IERC165(module).supportsInterface(type(ISafeProtocolHooks).interfaceId)
) {
revert ModuleDoesNotSupportExpectedInterfaceId(module, type(ISafeProtocolHooks).interfaceId);
// Maximum allowed value of moduleTypes is 31. i.e. 2^0 (Plugin) + 2^1 (Function Handler) + 2^2 (Hooks) + 2^3 (Signature Validator hooks) + 2^4 (Signature Validator)
if (moduleTypes > 31) {
revert InvalidModuleType(module, moduleTypes);
}

if (
moduleTypes & MODULE_TYPE_PLUGIN == MODULE_TYPE_PLUGIN &&
!IERC165(module).supportsInterface(type(ISafeProtocolPlugin).interfaceId)
) {
revert ModuleDoesNotSupportExpectedInterfaceId(module, type(ISafeProtocolPlugin).interfaceId);
}

if (
moduleTypes & MODULE_TYPE_FUNCTION_HANDLER == MODULE_TYPE_FUNCTION_HANDLER &&
!IERC165(module).supportsInterface(type(ISafeProtocolFunctionHandler).interfaceId)
) {
revert ModuleDoesNotSupportExpectedInterfaceId(module, type(ISafeProtocolFunctionHandler).interfaceId);
}
optionalCheckInterfaceSupport(module, moduleTypes, MODULE_TYPE_PLUGIN, type(ISafeProtocolPlugin).interfaceId);
optionalCheckInterfaceSupport(module, moduleTypes, MODULE_TYPE_FUNCTION_HANDLER, type(ISafeProtocolFunctionHandler).interfaceId);
optionalCheckInterfaceSupport(module, moduleTypes, MODULE_TYPE_HOOKS, type(ISafeProtocolHooks).interfaceId);
optionalCheckInterfaceSupport(
module,
moduleTypes,
MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS,
type(ISafeProtocolSignatureValidatorHooks).interfaceId
);
optionalCheckInterfaceSupport(
module,
moduleTypes,
MODULE_TYPE_SIGNATURE_VALIDATOR,
type(ISafeProtocolSignatureValidator).interfaceId
);

listedModules[module] = ModuleInfo(uint64(block.timestamp), 0, moduleTypes);
emit ModuleAdded(module);
}

/**
* @notice This function checks if module supports expected interfaceId. This function will revert if module does not support expected interfaceId.
* @param module Address of the module
* @param moduleTypes uint8 representing the types of module
* @param moduleTypeToCheck uint8 representing the type of module to check
* @param interfaceId bytes4 representing the interfaceId to check
*/
function optionalCheckInterfaceSupport(address module, uint8 moduleTypes, uint8 moduleTypeToCheck, bytes4 interfaceId) internal view {
if (moduleTypes & moduleTypeToCheck == moduleTypeToCheck && !IERC165(module).supportsInterface(interfaceId)) {
revert ModuleDoesNotSupportExpectedInterfaceId(module, interfaceId);
}
}

/**
* @notice Allows only owner to flad a module. Only previously added module can be flagged.
* This function does not permit flagging a module twice.
Expand Down
231 changes: 231 additions & 0 deletions contracts/SignatureValidatorManager.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,231 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.18;
import {ISafeProtocolSignatureValidator} from "./interfaces/Modules.sol";
import {RegistryManager} from "./base/RegistryManager.sol";
import {ISafeProtocolFunctionHandler, ISafeProtocolSignatureValidatorHooks} from "./interfaces/Modules.sol";
import {MODULE_TYPE_SIGNATURE_VALIDATOR, MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS} from "./common/Constants.sol";
import {IAccount} from "./interfaces/Accounts.sol";
import {ISafeProtocolSignatureValidatorManager} from "./interfaces/Manager.sol";
import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";

/**
* @title SignatureValidatorManager
* @notice This contract facilitates signature validation. It maintains the signature validator(s) per account per domain or uses a default validation scheme based on the content of the data passed.
* This contract follows the Safe{Core} Protocol specification for signature validation. For more details on specification refer to https://github.com/safe-global/safe-core-protocol-specs.
* Implementaion of this contract is inspired by this pull request: https://github.com/rndlabs/safe-contracts/pull/1/files.
* Expected setup to use this contract for signature validation is as follows:
* - Account enables SafeProtocolManager as a fallback handler
* - Account sets SignatureValidatorManager as a function handler for a function selector e.g. 0x1626ba7e i.e. bytes4(keccak256("isValidSignature(bytes32,bytes)")
* @dev SignatureValidatorManager inherits RegistryManager leading to possible state drift of Registry address with the SafeProtocolManager contract.
* Do not set this contract as a fallback handler of a Safe account. Using this as a fallback handler would allow unauthorised setting of signature validator and signature validator hooks.
*/

contract SignatureValidatorManager is RegistryManager, ISafeProtocolFunctionHandler, ISafeProtocolSignatureValidatorManager {
constructor(address _registry, address _initialOwner) RegistryManager(_registry, _initialOwner) {}

// constants
// Signature selector bytes4(keccak256("Account712Signature(bytes32,bytes32,bytes)"));
bytes4 public constant SIGNATURE_VALIDATOR_SELECTOR = 0xb5c726cb;

// keccak256("SafeMessage(bytes message)");
bytes32 private constant ACCOUNT_MSG_TYPEHASH = 0x60b3cbf8b4a223d68d641b3b6ddf9a298e7f33710cf3d3a9d1146b5a6150fbca;

// keccak256(
// "EIP712Domain(uint256 chainId,address verifyingContract)"
// );
bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = 0x47e79534a245952e8b16893a336b85a3d9ea9fa8c573f3d803afb92a79469218;

// Storage
/**
* @notice Mapping to account address => domain separator => signature validator contract
*/
mapping(address => mapping(bytes32 => address)) public signatureValidators;

/**
* @notice Mapping to account address => signature validator hooks contract
*/
mapping(address => address) public signatureValidatorHooks;

// Events
/**
* @notice Only one type of event is emitted for simplicity rather one for each individual case: remvoing, updating, adding new signature validator.
*/
event SignatureValidatorChanged(address indexed account, bytes32 indexed domainSeparator, address indexed signatureValidator);

/**
* @notice Only one type of event is emitted for simplicity rather one for each individual case: remvoing, updating, adding new signature validator hooks.
*/
event SignatureValidatorHooksChanged(address indexed account, address indexed signatureValidatorHooks);

// Errors
error SignatureValidatorNotSet(address account);
error InvalidMessageHash(bytes32 messageHash);

/**
* @notice Sets the signature validator contract for an account
* @param signatureValidator Address of the signature validator contract
*/
function setSignatureValidator(bytes32 domainSeparator, address signatureValidator) external {
if (signatureValidator != address(0)) {
checkPermittedModule(signatureValidator, MODULE_TYPE_SIGNATURE_VALIDATOR);

if (!ISafeProtocolSignatureValidator(signatureValidator).supportsInterface(type(ISafeProtocolSignatureValidator).interfaceId))
revert ContractDoesNotImplementValidInterfaceId(signatureValidator);
}
signatureValidators[msg.sender][domainSeparator] = signatureValidator;

emit SignatureValidatorChanged(msg.sender, domainSeparator, signatureValidator);
}

/**
* @notice Sets the signature validator hooks for an account
* @param signatureValidatorHooksAddress Address of the signature validator hooks contract
*/
function setSignatureValidatorHooks(address signatureValidatorHooksAddress) external override {
if (signatureValidatorHooksAddress != address(0)) {
checkPermittedModule(signatureValidatorHooksAddress, MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS);

if (
!ISafeProtocolSignatureValidatorHooks(signatureValidatorHooksAddress).supportsInterface(
type(ISafeProtocolSignatureValidatorHooks).interfaceId
)
) revert ContractDoesNotImplementValidInterfaceId(signatureValidatorHooksAddress);
}
signatureValidatorHooks[msg.sender] = signatureValidatorHooksAddress;

emit SignatureValidatorHooksChanged(msg.sender, signatureValidatorHooksAddress);
}

/**
* @notice A view function that the Manager will call when an account has enabled this contract as a function handler in the Manager
* @param account Address of the account whose signature validator is to be used
* @param sender Address requesting signature validation
* @param data Calldata containing the 4 bytes function selector, 32 bytes message hash and payload.
* Layout of data:
* 0x00 to 0x04 - 4 bytes function selector when this contract is set as a function handler in the SafeProtocolManager i.e. 0x1626ba7e
* 0x04 to 0x24 - 32 bytes hash of the message used for signing
* 0x24 to end - bytes containing signatures or signatureData either one of the below:
* If first 4 bytes of signatureData are 0xb5c726cb i.e. bytes4(keccak256("Account712Signature(bytes32,bytes32,bytes)")); then it will be interpreted as follows:
* payload = abi.encodeWithSelector(0xb5c726cb, abi.encode(domainSeparator, structHash, signatures)
* Layout of `data` parameter in this case:
* 0x00 to 0x04 - 4 bytes function selector when this contract is set as a function handler in the SafeProtocolManager i.e. 0x1626ba7e
* 0x04 to 0x24 - 32 bytes hash of the signed message
* 0x24 to 0x44 - 32 bytes offset to the start of `bytes` parameter
* 0x44 to 0x64 - 32 bytes length of `bytes` parameter
* 0x64 to 0x68 - 4 bytes of Signature selector
* 0x68 to 0x88 - 32 bytes domain separator
* 0x88 to 0xa8 - 32 bytes struct hash
* 0xa8 to end - contains offset, length of bytes, and actual bytes containing signatures
* Else:
* bytes containing signature data
* default validation flow will be used which will depend on the account implementation
*
*/
function handle(
address account,
address sender,
uint256 /* value */,
bytes calldata data
) external view override returns (bytes memory) {
// Skip first 4 bytes of data as it contains function selector
(bytes32 messageHash, bytes memory signatureData) = abi.decode(data[0x4:], (bytes32, bytes));

address signatureValidatorHooksAddress = signatureValidatorHooks[account];
bytes memory prevalidationData;
bytes memory returnData;

if (signatureValidatorHooksAddress != address(0)) {
checkPermittedModule(signatureValidatorHooksAddress, MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS);
prevalidationData = ISafeProtocolSignatureValidatorHooks(signatureValidatorHooksAddress).preValidationHook(
account,
sender,
data
);
}

if (bytes4(data[0x64:0x68]) == SIGNATURE_VALIDATOR_SELECTOR) {
returnData = abi.encode(validateWithSignatureValdiator(account, sender, messageHash, data[0x68:]));
} else {
returnData = defaultValidator(account, messageHash, signatureData);
}

if (signatureValidatorHooksAddress != address(0)) {
ISafeProtocolSignatureValidatorHooks(signatureValidatorHooksAddress).postValidationHook(account, prevalidationData);
}
return returnData;
}

/**
* @notice A view function for default signature validation flow.
* @param account Address of the account whose signature is to be validated. Account should support function `checkSignatures(bytes32, bytes, bytes)`
* @param hash bytes32 hash of the data that is signed
* @param signatures Arbitrary length bytes array containing the signatures
*/
function defaultValidator(address account, bytes32 hash, bytes memory signatures) internal view returns (bytes memory) {
bytes memory messageData = abi.encodePacked(
bytes1(0x19),
bytes1(0x01),
keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, block.chainid, account)),
keccak256(abi.encode(ACCOUNT_MSG_TYPEHASH, keccak256(abi.encode(hash))))
);
bytes32 messageHash = keccak256(messageData);
IAccount(account).checkSignatures(messageHash, messageData, signatures);
// bytes4(keccak256("isValidSignature(bytes32,bytes)")
return abi.encode(0x1626ba7e);
}

/**
*
* @param account Address of the account whose signature is to be validated
* @param sender Address of the entitty that requested for signature validation
* @param messageHash Hash of the message that is signed
* @param data Arbitrary length bytes array containing the domain separator, struct hash and signatures
*/
function validateWithSignatureValdiator(
address account,
address sender,
bytes32 messageHash,
bytes calldata data
) internal view returns (bytes4) {
(bytes32 domainSeparator, bytes32 structHash, bytes memory signatures) = abi.decode(data, (bytes32, bytes32, bytes));

if (keccak256(abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator, structHash)) != messageHash) {
revert InvalidMessageHash(messageHash);
}

address signatureValidator = signatureValidators[account][domainSeparator];
if (signatureValidator == address(0)) {
revert SignatureValidatorNotSet(account);
}

checkPermittedModule(signatureValidator, MODULE_TYPE_SIGNATURE_VALIDATOR);

return
ISafeProtocolSignatureValidator(signatureValidator).isValidSignature(
account,
sender,
messageHash,
domainSeparator,
structHash,
signatures
);
}

/**
* @notice A function that returns module information.
* @return providerType uint256 Type of metadata provider
* @return location Arbitrary length bytes data containing the location of the metadata provider
*/
function metadataProvider() external view override returns (uint256 providerType, bytes memory location) {}

/**
* @param interfaceId bytes4 interface id to be checked
* @return true if interface is supported
*/
function supportsInterface(bytes4 interfaceId) external view override returns (bool) {
return
interfaceId == type(IERC165).interfaceId ||
interfaceId == type(ISafeProtocolSignatureValidatorManager).interfaceId ||
interfaceId == type(ISafeProtocolFunctionHandler).interfaceId;
}
}
2 changes: 2 additions & 0 deletions contracts/common/Constants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ uint8 constant PLUGIN_PERMISSION_EXECUTE_DELEGATECALL = 4;
uint8 constant MODULE_TYPE_PLUGIN = 1;
uint8 constant MODULE_TYPE_FUNCTION_HANDLER = 2;
uint8 constant MODULE_TYPE_HOOKS = 4;
uint8 constant MODULE_TYPE_SIGNATURE_VALIDATOR_HOOKS = 8;
uint8 constant MODULE_TYPE_SIGNATURE_VALIDATOR = 16;
2 changes: 2 additions & 0 deletions contracts/interfaces/Accounts.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@ interface IAccount {
bytes memory data,
uint8 operation
) external returns (bool success, bytes memory returnData);

function checkSignatures(bytes32 dataHash, bytes memory data, bytes memory signatures) external view;
}
13 changes: 13 additions & 0 deletions contracts/interfaces/Manager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,16 @@ interface ISafeProtocolManager {
*/
function executeRootAccess(address account, SafeRootAccess calldata rootAccess) external returns (bytes memory data);
}

interface ISafeProtocolSignatureValidatorManager {
/**
* @param domainSeparator bytes32 containing the domain for which Signature Validator contract should be used
* @param signatureValidatorContract Address of the Signature Validator Contract implementing ISafeProtocolSignatureValidator interface
*/
function setSignatureValidator(bytes32 domainSeparator, address signatureValidatorContract) external;

/**
* @param signatureValidatorHooksContract Address of the contract to be used as Hooks for Signature Validator implementing ISignatureValidatorHook interface
*/
function setSignatureValidatorHooks(address signatureValidatorHooksContract) external;
}
Loading

0 comments on commit 0f3ee32

Please sign in to comment.