mirror
/
openzeppelin-contracts
mirror of https://github.com/OpenZeppelin/openzeppelin-contracts
2
1
Fork 1
Code Issues Releases Wiki Activity

Remove redundant ECDSA constraint (#3591)

Browse Source 
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
pull/3627/head
t11s 3 years ago committed by GitHub
parent 2dc086563f
commit 324eda228c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 45 additions and 61 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 3
      CHANGELOG.md
  2. 7
      contracts/utils/cryptography/ECDSA.sol
  3. 96
      test/utils/cryptography/ECDSA.test.js

3
CHANGELOG.md
Unescape View File

@ -7,12 +7,13 @@
* `Address`: optimize `functionCall` functions by checking contract size only if there is no returned data. ([#3469](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3469))
* `GovernorCompatibilityBravo`: remove unused `using` statements ([#3506](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3506))
* `ERC20`: optimize `_transfer`, `_mint` and `_burn` by using `unchecked` arithmetic when possible. ([#3513](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3513))
* `ERC20FlashMint`: add an internal `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551))
* `ERC721`: optimize transfers by making approval clearing implicit instead of emitting an event. ([#3481](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3481))
* `ERC721`: optimize burn by making approval clearing implicit instead of emitting an event. ([#3538](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3538))
* `ReentrancyGuard`: Reduce code size impact of the modifier by using internal functions. ([#3515](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3515))
* `SafeCast`: optimize downcasting of signed integers. ([#3565](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3565))
* `ERC20FlashMint`: add an internal `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551))
* `VestingWallet`: remove unused library `Math.sol`. ([#3605](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3605))
* `ECDSA`: Remove redundant check on the `v` value. ([#3591](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3591))
### Compatibility Note

7
contracts/utils/cryptography/ECDSA.sol
Unescape View File

@ -17,7 +17,7 @@ library ECDSA {
InvalidSignature,
InvalidSignatureLength,
InvalidSignatureS,
InvalidSignatureV
InvalidSignatureV // Deprecated in v4.8
}
function _throwError(RecoverError error) private pure {
@ -29,8 +29,6 @@ library ECDSA {
revert("ECDSA: invalid signature length");
} else if (error == RecoverError.InvalidSignatureS) {
revert("ECDSA: invalid signature 's' value");
} else if (error == RecoverError.InvalidSignatureV) {
revert("ECDSA: invalid signature 'v' value");
}
}
@ -149,9 +147,6 @@ library ECDSA {
if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
return (address(0), RecoverError.InvalidSignatureS);
}
if (v != 27 && v != 28) {
return (address(0), RecoverError.InvalidSignatureV);
}
// If the signature is valid (and not malleable), return the signer address
address signer = ecrecover(hash, v, r, s);

96
test/utils/cryptography/ECDSA.test.js
Unescape View File

@ -98,45 +98,39 @@ contract('ECDSA', function (accounts) {
});
});
context('with v0 signature', function () {
context('with v=27 signature', function () {
// Signature generated outside ganache with method web3.eth.sign(signer, message)
const signer = '0x2cc1166f6212628A0deEf2B33BEFB2187D35b86c';
// eslint-disable-next-line max-len
const signatureWithoutVersion = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892';
const signatureWithoutV = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892';
it('reverts with 00 as version value', async function () {
const version = '00';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
await expectRevert(
this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)),
'ECDSA: invalid signature \'v\' value',
);
});
it('works with 27 as version value', async function () {
const version = '1b'; // 27 = 1b.
const signature = signatureWithoutVersion + version;
it('works with correct v value', async function () {
const v = '1b'; // 27 = 1b.
const signature = signatureWithoutV + v;
expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.equal(signer);
expect(await this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature))).to.equal(signer);
expect(await this.ecdsa.recover_r_vs(TEST_MESSAGE, ...split(to2098Format(signature)))).to.equal(signer);
});
it('reverts with wrong version', async function () {
// The last two hex digits are the signature version.
// The only valid values are 0, 1, 27 and 28.
const version = '02';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
await expectRevert(
this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)),
'ECDSA: invalid signature \'v\' value',
);
it('rejects incorrect v value', async function () {
const v = '1c'; // 28 = 1c.
const signature = signatureWithoutV + v;
expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.not.equal(signer);
expect(await this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature))).to.not.equal(signer);
expect(await this.ecdsa.recover_r_vs(TEST_MESSAGE, ...split(to2098Format(signature)))).to.not.equal(signer);
});
it('reverts wrong v values', async function () {
for (const v of ['00', '01']) {
const signature = signatureWithoutV + v;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature');
await expectRevert(this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)), 'ECDSA: invalid signature');
}
});
it('rejects short EIP2098 format', async function () {
const version = '1b'; // 27 = 1b.
const signature = signatureWithoutVersion + version;
const v = '1b'; // 27 = 1b.
const signature = signatureWithoutV + v;
await expectRevert(
this.ecdsa.recover(TEST_MESSAGE, to2098Format(signature)),
'ECDSA: invalid signature length',
@ -144,44 +138,38 @@ contract('ECDSA', function (accounts) {
});
});
context('with v1 signature', function () {
context('with v=28 signature', function () {
const signer = '0x1E318623aB09Fe6de3C9b8672098464Aeda9100E';
// eslint-disable-next-line max-len
const signatureWithoutVersion = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0';
const signatureWithoutV = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0';
it('reverts with 01 as version value', async function () {
const version = '01';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
await expectRevert(
this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)),
'ECDSA: invalid signature \'v\' value',
);
});
it('works with 28 as version value', async function () {
const version = '1c'; // 28 = 1c.
const signature = signatureWithoutVersion + version;
it('works with correct v value', async function () {
const v = '1c'; // 28 = 1c.
const signature = signatureWithoutV + v;
expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.equal(signer);
expect(await this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature))).to.equal(signer);
expect(await this.ecdsa.recover_r_vs(TEST_MESSAGE, ...split(to2098Format(signature)))).to.equal(signer);
});
it('reverts with wrong version', async function () {
// The last two hex digits are the signature version.
// The only valid values are 0, 1, 27 and 28.
const version = '02';
const signature = signatureWithoutVersion + version;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
await expectRevert(
this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)),
'ECDSA: invalid signature \'v\' value',
);
it('rejects incorrect v value', async function () {
const v = '1b'; // 27 = 1b.
const signature = signatureWithoutV + v;
expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.not.equal(signer);
expect(await this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature))).to.not.equal(signer);
expect(await this.ecdsa.recover_r_vs(TEST_MESSAGE, ...split(to2098Format(signature)))).to.not.equal(signer);
});
it('reverts invalid v values', async function () {
for (const v of ['00', '01']) {
const signature = signatureWithoutV + v;
await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature');
await expectRevert(this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)), 'ECDSA: invalid signature');
}
});
it('rejects short EIP2098 format', async function () {
const version = '1c'; // 27 = 1b.
const signature = signatureWithoutVersion + version;
const v = '1c'; // 27 = 1b.
const signature = signatureWithoutV + v;
await expectRevert(
this.ecdsa.recover(TEST_MESSAGE, to2098Format(signature)),
'ECDSA: invalid signature length',

Write Preview
Loading…
Cancel
Save