Skip to content
This repository has been archived by the owner on Nov 27, 2024. It is now read-only.

feat: Gasless Meta Transactions #208

Merged
merged 7 commits into from
Apr 10, 2023
Merged

feat: Gasless Meta Transactions #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bc43ff6
_createAction internal function
0xrajath Apr 3, 2023
b10d25c
createActionBySig()
0xrajath Apr 3, 2023
e822ba0
Fixing castApprovalBySig and castDisapprovalBySig
0xrajath Apr 3, 2023
7601542
CreateActionBySig test scaffold
0xrajath Apr 3, 2023
a85c083
using nonces for replay protection
0xrajath Apr 3, 2023
a512c3a
Scaffold for tests
0xrajath Apr 4, 2023
bf6b206
User + selector for nonce calculation
0xrajath Apr 6, 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
223 changes: 171 additions & 52 deletions src/VertexCore.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,21 @@ contract VertexCore is Initializable {
// =============================================================

/// @notice EIP-712 base typehash.
bytes32 public constant DOMAIN_TYPEHASH =
keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");
bytes32 public constant EIP712_DOMAIN_TYPEHASH =
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");

/// @notice EIP-712 approval typehash.
bytes32 public constant APPROVAL_EMITTED_TYPEHASH = keccak256("ApprovalCast(uint256 id,address policyholder)");
/// @notice EIP-712 createAction typehash.
bytes32 public constant CREATE_ACTION_TYPEHASH = keccak256(
"CreateAction(uint8 role,address strategy,address target,uint256 value,bytes4 selector,bytes data,address policyholder,uint256 nonce)"
);

/// @notice EIP-712 castApproval typehash.
bytes32 public constant CAST_APPROVAL_TYPEHASH =
keccak256("CastApproval(uint256 actionId,uint8 role,string reason,address policyholder,uint256 nonce)");

/// @notice EIP-712 disapproval typehash.
bytes32 public constant DISAPPROVAL_EMITTED_TYPEHASH = keccak256("DisapprovalCast(uint256 id,address policyholder)");
/// @notice EIP-712 castDisapproval typehash.
bytes32 public constant CAST_DISAPPROVAL_TYPEHASH =
keccak256("CastDisapproval(uint256 actionId,uint8 role,string reason,address policyholder,uint256 nonce)");

/// @notice Equivalent to 100%, but scaled for precision
uint256 internal constant ONE_HUNDRED_IN_BPS = 10_000;
Expand Down Expand Up @@ -114,6 +121,11 @@ contract VertexCore is Initializable {
/// @notice Mapping of all authorized strategies.
mapping(VertexStrategy => bool) public authorizedStrategies;

/// @notice Mapping of users to function selectors to current nonces for EIP-712 signatures.
/// @dev This is used to prevent replay attacks by incrementing the nonce for each operation (createAction,
/// castApproval and castDisapproval) signed by the policyholder.
mapping(address => mapping(bytes4 => uint256)) public nonces;

// ======================================================
// ======== Contract Creation and Initialization ========
// ======================================================
Expand Down Expand Up @@ -148,7 +160,8 @@ contract VertexCore is Initializable {
// ===========================================

/// @notice Creates an action. The creator needs to hold a policy with the permissionId of the provided
/// strategy, target, selector.
/// {target, selector, strategy}.
/// @param role The role that will be used to determine the permissionId of the policy holder.
/// @param strategy The VertexStrategy contract that will determine how the action is executed.
/// @param target The contract called when the action is executed.
/// @param value The value in wei to be sent when the action is executed.
Expand All @@ -163,42 +176,60 @@ contract VertexCore is Initializable {
bytes4 selector,
bytes calldata data
) external returns (uint256 actionId) {
if (!authorizedStrategies[strategy]) revert InvalidStrategy();

PermissionData memory permission = PermissionData(target, selector, strategy);
bytes32 permissionId = keccak256(abi.encode(permission));

// Typically (such as in Governor contracts) this should check that the caller has permission
// at `block.number|timestamp - 1` but here we're just checking if the caller *currently* has
// permission. Technically this introduces a race condition if e.g. an action to revoke a role
// from someone (or revoke a permission from a role) is ready to be executed at the same time as
// an action is created, as the order of transactions in the block then affects if action
// creation would succeed. However, we are ok with this tradeoff because it means we don't need
// to checkpoint the `canCreateAction` mapping which is simpler and cheaper, and in practice
// this race condition is unlikely to matter.
if (!policy.hasPermissionId(msg.sender, role, permissionId)) revert PolicyholderDoesNotHavePermission();

actionId = actionsCount;
Action storage newAction = actions[actionId];

// Revert if the policy has no supply for any provided roles.
(uint256 approvalPolicySupply, uint256 disapprovalPolicySupply) = _assertNonZeroRoleSupplies(strategy);

newAction.creator = msg.sender;
newAction.strategy = strategy;
newAction.target = target;
newAction.value = value;
newAction.selector = selector;
newAction.data = data;
newAction.creationTime = block.timestamp;
newAction.approvalPolicySupply = approvalPolicySupply;
newAction.disapprovalPolicySupply = disapprovalPolicySupply;

unchecked {
++actionsCount;
}
actionId = _createAction(msg.sender, role, strategy, target, value, selector, data);
}

emit ActionCreated(actionId, msg.sender, strategy, target, value, selector, data);
/// @notice Creates an action via an off-chain signature. The creator needs to hold a policy with the permissionId of
/// the provided {target, selector, strategy}.
/// @param role The role that will be used to determine the permissionId of the policy holder.
/// @param strategy The VertexStrategy contract that will determine how the action is executed.
/// @param target The contract called when the action is executed.
/// @param value The value in wei to be sent when the action is executed.
/// @param selector The function selector that will be called when the action is executed.
/// @param data The encoded arguments to be passed to the function that is called when the action is executed.
/// @param user The user that signed the message.
/// @param v ECDSA signature component: Parity of the `y` coordinate of point `R`
/// @param r ECDSA signature component: x-coordinate of `R`
/// @param s ECDSA signature component: `s` value of the signature
/// @return actionId actionId of the newly created action.
function createActionBySig(
uint8 role,
VertexStrategy strategy,
address target,
uint256 value,
bytes4 selector,
bytes calldata data,
address user,
uint8 v,
bytes32 r,
bytes32 s
) external returns (uint256 actionId) {
bytes32 digest = keccak256(
0xrajath marked this conversation as resolved.
Show resolved Hide resolved
abi.encodePacked(
0xrajath marked this conversation as resolved.
Show resolved Hide resolved
"\x19\x01",
keccak256(
abi.encode(
EIP712_DOMAIN_TYPEHASH, keccak256(bytes(name)), keccak256(bytes("1")), block.chainid, address(this)
)
),
keccak256(
abi.encode(
CREATE_ACTION_TYPEHASH,
role,
address(strategy),
target,
value,
selector,
keccak256(data),
user,
_useNonce(user, msg.sig)
)
)
)
);
address signer = ecrecover(digest, v, r, s);
0xrajath marked this conversation as resolved.
Show resolved Hide resolved
if (signer == address(0) || signer != user) revert InvalidSignature();
actionId = _createAction(signer, role, strategy, target, value, selector, data);
}

/// @notice Queue an action by actionId if it's in Approved state.
Expand Down Expand Up @@ -272,20 +303,36 @@ contract VertexCore is Initializable {
/// @notice How policyholders add their support of the approval of an action via an off-chain signature.
/// @param actionId The id of the action.
/// @param role The role the policyholder uses to cast their approval.
/// @param reason The reason given for the approval by the policyholder.
/// @param user The user that signed the message.
/// @param v ECDSA signature component: Parity of the `y` coordinate of point `R`
/// @param r ECDSA signature component: x-coordinate of `R`
/// @param s ECDSA signature component: `s` value of the signature
function castApprovalBySig(uint256 actionId, uint8 role, uint8 v, bytes32 r, bytes32 s) external {
function castApprovalBySig(
uint256 actionId,
uint8 role,
string calldata reason,
address user,
uint8 v,
bytes32 r,
bytes32 s
) external {
bytes32 digest = keccak256(
abi.encodePacked(
"\x19\x01",
keccak256(abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name)), block.chainid, address(this))),
keccak256(abi.encode(APPROVAL_EMITTED_TYPEHASH, actionId, msg.sender))
keccak256(
abi.encode(
EIP712_DOMAIN_TYPEHASH, keccak256(bytes(name)), keccak256(bytes("1")), block.chainid, address(this)
)
),
keccak256(
abi.encode(CAST_APPROVAL_TYPEHASH, actionId, role, keccak256(bytes(reason)), user, _useNonce(user, msg.sig))
)
)
);
address signer = ecrecover(digest, v, r, s);
if (signer == address(0)) revert InvalidSignature();
return _castApproval(signer, role, actionId, "");
if (signer == address(0) || signer != user) revert InvalidSignature();
return _castApproval(signer, role, actionId, reason);
}

/// @notice How policyholders add their support of the disapproval of an action.
Expand All @@ -306,20 +353,38 @@ contract VertexCore is Initializable {
/// @notice How policyholders add their support of the disapproval of an action via an off-chain signature.
/// @param actionId The id of the action.
/// @param role The role the policyholder uses to cast their disapproval.
/// @param reason The reason given for the approval by the policyholder.
/// @param user The user that signed the message.
/// @param v ECDSA signature component: Parity of the `y` coordinate of point `R`
/// @param r ECDSA signature component: x-coordinate of `R`
/// @param s ECDSA signature component: `s` value of the signature
function castDisapprovalBySig(uint256 actionId, uint8 role, uint8 v, bytes32 r, bytes32 s) external {
function castDisapprovalBySig(
uint256 actionId,
uint8 role,
string calldata reason,
address user,
uint8 v,
bytes32 r,
bytes32 s
) external {
bytes32 digest = keccak256(
abi.encodePacked(
"\x19\x01",
keccak256(abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name)), block.chainid, address(this))),
keccak256(abi.encode(DISAPPROVAL_EMITTED_TYPEHASH, actionId, msg.sender))
keccak256(
abi.encode(
EIP712_DOMAIN_TYPEHASH, keccak256(bytes(name)), keccak256(bytes("1")), block.chainid, address(this)
)
),
keccak256(
abi.encode(
CAST_DISAPPROVAL_TYPEHASH, actionId, role, keccak256(bytes(reason)), user, _useNonce(user, msg.sig)
)
)
)
);
address signer = ecrecover(digest, v, r, s);
if (signer == address(0)) revert InvalidSignature();
return _castDisapproval(signer, role, actionId, "");
if (signer == address(0) || signer != user) revert InvalidSignature();
return _castDisapproval(signer, role, actionId, reason);
}

/// @notice Deploy new strategies and add them to the mapping of authorized strategies.
Expand Down Expand Up @@ -396,6 +461,53 @@ contract VertexCore is Initializable {
// ======== Internal Logic ========
// ================================

function _createAction(
address policyholder,
uint8 role,
VertexStrategy strategy,
address target,
uint256 value,
bytes4 selector,
bytes calldata data
) internal returns (uint256 actionId) {
if (!authorizedStrategies[strategy]) revert InvalidStrategy();

PermissionData memory permission = PermissionData(target, selector, strategy);
bytes32 permissionId = keccak256(abi.encode(permission));

// Typically (such as in Governor contracts) this should check that the caller has permission
// at `block.number|timestamp - 1` but here we're just checking if the caller *currently* has
// permission. Technically this introduces a race condition if e.g. an action to revoke a role
// from someone (or revoke a permission from a role) is ready to be executed at the same time as
// an action is created, as the order of transactions in the block then affects if action
// creation would succeed. However, we are ok with this tradeoff because it means we don't need
// to checkpoint the `canCreateAction` mapping which is simpler and cheaper, and in practice
// this race condition is unlikely to matter.
if (!policy.hasPermissionId(policyholder, role, permissionId)) revert PolicyholderDoesNotHavePermission();

actionId = actionsCount;
Action storage newAction = actions[actionId];

// Revert if the policy has no supply for any provided roles.
(uint256 approvalPolicySupply, uint256 disapprovalPolicySupply) = _assertNonZeroRoleSupplies(strategy);

newAction.creator = policyholder;
newAction.strategy = strategy;
newAction.target = target;
newAction.value = value;
newAction.selector = selector;
newAction.data = data;
newAction.creationTime = block.timestamp;
newAction.approvalPolicySupply = approvalPolicySupply;
newAction.disapprovalPolicySupply = disapprovalPolicySupply;

unchecked {
++actionsCount;
}

emit ActionCreated(actionId, policyholder, strategy, target, value, selector, data);
}

function _castApproval(address policyholder, uint8 role, uint256 actionId, string memory reason) internal {
if (getActionState(actionId) != ActionState.Active) revert ActionNotActive();
bool hasApproved = approvals[actionId][policyholder];
Expand Down Expand Up @@ -502,4 +614,11 @@ contract VertexCore is Initializable {
disapprovalPolicySupply = policy.getSupply(disapprovalRole);
if (disapprovalPolicySupply == 0) revert RoleHasZeroSupply(disapprovalRole);
}

function _useNonce(address user, bytes4 selector) internal returns (uint256 nonce) {
nonce = nonces[user][selector];
unchecked {
nonces[user][selector] = nonce + 1;
}
}
}
70 changes: 66 additions & 4 deletions test/VertexCore.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,36 @@ contract CreateAction is VertexCoreTest {
}
}

contract CreateActionBySig is VertexCoreTest {
function test_SuccessfulCreateActionBySignature() public {
// TODO
// This is a happy path test.
// Assert changes to Action storage.
// Assert event emission.
}

function test_CheckNonceIncrements() public {
// TODO
// This is a happy path test.
// Assert that nonce increments
}

function test_OperationCannotBeReplayed() public {
// TODO
// Check that operation with same parameters cannot be replayed.
}

function test_RevertIf_SignerIsNotPolicyHolder() public {
// TODO
// Reverts if user!=signer
}

function test_RevertIf_SignerIsZeroAddress() public {
// TODO
// Reverts if signer == address(0)
}
}

contract CancelAction is VertexCoreTest {
function setUp() public override {
VertexCoreTest.setUp();
Expand Down Expand Up @@ -848,9 +878,25 @@ contract CastApprovalBySig is VertexCoreTest {
// Assert event emission.
}

function test_RevertIf_InvalidPolicyholder() public {
function test_CheckNonceIncrements() public {
// TODO
// https://github.com/llama-community/vertex-v1/issues/62
// This is a happy path test.
// Assert that nonce increments
}

function test_OperationCannotBeReplayed() public {
// TODO
// Check that operation with same parameters cannot be replayed.
}

function test_RevertIf_SignerIsNotPolicyHolder() public {
// TODO
// Reverts if user!=signer
}

function test_RevertIf_SignerIsZeroAddress() public {
// TODO
// Reverts if signer == address(0)
}
}

Expand Down Expand Up @@ -929,9 +975,25 @@ contract CastDisapprovalBySig is VertexCoreTest {
// Assert event emission.
}

function test_RevertIf_CallerIsNotPolicyHolder() public {
function test_CheckNonceIncrements() public {
// TODO
// This is a happy path test.
// Assert that nonce increments
}

function test_OperationCannotBeReplayed() public {
// TODO
// Check that operation with same parameters cannot be replayed.
}

function test_RevertIf_SignerIsNotPolicyHolder() public {
// TODO
// Reverts if user!=signer
}

function test_RevertIf_SignerIsZeroAddress() public {
// TODO
// https://github.com/llama-community/vertex-v1/issues/62
// Reverts if signer == address(0)
}
}

Expand Down