Skip to content

Commit

Permalink
Update forge and change visibility in fuzz tests (OpenZeppelin#5103)
Browse files Browse the repository at this point in the history
Co-authored-by: cairo <cairoeth@protonmail.com>
Signed-off-by: Hadrien Croubois <hadrien.croubois@gmail.com>
  • Loading branch information
2 people authored and Amxx committed Nov 30, 2024
1 parent 07231f0 commit b6813a8
Show file tree
Hide file tree
Showing 16 changed files with 74 additions and 70 deletions.
2 changes: 1 addition & 1 deletion lib/forge-std
Submodule forge-std updated 6 files
+193 −0 CONTRIBUTING.md
+16 −0 README.md
+1 −1 package.json
+12 −1 scripts/vm.py
+126 −16 src/Vm.sol
+2 −2 test/Vm.t.sol
2 changes: 1 addition & 1 deletion scripts/generate/templates/Checkpoints.t.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function _prepareKeys(${opts.keyTypeName}[] memory keys, ${opts.keyTypeName} max
}
}
function _assertLatestCheckpoint(bool exist, ${opts.keyTypeName} key, ${opts.valueTypeName} value) internal {
function _assertLatestCheckpoint(bool exist, ${opts.keyTypeName} key, ${opts.valueTypeName} value) internal view {
(bool _exist, ${opts.keyTypeName} _key, ${opts.valueTypeName} _value) = _ckpts.latestCheckpoint();
assertEq(_exist, exist);
assertEq(_key, key);
Expand Down
8 changes: 4 additions & 4 deletions scripts/generate/templates/SlotDerivation.t.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function _assertDeriveArray(uint256 length, uint256 offset) public {
const mapping = ({ type, name }) => `\
mapping(${type} => bytes) private _${type}Mapping;
function testSymbolicDeriveMapping${name}(${type} key) public {
function testSymbolicDeriveMapping${name}(${type} key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _${type}Mapping.slot
Expand Down Expand Up @@ -76,15 +76,15 @@ function testSymbolicDeriveMapping${name}Dirty(bytes32 dirtyKey) public {
const boundedMapping = ({ type, name }) => `\
mapping(${type} => bytes) private _${type}Mapping;
function testDeriveMapping${name}(${type} memory key) public {
function testDeriveMapping${name}(${type} memory key) public view {
_assertDeriveMapping${name}(key);
}
function symbolicDeriveMapping${name}() public {
function symbolicDeriveMapping${name}() public view {
_assertDeriveMapping${name}(svm.create${name}(256, "DeriveMapping${name}Input"));
}
function _assertDeriveMapping${name}(${type} memory key) internal {
function _assertDeriveMapping${name}(${type} memory key) internal view {
bytes32 baseSlot;
assembly {
baseSlot := _${type}Mapping.slot
Expand Down
8 changes: 6 additions & 2 deletions test/governance/Governor.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import {Governor} from "@openzeppelin/contracts/governance/Governor.sol";
contract GovernorInternalTest is Test, Governor {
constructor() Governor("") {}

function testValidDescriptionForProposer(string memory description, address proposer, bool includeProposer) public {
function testValidDescriptionForProposer(
string memory description,
address proposer,
bool includeProposer
) public view {
if (includeProposer) {
description = string.concat(description, "#proposer=", Strings.toHexString(proposer));
}
Expand All @@ -20,7 +24,7 @@ contract GovernorInternalTest is Test, Governor {
string memory description,
address commitProposer,
address actualProposer
) public {
) public view {
vm.assume(commitProposer != actualProposer);
description = string.concat(description, "#proposer=", Strings.toHexString(commitProposer));
assertFalse(_isValidDescriptionForProposer(actualProposer, description));
Expand Down
6 changes: 3 additions & 3 deletions test/proxy/Clones.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ contract ClonesTest is Test {
return 42;
}

function testSymbolicPredictDeterministicAddressSpillage(address implementation, bytes32 salt) public {
function testSymbolicPredictDeterministicAddressSpillage(address implementation, bytes32 salt) public view {
address predicted = Clones.predictDeterministicAddress(implementation, salt);
bytes32 spillage;
assembly ("memory-safe") {
Expand All @@ -23,7 +23,7 @@ contract ClonesTest is Test {
address implementation,
bytes32 salt,
bytes memory args
) public {
) public view {
vm.assume(args.length < 0xbfd3);

address predicted = Clones.predictDeterministicAddressWithImmutableArgs(implementation, args, salt);
Expand Down Expand Up @@ -59,7 +59,7 @@ contract ClonesTest is Test {
assertEq(ClonesTest(cloneDirty).getNumber(), this.getNumber());
}

function testPredictDeterministicAddressDirty(bytes32 salt) external {
function testPredictDeterministicAddressDirty(bytes32 salt) external view {
address predictClean = Clones.predictDeterministicAddress(address(this), salt);
address predictDirty = Clones.predictDeterministicAddress(_dirty(address(this)), salt);

Expand Down
6 changes: 3 additions & 3 deletions test/utils/Arrays.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import {SymTest} from "halmos-cheatcodes/SymTest.sol";
import {Arrays} from "@openzeppelin/contracts/utils/Arrays.sol";

contract ArraysTest is Test, SymTest {
function testSort(uint256[] memory values) public {
function testSort(uint256[] memory values) public pure {
Arrays.sort(values);
_assertSort(values);
}

function symbolicSort() public {
function symbolicSort() public pure {
uint256[] memory values = new uint256[](3);
for (uint256 i = 0; i < 3; i++) {
values[i] = svm.createUint256("arrayElement");
Expand All @@ -23,7 +23,7 @@ contract ArraysTest is Test, SymTest {

/// Asserts

function _assertSort(uint256[] memory values) internal {
function _assertSort(uint256[] memory values) internal pure {
for (uint256 i = 1; i < values.length; ++i) {
assertLe(values[i - 1], values[i]);
}
Expand Down
4 changes: 2 additions & 2 deletions test/utils/Base64.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import {Test} from "forge-std/Test.sol";
import {Base64} from "@openzeppelin/contracts/utils/Base64.sol";

contract Base64Test is Test {
function testEncode(bytes memory input) external {
function testEncode(bytes memory input) external pure {
assertEq(Base64.encode(input), vm.toBase64(input));
}

function testEncodeURL(bytes memory input) external {
function testEncodeURL(bytes memory input) external pure {
assertEq(Base64.encodeURL(input), _removePadding(vm.toBase64URL(input)));
}

Expand Down
2 changes: 1 addition & 1 deletion test/utils/Create2.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {Test} from "forge-std/Test.sol";
import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";

contract Create2Test is Test {
function testSymbolicComputeAddressSpillage(bytes32 salt, bytes32 bytecodeHash, address deployer) public {
function testSymbolicComputeAddressSpillage(bytes32 salt, bytes32 bytecodeHash, address deployer) public pure {
address predicted = Create2.computeAddress(salt, bytecodeHash, deployer);
bytes32 spillage;
assembly ("memory-safe") {
Expand Down
12 changes: 6 additions & 6 deletions test/utils/ShortStrings.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import {ShortStrings, ShortString} from "@openzeppelin/contracts/utils/ShortStri
contract ShortStringsTest is Test, SymTest {
string _fallback;

function testRoundtripShort(string memory input) external {
function testRoundtripShort(string memory input) external pure {
vm.assume(_isShort(input));
_assertRoundtripShort(input);
}

function symbolicRoundtripShort() external {
function symbolicRoundtripShort() external pure {
string memory input = svm.createString(31, "RoundtripShortInput");
_assertRoundtripShort(input);
}
Expand All @@ -41,12 +41,12 @@ contract ShortStringsTest is Test, SymTest {
_assertRevertLong(input);
}

function testLengthShort(string memory input) external {
function testLengthShort(string memory input) external pure {
vm.assume(_isShort(input));
_assertLengthShort(input);
}

function symbolicLengthShort() external {
function symbolicLengthShort() external pure {
string memory input = svm.createString(31, "LengthShortInput");
_assertLengthShort(input);
}
Expand All @@ -66,7 +66,7 @@ contract ShortStringsTest is Test, SymTest {

/// Assertions

function _assertRoundtripShort(string memory input) internal {
function _assertRoundtripShort(string memory input) internal pure {
ShortString short = ShortStrings.toShortString(input);
string memory output = ShortStrings.toString(short);
assertEq(input, output);
Expand All @@ -84,7 +84,7 @@ contract ShortStringsTest is Test, SymTest {
this.toShortString(input);
}

function _assertLengthShort(string memory input) internal {
function _assertLengthShort(string memory input) internal pure {
ShortString short = ShortStrings.toShortString(input);
uint256 shortLength = ShortStrings.byteLength(short);
uint256 inputLength = bytes(input).length;
Expand Down
28 changes: 14 additions & 14 deletions test/utils/SlotDerivation.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract SlotDerivationTest is Test, SymTest {

mapping(address => bytes) private _addressMapping;

function testSymbolicDeriveMappingAddress(address key) public {
function testSymbolicDeriveMappingAddress(address key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _addressMapping.slot
Expand All @@ -59,7 +59,7 @@ contract SlotDerivationTest is Test, SymTest {

mapping(bool => bytes) private _boolMapping;

function testSymbolicDeriveMappingBoolean(bool key) public {
function testSymbolicDeriveMappingBoolean(bool key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _boolMapping.slot
Expand All @@ -76,7 +76,7 @@ contract SlotDerivationTest is Test, SymTest {

mapping(bytes32 => bytes) private _bytes32Mapping;

function testSymbolicDeriveMappingBytes32(bytes32 key) public {
function testSymbolicDeriveMappingBytes32(bytes32 key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _bytes32Mapping.slot
Expand All @@ -93,7 +93,7 @@ contract SlotDerivationTest is Test, SymTest {

mapping(bytes4 => bytes) private _bytes4Mapping;

function testSymbolicDeriveMappingBytes4(bytes4 key) public {
function testSymbolicDeriveMappingBytes4(bytes4 key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _bytes4Mapping.slot
Expand All @@ -110,7 +110,7 @@ contract SlotDerivationTest is Test, SymTest {

mapping(uint256 => bytes) private _uint256Mapping;

function testSymbolicDeriveMappingUint256(uint256 key) public {
function testSymbolicDeriveMappingUint256(uint256 key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _uint256Mapping.slot
Expand All @@ -127,7 +127,7 @@ contract SlotDerivationTest is Test, SymTest {

mapping(uint32 => bytes) private _uint32Mapping;

function testSymbolicDeriveMappingUint32(uint32 key) public {
function testSymbolicDeriveMappingUint32(uint32 key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _uint32Mapping.slot
Expand All @@ -144,7 +144,7 @@ contract SlotDerivationTest is Test, SymTest {

mapping(int256 => bytes) private _int256Mapping;

function testSymbolicDeriveMappingInt256(int256 key) public {
function testSymbolicDeriveMappingInt256(int256 key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _int256Mapping.slot
Expand All @@ -161,7 +161,7 @@ contract SlotDerivationTest is Test, SymTest {

mapping(int32 => bytes) private _int32Mapping;

function testSymbolicDeriveMappingInt32(int32 key) public {
function testSymbolicDeriveMappingInt32(int32 key) public view {
bytes32 baseSlot;
assembly {
baseSlot := _int32Mapping.slot
Expand All @@ -178,15 +178,15 @@ contract SlotDerivationTest is Test, SymTest {

mapping(string => bytes) private _stringMapping;

function testDeriveMappingString(string memory key) public {
function testDeriveMappingString(string memory key) public view {
_assertDeriveMappingString(key);
}

function symbolicDeriveMappingString() public {
function symbolicDeriveMappingString() public view {
_assertDeriveMappingString(svm.createString(256, "DeriveMappingStringInput"));
}

function _assertDeriveMappingString(string memory key) internal {
function _assertDeriveMappingString(string memory key) internal view {
bytes32 baseSlot;
assembly {
baseSlot := _stringMapping.slot
Expand All @@ -203,15 +203,15 @@ contract SlotDerivationTest is Test, SymTest {

mapping(bytes => bytes) private _bytesMapping;

function testDeriveMappingBytes(bytes memory key) public {
function testDeriveMappingBytes(bytes memory key) public view {
_assertDeriveMappingBytes(key);
}

function symbolicDeriveMappingBytes() public {
function symbolicDeriveMappingBytes() public view {
_assertDeriveMappingBytes(svm.createBytes(256, "DeriveMappingBytesInput"));
}

function _assertDeriveMappingBytes(bytes memory key) internal {
function _assertDeriveMappingBytes(bytes memory key) internal view {
bytes32 baseSlot;
assembly {
baseSlot := _bytesMapping.slot
Expand Down
8 changes: 4 additions & 4 deletions test/utils/Strings.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@ import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
contract StringsTest is Test {
using Strings for *;

function testParse(uint256 value) external {
function testParse(uint256 value) external pure {
assertEq(value, value.toString().parseUint());
}

function testParseSigned(int256 value) external {
function testParseSigned(int256 value) external pure {
assertEq(value, value.toStringSigned().parseInt());
}

function testParseHex(uint256 value) external {
function testParseHex(uint256 value) external pure {
assertEq(value, value.toHexString().parseHexUint());
}

function testParseChecksumHex(address value) external {
function testParseChecksumHex(address value) external pure {
assertEq(value, value.toChecksumHexString().parseAddress());
}
}
4 changes: 2 additions & 2 deletions test/utils/cryptography/P256.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";

contract P256Test is Test {
/// forge-config: default.fuzz.runs = 512
function testVerify(bytes32 digest, uint256 seed) public {
function testVerify(bytes32 digest, uint256 seed) public view {
uint256 privateKey = _asPrivateKey(seed);

(uint256 x, uint256 y) = vm.publicKeyP256(privateKey);
Expand All @@ -20,7 +20,7 @@ contract P256Test is Test {
}

/// forge-config: default.fuzz.runs = 512
function testRecover(bytes32 digest, uint256 seed) public {
function testRecover(bytes32 digest, uint256 seed) public view {
uint256 privateKey = _asPrivateKey(seed);

(uint256 x, uint256 y) = vm.publicKeyP256(privateKey);
Expand Down
Loading

0 comments on commit b6813a8

Please sign in to comment.