Skip to content

Commit 4f3422c

Browse files
k06ak06a
committed
Introduce SequentialOperations and ParallelOperations between EIP712 and signature-based operations like ERC20Permit, Votes, Governor, MinimalForwarder
1 parent 484a7ac commit 4f3422c

File tree

12 files changed

+188
-69
lines changed

12 files changed

+188
-69
lines changed

‎contracts/governance/Governor.sol‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pragma solidity ^0.8.0;
66
import "../token/ERC721/IERC721Receiver.sol";
77
import "../token/ERC1155/IERC1155Receiver.sol";
88
import "../utils/cryptography/ECDSA.sol";
9-
import "../utils/cryptography/EIP712.sol";
9+
import "../utils/cryptography/ParallelOperations.sol";
1010
import "../utils/introspection/ERC165.sol";
1111
import "../utils/math/SafeCast.sol";
1212
import "../utils/structs/DoubleEndedQueue.sol";
@@ -26,7 +26,7 @@ import "./IGovernor.sol";
2626
*
2727
* _Available since v4.3._
2828
*/
29-
abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receiver, IERC1155Receiver {
29+
abstract contract Governor is Context, ERC165, ParallelOperations, IGovernor, IERC721Receiver, IERC1155Receiver {
3030
using DoubleEndedQueue for DoubleEndedQueue.Bytes32Deque;
3131
using SafeCast for uint256;
3232
using Timers for Timers.BlockNumber;
@@ -450,8 +450,10 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
450450
bytes32 r,
451451
bytes32 s
452452
) public virtual override returns (uint256) {
453-
address voter = ECDSA.recover(
454-
_hashTypedDataV4(keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support))),
453+
address voter = _validateParallelOperation(
454+
BALLOT_TYPEHASH,
455+
keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support)),
456+
proposalId, // use proposalId as nonce
455457
v,
456458
r,
457459
s
@@ -471,23 +473,14 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
471473
bytes32 r,
472474
bytes32 s
473475
) public virtual override returns (uint256) {
474-
address voter = ECDSA.recover(
475-
_hashTypedDataV4(
476-
keccak256(
477-
abi.encode(
478-
EXTENDED_BALLOT_TYPEHASH,
479-
proposalId,
480-
support,
481-
keccak256(bytes(reason)),
482-
keccak256(params)
483-
)
484-
)
485-
),
476+
address voter = _validateParallelOperation(
477+
EXTENDED_BALLOT_TYPEHASH,
478+
keccak256(abi.encode(EXTENDED_BALLOT_TYPEHASH, proposalId, support, keccak256(bytes(reason)), keccak256(params))),
479+
proposalId, // use proposalId as nonce
486480
v,
487481
r,
488482
s
489483
);
490-
491484
return _castVote(proposalId, voter, support, reason, params);
492485
}
493486

‎contracts/governance/utils/Votes.sol‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ pragma solidity ^0.8.0;
55
import "../../utils/Context.sol";
66
import "../../utils/Nonces.sol";
77
import "../../utils/Checkpoints.sol";
8-
import "../../utils/cryptography/EIP712.sol";
8+
import "../../utils/cryptography/SequentialOperations.sol";
99
import "./IVotes.sol";
1010
import "../../utils/math/SafeCast.sol";
1111

@@ -29,17 +29,15 @@ import "../../utils/math/SafeCast.sol";
2929
*
3030
* _Available since v4.5._
3131
*/
32-
abstract contract Votes is IVotes, Context, EIP712 {
32+
abstract contract Votes is IVotes, Context, SequentialOperations {
3333
using Checkpoints for Checkpoints.History;
34-
using Nonces for Nonces.Data;
3534

3635
bytes32 private constant _DELEGATION_TYPEHASH =
3736
keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
3837

3938
mapping(address => address) private _delegation;
4039
mapping(address => Checkpoints.History) private _delegateCheckpoints;
4140
Checkpoints.History private _totalCheckpoints;
42-
Nonces.Data private _nonces;
4341

4442
/**
4543
* @dev Returns the current amount of votes that `account` has.
@@ -86,7 +84,7 @@ abstract contract Votes is IVotes, Context, EIP712 {
8684
* @dev Returns the delegation nonce for `owner`.
8785
*/
8886
function delegationNonces(address owner) public view virtual override returns (uint256) {
89-
return _nonces.nonces(owner);
87+
return operationNonces(_DELEGATION_TYPEHASH, owner);
9088
}
9189

9290
/**
@@ -116,13 +114,14 @@ abstract contract Votes is IVotes, Context, EIP712 {
116114
bytes32 s
117115
) public virtual override {
118116
require(block.timestamp <= expiry, "Votes: signature expired");
119-
address signer = ECDSA.recover(
120-
_hashTypedDataV4(keccak256(abi.encode(_DELEGATION_TYPEHASH, delegatee, nonce, expiry))),
117+
address signer = _validateSequentialOperation(
118+
_DELEGATION_TYPEHASH,
119+
keccak256(abi.encode(_DELEGATION_TYPEHASH, delegatee, nonce, expiry)),
120+
nonce,
121121
v,
122122
r,
123123
s
124124
);
125-
require(nonce == _nonces.useNonce(signer), "Votes: invalid nonce");
126125
_delegate(signer, delegatee);
127126
}
128127

‎contracts/metatx/MinimalForwarder.sol‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
pragma solidity ^0.8.0;
55

66
import "../utils/cryptography/ECDSA.sol";
7-
import "../utils/cryptography/EIP712.sol";
7+
import "../utils/cryptography/SequentialOperations.sol";
88

99
/**
1010
* @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}.
@@ -14,7 +14,7 @@ import "../utils/cryptography/EIP712.sol";
1414
* functioning forwarding system with good properties requires more complexity. We suggest you look at other projects
1515
* such as the GSN which do have the goal of building a system like that.
1616
*/
17-
contract MinimalForwarder is EIP712 {
17+
contract MinimalForwarder is SequentialOperations {
1818
using ECDSA for bytes32;
1919

2020
struct ForwardRequest {
@@ -29,19 +29,20 @@ contract MinimalForwarder is EIP712 {
2929
bytes32 private constant _TYPEHASH =
3030
keccak256("ForwardRequest(address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data)");
3131

32-
mapping(address => uint256) private _nonces;
33-
3432
constructor() EIP712("MinimalForwarder", "0.0.1") {}
3533

3634
function getNonce(address from) public view returns (uint256) {
37-
return _nonces[from];
35+
return operationNonces(_TYPEHASH, from);
3836
}
3937

40-
function verify(ForwardRequest calldata req, bytes calldata signature) public view returns (bool) {
41-
address signer = _hashTypedDataV4(
42-
keccak256(abi.encode(_TYPEHASH, req.from, req.to, req.value, req.gas, req.nonce, keccak256(req.data)))
43-
).recover(signature);
44-
return _nonces[req.from] == req.nonce && signer == req.from;
38+
function verify(ForwardRequest calldata req, bytes calldata signature) public returns (bool) {
39+
address signer = _validateSequentialOperation(
40+
_TYPEHASH,
41+
keccak256(abi.encode(_TYPEHASH, req.from, req.to, req.value, req.gas, req.nonce, keccak256(req.data))),
42+
req.nonce,
43+
signature
44+
);
45+
return signer == req.from;
4546
}
4647

4748
function execute(ForwardRequest calldata req, bytes calldata signature)
@@ -50,8 +51,7 @@ contract MinimalForwarder is EIP712 {
5051
returns (bool, bytes memory)
5152
{
5253
require(verify(req, signature), "MinimalForwarder: signature does not match request");
53-
_nonces[req.from] = req.nonce + 1;
54-
54+
5555
(bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}(
5656
abi.encodePacked(req.data, req.from)
5757
);

‎contracts/token/ERC20/extensions/ERC20Permit.sol‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pragma solidity ^0.8.0;
66
import "./IERC20Permit.sol";
77
import "../ERC20.sol";
88
import "../../../utils/cryptography/ECDSA.sol";
9-
import "../../../utils/cryptography/EIP712.sol";
9+
import "../../../utils/cryptography/SequentialOperations.sol";
1010
import "../../../utils/Nonces.sol";
1111

1212
/**
@@ -19,9 +19,7 @@ import "../../../utils/Nonces.sol";
1919
*
2020
* _Available since v3.4._
2121
*/
22-
abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
23-
using Nonces for Nonces.Data;
24-
22+
abstract contract ERC20Permit is ERC20, IERC20Permit, SequentialOperations {
2523
// solhint-disable-next-line var-name-mixedcase
2624
bytes32 private constant _PERMIT_TYPEHASH =
2725
keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
@@ -33,7 +31,6 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
3331
*/
3432
// solhint-disable-next-line var-name-mixedcase
3533
bytes32 private _PERMIT_TYPEHASH_DEPRECATED_SLOT;
36-
Nonces.Data private _nonces;
3734

3835
/**
3936
* @dev Initializes the {EIP712} domain separator using the `name` parameter, and setting `version` to `"1"`.
@@ -55,22 +52,24 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
5552
bytes32 s
5653
) public virtual override {
5754
require(block.timestamp <= deadline, "ERC20Permit: expired deadline");
58-
59-
bytes32 structHash = keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _nonces.useNonce(owner), deadline));
60-
61-
bytes32 hash = _hashTypedDataV4(structHash);
62-
63-
address signer = ECDSA.recover(hash, v, r, s);
64-
require(signer == owner, "ERC20Permit: invalid signature");
65-
55+
uint256 nonce = operationNonces(_PERMIT_TYPEHASH, owner);
56+
address signer = _validateSequentialOperation(
57+
_PERMIT_TYPEHASH,
58+
keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, nonce, deadline)),
59+
nonce,
60+
v,
61+
r,
62+
s
63+
);
64+
require(owner == signer, "ERC20Permit: invalid signature");
6665
_approve(owner, spender, value);
6766
}
6867

6968
/**
7069
* @dev See {IERC20Permit-nonces}.
7170
*/
7271
function nonces(address owner) public view virtual override returns (uint256) {
73-
return _nonces.nonces(owner);
72+
return operationNonces(_PERMIT_TYPEHASH, owner);
7473
}
7574

7675
/**

‎contracts/utils/cryptography/ParallelOperations.sol‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.0;
3+
4+
import "../Context.sol";
5+
import "../Nonces.sol";
6+
import "./EIP712.sol";
7+
import "./ECDSA.sol";
8+
9+
/**
10+
* @dev Provides tracking nonces per address and operation. Operation ids should be unique.
11+
*/
12+
abstract contract ParallelOperations is Context, EIP712 {
13+
mapping(bytes32 => mapping(address => mapping(uint256 => bool))) private _usedOperationIds;
14+
15+
function isOperationIdAvailable(bytes32 operationTypehash, address owner, uint256 operationId) public view virtual returns (bool) {
16+
return !_usedOperationIds[operationTypehash][owner][operationId];
17+
}
18+
19+
function useOperationId(bytes32 operationTypehash, uint256 operationId) public virtual {
20+
_useOperationId(operationTypehash, _msgSender(), operationId);
21+
}
22+
23+
function _validateParallelOperation(
24+
bytes32 operationTypehash,
25+
bytes32 operationHash,
26+
uint256 operationId,
27+
bytes memory signature
28+
) internal virtual returns(address signer) {
29+
signer = ECDSA.recover(_hashTypedDataV4(operationHash), signature);
30+
_useOperationId(operationTypehash, signer, operationId);
31+
}
32+
33+
function _validateParallelOperation(
34+
bytes32 operationTypehash,
35+
bytes32 operationHash,
36+
uint256 operationId,
37+
uint8 v,
38+
bytes32 r,
39+
bytes32 s
40+
) internal virtual returns(address signer) {
41+
signer = ECDSA.recover(_hashTypedDataV4(operationHash), v, r, s);
42+
_useOperationId(operationTypehash, signer, operationId);
43+
}
44+
45+
function _validateParallelOperation(
46+
bytes32 operationTypehash,
47+
bytes32 operationHash,
48+
uint256 operationId,
49+
bytes32 r,
50+
bytes32 vs
51+
) internal virtual returns(address signer) {
52+
signer = ECDSA.recover(_hashTypedDataV4(operationHash), r, vs);
53+
_useOperationId(operationTypehash, signer, operationId);
54+
}
55+
56+
/// @dev Method made non-virtual to deny changing logic of parallel operations invalidation.
57+
function _useOperationId(bytes32 operationTypehash, address owner, uint256 operationId) internal {
58+
require(
59+
!_usedOperationIds[operationTypehash][owner][operationId],
60+
"ParallelOperations: invalid operation id"
61+
);
62+
_usedOperationIds[operationTypehash][owner][operationId] = true;
63+
}
64+
}

‎contracts/utils/cryptography/SequentialOperations.sol‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.0;
3+
4+
import "../Context.sol";
5+
import "../Nonces.sol";
6+
import "./EIP712.sol";
7+
import "./ECDSA.sol";
8+
9+
/**
10+
* @dev Provides tracking nonces per address and operation. Nonces will only increment.
11+
*/
12+
abstract contract SequentialOperations is Context, EIP712 {
13+
using Nonces for Nonces.Data;
14+
15+
mapping(bytes32 => Nonces.Data) private _nonces;
16+
17+
function operationNonces(bytes32 operationTypehash, address owner) public view virtual returns (uint256) {
18+
return _nonces[operationTypehash].nonces(owner);
19+
}
20+
21+
function useOperationNonce(bytes32 operationTypehash, uint256 nonce) public virtual {
22+
_useOperationNonce(operationTypehash, _msgSender(), nonce);
23+
}
24+
25+
function _validateSequentialOperation(
26+
bytes32 operationTypehash,
27+
bytes32 operationHash,
28+
uint256 nonce,
29+
bytes memory signature
30+
) internal virtual returns(address signer) {
31+
signer = ECDSA.recover(_hashTypedDataV4(operationHash), signature);
32+
_useOperationNonce(operationTypehash, signer, nonce);
33+
}
34+
35+
function _validateSequentialOperation(
36+
bytes32 operationTypehash,
37+
bytes32 operationHash,
38+
uint256 nonce,
39+
uint8 v,
40+
bytes32 r,
41+
bytes32 s
42+
) internal virtual returns(address signer) {
43+
signer = ECDSA.recover(_hashTypedDataV4(operationHash), v, r, s);
44+
_useOperationNonce(operationTypehash, signer, nonce);
45+
}
46+
47+
function _validateSequentialOperation(
48+
bytes32 operationTypehash,
49+
bytes32 operationHash,
50+
uint256 nonce,
51+
bytes32 r,
52+
bytes32 vs
53+
) internal virtual returns(address signer) {
54+
signer = ECDSA.recover(_hashTypedDataV4(operationHash), r, vs);
55+
_useOperationNonce(operationTypehash, signer, nonce);
56+
}
57+
58+
/// @dev Method made non-virtual to deny changing logic of sequential operations invalidation.
59+
function _useOperationNonce(bytes32 operationTypehash, address owner, uint256 nonce) internal {
60+
require(nonce == _nonces[operationTypehash].useNonce(owner), "SequentialOperations: invalid nonce");
61+
}
62+
}

‎test/governance/utils/Votes.behavior.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ function shouldBehaveLikeVotes () {
9595

9696
await expectRevert(
9797
this.votes.delegateBySig(delegatorAddress, nonce, MAX_UINT256, v, r, s),
98-
'Votes: invalid nonce',
98+
'SequentialOperations: invalid nonce',
9999
);
100100
});
101101

@@ -127,7 +127,7 @@ function shouldBehaveLikeVotes () {
127127
));
128128
await expectRevert(
129129
this.votes.delegateBySig(delegatorAddress, nonce + 1, MAX_UINT256, v, r, s),
130-
'Votes: invalid nonce',
130+
'SequentialOperations: invalid nonce',
131131
);
132132
});
133133

‎test/metatx/ERC2771Context.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ contract('ERC2771Context', function (accounts) {
7676
};
7777

7878
const sign = ethSigUtil.signTypedMessage(this.wallet.getPrivateKey(), { data: { ...this.data, message: req } });
79-
expect(await this.forwarder.verify(req, sign)).to.equal(true);
79+
expect(await this.forwarder.contract.methods.verify(req, sign).call()).to.equal(true);
8080

8181
const { tx } = await this.forwarder.execute(req, sign);
8282
await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Sender', { sender: this.sender });
@@ -99,7 +99,7 @@ contract('ERC2771Context', function (accounts) {
9999
};
100100

101101
const sign = ethSigUtil.signTypedMessage(this.wallet.getPrivateKey(), { data: { ...this.data, message: req } });
102-
expect(await this.forwarder.verify(req, sign)).to.equal(true);
102+
expect(await this.forwarder.contract.methods.verify(req, sign).call()).to.equal(true);
103103

104104
const { tx } = await this.forwarder.execute(req, sign);
105105
await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Data', { data, integerValue, stringValue });

0 commit comments

Comments
 (0)