Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isValidERC1271SignatureNow to SignatureChecker library #3932

Merged
5 changes: 5 additions & 0 deletions .changeset/slimy-knives-hug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`SignatureChecker`: Add `isValidERC1271SignatureNow` for checking a signature directly against a smart contract using ERC-1271.
19 changes: 16 additions & 3 deletions contracts/utils/cryptography/SignatureChecker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,23 @@ library SignatureChecker {
*/
function isValidSignatureNow(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) {
(address recovered, ECDSA.RecoverError error) = ECDSA.tryRecover(hash, signature);
if (error == ECDSA.RecoverError.NoError && recovered == signer) {
return true;
}
return
(error == ECDSA.RecoverError.NoError && recovered == signer) ||
isValidERC1271SignatureNow(signer, hash, signature);
}

/**
* @dev Checks if a signature is valid for a given signer and data hash. The signature is validated
* against the signer smart contract using ERC1271.
*
* 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 isValidERC1271SignatureNow(
address signer,
bytes32 hash,
bytes memory signature
) internal view returns (bool) {
(bool success, bytes memory result) = signer.staticcall(
abi.encodeWithSelector(IERC1271.isValidSignature.selector, hash, signature)
);
Expand Down
76 changes: 40 additions & 36 deletions test/utils/cryptography/SignatureChecker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,44 +40,48 @@ contract('SignatureChecker (ERC1271)', function (accounts) {
});

context('ERC1271 wallet', function () {
it('with matching signer and signature', async function () {
expect(
await this.signaturechecker.$isValidSignatureNow(
this.wallet.address,
toEthSignedMessageHash(TEST_MESSAGE),
this.signature,
),
).to.equal(true);
});
for (const signature of ['isValidERC1271SignatureNow', 'isValidSignatureNow']) {
context(signature, function () {
it('with matching signer and signature', async function () {
expect(
await this.signaturechecker[`$${signature}`](
this.wallet.address,
toEthSignedMessageHash(TEST_MESSAGE),
this.signature,
),
).to.equal(true);
});

it('with invalid signer', async function () {
expect(
await this.signaturechecker.$isValidSignatureNow(
this.signaturechecker.address,
toEthSignedMessageHash(TEST_MESSAGE),
this.signature,
),
).to.equal(false);
});
it('with invalid signer', async function () {
expect(
await this.signaturechecker[`$${signature}`](
this.signaturechecker.address,
toEthSignedMessageHash(TEST_MESSAGE),
this.signature,
),
).to.equal(false);
});

it('with invalid signature', async function () {
expect(
await this.signaturechecker.$isValidSignatureNow(
this.wallet.address,
toEthSignedMessageHash(WRONG_MESSAGE),
this.signature,
),
).to.equal(false);
});
it('with invalid signature', async function () {
expect(
await this.signaturechecker[`$${signature}`](
this.wallet.address,
toEthSignedMessageHash(WRONG_MESSAGE),
this.signature,
),
).to.equal(false);
});

it('with malicious wallet', async function () {
expect(
await this.signaturechecker.$isValidSignatureNow(
this.malicious.address,
toEthSignedMessageHash(TEST_MESSAGE),
this.signature,
),
).to.equal(false);
});
it('with malicious wallet', async function () {
expect(
await this.signaturechecker[`$${signature}`](
this.malicious.address,
toEthSignedMessageHash(TEST_MESSAGE),
this.signature,
),
).to.equal(false);
});
});
}
});
});