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.

Sign up for GitHub

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

Rename fuzz to convention #48

Merged
merged 11 commits into from
Aug 18, 2023
Merged

Rename fuzz to convention #48

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
cf4a9e1
Rename fuzz to convention and skip fuzz in snapshot
matYang Aug 17, 2023
85f6194
update CI
matYang Aug 17, 2023
2e545cd
no longer skip fuzz tests in snapshot
matYang Aug 17, 2023
78860b6
add fuzz results back to snapshot
matYang Aug 17, 2023
9aa3c33
lower USDC fuzz test runs
matYang Aug 17, 2023
f0ef89e
Merge branch 'ccip-develop' into chore/exclude-fuzz-tests-from-snapshot
matYang Aug 17, 2023
7f88e86
resnapshot
matYang Aug 17, 2023
a862b83
update comment
matYang Aug 17, 2023
a4e6e23
Merge branch 'ccip-develop' into chore/exclude-fuzz-tests-from-snapshot
matYang Aug 18, 2023
69ea8d1
update snapshot
matYang Aug 18, 2023
13ebc76
Merge branch 'ccip-develop' into chore/exclude-fuzz-tests-from-snapshot
RensR Aug 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/solidity-foundry.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ jobs:

- name: Run Forge snapshot
run: |
forge snapshot --check gas-snapshots/${{ matrix.product }}.gas-snapshot
forge snapshot --no-match-test testFuzz_ --check gas-snapshots/${{ matrix.product }}.gas-snapshot
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to not edit CI to hide failures

Copy link
Collaborator Author

@matYang matYang Aug 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snapshot results are like:

MerkleMultiProofTest:testMerkleMulti1of4(bytes32,bytes32,bytes32) (runs: 256, μ: 7037, ~: 7036)
MerkleMultiProofTest:testMerkleMulti2of4(bytes32,bytes32,bytes32,bytes32) (runs: 256, μ: 8714, ~: 8719)
MerkleMultiProofTest:testMerkleMulti3of4(bytes32,bytes32,bytes32,bytes32) (runs: 256, μ: 8795, ~: 8792)
MerkleMultiProofTest:testMerkleMulti4of4(bytes32,bytes32,bytes32,bytes32) (runs: 256, μ: 8772, ~: 8768)

With as much info I can find, these values can be flaky. It would make sense to exclude these tests from snapshot.
foundry-rs/foundry#1081
foundry-rs/foundry#4517
foundry-rs/foundry#5262

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion and further investigation, latest foundry snapshot command does use a static seed, which should lead to deterministic inputs and overall consistent fuzz snapshot results.

id: snapshot
working-directory: contracts
env:
Expand Down
2 changes: 1 addition & 1 deletion contracts/GNUmakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ ALL_FOUNDRY_PRODUCTS = vrf automation llo-feeds functions automation-dev shared
# make FOUNDRY_PROFILE=llo-feeds snapshot
.PHONY: snapshot
snapshot: ## Make a snapshot for a specific product.
export FOUNDRY_PROFILE=$(FOUNDRY_PROFILE) && forge snapshot --snap gas-snapshots/$(FOUNDRY_PROFILE).gas-snapshot
export FOUNDRY_PROFILE=$(FOUNDRY_PROFILE) && forge snapshot --no-match-test testFuzz_ --snap gas-snapshots/$(FOUNDRY_PROFILE).gas-snapshot

.PHONY: snapshot-all
snapshot-all: ## Make a snapshot for all products.
Expand Down
82 changes: 30 additions & 52 deletions contracts/gas-snapshots/ccip.gas-snapshot
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions contracts/src/v0.8/ccip/test/applications/PingPongDapp.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,19 @@ contract PingPong_ccipReceive is PingPongDappSetup {
}

contract PingPong_plumbing is PingPongDappSetup {
function testCounterPartChainSelectorSuccess(uint64 chainSelector) public {
function testFuzz_CounterPartChainSelectorSuccess(uint64 chainSelector) public {
s_pingPong.setCounterpartChainSelector(chainSelector);

assertEq(s_pingPong.getCounterpartChainSelector(), chainSelector);
}

function testCounterPartAddressSuccess(address counterpartAddress) public {
function testFuzz_CounterPartAddressSuccess(address counterpartAddress) public {
s_pingPong.setCounterpartAddress(counterpartAddress);

assertEq(s_pingPong.getCounterpartAddress(), counterpartAddress);
}

function testCounterPartAddressSuccess(uint64 chainSelector, address counterpartAddress) public {
function testFuzz_CounterPartAddressSuccess(uint64 chainSelector, address counterpartAddress) public {
s_pingPong.setCounterpart(chainSelector, counterpartAddress);

assertEq(s_pingPong.getCounterpartAddress(), counterpartAddress);
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/v0.8/ccip/test/commitStore/CommitStore.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ contract CommitStore_constructor is PriceRegistrySetup, OCR2BaseSetup {

/// @notice #setMinSeqNr
contract CommitStore_setMinSeqNr is CommitStoreSetup {
function testSetMinSeqNrSuccess(uint64 minSeqNr) public {
function testFuzz_SetMinSeqNrSuccess(uint64 minSeqNr) public {
s_commitStore.setMinSeqNr(minSeqNr);

assertEq(s_commitStore.getExpectedNextSequenceNumber(), minSeqNr);
Expand All @@ -161,7 +161,7 @@ contract CommitStore_setMinSeqNr is CommitStoreSetup {

/// @notice #setDynamicConfig
contract CommitStore_setDynamicConfig is CommitStoreSetup {
function testSetDynamicConfigSuccess(address priceRegistry) public {
function testFuzz_SetDynamicConfigSuccess(address priceRegistry) public {
vm.assume(priceRegistry != address(0));
CommitStore.StaticConfig memory staticConfig = s_commitStore.getStaticConfig();
CommitStore.DynamicConfig memory dynamicConfig = CommitStore.DynamicConfig({priceRegistry: priceRegistry});
Expand Down
10 changes: 5 additions & 5 deletions contracts/src/v0.8/ccip/test/libraries/MerkleMultiProof.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ contract MerkleMultiProofTest is BaseTest {
assertEq(expectedRoot, root);
}

function testMerkleRoot2(bytes32 left, bytes32 right) public {
function testFuzz_MerkleRoot2(bytes32 left, bytes32 right) public {
bytes32[] memory leaves = new bytes32[](2);
leaves[0] = left;
leaves[1] = right;
Expand All @@ -91,7 +91,7 @@ contract MerkleMultiProofTest is BaseTest {
assertEq(root, expectedRoot);
}

function testMerkleMulti1of4(bytes32 leaf1, bytes32 proof1, bytes32 proof2) public {
function testFuzz_MerkleMulti1of4(bytes32 leaf1, bytes32 proof1, bytes32 proof2) public {
bytes32[] memory leaves = new bytes32[](1);
leaves[0] = leaf1;
bytes32[] memory proofs = new bytes32[](2);
Expand All @@ -106,7 +106,7 @@ contract MerkleMultiProofTest is BaseTest {
assertEq(MerkleMultiProof.merkleRoot(leaves, proofs, 0), result);
}

function testMerkleMulti2of4(bytes32 leaf1, bytes32 leaf2, bytes32 proof1, bytes32 proof2) public {
function testFuzz_MerkleMulti2of4(bytes32 leaf1, bytes32 leaf2, bytes32 proof1, bytes32 proof2) public {
bytes32[] memory leaves = new bytes32[](2);
leaves[0] = leaf1;
leaves[1] = leaf2;
Expand All @@ -124,7 +124,7 @@ contract MerkleMultiProofTest is BaseTest {
assertEq(MerkleMultiProof.merkleRoot(leaves, proofs, 4), finalResult);
}

function testMerkleMulti3of4(bytes32 leaf1, bytes32 leaf2, bytes32 leaf3, bytes32 proof) public {
function testFuzz_MerkleMulti3of4(bytes32 leaf1, bytes32 leaf2, bytes32 leaf3, bytes32 proof) public {
bytes32[] memory leaves = new bytes32[](3);
leaves[0] = leaf1;
leaves[1] = leaf2;
Expand All @@ -142,7 +142,7 @@ contract MerkleMultiProofTest is BaseTest {
assertEq(MerkleMultiProof.merkleRoot(leaves, proofs, 5), finalResult);
}

function testMerkleMulti4of4(bytes32 leaf1, bytes32 leaf2, bytes32 leaf3, bytes32 leaf4) public {
function testFuzz_MerkleMulti4of4(bytes32 leaf1, bytes32 leaf2, bytes32 leaf3, bytes32 leaf4) public {
bytes32[] memory leaves = new bytes32[](4);
leaves[0] = leaf1;
leaves[1] = leaf2;
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/ccip/test/offRamp/EVM2EVMOffRamp.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1002,7 +1002,7 @@ contract EVM2EVMOffRamp_getExecutionState is EVM2EVMOffRampSetup {

/// forge-config: default.fuzz.runs = 32
/// forge-config: ccip.fuzz.runs = 32
function testDifferentialSuccess_fuzz(uint16[500] memory seqNums, uint8[500] memory values) public {
function testFuzz_DifferentialSuccess(uint16[500] memory seqNums, uint8[500] memory values) public {
for (uint256 i = 0; i < seqNums.length; ++i) {
// Only use the first three slots. This makes sure existing slots get overwritten
// as the tests uses 500 sequence numbers.
Expand Down
6 changes: 3 additions & 3 deletions contracts/src/v0.8/ccip/test/onRamp/EVM2EVMOnRamp.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ contract EVM2EVMOnRamp_constructor is EVM2EVMOnRampSetup {
}

contract EVM2EVMOnRamp_payNops_fuzz is EVM2EVMOnRampSetup {
function test_fuzz_NopPayNopsSuccess(uint96 nopFeesJuels) public {
function testFuzz_NopPayNopsSuccess(uint96 nopFeesJuels) public {
(EVM2EVMOnRamp.NopAndWeight[] memory nopsAndWeights, uint256 weightsTotal) = s_onRamp.getNops();
// To avoid NoFeesToPay
vm.assume(nopFeesJuels > weightsTotal);
Expand Down Expand Up @@ -365,7 +365,7 @@ contract EVM2EVMOnRamp_forwardFromRouter is EVM2EVMOnRampSetup {
}

// Make sure any valid sender, receiver and feeAmount can be handled.
function test_fuzz_ForwardFromRouterSuccess(address originalSender, address receiver, uint96 feeTokenAmount) public {
function testFuzz_ForwardFromRouterSuccess(address originalSender, address receiver, uint96 feeTokenAmount) public {
// To avoid RouterMustSetOriginalSender
vm.assume(originalSender != address(0));
vm.assume(uint160(receiver) >= 10);
Expand Down Expand Up @@ -846,7 +846,7 @@ contract EVM2EVMOnRamp_getTokenTransferFeeUSD is EVM2EVMOnRamp_getFeeSetup {
assertEq(configUSDToValue(feeTokenConfig.minTokenTransferFeeUSD), feeAmount);
}

function testTokenTransferFeeDuplicateTokens(uint256 transfers, uint256 amount) public {
function testFuzz_TokenTransferFeeDuplicateTokens(uint256 transfers, uint256 amount) public {
// It shouldn't be possible to pay materially lower fees by splitting up the transfers.
// Note it is possible to pay higher fees since the minimum fees are added.
EVM2EVMOnRamp.DynamicConfig memory dynamicConfig = s_onRamp.getDynamicConfig();
Expand Down
14 changes: 7 additions & 7 deletions contracts/src/v0.8/ccip/test/pools/LockReleaseTokenPool.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract LockReleaseTokenPool_lockOrBurn is LockReleaseTokenPoolSetup {
event Locked(address indexed sender, uint256 amount);
event TokensConsumed(uint256 tokens);

function testLockOrBurnNoAllowListSuccess(uint256 amount) public {
function testFuzz_LockOrBurnNoAllowListSuccess(uint256 amount) public {
amount = bound(amount, 1, rateLimiterConfig().capacity);
changePrank(s_allowedOnRamp);

Expand Down Expand Up @@ -96,7 +96,7 @@ contract LockReleaseTokenPool_releaseOrMint is LockReleaseTokenPoolSetup {
event TokensConsumed(uint256 tokens);
event Released(address indexed sender, address indexed recipient, uint256 amount);

function testReleaseOrMintSuccess(address recipient, uint256 amount) public {
function testFuzz_ReleaseOrMintSuccess(address recipient, uint256 amount) public {
// Since the owner already has tokens this would break the checks
vm.assume(recipient != OWNER);
vm.assume(recipient != address(0));
Expand Down Expand Up @@ -138,7 +138,7 @@ contract LockReleaseTokenPool_releaseOrMint is LockReleaseTokenPoolSetup {
}

contract LockReleaseTokenPool_getProvidedLiquidity is LockReleaseTokenPoolSetup {
function testGetProvidedLiquiditySuccess(uint256 amount) public {
function testFuzz_GetProvidedLiquiditySuccess(uint256 amount) public {
s_token.approve(address(s_lockReleaseTokenPool), amount);

s_lockReleaseTokenPool.addLiquidity(amount);
Expand All @@ -148,7 +148,7 @@ contract LockReleaseTokenPool_getProvidedLiquidity is LockReleaseTokenPoolSetup
}

contract LockReleaseTokenPool_addLiquidity is LockReleaseTokenPoolSetup {
function testAddLiquiditySuccess(uint256 amount) public {
function testFuzz_AddLiquiditySuccess(uint256 amount) public {
uint256 balancePre = s_token.balanceOf(OWNER);
s_token.approve(address(s_lockReleaseTokenPool), amount);

Expand All @@ -160,15 +160,15 @@ contract LockReleaseTokenPool_addLiquidity is LockReleaseTokenPoolSetup {

// Reverts

function testExceedsAllowance(uint256 amount) public {
function testFuzz_ExceedsAllowance(uint256 amount) public {
vm.assume(amount > 0);
vm.expectRevert("ERC20: insufficient allowance");
s_lockReleaseTokenPool.addLiquidity(amount);
}
}

contract LockReleaseTokenPool_removeLiquidity is LockReleaseTokenPoolSetup {
function testRemoveLiquiditySuccess(uint256 amount) public {
function testFuzz_RemoveLiquiditySuccess(uint256 amount) public {
uint256 balancePre = s_token.balanceOf(OWNER);
s_token.approve(address(s_lockReleaseTokenPool), amount);
s_lockReleaseTokenPool.addLiquidity(amount);
Expand All @@ -179,7 +179,7 @@ contract LockReleaseTokenPool_removeLiquidity is LockReleaseTokenPoolSetup {
}

// Reverts
function testWithdrawalTooHighReverts(uint256 balance, uint256 withdrawal) public {
function testFuzz_WithdrawalTooHighReverts(uint256 balance, uint256 withdrawal) public {
vm.assume(balance < withdrawal);
s_token.approve(address(s_lockReleaseTokenPool), balance);
s_lockReleaseTokenPool.addLiquidity(balance);
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/ccip/test/pools/ThirdPartyBurnMintTokenPool.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ contract ThirdPartyBurnMintTokenPool_lockOrBurn is ThirdPartyBurnMintTokenPoolSe
deal(address(s_token), address(s_thirdPartyPoolWithAllowList), type(uint256).max);
}

function testLockOrBurnNoAllowListSuccess(uint256 amount) public {
function testFuzz_LockOrBurnNoAllowListSuccess(uint256 amount) public {
amount = bound(amount, 1, rateLimiterConfig().capacity);
changePrank(s_allowedOnRamp);

Expand Down
4 changes: 2 additions & 2 deletions contracts/src/v0.8/ccip/test/pools/TokenPool.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ contract TokenPool_setOnRampRateLimiterConfig is TokenPoolSetup {
s_tokenPool.applyRampUpdates(onRampUpdates1, new TokenPool.RampUpdate[](0));
}

function testSetRateLimiterConfigSuccess(uint128 capacity, uint128 rate, uint32 newTime) public {
function testFuzz_SetRateLimiterConfigSuccess(uint128 capacity, uint128 rate, uint32 newTime) public {
// Bucket updates only work on increasing time
vm.assume(newTime >= block.timestamp);
vm.warp(newTime);
Expand Down Expand Up @@ -203,7 +203,7 @@ contract TokenPool_setOffRampRateLimiterConfig is TokenPoolSetup {
s_tokenPool.applyRampUpdates(new TokenPool.RampUpdate[](0), offRampUpdates1);
}

function testSetRateLimiterConfigSuccess(uint128 capacity, uint128 rate, uint32 newTime) public {
function testFuzz_SetRateLimiterConfigSuccess(uint128 capacity, uint128 rate, uint32 newTime) public {
// Bucket updates only work on increasing time
vm.assume(newTime >= block.timestamp);
vm.warp(newTime);
Expand Down
8 changes: 4 additions & 4 deletions contracts/src/v0.8/ccip/test/pools/USDCTokenPool.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ contract USDCTokenPool_lockOrBurn is USDCTokenPoolSetup {
event Burned(address indexed sender, uint256 amount);
event TokensConsumed(uint256 tokens);

function testLockOrBurnSuccess(bytes32 destinationReceiver, uint256 amount) public {
function testFuzz_LockOrBurnSuccess(bytes32 destinationReceiver, uint256 amount) public {
vm.assume(amount < rateLimiterConfig().capacity);
vm.assume(amount > 0);
changePrank(s_routerAllowedOnRamp);
Expand Down Expand Up @@ -136,7 +136,7 @@ contract USDCTokenPool_lockOrBurn is USDCTokenPoolSetup {
assertEq(s_mockUSDC.s_nonce() - 1, nonce);
}

function testLockOrBurnWithAllowListSuccess(bytes32 destinationReceiver, uint256 amount) public {
function testFuzz_LockOrBurnWithAllowListSuccess(bytes32 destinationReceiver, uint256 amount) public {
vm.assume(amount < rateLimiterConfig().capacity);
vm.assume(amount > 0);
changePrank(s_routerAllowedOnRamp);
Expand Down Expand Up @@ -203,7 +203,7 @@ contract USDCTokenPool_lockOrBurn is USDCTokenPoolSetup {
contract USDCTokenPool_releaseOrMint is USDCTokenPoolSetup {
event Minted(address indexed sender, address indexed recipient, uint256 amount);

function testReleaseOrMintSuccess(address receiver, uint256 amount) public {
function testFuzz_ReleaseOrMintSuccess(address receiver, uint256 amount) public {
amount = bound(amount, 0, rateLimiterConfig().capacity);
changePrank(s_routerAllowedOffRamp);

Expand Down Expand Up @@ -267,7 +267,7 @@ contract USDCTokenPool_setDomains is USDCTokenPoolSetup {

mapping(uint64 destChainSelector => USDCTokenPool.Domain domain) private s_chainToDomain;

function testSetDomainsSuccess(
function testFuzz_SetDomainsSuccess(
bytes32[10] calldata allowedCallers,
uint32[10] calldata domainIdentifiers,
uint64[10] calldata destChainSelectors
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/ccip/test/priceRegistry/PriceRegistry.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ contract PriceRegistry_convertTokenAmount is PriceRegistrySetup {
assertEq(s_priceRegistry.convertTokenAmount(s_weth, amount, s_sourceTokens[0]), expected);
}

function test_fuzz_ConvertTokenAmountSuccess(
function testFuzz_ConvertTokenAmountSuccess(
uint256 feeTokenAmount,
uint192 usdPerFeeToken,
uint160 usdPerLinkToken,
Expand Down
8 changes: 4 additions & 4 deletions contracts/src/v0.8/ccip/test/router/Router.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ contract Router_ccipSend is EVM2EVMOnRampSetup {
s_sourceRouter.ccipSend(wrongChain, message);
}

function testUnsupportedFeeTokenReverts(address wrongFeeToken) public {
function testFuzz_UnsupportedFeeTokenReverts(address wrongFeeToken) public {
// We have three fee tokens set, all others should revert.
vm.assume(address(s_sourceFeeToken) != wrongFeeToken);
vm.assume(address(s_sourceRouter.getWrappedNative()) != wrongFeeToken);
Expand All @@ -327,7 +327,7 @@ contract Router_ccipSend is EVM2EVMOnRampSetup {
s_sourceRouter.ccipSend(DEST_CHAIN_ID, message);
}

function testUnsupportedTokenReverts(address wrongToken) public {
function testFuzz_UnsupportedTokenReverts(address wrongToken) public {
for (uint256 i = 0; i < s_sourceTokens.length; ++i) {
vm.assume(address(s_sourceTokens[i]) != wrongToken);
}
Expand Down Expand Up @@ -483,7 +483,7 @@ contract Router_applyRampUpdates is RouterSetup {

/// @notice #setWrappedNative
contract Router_setWrappedNative is EVM2EVMOnRampSetup {
function testSetWrappedNativeSuccess(address wrappedNative) public {
function testFuzz_SetWrappedNativeSuccess(address wrappedNative) public {
s_sourceRouter.setWrappedNative(wrappedNative);
assertEq(wrappedNative, s_sourceRouter.getWrappedNative());
}
Expand Down Expand Up @@ -611,7 +611,7 @@ contract Router_routeMessage is EVM2EVMOffRampSetup {
assertEq("", retData);
}

function test_fuzz_ExecutionEventSuccess(bytes calldata error) public {
function testFuzz_ExecutionEventSuccess(bytes calldata error) public {
Client.Any2EVMMessage memory message = generateReceiverMessage(SOURCE_CHAIN_ID);
s_reverting_receiver.setErr(error);

Expand Down