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.

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

[🔥A1] Modifier.sol Transactions with EIP-712 and EIP-1271 Signature Authentication #137

Merged
merged 41 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
5d288d7
First crack at the base class that does the EIP712 appended signature…
cristovaoth Sep 27, 2023
868ce59
Include EIP712 signature auth logic into Modifier.sol,
cristovaoth Sep 28, 2023
e975b8d
Add tests ensuring Modifier is managing nonce correctly
cristovaoth Sep 28, 2023
df35063
Naming improvements
cristovaoth Sep 28, 2023
3f71d04
Use block.chainid instead of opening an assembley block
cristovaoth Sep 28, 2023
4ac7bd7
Move moduleOnly only to more straigthforward shape
cristovaoth Sep 28, 2023
f45187e
Signer should not define the relayer's message value.
cristovaoth Sep 28, 2023
f6d7d08
Rename root type for EIP712 data structure
cristovaoth Sep 28, 2023
2b30406
Add natspec
cristovaoth Sep 28, 2023
72b4f25
Extract badSignature checks to their own test cases, instead of mixed…
cristovaoth Sep 28, 2023
12b96e5
Rename helper function
cristovaoth Sep 28, 2023
cd85276
Renames in preparation for eip-1271 signatures
cristovaoth Sep 29, 2023
148fb8a
Add eip-1271 support
cristovaoth Sep 30, 2023
300c80a
Rename helper function
cristovaoth Sep 30, 2023
557e372
Change visibility of moduleTxHash. Comments
cristovaoth Oct 1, 2023
09cdbb2
Update comments. Use v,s,r order. Add test cases
cristovaoth Oct 1, 2023
e10e352
Adjust comments
cristovaoth Oct 1, 2023
d05f894
Adjust comments
cristovaoth Oct 2, 2023
7c6c8ef
Introduce GuardableModule and GuardableModifier - this makes Module a…
cristovaoth Oct 4, 2023
446cd08
GuardableModifier tests
cristovaoth Oct 7, 2023
cc0e322
Bumping OpenZeppeling for bytecode optimized base classes
cristovaoth Oct 7, 2023
f098f7d
remove import
cristovaoth Oct 7, 2023
2cf6cda
Mark exec functions as virtual for Guardable*
cristovaoth Oct 8, 2023
d2a122d
Prettier
cristovaoth Oct 8, 2023
022394c
Make execTransactionFromModule virtual
cristovaoth Oct 9, 2023
fc032b7
Add comment to Modifier.sol
cristovaoth Oct 9, 2023
7960593
Break down Modifier authorization logic in reusable parts
cristovaoth Oct 12, 2023
9093ce3
Add failing tests
cristovaoth Oct 12, 2023
9aaae1b
Changing the base SignatureChecker to work with user proposed salt, i…
cristovaoth Oct 12, 2023
ea645eb
Introduce execution tracking into Modifier.sol based on executed and …
cristovaoth Oct 12, 2023
dd9e84c
Rename auxiliary function
cristovaoth Oct 12, 2023
908adeb
Moving the newly introducedd errors into ExecutionTracker
cristovaoth Oct 12, 2023
cd40d32
Fully cover guard with tests
cristovaoth Oct 12, 2023
91d32ee
Make sentOrSigned by an internal helper function for Modifiers
cristovaoth Oct 13, 2023
c4211a4
Make moduleTxSignedBy return always the hash, even when no signer fou…
cristovaoth Oct 18, 2023
41f16eb
Consolidate hash tracking into one mapping
cristovaoth Oct 18, 2023
9c1306e
Ensure contract specific signature is sliced correctly
cristovaoth Oct 19, 2023
637dcf1
Simplify splitSignature, and pull up logic that assembles the isValid…
cristovaoth Oct 25, 2023
64971c5
Add test for contract specific signature empty
cristovaoth Oct 25, 2023
9f65e4d
Remove aux variable
cristovaoth Oct 25, 2023
267725a
Consistently return hash from moduleTxSignedBy()
cristovaoth Oct 26, 2023
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
92 changes: 92 additions & 0 deletions contracts/core/GuardableModifier.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity >=0.7.0 <0.9.0;

import "../guard/Guardable.sol";
import "./Modifier.sol";

abstract contract GuardableModifier is Module, Guardable, Modifier {
/// @dev Passes a transaction to be executed by the avatar.
/// @notice Can only be called by this contract.
/// @param to Destination address of module transaction.
/// @param value Ether value of module transaction.
/// @param data Data payload of module transaction.
/// @param operation Operation type of module transaction: 0 == call, 1 == delegate call.
function exec(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation
) internal virtual override returns (bool success) {
address currentGuard = guard;
if (currentGuard != address(0)) {
IGuard(currentGuard).checkTransaction(
/// Transaction info used by module transactions.
to,
value,
data,
operation,
/// Zero out the redundant transaction information only used for Safe multisig transctions.
0,
0,
0,
address(0),
payable(0),
"",
sentOrSignedBy()
);
}
success = IAvatar(target).execTransactionFromModule(
to,
value,
data,
operation
);
if (currentGuard != address(0)) {
IGuard(currentGuard).checkAfterExecution(bytes32(0), success);
}
}

/// @dev Passes a transaction to be executed by the target and returns data.
/// @notice Can only be called by this contract.
/// @param to Destination address of module transaction.
/// @param value Ether value of module transaction.
/// @param data Data payload of module transaction.
/// @param operation Operation type of module transaction: 0 == call, 1 == delegate call.
function execAndReturnData(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation
)
internal
virtual
override
returns (bool success, bytes memory returnData)
{
address currentGuard = guard;
if (currentGuard != address(0)) {
IGuard(currentGuard).checkTransaction(
/// Transaction info used by module transactions.
to,
value,
data,
operation,
/// Zero out the redundant transaction information only used for Safe multisig transctions.
0,
0,
0,
address(0),
payable(0),
"",
sentOrSignedBy()
);
}

(success, returnData) = IAvatar(target)
.execTransactionFromModuleReturnData(to, value, data, operation);

if (currentGuard != address(0)) {
IGuard(currentGuard).checkAfterExecution(bytes32(0), success);
}
}
}
93 changes: 93 additions & 0 deletions contracts/core/GuardableModule.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity >=0.7.0 <0.9.0;

import "../guard/Guardable.sol";
import "./Module.sol";

/// @title GuardableModule - A contract that can pass messages to a Module Manager contract if enabled by that contract.
abstract contract GuardableModule is Module, Guardable {
/// @dev Passes a transaction to be executed by the avatar.
/// @notice Can only be called by this contract.
/// @param to Destination address of module transaction.
/// @param value Ether value of module transaction.
/// @param data Data payload of module transaction.
/// @param operation Operation type of module transaction: 0 == call, 1 == delegate call.
function exec(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation
) internal override returns (bool success) {
address currentGuard = guard;
if (currentGuard != address(0)) {
IGuard(currentGuard).checkTransaction(
/// Transaction info used by module transactions.
to,
value,
data,
operation,
/// Zero out the redundant transaction information only used for Safe multisig transctions.
0,
0,
0,
address(0),
payable(0),
"",
msg.sender
);
}
success = IAvatar(target).execTransactionFromModule(
to,
value,
data,
operation
);
if (currentGuard != address(0)) {
IGuard(currentGuard).checkAfterExecution(bytes32(0), success);
}
}

/// @dev Passes a transaction to be executed by the target and returns data.
/// @notice Can only be called by this contract.
/// @param to Destination address of module transaction.
/// @param value Ether value of module transaction.
/// @param data Data payload of module transaction.
/// @param operation Operation type of module transaction: 0 == call, 1 == delegate call.
function execAndReturnData(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation
)
internal
virtual
override
returns (bool success, bytes memory returnData)
{
address currentGuard = guard;
if (currentGuard != address(0)) {
IGuard(currentGuard).checkTransaction(
/// Transaction info used by module transactions.
to,
value,
data,
operation,
/// Zero out the redundant transaction information only used for Safe multisig transctions.
0,
0,
0,
address(0),
payable(0),
"",
msg.sender
);
}

(success, returnData) = IAvatar(target)
.execTransactionFromModuleReturnData(to, value, data, operation);

if (currentGuard != address(0)) {
IGuard(currentGuard).checkAfterExecution(bytes32(0), success);
}
}
}
57 changes: 44 additions & 13 deletions contracts/core/Modifier.sol
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
// SPDX-License-Identifier: LGPL-3.0-only

/// @title Modifier Interface - A contract that sits between a Module and an Avatar and enforce some additional logic.
pragma solidity >=0.7.0 <0.9.0;

import "../interfaces/IAvatar.sol";
import "../signature/ExecutionTracker.sol";
import "../signature/SignatureChecker.sol";
import "./Module.sol";

abstract contract Modifier is Module, IAvatar {
/// @title Modifier Interface - A contract that sits between a Module and an Avatar and enforce some additional logic.
abstract contract Modifier is
Module,
ExecutionTracker,
SignatureChecker,
IAvatar
{
address internal constant SENTINEL_MODULES = address(0x1);
/// Mapping of modules.
mapping(address => address) internal modules;
Expand All @@ -32,8 +38,10 @@ abstract contract Modifier is Module, IAvatar {

/*
--------------------------------------------------
You must override at least one of following two virtual functions,
You must override both of the following virtual functions,
execTransactionFromModule() and execTransactionFromModuleReturnData().
It is recommended that implementations of both functions make use the
onlyModule modifier.
*/

/// @dev Passes a transaction to the modifier.
Expand All @@ -47,7 +55,7 @@ abstract contract Modifier is Module, IAvatar {
uint256 value,
bytes calldata data,
Enum.Operation operation
) public virtual override moduleOnly returns (bool success) {}
) public virtual returns (bool success);

/// @dev Passes a transaction to the modifier, expects return data.
/// @notice Can only be called by enabled modules.
Expand All @@ -60,23 +68,46 @@ abstract contract Modifier is Module, IAvatar {
uint256 value,
bytes calldata data,
Enum.Operation operation
)
public
virtual
override
moduleOnly
returns (bool success, bytes memory returnData)
{}
) public virtual returns (bool success, bytes memory returnData);

/*
--------------------------------------------------
*/

modifier moduleOnly() {
if (modules[msg.sender] == address(0)) revert NotAuthorized(msg.sender);
if (modules[msg.sender] == address(0)) {
cristovaoth marked this conversation as resolved.
Show resolved Hide resolved
(bytes32 hash, address signer) = moduleTxSignedBy();

// is the signer a module?
if (modules[signer] == address(0)) {
revert NotAuthorized(msg.sender);
}

// is the provided signature fresh?
if (consumed[signer][hash]) {
revert HashAlreadyConsumed(hash);
}

consumed[signer][hash] = true;
emit HashExecuted(hash);
}

_;
}

function sentOrSignedBy() internal view returns (address) {
if (modules[msg.sender] != address(0)) {
return msg.sender;
}

(, address signer) = moduleTxSignedBy();
if (modules[signer] != address(0)) {
return signer;
}

return address(0);
}

/// @dev Disables a module on the modifier.
/// @notice This can only be called by the owner.
/// @param prevModule Module that pointed to the module to be removed in the linked list.
Expand Down
62 changes: 15 additions & 47 deletions contracts/core/Module.sol
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
// SPDX-License-Identifier: LGPL-3.0-only

/// @title Module Interface - A contract that can pass messages to a Module Manager contract if enabled by that contract.
pragma solidity >=0.7.0 <0.9.0;

import "../interfaces/IAvatar.sol";
import "../factory/FactoryFriendly.sol";
import "../guard/Guardable.sol";
import "../interfaces/IAvatar.sol";

abstract contract Module is FactoryFriendly, Guardable {
/// @title Module Interface - A contract that can pass messages to a Module Manager contract if enabled by that contract.
abstract contract Module is FactoryFriendly {
/// @dev Address that will ultimately execute function calls.
address public avatar;
/// @dev Address that this module will pass transactions to.
Expand Down Expand Up @@ -45,8 +43,14 @@ abstract contract Module is FactoryFriendly, Guardable {
uint256 value,
bytes memory data,
Enum.Operation operation
) internal returns (bool success) {
(success, ) = _exec(to, value, data, operation);
) internal virtual returns (bool success) {
return
IAvatar(target).execTransactionFromModule(
to,
value,
data,
operation
);
}

/// @dev Passes a transaction to be executed by the target and returns data.
Expand All @@ -60,49 +64,13 @@ abstract contract Module is FactoryFriendly, Guardable {
uint256 value,
bytes memory data,
Enum.Operation operation
) internal returns (bool success, bytes memory returnData) {
(success, returnData) = _exec(to, value, data, operation);
}

function _exec(
address to,
uint256 value,
bytes memory data,
Enum.Operation operation
) private returns (bool success, bytes memory returnData) {
address currentGuard = guard;
if (currentGuard != address(0)) {
IGuard(currentGuard).checkTransaction(
/// Transaction info used by module transactions.
) internal virtual returns (bool success, bytes memory returnData) {
return
IAvatar(target).execTransactionFromModuleReturnData(
to,
value,
data,
operation,
/// Zero out the redundant transaction information only used for Safe multisig transctions.
0,
0,
0,
address(0),
payable(0),
"",
msg.sender
operation
);
(success, returnData) = IAvatar(target)
.execTransactionFromModuleReturnData(
to,
value,
data,
operation
);
IGuard(currentGuard).checkAfterExecution("", success);
} else {
(success, returnData) = IAvatar(target)
.execTransactionFromModuleReturnData(
to,
value,
data,
operation
);
}
}
}
8 changes: 4 additions & 4 deletions contracts/factory/ModuleProxyFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ contract ModuleProxyFactory {
/// @notice Initialization failed.
error FailedInitialization();

function createProxy(address target, bytes32 salt)
internal
returns (address result)
{
function createProxy(
address target,
bytes32 salt
) internal returns (address result) {
if (address(target) == address(0)) revert ZeroAddress(target);
if (address(target).code.length == 0) revert TargetHasNoCode(target);
bytes memory deployment = abi.encodePacked(
Expand Down
Loading
Loading