diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 261372f0c..3b570418f 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -27,7 +27,7 @@ library SignatureChecker { * NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus * change through time. It could return true at block N and false at block N+1 (or the opposite). * - * NOTE: For an extended version of this function that supports ERC-7913 signatures, see {isValidERC7913SignatureNow}. + * NOTE: For an extended version of this function that supports ERC-7913 signatures, see {isValidSignatureNow-bytes-bytes32-bytes-}. */ function isValidSignatureNow(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) { if (signer.code.length == 0) { @@ -73,7 +73,7 @@ library SignatureChecker { * NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus * change through time. It could return true at block N and false at block N+1 (or the opposite). */ - function isValidERC7913SignatureNow( + function isValidSignatureNow( bytes memory signer, bytes32 hash, bytes memory signature @@ -102,7 +102,7 @@ library SignatureChecker { * NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus * change through time. It could return true at block N and false at block N+1 (or the opposite). */ - function areValidERC7913SignaturesNow( + function areValidSignaturesNow( bytes32 hash, bytes[] memory signers, bytes[] memory signatures @@ -115,7 +115,7 @@ library SignatureChecker { bytes memory signer = signers[i]; // If one of the signatures is invalid, reject the batch - if (!isValidERC7913SignatureNow(signer, hash, signatures[i])) return false; + if (!isValidSignatureNow(signer, hash, signatures[i])) return false; bytes32 id = keccak256(signer); // If the current signer ID is greater than all previous IDs, then this is a new signer. diff --git a/contracts/utils/cryptography/signers/MultiSignerERC7913.sol b/contracts/utils/cryptography/signers/MultiSignerERC7913.sol index a23c82dcb..b83f5520f 100644 --- a/contracts/utils/cryptography/signers/MultiSignerERC7913.sol +++ b/contracts/utils/cryptography/signers/MultiSignerERC7913.sol @@ -209,7 +209,7 @@ abstract contract MultiSignerERC7913 is AbstractSigner { * Returns whether whether the signers are authorized and the signatures are valid for the given hash. * * IMPORTANT: Sorting the signers by their `keccak256` hash will improve the gas efficiency of this function. - * See {SignatureChecker-areValidERC7913SignaturesNow} for more details. + * See {SignatureChecker-areValidSignaturesNow-bytes32-bytes[]-bytes[]} for more details. * * Requirements: * @@ -225,7 +225,7 @@ abstract contract MultiSignerERC7913 is AbstractSigner { return false; } } - return hash.areValidERC7913SignaturesNow(signers, signatures); + return hash.areValidSignaturesNow(signers, signatures); } /** diff --git a/contracts/utils/cryptography/signers/SignerERC7913.sol b/contracts/utils/cryptography/signers/SignerERC7913.sol index 107b3959f..ecc387536 100644 --- a/contracts/utils/cryptography/signers/SignerERC7913.sol +++ b/contracts/utils/cryptography/signers/SignerERC7913.sol @@ -41,11 +41,14 @@ abstract contract SignerERC7913 is AbstractSigner { _signer = signer_; } - /// @dev Verifies a signature using {SignatureChecker-isValidERC7913SignatureNow} with {signer}, `hash` and `signature`. + /** + * @dev Verifies a signature using {SignatureChecker-isValidSignatureNow-bytes-bytes32-bytes-} + * with {signer}, `hash` and `signature`. + */ function _rawSignatureValidation( bytes32 hash, bytes calldata signature ) internal view virtual override returns (bool) { - return SignatureChecker.isValidERC7913SignatureNow(signer(), hash, signature); + return SignatureChecker.isValidSignatureNow(signer(), hash, signature); } } diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc index af9cb69bf..7aae15b9d 100644 --- a/docs/modules/ROOT/pages/utilities.adoc +++ b/docs/modules/ROOT/pages/utilities.adoc @@ -142,7 +142,7 @@ A signer is represented as a `bytes` object that concatenates a verifier address [source,solidity] ---- function _verifyERC7913Signature(bytes memory signer, bytes32 hash, bytes memory signature) internal view returns (bool) { - return SignatureChecker.isValidERC7913SignatureNow(signer, hash, signature); + return SignatureChecker.isValidSignatureNow(signer, hash, signature); } ---- @@ -163,7 +163,7 @@ function _verifyMultipleSignatures( bytes[] memory signers, bytes[] memory signatures ) internal view returns (bool) { - return SignatureChecker.areValidERC7913SignaturesNow(hash, signers, signatures); + return SignatureChecker.areValidSignaturesNow(hash, signers, signatures); } ---- diff --git a/test/utils/cryptography/SignatureChecker.test.js b/test/utils/cryptography/SignatureChecker.test.js index e767adfa8..143619619 100644 --- a/test/utils/cryptography/SignatureChecker.test.js +++ b/test/utils/cryptography/SignatureChecker.test.js @@ -33,18 +33,21 @@ describe('SignatureChecker (ERC1271)', function () { describe('EOA account', function () { it('with matching signer and signature', async function () { - await expect(this.mock.$isValidSignatureNow(this.signer, TEST_MESSAGE_HASH, this.signature)).to.eventually.be - .true; + await expect( + this.mock.$isValidSignatureNow(ethers.Typed.address(this.signer.address), TEST_MESSAGE_HASH, this.signature), + ).to.eventually.be.true; }); it('with invalid signer', async function () { - await expect(this.mock.$isValidSignatureNow(this.other, TEST_MESSAGE_HASH, this.signature)).to.eventually.be - .false; + await expect( + this.mock.$isValidSignatureNow(ethers.Typed.address(this.other.address), TEST_MESSAGE_HASH, this.signature), + ).to.eventually.be.false; }); it('with invalid signature', async function () { - await expect(this.mock.$isValidSignatureNow(this.signer, WRONG_MESSAGE_HASH, this.signature)).to.eventually.be - .false; + await expect( + this.mock.$isValidSignatureNow(ethers.Typed.address(this.signer.address), WRONG_MESSAGE_HASH, this.signature), + ).to.eventually.be.false; }); }); @@ -52,55 +55,76 @@ describe('SignatureChecker (ERC1271)', function () { for (const fn of ['isValidERC1271SignatureNow', 'isValidSignatureNow']) { describe(fn, function () { it('with matching signer and signature', async function () { - await expect(this.mock.getFunction(`$${fn}`)(this.wallet, TEST_MESSAGE_HASH, this.signature)).to.eventually.be - .true; + await expect( + this.mock.getFunction(`$${fn}`)( + ethers.Typed.address(this.wallet.target), + TEST_MESSAGE_HASH, + this.signature, + ), + ).to.eventually.be.true; }); it('with invalid signer', async function () { - await expect(this.mock.getFunction(`$${fn}`)(this.mock, TEST_MESSAGE_HASH, this.signature)).to.eventually.be - .false; + await expect( + this.mock.getFunction(`$${fn}`)(ethers.Typed.address(this.mock.target), TEST_MESSAGE_HASH, this.signature), + ).to.eventually.be.false; }); it('with identity precompile', async function () { - await expect(this.mock.getFunction(`$${fn}`)(precompile.identity, TEST_MESSAGE_HASH, this.signature)).to - .eventually.be.false; + await expect( + this.mock.getFunction(`$${fn}`)( + ethers.Typed.address(precompile.identity), + TEST_MESSAGE_HASH, + this.signature, + ), + ).to.eventually.be.false; }); it('with invalid signature', async function () { - await expect(this.mock.getFunction(`$${fn}`)(this.wallet, WRONG_MESSAGE_HASH, this.signature)).to.eventually - .be.false; + await expect( + this.mock.getFunction(`$${fn}`)( + ethers.Typed.address(this.wallet.target), + WRONG_MESSAGE_HASH, + this.signature, + ), + ).to.eventually.be.false; }); it('with malicious wallet', async function () { - await expect(this.mock.getFunction(`$${fn}`)(this.malicious, TEST_MESSAGE_HASH, this.signature)).to.eventually - .be.false; + await expect( + this.mock.getFunction(`$${fn}`)( + ethers.Typed.address(this.malicious.target), + TEST_MESSAGE_HASH, + this.signature, + ), + ).to.eventually.be.false; }); }); } }); describe('ERC7913', function () { - describe('isValidERC7913SignatureNow', function () { + describe('isValidSignatureNow', function () { describe('with EOA signer', function () { it('with matching signer and signature', async function () { const eoaSigner = ethers.zeroPadValue(this.signer.address, 20); const signature = await this.signer.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(eoaSigner, TEST_MESSAGE_HASH, signature)).to.eventually.be - .true; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature)).to + .eventually.be.true; }); it('with invalid signer', async function () { const eoaSigner = ethers.zeroPadValue(this.other.address, 20); const signature = await this.signer.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(eoaSigner, TEST_MESSAGE_HASH, signature)).to.eventually.be - .false; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature)).to + .eventually.be.false; }); it('with invalid signature', async function () { const eoaSigner = ethers.zeroPadValue(this.signer.address, 20); const signature = await this.signer.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(eoaSigner, WRONG_MESSAGE_HASH, signature)).to.eventually.be - .false; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), WRONG_MESSAGE_HASH, signature)).to + .eventually.be.false; }); }); @@ -108,22 +132,22 @@ describe('SignatureChecker (ERC1271)', function () { it('with matching signer and signature', async function () { const walletSigner = ethers.zeroPadValue(this.wallet.target, 20); const signature = await this.signer.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(walletSigner, TEST_MESSAGE_HASH, signature)).to.eventually - .be.true; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.true; }); it('with invalid signer', async function () { const walletSigner = ethers.zeroPadValue(this.mock.target, 20); const signature = await this.signer.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(walletSigner, TEST_MESSAGE_HASH, signature)).to.eventually - .be.false; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); it('with invalid signature', async function () { const walletSigner = ethers.zeroPadValue(this.wallet.target, 20); const signature = await this.signer.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(walletSigner, WRONG_MESSAGE_HASH, signature)).to.eventually - .be.false; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), WRONG_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); }); @@ -136,8 +160,8 @@ describe('SignatureChecker (ERC1271)', function () { ]); const signature = await aliceP256.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(signer, TEST_MESSAGE_HASH, signature)).to.eventually.be - .true; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to + .eventually.be.true; }); it('with invalid verifier', async function () { @@ -148,16 +172,16 @@ describe('SignatureChecker (ERC1271)', function () { ]); const signature = await aliceP256.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(signer, TEST_MESSAGE_HASH, signature)).to.eventually.be - .false; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to + .eventually.be.false; }); it('with invalid key', async function () { const signer = ethers.concat([this.verifier.target, ethers.randomBytes(32)]); const signature = await aliceP256.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(signer, TEST_MESSAGE_HASH, signature)).to.eventually.be - .false; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to + .eventually.be.false; }); it('with invalid signature', async function () { @@ -168,20 +192,20 @@ describe('SignatureChecker (ERC1271)', function () { ]); const signature = ethers.randomBytes(65); // invalid (random) signature - await expect(this.mock.$isValidERC7913SignatureNow(signer, TEST_MESSAGE_HASH, signature)).to.eventually.be - .false; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to + .eventually.be.false; }); it('with signer too short', async function () { const signer = ethers.randomBytes(19); // too short const signature = await aliceP256.signMessage(TEST_MESSAGE); - await expect(this.mock.$isValidERC7913SignatureNow(signer, TEST_MESSAGE_HASH, signature)).to.eventually.be - .false; + await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to + .eventually.be.false; }); }); }); - describe('areValidERC7913SignaturesNow', function () { + describe('areValidSignaturesNow', function () { const sortSigners = (...signers) => signers.sort(({ signer: a }, { signer: b }) => ethers.keccak256(b) - ethers.keccak256(a)); @@ -189,8 +213,7 @@ describe('SignatureChecker (ERC1271)', function () { const signer = ethers.zeroPadValue(this.signer.address, 20); const signature = await this.signer.signMessage(TEST_MESSAGE); - await expect(this.mock.$areValidERC7913SignaturesNow(TEST_MESSAGE_HASH, [signer], [signature])).to.eventually.be - .true; + await expect(this.mock.$areValidSignaturesNow(TEST_MESSAGE_HASH, [signer], [signature])).to.eventually.be.true; }); it('should validate multiple signatures with different signer types', async function () { @@ -214,7 +237,7 @@ describe('SignatureChecker (ERC1271)', function () { ); await expect( - this.mock.$areValidERC7913SignaturesNow( + this.mock.$areValidSignaturesNow( TEST_MESSAGE_HASH, signers.map(({ signer }) => signer), signers.map(({ signature }) => signature), @@ -235,7 +258,7 @@ describe('SignatureChecker (ERC1271)', function () { ); await expect( - this.mock.$areValidERC7913SignaturesNow( + this.mock.$areValidSignaturesNow( TEST_MESSAGE_HASH, signers.map(({ signer }) => signer), signers.map(({ signature }) => signature), @@ -256,7 +279,7 @@ describe('SignatureChecker (ERC1271)', function () { ); await expect( - this.mock.$areValidERC7913SignaturesNow( + this.mock.$areValidSignaturesNow( TEST_MESSAGE_HASH, signers.map(({ signer }) => signer), signers.map(({ signature }) => signature), @@ -285,7 +308,7 @@ describe('SignatureChecker (ERC1271)', function () { ); await expect( - this.mock.$areValidERC7913SignaturesNow( + this.mock.$areValidSignaturesNow( TEST_MESSAGE_HASH, signers.map(({ signer }) => signer), signers.map(({ signature }) => signature), @@ -314,7 +337,7 @@ describe('SignatureChecker (ERC1271)', function () { ).reverse(); // reverse await expect( - this.mock.$areValidERC7913SignaturesNow( + this.mock.$areValidSignaturesNow( TEST_MESSAGE_HASH, signers.map(({ signer }) => signer), signers.map(({ signature }) => signature), @@ -335,7 +358,7 @@ describe('SignatureChecker (ERC1271)', function () { ); await expect( - this.mock.$areValidERC7913SignaturesNow( + this.mock.$areValidSignaturesNow( TEST_MESSAGE_HASH, signers.map(({ signer }) => signer), signers.map(({ signature }) => signature), @@ -356,7 +379,7 @@ describe('SignatureChecker (ERC1271)', function () { ); await expect( - this.mock.$areValidERC7913SignaturesNow( + this.mock.$areValidSignaturesNow( TEST_MESSAGE_HASH, signers.map(({ signer }) => signer), signers.map(({ signature }) => signature), @@ -377,7 +400,7 @@ describe('SignatureChecker (ERC1271)', function () { ); await expect( - this.mock.$areValidERC7913SignaturesNow( + this.mock.$areValidSignaturesNow( TEST_MESSAGE_HASH, signers.map(({ signer }) => signer), signers.map(({ signature }) => signature).slice(1), @@ -386,7 +409,7 @@ describe('SignatureChecker (ERC1271)', function () { }); it('should pass with empty arrays', async function () { - await expect(this.mock.$areValidERC7913SignaturesNow(TEST_MESSAGE_HASH, [], [])).to.eventually.be.true; + await expect(this.mock.$areValidSignaturesNow(TEST_MESSAGE_HASH, [], [])).to.eventually.be.true; }); }); });