From 9372812c18bdef57e31ca84b7a0490e0d3089979 Mon Sep 17 00:00:00 2001 From: garyghayrat Date: Tue, 9 Jan 2024 12:30:53 -0500 Subject: [PATCH] Update comments --- src/ERC6538Registry.sol | 28 +++++++++++++++------------- test/Deploy.t.sol | 2 +- test/ERC6538Registry.t.sol | 7 +++---- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/ERC6538Registry.sol b/src/ERC6538Registry.sol index e901f58..2185781 100644 --- a/src/ERC6538Registry.sol +++ b/src/ERC6538Registry.sol @@ -189,7 +189,9 @@ contract ERC6538Registry is IERC6538Registry { bytes32 r; bytes32 s; uint8 v; - assembly { + // 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))) @@ -208,22 +210,22 @@ contract ERC6538Registry is IERC6538Registry { 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. + // 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 + // 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)); diff --git a/test/Deploy.t.sol b/test/Deploy.t.sol index 295fff4..8e7f39b 100644 --- a/test/Deploy.t.sol +++ b/test/Deploy.t.sol @@ -27,7 +27,7 @@ contract DeployTest is Test, Deploy { require(erc5564ComputedAddress.code.length == 0); require(erc6538ComputedAddress.code.length == 0); - /// Run deploy script + // Run deploy script Deploy.run(); assertTrue(erc5564ComputedAddress.code.length > 0); diff --git a/test/ERC6538Registry.t.sol b/test/ERC6538Registry.t.sol index 414ff34..7e18560 100644 --- a/test/ERC6538Registry.t.sol +++ b/test/ERC6538Registry.t.sol @@ -194,10 +194,9 @@ contract ERC1271MockContract { bytes32 r; bytes32 s; uint8 v; - // ecrecover takes the signature parameters, and the only way to get them - // currently is to use assembly. - /// @solidity memory-safe-assembly - assembly { + // 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)))