Skip to content

Commit

Permalink
Simplify ERC712,1271 signature verification process
Browse files Browse the repository at this point in the history
  • Loading branch information
@garyghayrat
garyghayrat committed Jan 11, 2024
1 parent b2af4c6 commit a5898bd
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 146 deletions.
174 changes: 40 additions & 134 deletions src/ERC6538Registry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,38 @@ contract ERC6538Registry {
address registrant,
uint256 schemeId,
bytes memory signature,
bytes memory stealthMetaAddress
bytes memory stealthMetaAddress,
uint8 v,
bytes32 r,
bytes32 s
) external {
bytes32 digest = _hashTypedDataV4(
keccak256(
abi.encode(TYPE_HASH, registrant, schemeId, stealthMetaAddress, nonceOf[registrant]++)
)
bytes32 dataHash;
address recoveredAddress;

unchecked {
dataHash = keccak256(
abi.encodePacked(
"\x19\x01",
domainSeparator,
keccak256(
abi.encode(TYPE_HASH, registrant, schemeId, stealthMetaAddress, nonceOf[registrant]++)
)
)
);
recoveredAddress = ecrecover(dataHash, v, r, s);
}

require(
(
(recoveredAddress != address(0) && recoveredAddress == registrant)
|| (
IERC1271(registrant).isValidSignature(dataHash, signature)
== IERC1271.isValidSignature.selector
)
),
"Invalid signature"
);
require(isValidSignatureNow(registrant, digest, signature), "Invalid signature");

stealthMetaAddressOf[registrant][schemeId] = stealthMetaAddress;
emit StealthMetaAddressSet(registrant, schemeId, stealthMetaAddress);
}
Expand All @@ -86,134 +110,16 @@ contract ERC6538Registry {
function incrementNonce() external {
nonceOf[msg.sender]++;
}
}

/// @notice Returns the hash of the fully encoded EIP712 message for this domain.
/// @dev The following code is modified from OpenZeppelin's `EIP712.sol` file. Permalink:
/// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/e70a0118ef10773457f670671baefad2c5ea610d/contracts/utils/cryptography/EIP712.sol
/// @param structHash The hash of the struct containing the message data, as defined in
/// https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct
function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) {
return toTypedDataHash(domainSeparator, structHash);
}

/// @dev Returns the keccak256 digest of an EIP-712 typed data (ERC-191 version `0x01`).
/// @dev The digest is calculated from a `domainSeparator` and a `structHash`, by prefixing them
/// with `\x19\x01` and hashing the result. It corresponds to the hash signed by the
/// https://eips.ethereum.org/EIPS/eip-712[`eth_signTypedData`] JSON-RPC method as part of
/// EIP-712.
/// @dev The following code is from OpenZeppelin's `MessageHashUtils.sol` file. Permalink:
/// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/e70a0118ef10773457f670671baefad2c5ea610d/contracts/utils/cryptography/MessageHashUtils.sol
/// @param contractDomainSeparator The domain separator.
/// @param structHash The hash of the struct containing the message data, as defined in
/// https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct
function toTypedDataHash(bytes32 contractDomainSeparator, bytes32 structHash)
internal
pure
returns (bytes32 digest)
{
/// @solidity memory-safe-assembly
assembly {
let ptr := mload(0x40)
mstore(ptr, hex"1901")
mstore(add(ptr, 0x02), contractDomainSeparator)
mstore(add(ptr, 0x22), structHash)
digest := keccak256(ptr, 0x42)
}
}

/// @notice Checks if a signature is valid for a given signer and data hash. If the signer is a
/// smart contract, the signature is validated against that smart contract using ERC1271,
/// otherwise it's validated using `tryRecover`.
/// @dev The following code is from OpenZeppelin's `SignatureChecker.sol` file. Permalink:
/// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/e70a0118ef10773457f670671baefad2c5ea610d/contracts/utils/cryptography/SignatureChecker.sol
/// @param signer The address that should have signed the message data.
/// @param hash The digest of message data.
/// @param signature The signature provided by the registrant.
function isValidSignatureNow(address signer, bytes32 hash, bytes memory signature)
internal
view
returns (bool)
{
(address recovered, RecoverError error,) = tryRecover(hash, signature);
return (error == RecoverError.NoError && recovered == signer)
|| isValidERC1271SignatureNow(signer, hash, signature);
}

/// @notice Checks if a signature is valid for a given signer and data hash. The signature is
/// validated against the signer smart contract using ERC1271.
/// @dev The following code is modified from OpenZeppelin's `SignatureChecker.sol` file. Permalink:
/// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/e70a0118ef10773457f670671baefad2c5ea610d/contracts/utils/cryptography/SignatureChecker.sol
/// @param signer The address that should have signed the message data.
/// @param hash The digest of message data.
/// @param signature The signature provided by the registrant.
function isValidERC1271SignatureNow(address signer, bytes32 hash, bytes memory signature)
internal
/// @notice Interface of the ERC1271 standard signature validation method for contracts as defined
/// in https://eips.ethereum.org/EIPS/eip-1271[ERC-1271].
interface IERC1271 {
/// @dev Should return whether the signature provided is valid for the provided data
/// @param hash Hash of the data to be signed
/// @param signature Signature byte array associated with _data
function isValidSignature(bytes32 hash, bytes memory signature)
external
view
returns (bool)
{
(bool success, bytes memory result) = signer.staticcall(
abi.encodeWithSelector(bytes4(keccak256("isValidSignature(bytes32,bytes)")), hash, signature)
);
return (
success && result.length >= 32
&& abi.decode(result, (bytes32))
== bytes32(bytes4(keccak256("isValidSignature(bytes32,bytes)")))
);
}

/// @dev The following code is from OpenZeppelin's `ECDSA.sol` file. Permalink:
/// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/e70a0118ef10773457f670671baefad2c5ea610d/contracts/utils/cryptography/ECDSA.sol
/// @param hash The digest of message data.
/// @param signature The signature provided by the registrant.
function tryRecover(bytes32 hash, bytes memory signature)
internal
pure
returns (address, RecoverError, bytes32)
{
if (signature.length == 65) {
bytes32 r;
bytes32 s;
uint8 v;
// ecrecover takes the signature parameters, and the only way to get them currently is to use
// assembly.
assembly ("memory-safe") {
r := mload(add(signature, 0x20))
s := mload(add(signature, 0x40))
v := byte(0, mload(add(signature, 0x60)))
}
return tryRecover(hash, v, r, s);
} else {
return (address(0), RecoverError.InvalidSignatureLength, bytes32(signature.length));
}
}

/// @notice Recover the signer's address using `v`, `r` and `s` signature fields.
/// @dev The following code is from OpenZeppelin's `ECDSA.sol` file. Permalink:
/// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/e70a0118ef10773457f670671baefad2c5ea610d/contracts/utils/cryptography/ECDSA.sol
function tryRecover(bytes32 hash, uint8 v, bytes32 r, bytes32 s)
internal
pure
returns (address, RecoverError, bytes32)
{
// EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make
// the signature unique. Appendix F in the Ethereum Yellow paper
// (https://ethereum.github.io/yellowpaper/paper.pdf), defines the valid range for s in (301):
// 0 < s < secp256k1n ÷ 2 + 1, and for v in (302): v ∈ {27, 28}. Most signatures from
// current libraries generate a unique signature with an s-value in the lower half order.

// If your library generates malleable signatures, such as s-values in the upper range,
// calculate a new s-value with
// 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27
// to 28 or vice versa. If your library also generates signatures with 0/1 for v instead 27/28,
// add 27 to v to accept these malleable signatures as well.
if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
return (address(0), RecoverError.InvalidSignatureS, s);
}

// If the signature is valid (and not malleable), return the signer address
address signer = ecrecover(hash, v, r, s);
if (signer == address(0)) return (address(0), RecoverError.InvalidSignature, bytes32(0));

return (signer, RecoverError.NoError, bytes32(0));
}
returns (bytes4 magicValue);
}
39 changes: 27 additions & 12 deletions test/ERC6538Registry.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ contract RegisterKeysOnBehalf_Address is ERC6538RegistryTest {

vm.expectEmit(true, true, true, true);
emit StealthMetaAddressSet(alice, schemeId, stealthMetaAddress);
registry.registerKeysOnBehalf(alice, schemeId, signature, stealthMetaAddress);
registry.registerKeysOnBehalf(alice, schemeId, signature, stealthMetaAddress, v, r, s);
}

function testFuzz_ERC1271SignatureIsValid(
Expand All @@ -91,7 +91,7 @@ contract RegisterKeysOnBehalf_Address is ERC6538RegistryTest {

vm.expectEmit(true, true, true, true);
emit StealthMetaAddressSet(registrant, schemeId, stealthMetaAddress);
registry.registerKeysOnBehalf(registrant, schemeId, signature, stealthMetaAddress);
registry.registerKeysOnBehalf(registrant, schemeId, signature, stealthMetaAddress, v, r, s);
}

function testFuzz_UpdateStealthMetaAddress(
Expand All @@ -112,7 +112,7 @@ contract RegisterKeysOnBehalf_Address is ERC6538RegistryTest {

vm.expectEmit(true, true, true, true);
emit StealthMetaAddressSet(alice, schemeId, stealthMetaAddress);
registry.registerKeysOnBehalf(alice, schemeId, signature, stealthMetaAddress);
registry.registerKeysOnBehalf(alice, schemeId, signature, stealthMetaAddress, v, r, s);
}
}

Expand All @@ -123,12 +123,14 @@ contract RegisterKeysOnBehalf_Address is ERC6538RegistryTest {
bytes memory stealthMetaAddress
) external {
(address alice, uint256 alicePk) = makeAddrAndKey(name);
bytes32 hash = keccak256(abi.encode(alice, schemeId, stealthMetaAddress, 0));
SigUtils.RegistrantInfo memory registrantInfo =
SigUtils.RegistrantInfo(alice, schemeId, stealthMetaAddress, 0 /* nonce */ );
bytes32 hash = sigUtils.getTypedDataHash(registrantInfo);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePk, hash);
bytes memory signature = abi.encodePacked(r, s, v);

vm.expectRevert("Invalid signature");
registry.registerKeysOnBehalf(bob, schemeId, signature, stealthMetaAddress);
vm.expectRevert();
registry.registerKeysOnBehalf(bob, schemeId, signature, stealthMetaAddress, v, r, s);
}

function testFuzz_RevertIf_WrongNonce(
Expand All @@ -139,17 +141,18 @@ contract RegisterKeysOnBehalf_Address is ERC6538RegistryTest {
) external {
vm.assume(nonce != 0);
(address alice, uint256 alicePk) = makeAddrAndKey(name);
bytes32 hash = keccak256(abi.encode(alice, schemeId, stealthMetaAddress, nonce));
SigUtils.RegistrantInfo memory registrantInfo =
SigUtils.RegistrantInfo(alice, schemeId, stealthMetaAddress, nonce);
bytes32 hash = sigUtils.getTypedDataHash(registrantInfo);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePk, hash);
bytes memory signature = abi.encodePacked(r, s, v);

vm.expectRevert("Invalid signature");
registry.registerKeysOnBehalf(alice, schemeId, signature, stealthMetaAddress);
vm.expectRevert();
registry.registerKeysOnBehalf(alice, schemeId, signature, stealthMetaAddress, v, r, s);
}

function test_RevertIf_NoSignatureIsProvided() external {
vm.expectRevert("Invalid signature");
registry.registerKeysOnBehalf(address(0), 0, "", "");
vm.expectRevert();
registry.registerKeysOnBehalf(address(0), 0, "", "", 0, 0, 0);
}
}

Expand Down Expand Up @@ -266,3 +269,15 @@ contract SigUtils {
return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getStructHash(_info)));
}
}
/// @notice Interface of the ERC1271 standard signature validation method for contracts as defined
/// in https://eips.ethereum.org/EIPS/eip-1271[ERC-1271].

interface IERC1271 {
/// @dev Should return whether the signature provided is valid for the provided data
/// @param hash Hash of the data to be signed
/// @param signature Signature byte array associated with _data
function isValidSignature(bytes32 hash, bytes memory signature)
external
view
returns (bytes4 magicValue);
}

0 comments on commit a5898bd

Please sign in to comment.