-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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 remaining fuzzing tests and remove pseudo-random fuzzing #5095
Draft
cairoeth
wants to merge
11
commits into
OpenZeppelin:master
Choose a base branch
from
cairoeth:improve-fuzzing-state
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
d7c79cd
GovernorCountingFractional: foundry fuzzing
cairoeth 0936f4a
AuthorityUtilsTest: foundry fuzzing
cairoeth 420ebeb
ERC721Consecutive: remove hardhat test
cairoeth 0a0a9e0
Strings: add foundry fuzz and verify
cairoeth 894e464
SignedMath: remove unit tests
cairoeth dea6861
forge install: forge-std
cairoeth 0ab5f84
Strings: remove addresses unit tests
cairoeth f9ebc6b
Strings: fix unit test
cairoeth 07c1235
Create2: add fuzz unit test
cairoeth 14f8223
ECDSA: add fuzzed unit tests
cairoeth b607f30
ECDSA: improve unit tests
cairoeth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Submodule forge-std
updated
32 files
+6 −12 | .github/workflows/ci.yml | |
+3 −1 | .github/workflows/sync.yml | |
+0 −3 | .gitmodules | |
+1 −1 | README.md | |
+3 −3 | foundry.toml | |
+0 −1 | lib/ds-test | |
+1 −1 | package.json | |
+1 −1 | scripts/vm.py | |
+518 −225 | src/StdAssertions.sol | |
+10 −8 | src/StdChains.sol | |
+9 −3 | src/StdInvariant.sol | |
+7 −11 | src/StdJson.sol | |
+201 −106 | src/StdStorage.sol | |
+179 −0 | src/StdToml.sol | |
+1 −1 | src/StdUtils.sol | |
+4 −4 | src/Test.sol | |
+644 −7 | src/Vm.sol | |
+51 −33 | src/mocks/MockERC20.sol | |
+46 −32 | src/mocks/MockERC721.sol | |
+39 −909 | test/StdAssertions.t.sol | |
+31 −26 | test/StdChains.t.sol | |
+19 −11 | test/StdCheats.t.sol | |
+49 −0 | test/StdJson.t.sol | |
+8 −8 | test/StdMath.t.sol | |
+159 −11 | test/StdStorage.t.sol | |
+49 −0 | test/StdToml.t.sol | |
+18 −18 | test/StdUtils.t.sol | |
+3 −3 | test/Vm.t.sol | |
+8 −0 | test/fixtures/test.json | |
+6 −0 | test/fixtures/test.toml | |
+1 −1 | test/mocks/MockERC20.t.sol | |
+1 −1 | test/mocks/MockERC721.t.sol |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
167 changes: 167 additions & 0 deletions
167
test/governance/extensions/GovernorCountingFractional.t.sol
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
}); | ||
|
||
Comment on lines
-145
to
-150
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this test removed ? |
||
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') | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
// 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 = vm.replace(Strings.toHexString(value), "0", ""); | ||
string memory expected = vm.replace(vm.toString(bytes32(value)), "0", ""); | ||
assertEq(actual, expected); | ||
} | ||
|
||
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)); | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is not so much fuzzing as it is testing 4 clearly different cases. Not saying fuzzing would not cover them, but we don't really need 5000 fuzzing runs to cover the same code.
Also, I'm not sure how that change would affect coverage.