Skip to content

Commit

Permalink
♻️ Make SignatureChecker Module-Friendly (#228)
Browse files Browse the repository at this point in the history
### 🕓 Changelog

This PR refactors the `SignatureChecker` contract to make it
module-friendly and ready for the breaking `0.4.0` release. Please note
that `SignatureChecker` is now
[EIP-7377](https://eips.ethereum.org/EIPS/eip-7377)-safe. Furthermore, I
add a custom interface `IERC1271` for usage via
[EIP-1271](https://eips.ethereum.org/EIPS/eip-1271).

---------

Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
  • Loading branch information
pcaversaccio authored Apr 9, 2024
1 parent 7d2a4e7 commit 40a74ab
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 129 deletions.
36 changes: 18 additions & 18 deletions .gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -641,24 +641,24 @@ OwnableTest:testRenounceOwnershipSuccess() (gas: 17978)
OwnableTest:testTransferOwnershipNonOwner() (gas: 12655)
OwnableTest:testTransferOwnershipSuccess() (gas: 22388)
OwnableTest:testTransferOwnershipToZeroAddress() (gas: 15742)
SignatureCheckerTest:testEIP1271NoIsValidSignatureFunction() (gas: 23546)
SignatureCheckerTest:testEIP1271WithInvalidSignature(bytes,string) (runs: 256, μ: 24133, ~: 24130)
SignatureCheckerTest:testEIP1271WithInvalidSignature1() (gas: 34508)
SignatureCheckerTest:testEIP1271WithInvalidSignature2() (gas: 30198)
SignatureCheckerTest:testEIP1271WithInvalidSigner() (gas: 34586)
SignatureCheckerTest:testEIP1271WithMaliciousWallet() (gas: 24479)
SignatureCheckerTest:testEIP1271WithValidSignature() (gas: 34495)
SignatureCheckerTest:testEOAWithInvalidSignature1() (gas: 21023)
SignatureCheckerTest:testEOAWithInvalidSignature2() (gas: 21413)
SignatureCheckerTest:testEOAWithInvalidSigner() (gas: 21115)
SignatureCheckerTest:testEOAWithTooHighSValue() (gas: 18580)
SignatureCheckerTest:testEOAWithValidSignature() (gas: 20240)
SignatureCheckerTest:testFuzzEIP1271WithInvalidSigner(string,string) (runs: 256, μ: 36498, ~: 36564)
SignatureCheckerTest:testFuzzEIP1271WithValidSignature(string) (runs: 256, μ: 35074, ~: 35068)
SignatureCheckerTest:testFuzzEOAWithInvalidSignature(bytes,string) (runs: 256, μ: 17538, ~: 17537)
SignatureCheckerTest:testFuzzEOAWithInvalidSigner(string,string) (runs: 256, μ: 22134, ~: 22195)
SignatureCheckerTest:testFuzzEOAWithValidSignature(string,string) (runs: 256, μ: 21253, ~: 21314)
SignatureCheckerTest:testInitialSetup() (gas: 8315)
SignatureCheckerTest:testEIP1271NoIsValidSignatureFunction() (gas: 20078)
SignatureCheckerTest:testEIP1271WithInvalidSignature(bytes,string) (runs: 256, μ: 24205, ~: 24202)
SignatureCheckerTest:testEIP1271WithInvalidSignature1() (gas: 31040)
SignatureCheckerTest:testEIP1271WithInvalidSignature2() (gas: 32729)
SignatureCheckerTest:testEIP1271WithInvalidSigner() (gas: 31118)
SignatureCheckerTest:testEIP1271WithMaliciousWallet() (gas: 21011)
SignatureCheckerTest:testEIP1271WithValidSignature() (gas: 31027)
SignatureCheckerTest:testEOAWithInvalidSignature1() (gas: 20578)
SignatureCheckerTest:testEOAWithInvalidSignature2() (gas: 24282)
SignatureCheckerTest:testEOAWithInvalidSigner() (gas: 20670)
SignatureCheckerTest:testEOAWithTooHighSValue() (gas: 21455)
SignatureCheckerTest:testEOAWithValidSignature() (gas: 20586)
SignatureCheckerTest:testFuzzEIP1271WithInvalidSigner(string,string) (runs: 256, μ: 33030, ~: 33096)
SignatureCheckerTest:testFuzzEIP1271WithValidSignature(string) (runs: 256, μ: 31606, ~: 31600)
SignatureCheckerTest:testFuzzEOAWithInvalidSignature(bytes,string) (runs: 256, μ: 17100, ~: 17101)
SignatureCheckerTest:testFuzzEOAWithInvalidSigner(string,string) (runs: 256, μ: 21689, ~: 21750)
SignatureCheckerTest:testFuzzEOAWithValidSignature(string,string) (runs: 256, μ: 21599, ~: 21660)
SignatureCheckerTest:testInitialSetup() (gas: 8292)
TimelockControllerInvariants:statefulFuzzExecutedLessThanOrEqualToScheduled() (runs: 256, calls: 3840, reverts: 1221)
TimelockControllerInvariants:statefulFuzzExecutedProposalCancellation() (runs: 256, calls: 3840, reverts: 1261)
TimelockControllerInvariants:statefulFuzzExecutingCancelledProposal() (runs: 256, calls: 3840, reverts: 1295)
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- [`Create2Address`](https://github.com/pcaversaccio/snekmate/blob/v0.1.0/src/snekmate/utils/Create2Address.vy): Make `Create2Address` module-friendly. ([#225](https://github.com/pcaversaccio/snekmate/pull/225))
- [`ECDSA`](https://github.com/pcaversaccio/snekmate/blob/v0.1.0/src/snekmate/utils/ECDSA.vy): Make `ECDSA` module-friendly. ([#227](https://github.com/pcaversaccio/snekmate/pull/227))
- [`MessageHashUtils`](https://github.com/pcaversaccio/snekmate/blob/v0.1.0/src/snekmate/utils/MessageHashUtils.vy): Move the `ECDSA` message hash methods to a separate `MessageHashUtils` library module. ([#227](https://github.com/pcaversaccio/snekmate/pull/227))
- [`SignatureChecker`](https://github.com/pcaversaccio/snekmate/blob/v0.1.0/src/snekmate/utils/SignatureChecker.vy): Make `SignatureChecker` module-friendly. ([#228](https://github.com/pcaversaccio/snekmate/pull/228))
- **Vyper Contract Deployer**
- [`VyperDeployer`](https://github.com/pcaversaccio/snekmate/blob/v0.1.0/lib/utils/VyperDeployer.sol): Improve error message in the event of a Vyper compilation error. ([#219](https://github.com/pcaversaccio/snekmate/pull/219))

Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ src
├── CreateAddressMock — "CreateAddress Module Reference Implementation"
├── Create2AddressMock — "Create2Address Module Reference Implementation"
├── ECDSAMock — "ECDSA Module Reference Implementation"
└── MessageHashUtilsMock — "MessageHashUtils Module Reference Implementation"
├── MessageHashUtilsMock — "MessageHashUtils Module Reference Implementation"
└── SignatureCheckerMock — "SignatureChecker Module Reference Implementation"
```

## 🎛 Installation
Expand Down
125 changes: 22 additions & 103 deletions src/snekmate/utils/SignatureChecker.vy
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
@license GNU Affero General Public License v3.0 only
@author pcaversaccio
@notice Signature verification helper functions that can be used
instead of `ECDSA.recover_sig` to seamlessly support both
instead of {ECDSA-_recover_sig} to seamlessly support both
ECDSA signatures from externally-owned accounts (EOAs) as
well as EIP-1271 (https://eips.ethereum.org/EIPS/eip-1271)
signatures from smart contract wallets like Argent and Gnosis
Safe. For strict EIP-1271 verification, i.e. only valid EIP-1271
signatures are verified, the function `is_valid_ERC1271_signature_now`
signatures from smart contract wallets like Argent and Safe.
For strict EIP-1271 verification, i.e. only valid EIP-1271
signatures are verified, the function `_is_valid_ERC1271_signature_now`
can be called. The implementation is inspired by OpenZeppelin's
implementation here:
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/SignatureChecker.sol.
Expand All @@ -22,18 +22,20 @@
"""


# @dev We import the `ECDSA` module.
# @notice Please note that the `ECDSA` module
# is stateless and therefore does not require
# the `uses` keyword for usage.
from . import ECDSA as ecdsa


# @dev The 4-byte function selector of `isValidSignature(bytes32,bytes)`.
# @notice If you declare a variable as `public`,
# Vyper automatically generates an `external`
# getter function for the variable.
IERC1271_ISVALIDSIGNATURE_SELECTOR: public(constant(bytes4)) = 0x1626BA7E


# @dev Constants used as part of the ECDSA recovery function.
_MALLEABILITY_THRESHOLD: constant(bytes32) = 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0
_SIGNATURE_INCREMENT: constant(bytes32) = 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF


@deploy
@payable
def __init__():
Expand All @@ -45,15 +47,15 @@ def __init__():
pass


@external
@internal
@view
def is_valid_signature_now(signer: address, hash: bytes32, signature: Bytes[65]) -> bool:
def _is_valid_signature_now(signer: address, hash: bytes32, signature: Bytes[65]) -> bool:
"""
@dev Checks if a signature `signature` is valid
for a given `signer` and message digest `hash`.
If the signer is a smart contract, the signature
is validated against that smart contract using
EIP-1271, otherwise it's validated using `ECDSA.recover_sig`.
EIP-1271, otherwise it's validated using {ECDSA-_recover_sig}.
@notice Unlike ECDSA signatures, contract signatures
are revocable and the result of this function
can therefore change over time. It could return
Expand All @@ -62,19 +64,22 @@ def is_valid_signature_now(signer: address, hash: bytes32, signature: Bytes[65])
@param signature The maximum 65-byte signature of `hash`.
@return bool The verification whether `signature` is valid
for the provided data.
@custom:security Since we avoid validating ECDSA signatures
when code is deployed at the signer's address,
it is safe if EIP-7377 (https://eips.ethereum.org/EIPS/eip-7377)
should be deployed one day.
"""
# First check: ECDSA case.
recovered: address = self._recover_sig(hash, signature)
if (recovered == signer):
return True
if (not(signer.is_contract)):
return ecdsa._recover_sig(hash, signature) == signer

# Second check: EIP-1271 case.
return self._is_valid_ERC1271_signature_now(signer, hash, signature)


@external
@internal
@view
def is_valid_ERC1271_signature_now(signer: address, hash: bytes32, signature: Bytes[65]) -> bool:
def _is_valid_ERC1271_signature_now(signer: address, hash: bytes32, signature: Bytes[65]) -> bool:
"""
@dev Checks if a signature `signature` is valid
for a given `signer` and message digest `hash`.
Expand All @@ -88,25 +93,6 @@ def is_valid_ERC1271_signature_now(signer: address, hash: bytes32, signature: By
@return bool The verification whether `signature` is valid
for the provided data.
"""
return self._is_valid_ERC1271_signature_now(signer, hash, signature)


@internal
@view
def _is_valid_ERC1271_signature_now(signer: address, hash: bytes32, signature: Bytes[65]) -> bool:
"""
@dev This `internal` function is equivalent to
`is_valid_ERC1271_signature_now`, and can be used
for strict EIP-1271 verification.
@notice Unlike ECDSA signatures, contract signatures
are revocable and the result of this function
can therefore change over time. It could return
`True` in block N and `False` in block N+1 (or the opposite).
@param hash The 32-byte message digest that was signed.
@param signature The maximum 65-byte signature of `hash`.
@return bool The verification whether `signature` is valid
for the provided data.
"""
success: bool = empty(bool)
return_data: Bytes[32] = b""
# The following low-level call does not revert, but instead
Expand All @@ -122,70 +108,3 @@ def _is_valid_ERC1271_signature_now(signer: address, hash: bytes32, signature: B
success, return_data = \
raw_call(signer, _abi_encode(hash, signature, method_id=IERC1271_ISVALIDSIGNATURE_SELECTOR), max_outsize=32, is_static_call=True, revert_on_failure=False)
return (success and (len(return_data) == 32) and (convert(return_data, bytes32) == convert(IERC1271_ISVALIDSIGNATURE_SELECTOR, bytes32)))


@internal
@pure
def _recover_sig(hash: bytes32, signature: Bytes[65]) -> address:
"""
@dev Sourced from {ECDSA-recover_sig}.
@notice See {ECDSA-recover_sig} for the
function docstring.
"""
sig_length: uint256 = len(signature)
# 65-byte case: r,s,v standard signature.
if (sig_length == 65):
r: uint256 = extract32(signature, empty(uint256), output_type=uint256)
s: uint256 = extract32(signature, 32, output_type=uint256)
v: uint256 = convert(slice(signature, 64, 1), uint256)
return self._try_recover_vrs(hash, v, r, s)
# 64-byte case: r,vs signature; see: https://eips.ethereum.org/EIPS/eip-2098.
elif (sig_length == 64):
r: uint256 = extract32(signature, empty(uint256), output_type=uint256)
vs: uint256 = extract32(signature, 32, output_type=uint256)
return self._try_recover_r_vs(hash, r, vs)
else:
return empty(address)


@internal
@pure
def _recover_vrs(hash: bytes32, v: uint256, r: uint256, s: uint256) -> address:
"""
@dev Sourced from {ECDSA-_recover_vrs}.
@notice See {ECDSA-_recover_vrs} for the
function docstring.
"""
return self._try_recover_vrs(hash, v, r, s)


@internal
@pure
def _try_recover_r_vs(hash: bytes32, r: uint256, vs: uint256) -> address:
"""
@dev Sourced from {ECDSA-_try_recover_r_vs}.
@notice See {ECDSA-_try_recover_r_vs} for the
function docstring.
"""
s: uint256 = vs & convert(_SIGNATURE_INCREMENT, uint256)
# We do not check for an overflow here since the shift operation
# `vs >> 255` results essentially in a `uint8` type (0 or 1) and
# we use `uint256` as result type.
v: uint256 = unsafe_add(vs >> 255, 27)
return self._try_recover_vrs(hash, v, r, s)


@internal
@pure
def _try_recover_vrs(hash: bytes32, v: uint256, r: uint256, s: uint256) -> address:
"""
@dev Sourced from {ECDSA-_try_recover_vrs}.
@notice See {ECDSA-_try_recover_vrs} for the
function docstring.
"""
assert s <= convert(_MALLEABILITY_THRESHOLD, uint256), "ECDSA: invalid signature `s` value"

signer: address = ecrecover(hash, v, r, s)
assert signer != empty(address), "ECDSA: invalid signature"

return signer
29 changes: 29 additions & 0 deletions src/snekmate/utils/interfaces/IERC1271.vyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# pragma version ~=0.4.0b6
"""
@title EIP-1271 Interface Definition
@custom:contract-name IERC1271
@license GNU Affero General Public License v3.0 only
@author pcaversaccio
@notice The ERC-1271 standard defines a method by which any
contract can verify whether a signature on behalf of
a particular contract is valid. The ERC-165 identifier
for this interface is `0x1626BA7E`. For more details,
please refer to:
https://eips.ethereum.org/EIPS/eip-1271#specification.

On how to use interfaces in Vyper, please visit:
https://vyper.readthedocs.io/en/latest/interfaces.html#interfaces.
"""


@external
@view
def isValidSignature(_hash: bytes32, _signature: Bytes[65]) -> bytes4:
"""
@dev Returns the 4-byte magic value `0x1626BA7E` if the
verification passes.
@param _hash The 32-byte message digest that was signed.
@param _signature The secp256k1 64/65-byte signature of `_hash`.
@return bytes4 The 4-byte magic value.
"""
return ...
79 changes: 79 additions & 0 deletions src/snekmate/utils/mocks/SignatureCheckerMock.vy
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# pragma version ~=0.4.0b6
"""
@title SignatureChecker Module Reference Implementation
@custom:contract-name SignatureCheckerMock
@license GNU Affero General Public License v3.0 only
@author pcaversaccio
"""


# @dev We import and initialise the `SignatureChecker` module.
from .. import SignatureChecker as sc
initializes: sc


# @dev We export (i.e. the runtime bytecode exposes these
# functions externally, allowing them to be called using
# the ABI encoding specification) the `external` getter
# function `IERC1271_ISVALIDSIGNATURE_SELECTOR` from the
# `SignatureChecker` module.
# @notice Please note that you must always also export (if
# required by the contract logic) `public` declared `constant`,
# `immutable`, and state variables, for which Vyper automatically
# generates an `external` getter function for the variable.
exports: sc.IERC1271_ISVALIDSIGNATURE_SELECTOR


@deploy
@payable
def __init__():
"""
@dev To omit the opcodes for checking the `msg.value`
in the creation-time EVM bytecode, the constructor
is declared as `payable`.
"""
sc.__init__()


@external
@view
def is_valid_signature_now(signer: address, hash: bytes32, signature: Bytes[65]) -> bool:
"""
@dev Checks if a signature `signature` is valid
for a given `signer` and message digest `hash`.
If the signer is a smart contract, the signature
is validated against that smart contract using
EIP-1271, otherwise it's validated using {ECDSA-_recover_sig}.
@notice Unlike ECDSA signatures, contract signatures
are revocable and the result of this function
can therefore change over time. It could return
`True` in block N and `False` in block N+1 (or the opposite).
@param hash The 32-byte message digest that was signed.
@param signature The maximum 65-byte signature of `hash`.
@return bool The verification whether `signature` is valid
for the provided data.
@custom:security Since we avoid validating ECDSA signatures
when code is deployed at the signer's address,
it is safe if EIP-7377 (https://eips.ethereum.org/EIPS/eip-7377)
should be deployed one day.
"""
return sc._is_valid_signature_now(signer, hash, signature)


@external
@view
def is_valid_ERC1271_signature_now(signer: address, hash: bytes32, signature: Bytes[65]) -> bool:
"""
@dev Checks if a signature `signature` is valid
for a given `signer` and message digest `hash`.
The signature is validated using EIP-1271.
@notice Unlike ECDSA signatures, contract signatures
are revocable and the result of this function
can therefore change over time. It could return
`True` in block N and `False` in block N+1 (or the opposite).
@param hash The 32-byte message digest that was signed.
@param signature The maximum 65-byte signature of `hash`.
@return bool The verification whether `signature` is valid
for the provided data.
"""
return sc._is_valid_ERC1271_signature_now(signer, hash, signature)
15 changes: 8 additions & 7 deletions test/utils/SignatureChecker.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ contract SignatureCheckerTest is Test {
function setUp() public {
signatureChecker = ISignatureChecker(
vyperDeployer.deployContract(
"src/snekmate/utils/",
"SignatureChecker"
"src/snekmate/utils/mocks/",
"SignatureCheckerMock"
)
);
}
Expand Down Expand Up @@ -195,11 +195,12 @@ contract SignatureCheckerTest is Test {
bytes32 hash = keccak256("WAGMI");
(, bytes32 r, bytes32 s) = vm.sign(key, hash);
bytes memory signatureInvalid = abi.encodePacked(r, s, bytes1(0xa0));
vm.expectRevert(bytes("ECDSA: invalid signature"));
signatureChecker.is_valid_signature_now(
walletAddr,
hash,
signatureInvalid
assertTrue(
!signatureChecker.is_valid_signature_now(
walletAddr,
hash,
signatureInvalid
)
);
assertTrue(
!signatureChecker.is_valid_ERC1271_signature_now(
Expand Down

0 comments on commit 40a74ab

Please sign in to comment.