diff --git a/.changeset/fine-teams-serve.md b/.changeset/fine-teams-serve.md new file mode 100644 index 00000000000..241d5fac807 --- /dev/null +++ b/.changeset/fine-teams-serve.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`SignatureChecker`: Add `isValidSignatureNowCalldata(bytes,bytes32,bytes)` and `areValidSignaturesNowCalldata(hash,bytes[],bytes[])` that take arguments in calldata rather than memory. diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index 036be785eb4..a95ef33188c 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -121,6 +121,28 @@ library SignatureChecker { } } + /** + * @dev Variant of {isValidSignatureNow-bytes-bytes32-bytes-} that takes a signature in calldata + */ + function isValidSignatureNowCalldata( + bytes calldata signer, + bytes32 hash, + bytes calldata signature + ) internal view returns (bool) { + if (signer.length < 20) { + return false; + } else if (signer.length == 20) { + return isValidSignatureNowCalldata(address(bytes20(signer)), hash, signature); + } else { + (bool success, bytes memory result) = address(bytes20(signer)).staticcall( + abi.encodeCall(IERC7913SignatureVerifier.verify, (signer[20:], hash, signature)) + ); + return (success && + result.length >= 32 && + abi.decode(result, (bytes32)) == bytes32(IERC7913SignatureVerifier.verify.selector)); + } + } + /** * @dev Verifies multiple ERC-7913 `signatures` for a given `hash` using a set of `signers`. * Returns `false` if the number of signers and signatures is not the same. @@ -161,4 +183,38 @@ library SignatureChecker { return true; } + + /** + * @dev Variant of {areValidSignaturesNow} that takes signers and signatures in calldata + */ + function areValidSignaturesNowCalldata( + bytes32 hash, + bytes[] calldata signers, + bytes[] calldata signatures + ) internal view returns (bool) { + if (signers.length != signatures.length) return false; + + bytes32 lastId = bytes32(0); + + for (uint256 i = 0; i < signers.length; ++i) { + bytes calldata signer = signers[i]; + + // If one of the signatures is invalid, reject the batch + if (!isValidSignatureNowCalldata(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. + if (lastId < id) { + lastId = id; + } else { + // If this signer id is not greater than all the previous ones, verify that it is not a duplicate of a previous one + // This loop is never executed if the signers are ordered by id. + for (uint256 j = 0; j < i; ++j) { + if (id == keccak256(signers[j])) return false; + } + } + } + + return true; + } } diff --git a/test/utils/cryptography/SignatureChecker.test.js b/test/utils/cryptography/SignatureChecker.test.js index 82561d53877..23e1fb212bd 100644 --- a/test/utils/cryptography/SignatureChecker.test.js +++ b/test/utils/cryptography/SignatureChecker.test.js @@ -36,24 +36,39 @@ describe('SignatureChecker (ERC1271)', function () { await expect( this.mock.$isValidSignatureNow(ethers.Typed.address(this.signer.address), TEST_MESSAGE_HASH, this.signature), ).to.eventually.be.true; - await expect(this.mock.$isValidSignatureNowCalldata(this.signer.address, TEST_MESSAGE_HASH, this.signature)).to - .eventually.be.true; + await expect( + this.mock.$isValidSignatureNowCalldata( + 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(ethers.Typed.address(this.other.address), TEST_MESSAGE_HASH, this.signature), ).to.eventually.be.false; - await expect(this.mock.$isValidSignatureNowCalldata(this.other.address, TEST_MESSAGE_HASH, this.signature)).to - .eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata( + 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(ethers.Typed.address(this.signer.address), WRONG_MESSAGE_HASH, this.signature), ).to.eventually.be.false; - await expect(this.mock.$isValidSignatureNowCalldata(this.signer.address, WRONG_MESSAGE_HASH, this.signature)).to - .eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata( + ethers.Typed.address(this.signer.address), + WRONG_MESSAGE_HASH, + this.signature, + ), + ).to.eventually.be.false; }); }); @@ -117,6 +132,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature)).to .eventually.be.true; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature), + ).to.eventually.be.true; }); it('with invalid signer', async function () { @@ -124,6 +142,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(eoaSigner), TEST_MESSAGE_HASH, signature), + ).to.eventually.be.false; }); it('with invalid signature', async function () { @@ -131,6 +152,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(eoaSigner), WRONG_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(eoaSigner), WRONG_MESSAGE_HASH, signature), + ).to.eventually.be.false; }); }); @@ -140,6 +164,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature)) .to.eventually.be.true; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature), + ).to.eventually.be.true; }); it('with invalid signer', async function () { @@ -147,6 +174,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature)) .to.eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(walletSigner), TEST_MESSAGE_HASH, signature), + ).to.eventually.be.false; }); it('with invalid signature', async function () { @@ -154,6 +184,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(walletSigner), WRONG_MESSAGE_HASH, signature)) .to.eventually.be.false; + await expect( + this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(walletSigner), WRONG_MESSAGE_HASH, signature), + ).to.eventually.be.false; }); }); @@ -168,6 +201,8 @@ describe('SignatureChecker (ERC1271)', function () { await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.true; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.true; }); it('with invalid verifier', async function () { @@ -180,6 +215,8 @@ describe('SignatureChecker (ERC1271)', function () { await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); it('with invalid key', async function () { @@ -188,6 +225,8 @@ describe('SignatureChecker (ERC1271)', function () { await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); it('with invalid signature', async function () { @@ -200,6 +239,8 @@ describe('SignatureChecker (ERC1271)', function () { await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); it('with signer too short', async function () { @@ -207,6 +248,8 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await aliceP256.signMessage(TEST_MESSAGE); await expect(this.mock.$isValidSignatureNow(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)).to .eventually.be.false; + await expect(this.mock.$isValidSignatureNowCalldata(ethers.Typed.bytes(signer), TEST_MESSAGE_HASH, signature)) + .to.eventually.be.false; }); }); }); @@ -220,6 +263,9 @@ describe('SignatureChecker (ERC1271)', function () { const signature = await this.signer.signMessage(TEST_MESSAGE); await expect(this.mock.$areValidSignaturesNow(TEST_MESSAGE_HASH, [signer], [signature])).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata(TEST_MESSAGE_HASH, [ethers.Typed.bytes(signer)], [signature]), + ).to.eventually.be.true; }); it('should validate multiple signatures with different signer types', async function () { @@ -249,6 +295,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.true; }); it('should validate multiple EOA signatures', async function () { @@ -270,6 +323,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.true; }); it('should validate multiple ERC-1271 wallet signatures', async function () { @@ -291,6 +351,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.true; }); it('should validate multiple ERC-7913 signatures (ordered by ID)', async function () { @@ -320,6 +387,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.true; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.true; }); it('should validate multiple ERC-7913 signatures (unordered)', async function () { @@ -370,6 +444,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.false; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.false; }); it('should return false if there are duplicate signers', async function () { @@ -391,6 +472,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature), ), ).to.eventually.be.false; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature), + ), + ).to.eventually.be.false; }); it('should return false if signatures array length does not match signers array length', async function () { @@ -412,6 +500,13 @@ describe('SignatureChecker (ERC1271)', function () { signers.map(({ signature }) => signature).slice(1), ), ).to.eventually.be.false; + await expect( + this.mock.$areValidSignaturesNowCalldata( + TEST_MESSAGE_HASH, + signers.map(({ signer }) => ethers.Typed.bytes(signer)), + signers.map(({ signature }) => signature).slice(1), + ), + ).to.eventually.be.false; }); it('should pass with empty arrays', async function () {