From d7c79cd6924a3fab416dccc3fbb1418457ad7de7 Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 20:22:33 -0700 Subject: [PATCH 01/11] GovernorCountingFractional: foundry fuzzing --- .../GovernorCountingFractional.t.sol | 167 ++++++++++++++++++ .../GovernorCountingFractional.test.js | 80 --------- 2 files changed, 167 insertions(+), 80 deletions(-) create mode 100644 test/governance/extensions/GovernorCountingFractional.t.sol diff --git a/test/governance/extensions/GovernorCountingFractional.t.sol b/test/governance/extensions/GovernorCountingFractional.t.sol new file mode 100644 index 00000000000..2fe5d2a00a1 --- /dev/null +++ b/test/governance/extensions/GovernorCountingFractional.t.sol @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; + +import {GovernorFractionalMock} from "@openzeppelin/contracts/mocks/governance/GovernorFractionalMock.sol"; +import {ERC20VotesTimestampMock} from "@openzeppelin/contracts/mocks/token/ERC20VotesTimestampMock.sol"; + +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; +import {Governor} from "@openzeppelin/contracts/governance/Governor.sol"; +import {GovernorSettings} from "@openzeppelin/contracts/governance/extensions/GovernorSettings.sol"; +import {GovernorCountingFractional} from "@openzeppelin/contracts/governance/extensions/GovernorCountingFractional.sol"; +import {GovernorVotesQuorumFraction} from "@openzeppelin/contracts/governance/extensions/GovernorVotesQuorumFraction.sol"; +import {GovernorVotes} from "@openzeppelin/contracts/governance/extensions/GovernorVotes.sol"; + +import {IVotes} from "@openzeppelin/contracts/governance/utils/IVotes.sol"; + +contract GovernorMock is GovernorFractionalMock { + constructor( + string memory name, + uint48 votingDelay, + uint32 votingPeriod, + uint256 proposalThreshold, + address tokenAddress, + uint256 quorumNumeratorValue + ) + GovernorVotesQuorumFraction(quorumNumeratorValue) + GovernorVotes(IVotes(tokenAddress)) + GovernorSettings(votingDelay, votingPeriod, proposalThreshold) + Governor(name) + {} +} + +contract TokenMock is ERC20VotesTimestampMock { + constructor() ERC20("TokenMock", "TKN") EIP712("TokenMock", "1") {} + + function mint(address account, uint256 amount) public { + _mint(account, amount); + } +} + +contract GovernorCountingFractionalTest is Test { + GovernorFractionalMock internal _governor; + TokenMock internal _token; + + uint256 internal _proposerPrivateKey; + uint256 internal _voterPrivateKey; + + address internal _proposer; + address internal _voter; + + function setUp() public { + _token = new TokenMock(); + _governor = new GovernorMock("OZ-Governor", 0, 99999, 0, address(_token), 10); + + _proposerPrivateKey = 0xA11CE; + _voterPrivateKey = 0xB0B; + + _proposer = vm.addr(_proposerPrivateKey); + _voter = vm.addr(_voterPrivateKey); + } + + function createProposal() internal returns (uint256 proposalId) { + address[] memory targets = new address[](1); + targets[0] = address(this); + + uint256[] memory values = new uint256[](1); + values[0] = 0; + + bytes[] memory calldatas = new bytes[](1); + calldatas[0] = ""; + + vm.prank(_proposer); + proposalId = _governor.propose(targets, values, calldatas, "proposal description"); + + vm.warp(block.timestamp + 1); + } + + function testFractionalVotingTwice(uint32[3] memory votes) public { + uint256 sumVotes = uint256(votes[0]) + uint256(votes[1]) + uint256(votes[2]); + vm.assume(sumVotes > 0); + + _token.mint(_voter, sumVotes * 2); + + vm.prank(_voter); + _token.delegate(_voter); + + vm.warp(block.timestamp + 1); + + uint256 proposalId = createProposal(); + + (uint256 againstVotes, uint256 forVotes, uint256 abstainVotes) = _governor.proposalVotes(proposalId); + + assertTrue(againstVotes == 0 && forVotes == 0 && abstainVotes == 0); + assertEq(_governor.hasVoted(proposalId, _voter), false); + assertEq(_governor.usedVotes(proposalId, _voter), 0); + + vm.startPrank(_voter); + // TODO: check emitted events + _governor.castVoteWithReasonAndParams( + proposalId, + 255, + "no particular reason", + abi.encodePacked(uint128(votes[0]), uint128(votes[1]), uint128(votes[2])) + ); + _governor.castVoteWithReasonAndParams( + proposalId, + 255, + "no particular reason", + abi.encodePacked(uint128(votes[0]), uint128(votes[1]), uint128(votes[2])) + ); + + (againstVotes, forVotes, abstainVotes) = _governor.proposalVotes(proposalId); + + assertTrue( + againstVotes == uint256(votes[0]) * 2 && + forVotes == uint256(votes[1]) * 2 && + abstainVotes == uint256(votes[2]) * 2 + ); + assertEq(_governor.hasVoted(proposalId, _voter), true); + assertEq(_governor.usedVotes(proposalId, _voter), sumVotes * 2); + } + + function testFractionalThenNominal(uint32[3] memory fractional, uint160 nominal) public { + uint256 sumVotes = uint256(fractional[0]) + uint256(fractional[1]) + uint256(fractional[2]); + vm.assume(sumVotes > 0); + vm.assume(nominal > 0); + + _token.mint(_voter, sumVotes + nominal); + + vm.prank(_voter); + _token.delegate(_voter); + + vm.warp(block.timestamp + 1); + + uint256 proposalId = createProposal(); + + (uint256 againstVotes, uint256 forVotes, uint256 abstainVotes) = _governor.proposalVotes(proposalId); + + assertTrue(againstVotes == 0 && forVotes == 0 && abstainVotes == 0); + assertEq(_governor.hasVoted(proposalId, _voter), false); + assertEq(_governor.usedVotes(proposalId, _voter), 0); + + vm.startPrank(_voter); + // TODO: check emitted events + _governor.castVoteWithReasonAndParams( + proposalId, + 255, + "no particular reason", + abi.encodePacked(uint128(fractional[0]), uint128(fractional[1]), uint128(fractional[2])) + ); + + _governor.castVoteWithReason(proposalId, 0, "no particular reason"); + + (againstVotes, forVotes, abstainVotes) = _governor.proposalVotes(proposalId); + + assertTrue( + againstVotes == uint256(fractional[0]) + nominal && + forVotes == uint256(fractional[1]) && + abstainVotes == uint256(fractional[2]) + ); + assertEq(_governor.hasVoted(proposalId, _voter), true); + assertEq(_governor.usedVotes(proposalId, _voter), sumVotes + nominal); + } +} diff --git a/test/governance/extensions/GovernorCountingFractional.test.js b/test/governance/extensions/GovernorCountingFractional.test.js index 393dbad79d5..db5beaebd6d 100644 --- a/test/governance/extensions/GovernorCountingFractional.test.js +++ b/test/governance/extensions/GovernorCountingFractional.test.js @@ -4,7 +4,6 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); const { GovernorHelper } = require('../../helpers/governance'); const { VoteType } = require('../../helpers/enums'); -const { zip } = require('../../helpers/iterate'); const { sum } = require('../../helpers/math'); const TOKENS = [ @@ -94,85 +93,6 @@ describe('GovernorCountingFractional', function () { }); describe('voting with a fraction of the weight', function () { - it('twice', async function () { - await this.helper.connect(this.proposer).propose(); - await this.helper.waitForSnapshot(); - - expect(await this.mock.proposalVotes(this.proposal.id)).to.deep.equal([0n, 0n, 0n]); - expect(await this.mock.hasVoted(this.proposal.id, this.voter2)).to.equal(false); - expect(await this.mock.usedVotes(this.proposal.id, this.voter2)).to.equal(0n); - - const steps = [ - ['0', '2', '1'], - ['1', '0', '1'], - ].map(votes => votes.map(vote => ethers.parseEther(vote))); - - for (const votes of steps) { - const params = ethers.solidityPacked(['uint128', 'uint128', 'uint128'], votes); - await expect( - this.helper.connect(this.voter2).vote({ - support: VoteType.Parameters, - reason: 'no particular reason', - params, - }), - ) - .to.emit(this.mock, 'VoteCastWithParams') - .withArgs( - this.voter2, - this.proposal.id, - VoteType.Parameters, - sum(...votes), - 'no particular reason', - params, - ); - } - - expect(await this.mock.proposalVotes(this.proposal.id)).to.deep.equal(zip(...steps).map(v => sum(...v))); - expect(await this.mock.hasVoted(this.proposal.id, this.voter2)).to.equal(true); - expect(await this.mock.usedVotes(this.proposal.id, this.voter2)).to.equal(sum(...[].concat(...steps))); - }); - - it('fractional then nominal', async function () { - await this.helper.connect(this.proposer).propose(); - await this.helper.waitForSnapshot(); - - expect(await this.mock.proposalVotes(this.proposal.id)).to.deep.equal([0n, 0n, 0n]); - expect(await this.mock.hasVoted(this.proposal.id, this.voter2)).to.equal(false); - expect(await this.mock.usedVotes(this.proposal.id, this.voter2)).to.equal(0n); - - const weight = ethers.parseEther('7'); - const fractional = ['1', '2', '1'].map(ethers.parseEther); - - const params = ethers.solidityPacked(['uint128', 'uint128', 'uint128'], fractional); - await expect( - this.helper.connect(this.voter2).vote({ - support: VoteType.Parameters, - reason: 'no particular reason', - params, - }), - ) - .to.emit(this.mock, 'VoteCastWithParams') - .withArgs( - this.voter2, - this.proposal.id, - VoteType.Parameters, - sum(...fractional), - 'no particular reason', - params, - ); - - await expect(this.helper.connect(this.voter2).vote({ support: VoteType.Against })) - .to.emit(this.mock, 'VoteCast') - .withArgs(this.voter2, this.proposal.id, VoteType.Against, weight - sum(...fractional), ''); - - expect(await this.mock.proposalVotes(this.proposal.id)).to.deep.equal([ - weight - sum(...fractional.slice(1)), - ...fractional.slice(1), - ]); - expect(await this.mock.hasVoted(this.proposal.id, this.voter2)).to.equal(true); - expect(await this.mock.usedVotes(this.proposal.id, this.voter2)).to.equal(weight); - }); - it('revert if params spend more than available', async function () { await this.helper.connect(this.proposer).propose(); await this.helper.waitForSnapshot(); From 0936f4a0ba72e2714ab3f99244555d94061a7a68 Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 20:32:49 -0700 Subject: [PATCH 02/11] AuthorityUtilsTest: foundry fuzzing --- test/access/manager/AuthorityUtils.t.sol | 30 ++++++++++++++++++++++ test/access/manager/AuthorityUtils.test.js | 18 ------------- 2 files changed, 30 insertions(+), 18 deletions(-) create mode 100644 test/access/manager/AuthorityUtils.t.sol diff --git a/test/access/manager/AuthorityUtils.t.sol b/test/access/manager/AuthorityUtils.t.sol new file mode 100644 index 00000000000..c93aab743b0 --- /dev/null +++ b/test/access/manager/AuthorityUtils.t.sol @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; + +import {AuthorityUtils} from "@openzeppelin/contracts/access/manager/AuthorityUtils.sol"; +import {AuthorityDelayMock} from "@openzeppelin/contracts/mocks/AuthorityMock.sol"; + +contract AuthorityUtilsTest is Test { + AuthorityDelayMock internal _authority; + + function setUp() public { + _authority = new AuthorityDelayMock(); + } + + function testAuthorityReturnDelay(uint32 delay, bool immediate) public { + _authority._setImmediate(immediate); + _authority._setDelay(delay); + + (bool _immediate, uint32 _delay) = AuthorityUtils.canCallWithDelay( + address(_authority), + address(this), + address(this), + hex"12345678" + ); + assertEq(_immediate, immediate); + assertEq(_delay, delay); + } +} diff --git a/test/access/manager/AuthorityUtils.test.js b/test/access/manager/AuthorityUtils.test.js index 905913f14c2..c837aacd448 100644 --- a/test/access/manager/AuthorityUtils.test.js +++ b/test/access/manager/AuthorityUtils.test.js @@ -64,24 +64,6 @@ describe('AuthorityUtils', function () { }); }); - describe('when authority replies with a delay', function () { - beforeEach(async function () { - this.authority = this.authorityDelayMock; - }); - - for (const immediate of [true, false]) { - for (const delay of [0n, 42n]) { - it(`returns (immediate=${immediate}, delay=${delay})`, async function () { - await this.authority._setImmediate(immediate); - await this.authority._setDelay(delay); - const result = await this.mock.$canCallWithDelay(this.authority, this.user, this.other, '0x12345678'); - expect(result.immediate).to.equal(immediate); - expect(result.delay).to.equal(delay); - }); - } - } - }); - describe('when authority replies with empty data', function () { beforeEach(async function () { this.authority = this.authorityNoResponse; From 420ebeb2e3dc0403b1509769c7d091712caa268f Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 20:46:34 -0700 Subject: [PATCH 03/11] ERC721Consecutive: remove hardhat test Removing Hardhat test as it's tested and fuzzed with Foundry via `test_transfer` --- test/token/ERC721/extensions/ERC721Consecutive.test.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/token/ERC721/extensions/ERC721Consecutive.test.js b/test/token/ERC721/extensions/ERC721Consecutive.test.js index d2eda944ccd..8dbec91e828 100644 --- a/test/token/ERC721/extensions/ERC721Consecutive.test.js +++ b/test/token/ERC721/extensions/ERC721Consecutive.test.js @@ -142,12 +142,6 @@ describe('ERC721Consecutive', function () { describe('ERC721 behavior', function () { const tokenId = offset + 1n; - it('core takes over ownership on transfer', async function () { - await this.token.connect(this.alice).transferFrom(this.alice, this.receiver, tokenId); - - expect(await this.token.ownerOf(tokenId)).to.equal(this.receiver); - }); - it('tokens can be burned and re-minted #1', async function () { await expect(this.token.connect(this.alice).$_burn(tokenId)) .to.emit(this.token, 'Transfer') From 0a0a9e051c3a2b55b3fa00ea71088dab62291d54 Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 21:22:00 -0700 Subject: [PATCH 04/11] Strings: add foundry fuzz and verify --- test/utils/Strings.t.sol | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 test/utils/Strings.t.sol diff --git a/test/utils/Strings.t.sol b/test/utils/Strings.t.sol new file mode 100644 index 00000000000..e10d35ebd5e --- /dev/null +++ b/test/utils/Strings.t.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; + +import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; + +contract StringsTest is Test { + function testStringUint256(uint256 value) public { + string memory actual = Strings.toString(value); + assertEq(actual, vm.toString(value)); + } + + function testStringInt256(int256 value) public { + string memory actual = Strings.toStringSigned(value); + assertEq(actual, vm.toString(value)); + } + + function testHexStringUint256(uint256 value) public { + string memory actual = Strings.toHexString(value); + assertEq(actual, vm.toString(bytes32(value))); + } + + function testChecksumHexStringAddress(address value) public { + string memory actual = Strings.toChecksumHexString(value); + assertEq(actual, vm.toString(value)); + } +} From 894e4645e16e57a96c94e806edadfb14723100b5 Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 21:29:35 -0700 Subject: [PATCH 05/11] SignedMath: remove unit tests Remove `average` and `abs` unit tests which attempt to do fuzzing in favor of Foundry native-fuzzing, already found in `SignedMath.t.sol` --- test/utils/math/SignedMath.test.js | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/test/utils/math/SignedMath.test.js b/test/utils/math/SignedMath.test.js index 877f3b480c4..f13919c84ea 100644 --- a/test/utils/math/SignedMath.test.js +++ b/test/utils/math/SignedMath.test.js @@ -30,24 +30,4 @@ describe('SignedMath', function () { await testCommutative(this.mock.$min, -1234n, 5678n, min(-1234n, 5678n)); }); }); - - describe('average', function () { - it('is correctly calculated with various input', async function () { - for (const x of [ethers.MinInt256, -57417n, -42304n, -4n, -3n, 0n, 3n, 4n, 42304n, 57417n, ethers.MaxInt256]) { - for (const y of [ethers.MinInt256, -57417n, -42304n, -5n, -2n, 0n, 2n, 5n, 42304n, 57417n, ethers.MaxInt256]) { - expect(await this.mock.$average(x, y)).to.equal((x + y) / 2n); - } - } - }); - }); - - describe('abs', function () { - const abs = x => (x < 0n ? -x : x); - - for (const n of [ethers.MinInt256, ethers.MinInt256 + 1n, -1n, 0n, 1n, ethers.MaxInt256 - 1n, ethers.MaxInt256]) { - it(`correctly computes the absolute value of ${n}`, async function () { - expect(await this.mock.$abs(n)).to.equal(abs(n)); - }); - } - }); }); From dea68617457468fd763dd812616efcc130d8c7af Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 21:55:28 -0700 Subject: [PATCH 06/11] forge install: forge-std v1.8.2 --- lib/forge-std | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/forge-std b/lib/forge-std index ae570fec082..978ac6fadb6 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit ae570fec082bfe1c1f45b0acca4a2b4f84d345ce +Subproject commit 978ac6fadb62f5f0b723c996f64be52eddba6801 From 0ab5f8460e01093a388c57c7b9b99faa699f0284 Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 21:57:43 -0700 Subject: [PATCH 07/11] Strings: remove addresses unit tests Remove in favor of fuzzed unit test with Foundry --- test/utils/Strings.t.sol | 5 +++++ test/utils/Strings.test.js | 39 -------------------------------------- 2 files changed, 5 insertions(+), 39 deletions(-) diff --git a/test/utils/Strings.t.sol b/test/utils/Strings.t.sol index e10d35ebd5e..289cad919f7 100644 --- a/test/utils/Strings.t.sol +++ b/test/utils/Strings.t.sol @@ -22,6 +22,11 @@ contract StringsTest is Test { assertEq(actual, vm.toString(bytes32(value))); } + function testHexStringAddress(address value) public { + string memory actual = Strings.toHexString(value); + assertEq(actual, vm.toLowercase(vm.toString(value))); + } + function testChecksumHexStringAddress(address value) public { string memory actual = Strings.toChecksumHexString(value); assertEq(actual, vm.toString(value)); diff --git a/test/utils/Strings.test.js b/test/utils/Strings.test.js index 6353fd886db..b97a25d606f 100644 --- a/test/utils/Strings.test.js +++ b/test/utils/Strings.test.js @@ -108,45 +108,6 @@ describe('Strings', function () { }); }); - describe('addresses', function () { - const addresses = [ - '0xa9036907dccae6a1e0033479b12e837e5cf5a02f', // Random address - '0x0000e0ca771e21bd00057f54a68c30d400000000', // Leading and trailing zeros - // EIP-55 reference - '0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed', - '0xfB6916095ca1df60bB79Ce92cE3Ea74c37c5d359', - '0xdbF03B407c01E7cD3CBea99509d93f8DDDC8C6FB', - '0xD1220A0cf47c7B9Be7A2E6BA89F429762e7b9aDb', - '0xfb6916095ca1df60bb79ce92ce3ea74c37c5d359', - '0x52908400098527886E0F7030069857D2E4169EE7', - '0x8617E340B3D01FA5F11F306F4090FD50E238070D', - '0xde709f2102306220921060314715629080e2fb77', - '0x27b1fdb04752bbc536007a920d24acb045561c26', - '0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed', - '0xfB6916095ca1df60bB79Ce92cE3Ea74c37c5d359', - '0xdbF03B407c01E7cD3CBea99509d93f8DDDC8C6FB', - '0xD1220A0cf47c7B9Be7A2E6BA89F429762e7b9aDb', - ]; - - describe('toHexString', function () { - for (const addr of addresses) { - it(`converts ${addr}`, async function () { - expect(await this.mock.getFunction('$toHexString(address)')(addr)).to.equal(addr.toLowerCase()); - }); - } - }); - - describe('toChecksumHexString', function () { - for (const addr of addresses) { - it(`converts ${addr}`, async function () { - expect(await this.mock.getFunction('$toChecksumHexString(address)')(addr)).to.equal( - ethers.getAddress(addr.toLowerCase()), - ); - }); - } - }); - }); - describe('equal', function () { it('compares two empty strings', async function () { expect(await this.mock.$equal('', '')).to.be.true; From f9ebc6bd4865bd0cf08323f58f686ba5ec95c68e Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 22:14:00 -0700 Subject: [PATCH 08/11] Strings: fix unit test --- test/utils/Strings.t.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/utils/Strings.t.sol b/test/utils/Strings.t.sol index 289cad919f7..f1288d41a05 100644 --- a/test/utils/Strings.t.sol +++ b/test/utils/Strings.t.sol @@ -18,8 +18,9 @@ contract StringsTest is Test { } function testHexStringUint256(uint256 value) public { - string memory actual = Strings.toHexString(value); - assertEq(actual, vm.toString(bytes32(value))); + string memory actual = vm.replace(Strings.toHexString(value), "0", ""); + string memory expected = vm.replace(vm.toString(bytes32(value)), "0", ""); + assertEq(actual, expected); } function testHexStringAddress(address value) public { From 07c123518c95575544d717bb45e438f7802a7322 Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 22:28:55 -0700 Subject: [PATCH 09/11] Create2: add fuzz unit test --- test/utils/Create2.t.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/utils/Create2.t.sol b/test/utils/Create2.t.sol index e8e2269f68a..025c48d7d62 100644 --- a/test/utils/Create2.t.sol +++ b/test/utils/Create2.t.sol @@ -15,4 +15,10 @@ contract Create2Test is Test { } assertEq(spillage, bytes32(0)); } + + function testCreate2(bytes32 salt, bytes32 bytecodeHash, address deployer) public { + address actual = Create2.computeAddress(salt, bytecodeHash, deployer); + address expected = vm.computeCreate2Address(salt, bytecodeHash, deployer); + assertEq(actual, expected); + } } From 14f822319a9a76645c34e035c7b6d4a76a86c943 Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 22:37:35 -0700 Subject: [PATCH 10/11] ECDSA: add fuzzed unit tests --- test/utils/cryptography/ECDSA.t.sol | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 test/utils/cryptography/ECDSA.t.sol diff --git a/test/utils/cryptography/ECDSA.t.sol b/test/utils/cryptography/ECDSA.t.sol new file mode 100644 index 00000000000..c7a792eb96f --- /dev/null +++ b/test/utils/cryptography/ECDSA.t.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; + +contract ECDSATest is Test { + function testRecoverWithValidSignature(string calldata wallet, string calldata message) public { + (address signer, uint256 key) = makeAddrAndKey(wallet); + bytes32 hash = keccak256(abi.encode(message)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(key, hash); + bytes memory signature = abi.encodePacked(r, s, v); + assertEq(signer, ECDSA.recover(hash, signature)); + } + + function testRecoverWithWrongMessage(string calldata wallet, string calldata message, bytes32 digest) public { + (address signer, uint256 key) = makeAddrAndKey(wallet); + bytes32 hashCorrect = keccak256(abi.encode(message)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(key, hashCorrect); + bytes memory signature = abi.encodePacked(r, s, v); + address recoveredAddress = ECDSA.recover(digest, signature); + assertTrue(signer != recoveredAddress); + } +} From b607f30a236ec3fa448c88a2953dfed1cad4f644 Mon Sep 17 00:00:00 2001 From: cairo Date: Wed, 19 Jun 2024 23:12:42 -0700 Subject: [PATCH 11/11] ECDSA: improve unit tests --- test/utils/cryptography/ECDSA.t.sol | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/test/utils/cryptography/ECDSA.t.sol b/test/utils/cryptography/ECDSA.t.sol index c7a792eb96f..5c52bfa0d22 100644 --- a/test/utils/cryptography/ECDSA.t.sol +++ b/test/utils/cryptography/ECDSA.t.sol @@ -6,20 +6,23 @@ import {Test} from "forge-std/Test.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; contract ECDSATest is Test { - function testRecoverWithValidSignature(string calldata wallet, string calldata message) public { - (address signer, uint256 key) = makeAddrAndKey(wallet); + function testRecoverWithValidSignature(string calldata seed, string calldata message) public { + (address signer, uint256 key) = makeAddrAndKey(seed); + bytes32 hash = keccak256(abi.encode(message)); (uint8 v, bytes32 r, bytes32 s) = vm.sign(key, hash); + bytes memory signature = abi.encodePacked(r, s, v); assertEq(signer, ECDSA.recover(hash, signature)); } - function testRecoverWithWrongMessage(string calldata wallet, string calldata message, bytes32 digest) public { - (address signer, uint256 key) = makeAddrAndKey(wallet); - bytes32 hashCorrect = keccak256(abi.encode(message)); - (uint8 v, bytes32 r, bytes32 s) = vm.sign(key, hashCorrect); + function testRecoverWithWrongMessage(string calldata seed, string calldata message, bytes32 digest) public { + (address signer, uint256 key) = makeAddrAndKey(seed); + + bytes32 validDigest = keccak256(abi.encode(message)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(key, validDigest); + bytes memory signature = abi.encodePacked(r, s, v); - address recoveredAddress = ECDSA.recover(digest, signature); - assertTrue(signer != recoveredAddress); + assertTrue(signer != ECDSA.recover(digest, signature)); } }