From 092e1756123e71cb696041e00af0461fddf32601 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Fri, 16 Jun 2023 00:44:41 -0400 Subject: [PATCH 01/27] feat(v1): migration guide --- src/getting-started/migrating-to-v1.md | 328 +++++++++++++++++++++++++ 1 file changed, 328 insertions(+) create mode 100644 src/getting-started/migrating-to-v1.md diff --git a/src/getting-started/migrating-to-v1.md b/src/getting-started/migrating-to-v1.md new file mode 100644 index 000000000..03ae75f2c --- /dev/null +++ b/src/getting-started/migrating-to-v1.md @@ -0,0 +1,328 @@ +## Migrating to Foundry V1 + +Foundry V1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting semver. It's the culmination of over a year of work and more than 200 contributors that chimed in to make it happen. + +With the V1 release, some breaking changes have been made which might require you to tweak your tests, and some patterns are now discouraged and might emit warnings. This guide will help you understand what these changes are and how to handle them properly. + +### How do I upgrade to V1? + +To upgrade, you'll need to reinstall `foundryup`. To do so, open your terminal and run the following command: + +```sh +curl -L https://foundry.paradigm.xyz | bash +``` + +This will reinstall `foundryup`. You can then run `foundryup` on your terminal to download the latest `V1` version. + +#### Deprecated features: + +- Invariant keyword: The invariant keyword has now been deprecated, and the new keyword is now `statefulFuzz`. +- `testFail`: Using `testFail` to write failing tests is now discouraged. A warning will be thrown if used. As an alternative, refactor your tests to use `expectRevert`. +- It is now impossible to use the following cheatcodes on precompiles: `etch`, `load`, `store`, `deal`. + +### Non-breaking but important changes +- Snapshot failures between fuzz runs are now persisted, so workarounds are not needed anymore. +- Sensitive broadcast logs are now logged into the `.gitignore`d `cache` directory, instead of written to logs. +- The base fee calculation on Anvil is now entirely skipped if the fee is set to 0. +- `vm.startPrank` now overwrites the existing prank instead of just erroring. However, the already set prank must be used first. + + +### Breaking changes + +When it comes to breaking changes, there has been a slight rework for the assertion (`expect*`) cheatcodes that removes previously allowed unintended or confusing behavior. + +The biggest change to expect, is that these cheatcodes will now work only for the next call, not at the "test" level anymore. The philosophy behind this is that by making these cheatcodes more strict through this requirement, tests will be more accurate as the cheatcodes will now succeed under more strict conditions. + +#### `expectEmit` + +`expectEmit` previously allowed you to assert that a log was emitted during the execution of the test. With V1, the following changes have been made: + +- it now only works for the _next call_. This means if you used to declare all your expected emits at the top of the test, you may now have to move them to just before the next call you perform. Cheatcode calls do not count. As long as the events are emitted during the next call's context, they will be matched. +- The events have to be _ordered_. That means, if you're trying to match events [A, B, C], you must declare them in this order. + - Skipping events is possible. Therefore, if a function emits events [A, B, C, D, E] and you want to match [B, D, E], you just need to declare them in that order. + +To illustrate, see the following examples: + +```solidity + +contract Emitter { + event A(uint256 indexed a); + event B(uint256 indexed b); + event C(uint256 indexed c); + event D(uint256 indexed d); + event E(uint256 indexed e); + + /// emit() emits events [A, B, C, D, E] + function emit() external { + emit A(1); + emit B(2); + emit C(3); + emit D(4); + emit E(1); + } +} + +contract EmitTest is Test { + Emitter public emitter; + + event A(uint256 indexed a); + event B(uint256 indexed b); + event C(uint256 indexed c); + event D(uint256 indexed d); + event E(uint256 indexed e); + + function setUp() public { + emitter = new Emitter(); + } + + /// CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, + /// on the test. + /// emit() emits [A, B, C, D, E], and we're expecting [A, B, C, D, E]. + /// this passes. + function testExpectEmit() public { + cheats.expectEmit(true, false, false, true); + emit A(1); + cheats.expectEmit(true, false, false, true); + emit B(2); + cheats.expectEmit(true, false, false, true); + emit C(3); + cheats.expectEmit(true, false, false, true); + emit D(4); + cheats.expectEmit(true, false, false, true); + emit E(5); + emitter.emit(); + } + + /// CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, + /// on the test. + /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. + /// this passes. + function testExpectEmitWindow() public { + cheats.expectEmit(true, false, false, true); + emit B(2); + cheats.expectEmit(true, false, false, true); + emit D(4); + cheats.expectEmit(true, false, false, true); + emit E(5); + emitter.emit(); + } + + /// INCORRECT BEHAVIOR: Declare all your expectEmits in the wrong order. + /// emit() emits [A, B, C, D, E], and we're expecting [D, B, E]. + /// this fails, as D is emitted after B, not before. + function testExpectEmitWindowFailure() public { + cheats.expectEmit(true, false, false, true); + emit D(4); + cheats.expectEmit(true, false, false, true); + emit B(2); + cheats.expectEmit(true, false, false, true); + emit E(5); + emitter.emit(); + } + + /// CORRECT BEHAVIOR: Declare all your expectEmits in an internal function. + /// Calling a contract function internally is a JUMPI, not a call, + /// therefore it's a valid pattern, useful if you have a lot of events to expect. + /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. + /// this passes. + function testExpectEmitWithInternalFunction() public { + declareExpectedEvents(); + emitter.emit(); + } + + function declareExpectedEvents() internal { + cheats.expectEmit(true, false, false, true); + emit B(2); + cheats.expectEmit(true, false, false, true); + emit D(4); + cheats.expectEmit(true, false, false, true); + emit E(5); + } +} +``` + +As illustrated above, if you'd like to make your tests more brief, you can declare your expected events in an internal function in your test contract to remove visual clutter. This is not an external call, which means they won't count for detecting events. + +#### `expectCall` + +Just like `expectEmit`, `expectCall` has gotten a similar rework. Before, it worked at the test level, which meant you could declare the calls you expect to see upfront and then start calling external functions. The new behavior is the following one: + +- It now only works for the next _call's subcalls_. This means that the call(s) you expect need to happen inside an external call for them to be detected. Calling it at the test level (directly) won't count. + - This is explained through "depth": the `expectCall` cheatcode is called at a "test-level" depth, so we enforce that the calls we expect to see are made at a "deeper" depth to be able to count them properly. Depth is increased whenever we make an external call. Internal calls do not increase depth. + +To illustrate, see the following examples: + +```solidity +contract Protocol { + function doSomething() external { + // do something... + } +} + +contract Caller { + Protocol public protocol; + + function foo() public { + // do something... + } + + function baz() public { + // do something... + } + + function bar() public { + protocol.doSomething(); + // do something... + } +} + +contract ExpectCallTest is Test { + Caller public caller; + + function setUp() public { + caller = new Caller(); + } + + /// CORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, + /// So we declare our expected call, and then call `caller.bar()`, which will + /// call `protocol.doSomething()` internally. + /// `doSomething()` is nested inside `bar`, so this passes. + function testExpectCall() public { + vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething)); + // This will call bar internally, so this is valid. + caller.bar(); + } + + /// INCORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, + /// So we declare our expected call, and then call `protocol.doSomething()` directly. + /// `doSomething()` is not nested, but rather called at the test level. + /// This doesn't satisfy the depth requirement described above, + /// so this fails. + function testExpectCallFailure() public { + vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething)); + // We're calling doSomething() directly here. + // This is not inside the next call's subcall tree, but rather a test-level + // call. This fails. + caller.protocol.doSomething(); + } + + /// CORRECT BEHAVIOR: Sometimes, we want to directly expect the call we're gonna + /// call next, but we need to satisfy the depth requirement. In those cases, + /// this pattern can be used. + /// Calling exposed_callFoo using `this` will create a new call context which + /// will increase depth. In its subcall tree, we'll detect `exposed_callFoo`, + /// which will make the test pass. + function testExpectCallWithNest() public { + vm.expectCall(address(caller), abi.encodeWithSelector(caller.foo.selector)); + this.exposed_callFoo(); + } + + function exposed_callFoo() public { + caller.foo(); + } +} +``` + +#### `expectRevert` + +`expectRevert` currently allows to expect for reverts at the test level. This means that a revert can be declared at the top of the test and as long as a function in the test reverts, the test will be pass. The new behavior is the following: + +- `expectRevert` CANNOT be used with other `expect` cheatcodes. It will fail if it detects they're being used at the same time. + - The reasoning for this is that calls that revert are rolled back, and events and calls therefore never happen. This restrictions helps model real-world behavior. +- `expectRevert` now works only for the next call. If the next call does not revert, the cheatcode will fail and therefore the test will also fail. The depth of the revert does not matter: as long as the next call reverts, either inmediately or deeper into its call subtree, the cheatcode will succeed. + +To illustrate, see the following examples: + +```solidity + +contract Reverter { + function revertWithMessage(string memory message) public pure { + require(false, message); + } + + function doNotRevert() public { + // do something unrelated, do not revert. + } +} + +contract ExpectRevertTest is Test { + Reverter reverter; + + function setUp() public { + reverter = new Reverter(); + } + + /// CORRECT BEHAVIOR: We expect `revertWithMessage` to revert, + /// so we declare a expectRevert and then call the function. + function testExpectRevertString() public { + vm.expectRevert("revert"); + reverter.revertWithMessage("revert"); + } + + /// INCORRECT BEHAVIOR: We correctly expect that a function will revert in this + /// test, but we declare the expected revert before the wrong function, + /// `doNotRevert()`, which does not revert. The cheatcode therefore fails. + function testFailRevertNotOnImmediateNextCall() public { + // expectRevert should only work for the next call. However, + // we do not inmediately revert, so, + // we fail. + vm.expectRevert("revert"); + reverter.doNotRevert(); + reverter.revertWithMessage("revert"); + } +} +``` + +### Testing internal calls or libraries with `expectCall` and `expectRevert` + +With the changes to `expectCall` and `expectRevert`, library maintainers might need to modify tests that expect calls or reverts on library functions that might be marked as internal. As an external call needs to be forced to satisfy the depth requirement, an intermediate contract that exposes an identical API to the function that needs to be called can be used. + +See the following example for testing a revert on a library. + +```solidity +library MathLib { + function add(uint a, uint b) internal pure { + return a + b; + } +} + +// intermediate "mock" contract that calls the library function +// and returns the result. +contract MathLibMock { + function add(uint a, uint b) external { + return MathLib.add(a, b); + } +} + +contract MathLibTest is Test { + MathLibMock public mock; + + function setUp() public { + mock = new MathLibMock(); + } + + /// INCORRECT BEHAVIOR: MathLib.add will revert due to arithmetic errors, + /// but as the function is marked as internal, it'll be a test level revert + /// instead of a call revert that expectRevert is supposed to catch. + /// The test will fail, and the error will let you know that you have a + /// "dangling" expectRevert. + function testRevertOnMathLibWithNoMock() public { + vm.expectRevert(); + MathLib.add(2 ** 128 - 1, 2 ** 255 - 1); + } + + /// CORRECT BEHAVIOR: mock.add will revert due to arithmetic errors, + /// and it will be successfully detected by the `expectRevert` cheatcode. + function testRevertOnMathLibWithMock() public { + vm.expectRevert(); + mock.add(2 ** 128 - 1, 2 ** 255 - 1); + } +} +``` + +### Recommended approach for tackling codebase migrations + +- Start by fixing your simpler tests so you get the hang of how the cheatcodes are supposed to work. +- Separate tests using both `expectCall`/`expectEmit` and `expectRevert` into two versions: one using `expectCall`/`expectEmit` as the happy path, and one using `expectRevert` as the unhappy path. +- Move your `expectEmit`, `expectCall` and `expectRevert` declarations to exactly before the call you expect to match these. Remember that `expectRevert` cannot be used along with the other `expect` cheatcodes. +- If you need to abstract function calls for `expectCall`, or calls that might revert for `expectRevert`, consider using an intermediate contract that calls these functions to satisfy the depth requirements. \ No newline at end of file From 8d72f83f0b8e1fd88f590c1badc8a6de44494e6e Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Fri, 16 Jun 2023 00:49:52 -0400 Subject: [PATCH 02/27] chore: add migration guide to summary --- src/SUMMARY.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 36ad67873..23577f91b 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -6,6 +6,7 @@ - [Installation](./getting-started/installation.md) - [First Steps with Foundry](./getting-started/first-steps.md) +- [Migrating to V1](./getting-started/migrating-to-v1.md) # Projects From 16eafa7dfbe8104039034e4efa47fce57fd572c8 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Tue, 27 Jun 2023 09:36:06 -0400 Subject: [PATCH 03/27] chore: restructure into new section with subsection --- src/SUMMARY.md | 9 ++++++++- .../migrating-to-v1.md | 0 2 files changed, 8 insertions(+), 1 deletion(-) rename src/{getting-started => migrating-to-v1}/migrating-to-v1.md (100%) diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 23577f91b..55d58a450 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -6,7 +6,14 @@ - [Installation](./getting-started/installation.md) - [First Steps with Foundry](./getting-started/first-steps.md) -- [Migrating to V1](./getting-started/migrating-to-v1.md) + +# Migrating to V1 + +- [Upgrading to V1](./migrating-to-v1/migrating-to-v1.md) +- [Deprecated Features](./migrating-to-v1/migrating-to-v1.md) +- [V1 Changes](./migrating-to-v1/migrating-to-v1.md) + - [Non-breaking Changes](./migrating-to-v1/migrating-to-v1.md) + - [Breaking Changes](./migrating-to-v1/migrating-to-v1.md) # Projects diff --git a/src/getting-started/migrating-to-v1.md b/src/migrating-to-v1/migrating-to-v1.md similarity index 100% rename from src/getting-started/migrating-to-v1.md rename to src/migrating-to-v1/migrating-to-v1.md From 951034dc0c57a2de9a9374f247e9392c9f77973f Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Tue, 27 Jun 2023 10:15:18 -0400 Subject: [PATCH 04/27] chore: refactor into different pages --- src/SUMMARY.md | 11 +- src/migrating-to-v1/breaking-changes.md | 292 ++++++++++++++++++++ src/migrating-to-v1/deprecated-features.md | 67 +++++ src/migrating-to-v1/non-breaking-changes.md | 6 + src/migrating-to-v1/upgrading-to-v1.md | 17 ++ src/migrating-to-v1/v1-changes.md | 3 + 6 files changed, 391 insertions(+), 5 deletions(-) create mode 100644 src/migrating-to-v1/breaking-changes.md create mode 100644 src/migrating-to-v1/deprecated-features.md create mode 100644 src/migrating-to-v1/non-breaking-changes.md create mode 100644 src/migrating-to-v1/upgrading-to-v1.md create mode 100644 src/migrating-to-v1/v1-changes.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 55d58a450..358143918 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -9,11 +9,12 @@ # Migrating to V1 -- [Upgrading to V1](./migrating-to-v1/migrating-to-v1.md) -- [Deprecated Features](./migrating-to-v1/migrating-to-v1.md) -- [V1 Changes](./migrating-to-v1/migrating-to-v1.md) - - [Non-breaking Changes](./migrating-to-v1/migrating-to-v1.md) - - [Breaking Changes](./migrating-to-v1/migrating-to-v1.md) +- [Introduction] (./migrating-to-v1/migrating-to-v1.md) +- [Upgrading to V1](./migrating-to-v1/upgrading-to-v1.md) +- [weprecated Features](./migrating-to-v1/deprecated-features.md) +- [V1 Changes](./migrating-to-v1/v1-changes.md) + - [Non-breaking Changes](./migrating-to-v1/non-breaking-changes.md) + - [Breaking Changes](./migrating-to-v1/breaking-changes.md) # Projects diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md new file mode 100644 index 000000000..01d118325 --- /dev/null +++ b/src/migrating-to-v1/breaking-changes.md @@ -0,0 +1,292 @@ +## Breaking changes + +When it comes to breaking changes, there has been a slight rework for the assertion (`expect*`) cheatcodes that removes previously allowed unintended or confusing behavior. + +The biggest change to expect, is that these cheatcodes will now work only for the next call, not at the "test" level anymore. The philosophy behind this is that by making these cheatcodes more strict through this requirement, tests will be more accurate as the cheatcodes will now succeed under more strict conditions. + +#### `expectEmit` + +`expectEmit` previously allowed you to assert that a log was emitted during the execution of the test. With V1, the following changes have been made: + +- it now only works for the _next call_. This means if you used to declare all your expected emits at the top of the test, you may now have to move them to just before the next call you perform. Cheatcode calls do not count. As long as the events are emitted during the next call's context, they will be matched. +- The events have to be _ordered_. That means, if you're trying to match events [A, B, C], you must declare them in this order. + - Skipping events is possible. Therefore, if a function emits events [A, B, C, D, E] and you want to match [B, D, E], you just need to declare them in that order. + +To illustrate, see the following examples: + +```solidity + +contract Emitter { + event A(uint256 indexed a); + event B(uint256 indexed b); + event C(uint256 indexed c); + event D(uint256 indexed d); + event E(uint256 indexed e); + + /// emit() emits events [A, B, C, D, E] + function emit() external { + emit A(1); + emit B(2); + emit C(3); + emit D(4); + emit E(1); + } +} + +contract EmitTest is Test { + Emitter public emitter; + + event A(uint256 indexed a); + event B(uint256 indexed b); + event C(uint256 indexed c); + event D(uint256 indexed d); + event E(uint256 indexed e); + + function setUp() public { + emitter = new Emitter(); + } + + /// CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, + /// on the test. + /// emit() emits [A, B, C, D, E], and we're expecting [A, B, C, D, E]. + /// this passes. + function testExpectEmit() public { + cheats.expectEmit(true, false, false, true); + emit A(1); + cheats.expectEmit(true, false, false, true); + emit B(2); + cheats.expectEmit(true, false, false, true); + emit C(3); + cheats.expectEmit(true, false, false, true); + emit D(4); + cheats.expectEmit(true, false, false, true); + emit E(5); + emitter.emit(); + } + + /// CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, + /// on the test. + /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. + /// this passes. + function testExpectEmitWindow() public { + cheats.expectEmit(true, false, false, true); + emit B(2); + cheats.expectEmit(true, false, false, true); + emit D(4); + cheats.expectEmit(true, false, false, true); + emit E(5); + emitter.emit(); + } + + /// INCORRECT BEHAVIOR: Declare all your expectEmits in the wrong order. + /// emit() emits [A, B, C, D, E], and we're expecting [D, B, E]. + /// this fails, as D is emitted after B, not before. + function testExpectEmitWindowFailure() public { + cheats.expectEmit(true, false, false, true); + emit D(4); + cheats.expectEmit(true, false, false, true); + emit B(2); + cheats.expectEmit(true, false, false, true); + emit E(5); + emitter.emit(); + } + + /// CORRECT BEHAVIOR: Declare all your expectEmits in an internal function. + /// Calling a contract function internally is a JUMPI, not a call, + /// therefore it's a valid pattern, useful if you have a lot of events to expect. + /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. + /// this passes. + function testExpectEmitWithInternalFunction() public { + declareExpectedEvents(); + emitter.emit(); + } + + function declareExpectedEvents() internal { + cheats.expectEmit(true, false, false, true); + emit B(2); + cheats.expectEmit(true, false, false, true); + emit D(4); + cheats.expectEmit(true, false, false, true); + emit E(5); + } +} +``` + +As illustrated above, if you'd like to make your tests more brief, you can declare your expected events in an internal function in your test contract to remove visual clutter. This is not an external call, which means they won't count for detecting events. + +#### `expectCall` + +Just like `expectEmit`, `expectCall` has gotten a similar rework. Before, it worked at the test level, which meant you could declare the calls you expect to see upfront and then start calling external functions. The new behavior is the following one: + +- It now only works for the next _call's subcalls_. This means that the call(s) you expect need to happen inside an external call for them to be detected. Calling it at the test level (directly) won't count. + - This is explained through "depth": the `expectCall` cheatcode is called at a "test-level" depth, so we enforce that the calls we expect to see are made at a "deeper" depth to be able to count them properly. Depth is increased whenever we make an external call. Internal calls do not increase depth. + +To illustrate, see the following examples: + +```solidity +contract Protocol { + function doSomething() external { + // do something... + } +} + +contract Caller { + Protocol public protocol; + + function foo() public { + // do something... + } + + function baz() public { + // do something... + } + + function bar() public { + protocol.doSomething(); + // do something... + } +} + +contract ExpectCallTest is Test { + Caller public caller; + + function setUp() public { + caller = new Caller(); + } + + /// CORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, + /// So we declare our expected call, and then call `caller.bar()`, which will + /// call `protocol.doSomething()` internally. + /// `doSomething()` is nested inside `bar`, so this passes. + function testExpectCall() public { + vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething)); + // This will call bar internally, so this is valid. + caller.bar(); + } + + /// INCORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, + /// So we declare our expected call, and then call `protocol.doSomething()` directly. + /// `doSomething()` is not nested, but rather called at the test level. + /// This doesn't satisfy the depth requirement described above, + /// so this fails. + function testExpectCallFailure() public { + vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething)); + // We're calling doSomething() directly here. + // This is not inside the next call's subcall tree, but rather a test-level + // call. This fails. + caller.protocol.doSomething(); + } + + /// CORRECT BEHAVIOR: Sometimes, we want to directly expect the call we're gonna + /// call next, but we need to satisfy the depth requirement. In those cases, + /// this pattern can be used. + /// Calling exposed_callFoo using `this` will create a new call context which + /// will increase depth. In its subcall tree, we'll detect `exposed_callFoo`, + /// which will make the test pass. + function testExpectCallWithNest() public { + vm.expectCall(address(caller), abi.encodeWithSelector(caller.foo.selector)); + this.exposed_callFoo(); + } + + function exposed_callFoo() public { + caller.foo(); + } +} +``` + +#### `expectRevert` + +`expectRevert` currently allows to expect for reverts at the test level. This means that a revert can be declared at the top of the test and as long as a function in the test reverts, the test will be pass. The new behavior is the following: + +- `expectRevert` CANNOT be used with other `expect` cheatcodes. It will fail if it detects they're being used at the same time. + - The reasoning for this is that calls that revert are rolled back, and events and calls therefore never happen. This restrictions helps model real-world behavior. +- `expectRevert` now works only for the next call. If the next call does not revert, the cheatcode will fail and therefore the test will also fail. The depth of the revert does not matter: as long as the next call reverts, either inmediately or deeper into its call subtree, the cheatcode will succeed. + +To illustrate, see the following examples: + +```solidity + +contract Reverter { + function revertWithMessage(string memory message) public pure { + require(false, message); + } + + function doNotRevert() public { + // do something unrelated, do not revert. + } +} + +contract ExpectRevertTest is Test { + Reverter reverter; + + function setUp() public { + reverter = new Reverter(); + } + + /// CORRECT BEHAVIOR: We expect `revertWithMessage` to revert, + /// so we declare a expectRevert and then call the function. + function testExpectRevertString() public { + vm.expectRevert("revert"); + reverter.revertWithMessage("revert"); + } + + /// INCORRECT BEHAVIOR: We correctly expect that a function will revert in this + /// test, but we declare the expected revert before the wrong function, + /// `doNotRevert()`, which does not revert. The cheatcode therefore fails. + function testFailRevertNotOnImmediateNextCall() public { + // expectRevert should only work for the next call. However, + // we do not inmediately revert, so, + // we fail. + vm.expectRevert("revert"); + reverter.doNotRevert(); + reverter.revertWithMessage("revert"); + } +} +``` + +### Testing internal calls or libraries with `expectCall` and `expectRevert` + +With the changes to `expectCall` and `expectRevert`, library maintainers might need to modify tests that expect calls or reverts on library functions that might be marked as internal. As an external call needs to be forced to satisfy the depth requirement, an intermediate contract that exposes an identical API to the function that needs to be called can be used. + +See the following example for testing a revert on a library. + +```solidity +library MathLib { + function add(uint a, uint b) internal pure { + return a + b; + } +} + +// intermediate "mock" contract that calls the library function +// and returns the result. +contract MathLibMock { + function add(uint a, uint b) external { + return MathLib.add(a, b); + } +} + +contract MathLibTest is Test { + MathLibMock public mock; + + function setUp() public { + mock = new MathLibMock(); + } + + /// INCORRECT BEHAVIOR: MathLib.add will revert due to arithmetic errors, + /// but as the function is marked as internal, it'll be a test level revert + /// instead of a call revert that expectRevert is supposed to catch. + /// The test will fail, and the error will let you know that you have a + /// "dangling" expectRevert. + function testRevertOnMathLibWithNoMock() public { + vm.expectRevert(); + MathLib.add(2 ** 128 - 1, 2 ** 255 - 1); + } + + /// CORRECT BEHAVIOR: mock.add will revert due to arithmetic errors, + /// and it will be successfully detected by the `expectRevert` cheatcode. + function testRevertOnMathLibWithMock() public { + vm.expectRevert(); + mock.add(2 ** 128 - 1, 2 ** 255 - 1); + } +} +``` \ No newline at end of file diff --git a/src/migrating-to-v1/deprecated-features.md b/src/migrating-to-v1/deprecated-features.md new file mode 100644 index 000000000..a1797db51 --- /dev/null +++ b/src/migrating-to-v1/deprecated-features.md @@ -0,0 +1,67 @@ +## Deprecated features: + +With foundry V1, we've decided to go ahead and start deprecating features that are considered anti-practices or might be misleading for developers. Deprecated means that while you can continue using these features, it is now a discouraged practice and you should plan on incrementally migrating away from using these. + +### Removing the Invariant keyword + +The invariant keyword has now been deprecated, and the new keyword is now `statefulFuzz`. This means that now writing tests in this manner is valid: + +```solidity +/// Old, deprecated way of declaring an invariant test +function invariantTestEq() public { + // Assert your invariants... +} + +/// New +function statefulFuzzTestEq() public { + // Assert your invariants +} +``` + +### Deprecating testFail + +Using `testFail` to write failing tests is now discouraged. While it's usage is extremely common, we've decided to remove it because it can introduce footguns in a test. A warning will be thrown if it's used. + +The now-recommended pattern is to refactor the actions that will make your test revert and subsequently fail in an utility function, and use `expectRevert` to ensure that this function call failed. This way, the test failure is expected explicitly, instead of introducing the possibility that the test fails in an unintended way silently. + +An example: + +```solidity + +contract Mock { + function revert_() public { + revert("This reverts"); + } +} + +contract TestFailDeprecated is Test { + Mock public mock; + + function setUp() public { + mock = new Mock(); + } + + /// Deprecated way. + function testFailReverter() public { + mock.revert_(); + } + + /// New way, without using testFail. + /// The call to revert_ has been refactored and is now expected to fail + /// with expectRevert() + function testReverterReverts() public { + vm.expectRevert(); + // Call using this to increase depth + this.exposed_call_Reverter(); + } + + function exposed_callReverter() public { + mock.revert_(); + } + +} +``` + +### Removing cheatcode support on some precompiles + +It is now impossible to use the following cheatcodes on precompiles: `etch`, `load`, `store`, `deal`. \ No newline at end of file diff --git a/src/migrating-to-v1/non-breaking-changes.md b/src/migrating-to-v1/non-breaking-changes.md new file mode 100644 index 000000000..bb93a7aa2 --- /dev/null +++ b/src/migrating-to-v1/non-breaking-changes.md @@ -0,0 +1,6 @@ +## Non-breaking but important changes + +- Snapshot failures between fuzz runs are now persisted, so workarounds are not needed anymore. +- Sensitive broadcast logs are now logged into the `.gitignore`d `cache` directory, instead of written to logs. +- The base fee calculation on Anvil is now entirely skipped if the fee is set to 0. +- `vm.startPrank` now overwrites the existing prank instead of just erroring. However, the already set prank must be used first. \ No newline at end of file diff --git a/src/migrating-to-v1/upgrading-to-v1.md b/src/migrating-to-v1/upgrading-to-v1.md new file mode 100644 index 000000000..d492f411c --- /dev/null +++ b/src/migrating-to-v1/upgrading-to-v1.md @@ -0,0 +1,17 @@ +## Migrating to Foundry V1 + +Foundry V1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting semver. It's the culmination of over a year of work and more than 200 contributors that chimed in to make it happen. + +With the V1 release, some breaking changes have been made which might require you to tweak your tests, and some patterns are now discouraged and might emit warnings. This guide will help you understand what these changes are and how to handle them properly. + +### How do I upgrade to V1? + +To upgrade, you'll need to reinstall `foundryup`. To do so, open your terminal and run the following command: + +```sh +curl -L https://foundry.paradigm.xyz | bash +``` + +This will reinstall `foundryup`. You can then run `foundryup` on your terminal to download the latest `V1` version. + +After this, you're now up to date with the latest stable Foundry version. \ No newline at end of file diff --git a/src/migrating-to-v1/v1-changes.md b/src/migrating-to-v1/v1-changes.md new file mode 100644 index 000000000..805798317 --- /dev/null +++ b/src/migrating-to-v1/v1-changes.md @@ -0,0 +1,3 @@ +## Breaking and Non-Breaking Changes in V1 + +With the V1 release, some breaking changes have been made which might require you to tweak your tests, and some patterns are now discouraged and might emit warnings. This guide will help you understand what these changes are and how to handle them properly. \ No newline at end of file From 5ba9aa7fc4ac50309cb2f2fae9451e482d4adfd1 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Tue, 27 Jun 2023 10:19:54 -0400 Subject: [PATCH 05/27] chore: fix mdbook err --- src/SUMMARY.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 358143918..609c1d8ad 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -9,9 +9,9 @@ # Migrating to V1 -- [Introduction] (./migrating-to-v1/migrating-to-v1.md) +- [Introduction](./migrating-to-v1/migrating-to-v1.md) - [Upgrading to V1](./migrating-to-v1/upgrading-to-v1.md) -- [weprecated Features](./migrating-to-v1/deprecated-features.md) +- [Deprecated Features](./migrating-to-v1/deprecated-features.md) - [V1 Changes](./migrating-to-v1/v1-changes.md) - [Non-breaking Changes](./migrating-to-v1/non-breaking-changes.md) - [Breaking Changes](./migrating-to-v1/breaking-changes.md) From 95e97b9a8e222207e48b35bc1464e15c6b4ca2a4 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Tue, 27 Jun 2023 10:21:39 -0400 Subject: [PATCH 06/27] chore: remove duplicated info from introductory content --- src/migrating-to-v1/migrating-to-v1.md | 325 +------------------------ 1 file changed, 1 insertion(+), 324 deletions(-) diff --git a/src/migrating-to-v1/migrating-to-v1.md b/src/migrating-to-v1/migrating-to-v1.md index 03ae75f2c..6ce3d1c6b 100644 --- a/src/migrating-to-v1/migrating-to-v1.md +++ b/src/migrating-to-v1/migrating-to-v1.md @@ -2,327 +2,4 @@ Foundry V1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting semver. It's the culmination of over a year of work and more than 200 contributors that chimed in to make it happen. -With the V1 release, some breaking changes have been made which might require you to tweak your tests, and some patterns are now discouraged and might emit warnings. This guide will help you understand what these changes are and how to handle them properly. - -### How do I upgrade to V1? - -To upgrade, you'll need to reinstall `foundryup`. To do so, open your terminal and run the following command: - -```sh -curl -L https://foundry.paradigm.xyz | bash -``` - -This will reinstall `foundryup`. You can then run `foundryup` on your terminal to download the latest `V1` version. - -#### Deprecated features: - -- Invariant keyword: The invariant keyword has now been deprecated, and the new keyword is now `statefulFuzz`. -- `testFail`: Using `testFail` to write failing tests is now discouraged. A warning will be thrown if used. As an alternative, refactor your tests to use `expectRevert`. -- It is now impossible to use the following cheatcodes on precompiles: `etch`, `load`, `store`, `deal`. - -### Non-breaking but important changes -- Snapshot failures between fuzz runs are now persisted, so workarounds are not needed anymore. -- Sensitive broadcast logs are now logged into the `.gitignore`d `cache` directory, instead of written to logs. -- The base fee calculation on Anvil is now entirely skipped if the fee is set to 0. -- `vm.startPrank` now overwrites the existing prank instead of just erroring. However, the already set prank must be used first. - - -### Breaking changes - -When it comes to breaking changes, there has been a slight rework for the assertion (`expect*`) cheatcodes that removes previously allowed unintended or confusing behavior. - -The biggest change to expect, is that these cheatcodes will now work only for the next call, not at the "test" level anymore. The philosophy behind this is that by making these cheatcodes more strict through this requirement, tests will be more accurate as the cheatcodes will now succeed under more strict conditions. - -#### `expectEmit` - -`expectEmit` previously allowed you to assert that a log was emitted during the execution of the test. With V1, the following changes have been made: - -- it now only works for the _next call_. This means if you used to declare all your expected emits at the top of the test, you may now have to move them to just before the next call you perform. Cheatcode calls do not count. As long as the events are emitted during the next call's context, they will be matched. -- The events have to be _ordered_. That means, if you're trying to match events [A, B, C], you must declare them in this order. - - Skipping events is possible. Therefore, if a function emits events [A, B, C, D, E] and you want to match [B, D, E], you just need to declare them in that order. - -To illustrate, see the following examples: - -```solidity - -contract Emitter { - event A(uint256 indexed a); - event B(uint256 indexed b); - event C(uint256 indexed c); - event D(uint256 indexed d); - event E(uint256 indexed e); - - /// emit() emits events [A, B, C, D, E] - function emit() external { - emit A(1); - emit B(2); - emit C(3); - emit D(4); - emit E(1); - } -} - -contract EmitTest is Test { - Emitter public emitter; - - event A(uint256 indexed a); - event B(uint256 indexed b); - event C(uint256 indexed c); - event D(uint256 indexed d); - event E(uint256 indexed e); - - function setUp() public { - emitter = new Emitter(); - } - - /// CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, - /// on the test. - /// emit() emits [A, B, C, D, E], and we're expecting [A, B, C, D, E]. - /// this passes. - function testExpectEmit() public { - cheats.expectEmit(true, false, false, true); - emit A(1); - cheats.expectEmit(true, false, false, true); - emit B(2); - cheats.expectEmit(true, false, false, true); - emit C(3); - cheats.expectEmit(true, false, false, true); - emit D(4); - cheats.expectEmit(true, false, false, true); - emit E(5); - emitter.emit(); - } - - /// CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, - /// on the test. - /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. - /// this passes. - function testExpectEmitWindow() public { - cheats.expectEmit(true, false, false, true); - emit B(2); - cheats.expectEmit(true, false, false, true); - emit D(4); - cheats.expectEmit(true, false, false, true); - emit E(5); - emitter.emit(); - } - - /// INCORRECT BEHAVIOR: Declare all your expectEmits in the wrong order. - /// emit() emits [A, B, C, D, E], and we're expecting [D, B, E]. - /// this fails, as D is emitted after B, not before. - function testExpectEmitWindowFailure() public { - cheats.expectEmit(true, false, false, true); - emit D(4); - cheats.expectEmit(true, false, false, true); - emit B(2); - cheats.expectEmit(true, false, false, true); - emit E(5); - emitter.emit(); - } - - /// CORRECT BEHAVIOR: Declare all your expectEmits in an internal function. - /// Calling a contract function internally is a JUMPI, not a call, - /// therefore it's a valid pattern, useful if you have a lot of events to expect. - /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. - /// this passes. - function testExpectEmitWithInternalFunction() public { - declareExpectedEvents(); - emitter.emit(); - } - - function declareExpectedEvents() internal { - cheats.expectEmit(true, false, false, true); - emit B(2); - cheats.expectEmit(true, false, false, true); - emit D(4); - cheats.expectEmit(true, false, false, true); - emit E(5); - } -} -``` - -As illustrated above, if you'd like to make your tests more brief, you can declare your expected events in an internal function in your test contract to remove visual clutter. This is not an external call, which means they won't count for detecting events. - -#### `expectCall` - -Just like `expectEmit`, `expectCall` has gotten a similar rework. Before, it worked at the test level, which meant you could declare the calls you expect to see upfront and then start calling external functions. The new behavior is the following one: - -- It now only works for the next _call's subcalls_. This means that the call(s) you expect need to happen inside an external call for them to be detected. Calling it at the test level (directly) won't count. - - This is explained through "depth": the `expectCall` cheatcode is called at a "test-level" depth, so we enforce that the calls we expect to see are made at a "deeper" depth to be able to count them properly. Depth is increased whenever we make an external call. Internal calls do not increase depth. - -To illustrate, see the following examples: - -```solidity -contract Protocol { - function doSomething() external { - // do something... - } -} - -contract Caller { - Protocol public protocol; - - function foo() public { - // do something... - } - - function baz() public { - // do something... - } - - function bar() public { - protocol.doSomething(); - // do something... - } -} - -contract ExpectCallTest is Test { - Caller public caller; - - function setUp() public { - caller = new Caller(); - } - - /// CORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, - /// So we declare our expected call, and then call `caller.bar()`, which will - /// call `protocol.doSomething()` internally. - /// `doSomething()` is nested inside `bar`, so this passes. - function testExpectCall() public { - vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething)); - // This will call bar internally, so this is valid. - caller.bar(); - } - - /// INCORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, - /// So we declare our expected call, and then call `protocol.doSomething()` directly. - /// `doSomething()` is not nested, but rather called at the test level. - /// This doesn't satisfy the depth requirement described above, - /// so this fails. - function testExpectCallFailure() public { - vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething)); - // We're calling doSomething() directly here. - // This is not inside the next call's subcall tree, but rather a test-level - // call. This fails. - caller.protocol.doSomething(); - } - - /// CORRECT BEHAVIOR: Sometimes, we want to directly expect the call we're gonna - /// call next, but we need to satisfy the depth requirement. In those cases, - /// this pattern can be used. - /// Calling exposed_callFoo using `this` will create a new call context which - /// will increase depth. In its subcall tree, we'll detect `exposed_callFoo`, - /// which will make the test pass. - function testExpectCallWithNest() public { - vm.expectCall(address(caller), abi.encodeWithSelector(caller.foo.selector)); - this.exposed_callFoo(); - } - - function exposed_callFoo() public { - caller.foo(); - } -} -``` - -#### `expectRevert` - -`expectRevert` currently allows to expect for reverts at the test level. This means that a revert can be declared at the top of the test and as long as a function in the test reverts, the test will be pass. The new behavior is the following: - -- `expectRevert` CANNOT be used with other `expect` cheatcodes. It will fail if it detects they're being used at the same time. - - The reasoning for this is that calls that revert are rolled back, and events and calls therefore never happen. This restrictions helps model real-world behavior. -- `expectRevert` now works only for the next call. If the next call does not revert, the cheatcode will fail and therefore the test will also fail. The depth of the revert does not matter: as long as the next call reverts, either inmediately or deeper into its call subtree, the cheatcode will succeed. - -To illustrate, see the following examples: - -```solidity - -contract Reverter { - function revertWithMessage(string memory message) public pure { - require(false, message); - } - - function doNotRevert() public { - // do something unrelated, do not revert. - } -} - -contract ExpectRevertTest is Test { - Reverter reverter; - - function setUp() public { - reverter = new Reverter(); - } - - /// CORRECT BEHAVIOR: We expect `revertWithMessage` to revert, - /// so we declare a expectRevert and then call the function. - function testExpectRevertString() public { - vm.expectRevert("revert"); - reverter.revertWithMessage("revert"); - } - - /// INCORRECT BEHAVIOR: We correctly expect that a function will revert in this - /// test, but we declare the expected revert before the wrong function, - /// `doNotRevert()`, which does not revert. The cheatcode therefore fails. - function testFailRevertNotOnImmediateNextCall() public { - // expectRevert should only work for the next call. However, - // we do not inmediately revert, so, - // we fail. - vm.expectRevert("revert"); - reverter.doNotRevert(); - reverter.revertWithMessage("revert"); - } -} -``` - -### Testing internal calls or libraries with `expectCall` and `expectRevert` - -With the changes to `expectCall` and `expectRevert`, library maintainers might need to modify tests that expect calls or reverts on library functions that might be marked as internal. As an external call needs to be forced to satisfy the depth requirement, an intermediate contract that exposes an identical API to the function that needs to be called can be used. - -See the following example for testing a revert on a library. - -```solidity -library MathLib { - function add(uint a, uint b) internal pure { - return a + b; - } -} - -// intermediate "mock" contract that calls the library function -// and returns the result. -contract MathLibMock { - function add(uint a, uint b) external { - return MathLib.add(a, b); - } -} - -contract MathLibTest is Test { - MathLibMock public mock; - - function setUp() public { - mock = new MathLibMock(); - } - - /// INCORRECT BEHAVIOR: MathLib.add will revert due to arithmetic errors, - /// but as the function is marked as internal, it'll be a test level revert - /// instead of a call revert that expectRevert is supposed to catch. - /// The test will fail, and the error will let you know that you have a - /// "dangling" expectRevert. - function testRevertOnMathLibWithNoMock() public { - vm.expectRevert(); - MathLib.add(2 ** 128 - 1, 2 ** 255 - 1); - } - - /// CORRECT BEHAVIOR: mock.add will revert due to arithmetic errors, - /// and it will be successfully detected by the `expectRevert` cheatcode. - function testRevertOnMathLibWithMock() public { - vm.expectRevert(); - mock.add(2 ** 128 - 1, 2 ** 255 - 1); - } -} -``` - -### Recommended approach for tackling codebase migrations - -- Start by fixing your simpler tests so you get the hang of how the cheatcodes are supposed to work. -- Separate tests using both `expectCall`/`expectEmit` and `expectRevert` into two versions: one using `expectCall`/`expectEmit` as the happy path, and one using `expectRevert` as the unhappy path. -- Move your `expectEmit`, `expectCall` and `expectRevert` declarations to exactly before the call you expect to match these. Remember that `expectRevert` cannot be used along with the other `expect` cheatcodes. -- If you need to abstract function calls for `expectCall`, or calls that might revert for `expectRevert`, consider using an intermediate contract that calls these functions to satisfy the depth requirements. \ No newline at end of file +With the V1 release, some breaking changes have been made which might require you to tweak your tests, and some patterns are now discouraged and might emit warnings. This guide will help you understand what these changes are and how to handle them properly. \ No newline at end of file From 168f8d7539c41633ef5f159d9bb2fae44ba658fc Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 29 Jun 2023 00:32:42 -0400 Subject: [PATCH 07/27] chore: several changes and fixes on wording --- src/migrating-to-v1/breaking-changes.md | 8 ++++---- src/migrating-to-v1/deprecated-features.md | 6 +++--- src/migrating-to-v1/upgrading-to-v1.md | 8 +------- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 01d118325..eb6ed9de4 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -2,13 +2,13 @@ When it comes to breaking changes, there has been a slight rework for the assertion (`expect*`) cheatcodes that removes previously allowed unintended or confusing behavior. -The biggest change to expect, is that these cheatcodes will now work only for the next call, not at the "test" level anymore. The philosophy behind this is that by making these cheatcodes more strict through this requirement, tests will be more accurate as the cheatcodes will now succeed under more strict conditions. +The biggest change is that these cheatcodes will now work only for the next call, not at the "test" level anymore. The philosophy behind this is that by making these cheatcodes more strict through this requirement, tests will be more accurate as the cheatcodes will now only succeed under more strict conditions. #### `expectEmit` `expectEmit` previously allowed you to assert that a log was emitted during the execution of the test. With V1, the following changes have been made: -- it now only works for the _next call_. This means if you used to declare all your expected emits at the top of the test, you may now have to move them to just before the next call you perform. Cheatcode calls do not count. As long as the events are emitted during the next call's context, they will be matched. +- it now only works for the _next call_. This means if you used to declare all your expected emits at the top of the test, you may now have to move them to just before the next call you perform. Cheatcode calls are ignored. As long as the events are emitted during the next call's context, they will be matched. - The events have to be _ordered_. That means, if you're trying to match events [A, B, C], you must declare them in this order. - Skipping events is possible. Therefore, if a function emits events [A, B, C, D, E] and you want to match [B, D, E], you just need to declare them in that order. @@ -196,10 +196,10 @@ contract ExpectCallTest is Test { #### `expectRevert` -`expectRevert` currently allows to expect for reverts at the test level. This means that a revert can be declared at the top of the test and as long as a function in the test reverts, the test will be pass. The new behavior is the following: +`expectRevert` currently allows to expect for reverts at the test level. This means that a revert can be declared at the top of the test, and as long as a function in the test reverts, the test will pass. The new behavior is the following: - `expectRevert` CANNOT be used with other `expect` cheatcodes. It will fail if it detects they're being used at the same time. - - The reasoning for this is that calls that revert are rolled back, and events and calls therefore never happen. This restrictions helps model real-world behavior. + - The reasoning for this is that calls that revert are rolled back, and events and calls therefore never happen. This restrictions helps model real execution behavior. - `expectRevert` now works only for the next call. If the next call does not revert, the cheatcode will fail and therefore the test will also fail. The depth of the revert does not matter: as long as the next call reverts, either inmediately or deeper into its call subtree, the cheatcode will succeed. To illustrate, see the following examples: diff --git a/src/migrating-to-v1/deprecated-features.md b/src/migrating-to-v1/deprecated-features.md index a1797db51..5a840bca5 100644 --- a/src/migrating-to-v1/deprecated-features.md +++ b/src/migrating-to-v1/deprecated-features.md @@ -20,7 +20,7 @@ function statefulFuzzTestEq() public { ### Deprecating testFail -Using `testFail` to write failing tests is now discouraged. While it's usage is extremely common, we've decided to remove it because it can introduce footguns in a test. A warning will be thrown if it's used. +Using `testFail` to write failing tests is now discouraged. While it's usage is extremely common, we've decided to deprecate it because it can introduce footguns in a test. The now-recommended pattern is to refactor the actions that will make your test revert and subsequently fail in an utility function, and use `expectRevert` to ensure that this function call failed. This way, the test failure is expected explicitly, instead of introducing the possibility that the test fails in an unintended way silently. @@ -51,7 +51,7 @@ contract TestFailDeprecated is Test { /// with expectRevert() function testReverterReverts() public { vm.expectRevert(); - // Call using this to increase depth + // Call using `this` to increase depth this.exposed_call_Reverter(); } @@ -62,6 +62,6 @@ contract TestFailDeprecated is Test { } ``` -### Removing cheatcode support on some precompiles +### Removing cheatcodes support on some precompiles It is now impossible to use the following cheatcodes on precompiles: `etch`, `load`, `store`, `deal`. \ No newline at end of file diff --git a/src/migrating-to-v1/upgrading-to-v1.md b/src/migrating-to-v1/upgrading-to-v1.md index d492f411c..19488f848 100644 --- a/src/migrating-to-v1/upgrading-to-v1.md +++ b/src/migrating-to-v1/upgrading-to-v1.md @@ -1,10 +1,4 @@ -## Migrating to Foundry V1 - -Foundry V1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting semver. It's the culmination of over a year of work and more than 200 contributors that chimed in to make it happen. - -With the V1 release, some breaking changes have been made which might require you to tweak your tests, and some patterns are now discouraged and might emit warnings. This guide will help you understand what these changes are and how to handle them properly. - -### How do I upgrade to V1? +## Upgrading to Foundry V1 To upgrade, you'll need to reinstall `foundryup`. To do so, open your terminal and run the following command: From beb37ae1c537638cb342855ec2f84cf6e845139c Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Mon, 3 Jul 2023 00:27:53 -0400 Subject: [PATCH 08/27] chore: updates on what to expect --- src/migrating-to-v1/migrating-to-v1.md | 12 +++++++++++- src/migrating-to-v1/upgrading-to-v1.md | 10 +++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/migrating-to-v1/migrating-to-v1.md b/src/migrating-to-v1/migrating-to-v1.md index 6ce3d1c6b..f90f6a15b 100644 --- a/src/migrating-to-v1/migrating-to-v1.md +++ b/src/migrating-to-v1/migrating-to-v1.md @@ -2,4 +2,14 @@ Foundry V1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting semver. It's the culmination of over a year of work and more than 200 contributors that chimed in to make it happen. -With the V1 release, some breaking changes have been made which might require you to tweak your tests, and some patterns are now discouraged and might emit warnings. This guide will help you understand what these changes are and how to handle them properly. \ No newline at end of file +With the V1 release, very few breaking changes have been included, meaning migration should be mostly painless. This guide will help you understand what these changes are and how to handle them properly. + +### What to expect + +V1 mainly brings more strictness to several cheatcodes, which will be the main thing to migrate early on. While we recommend migrating as soon as possible to V1, It's possible to upgrade your local installation and migrate the tests, while maintaining nightly usage on CI to avoid any failures that might disrupt your workflow. + +Aside from this, V1 is still the same Foundry you know. + +### I need support. Where do I go? + +The [dev Telegram](https://t.me/foundry_rs) is available for any concerns you may have that are not covered in this guide. \ No newline at end of file diff --git a/src/migrating-to-v1/upgrading-to-v1.md b/src/migrating-to-v1/upgrading-to-v1.md index 19488f848..dc299a881 100644 --- a/src/migrating-to-v1/upgrading-to-v1.md +++ b/src/migrating-to-v1/upgrading-to-v1.md @@ -1,5 +1,7 @@ ## Upgrading to Foundry V1 +### Local Installation + To upgrade, you'll need to reinstall `foundryup`. To do so, open your terminal and run the following command: ```sh @@ -8,4 +10,10 @@ curl -L https://foundry.paradigm.xyz | bash This will reinstall `foundryup`. You can then run `foundryup` on your terminal to download the latest `V1` version. -After this, you're now up to date with the latest stable Foundry version. \ No newline at end of file +After this, you're now up to date with the latest stable Foundry version. + +### Upgrading your CI + +When it comes to CI, you can add a `version` input field with `v1.0.0` or your desired stable release. This will pin the CI to that stable version. + +If you do not update the CI, it will stay on nightly. \ No newline at end of file From bf177426b7e59ffb989a9014921f1f9ecee85178 Mon Sep 17 00:00:00 2001 From: evalir Date: Tue, 4 Jul 2023 13:26:07 -0400 Subject: [PATCH 09/27] Update src/migrating-to-v1/breaking-changes.md Co-authored-by: Matt Solomon --- src/migrating-to-v1/breaking-changes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index eb6ed9de4..e17d13c40 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -8,7 +8,7 @@ The biggest change is that these cheatcodes will now work only for the next call `expectEmit` previously allowed you to assert that a log was emitted during the execution of the test. With V1, the following changes have been made: -- it now only works for the _next call_. This means if you used to declare all your expected emits at the top of the test, you may now have to move them to just before the next call you perform. Cheatcode calls are ignored. As long as the events are emitted during the next call's context, they will be matched. +- It now only works for the _next call_. This means if you used to declare all your expected emits at the top of the test, you may now have to move them to just before the next call you perform. Cheatcode calls are ignored. As long as the events are emitted during the next call's execution, they will be matched. - The events have to be _ordered_. That means, if you're trying to match events [A, B, C], you must declare them in this order. - Skipping events is possible. Therefore, if a function emits events [A, B, C, D, E] and you want to match [B, D, E], you just need to declare them in that order. From d5a531be0becd0cddeac0e8c94ed4dc43468c4b2 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Tue, 4 Jul 2023 15:21:19 -0400 Subject: [PATCH 10/27] chore: switch up the expectEmit section --- src/migrating-to-v1/breaking-changes.md | 77 +++++++++++++------------ 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index e17d13c40..4c91a24ce 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -18,18 +18,18 @@ To illustrate, see the following examples: contract Emitter { event A(uint256 indexed a); - event B(uint256 indexed b); - event C(uint256 indexed c); - event D(uint256 indexed d); + event B(uint256 b); + event C(uint256 c1, uint256 c2); + event D(bytes32 d1, uint256 f2, address indexed d3); event E(uint256 indexed e); /// emit() emits events [A, B, C, D, E] function emit() external { emit A(1); emit B(2); - emit C(3); - emit D(4); - emit E(1); + emit C(3, 3); + emit D(bytes32("gm"), 4, address(0xc4f3)); + emit E(5); } } @@ -37,76 +37,77 @@ contract EmitTest is Test { Emitter public emitter; event A(uint256 indexed a); - event B(uint256 indexed b); - event C(uint256 indexed c); - event D(uint256 indexed d); + event B(uint256 b); + event C(uint256 c1, uint256 c2); + event D(bytes32 d1, uint256 f2, address indexed d3); event E(uint256 indexed e); function setUp() public { emitter = new Emitter(); } - /// CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, - /// on the test. - /// emit() emits [A, B, C, D, E], and we're expecting [A, B, C, D, E]. - /// this passes. + // CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, + // on the test. + // emit() emits [A, B, C, D, E], and we're expecting [A, B, C, D, E]. + // This passes. function testExpectEmit() public { - cheats.expectEmit(true, false, false, true); + vm.expectEmit(true, false, false, true); emit A(1); - cheats.expectEmit(true, false, false, true); + vm.expectEmit(false, false, false, true); emit B(2); - cheats.expectEmit(true, false, false, true); - emit C(3); - cheats.expectEmit(true, false, false, true); - emit D(4); - cheats.expectEmit(true, false, false, true); + vm.expectEmit(false, false, false, true); + emit C(3, 3); + // It's also possible to skip all parameters entirely and just call the cheatcode. + vm.expectEmit(); + emit D(bytes32("gm"), 4, address(0xc4f3)); + vm.expectEmit(); emit E(5); emitter.emit(); } - /// CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, - /// on the test. - /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. - /// this passes. + // CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, + // on the test. + // emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. + // This passes. function testExpectEmitWindow() public { - cheats.expectEmit(true, false, false, true); + vm.expectEmit(); emit B(2); - cheats.expectEmit(true, false, false, true); - emit D(4); - cheats.expectEmit(true, false, false, true); + vm.expectEmit(); + emit D(bytes32("gm"), 4, address(0xc4f3)); + vm.expectEmit(); emit E(5); emitter.emit(); } /// INCORRECT BEHAVIOR: Declare all your expectEmits in the wrong order. /// emit() emits [A, B, C, D, E], and we're expecting [D, B, E]. - /// this fails, as D is emitted after B, not before. + /// This fails, as D is emitted after B, not before. function testExpectEmitWindowFailure() public { - cheats.expectEmit(true, false, false, true); - emit D(4); - cheats.expectEmit(true, false, false, true); + vm.expectEmit(); + emit D(bytes32("gm"), 4, address(0xc4f3)); + vm.expectEmit(); emit B(2); - cheats.expectEmit(true, false, false, true); + vm.expectEmit(); emit E(5); emitter.emit(); } /// CORRECT BEHAVIOR: Declare all your expectEmits in an internal function. - /// Calling a contract function internally is a JUMPI, not a call, + /// Calling a contract function internally is a JUMP, not a call, /// therefore it's a valid pattern, useful if you have a lot of events to expect. /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. - /// this passes. + /// This passes. function testExpectEmitWithInternalFunction() public { declareExpectedEvents(); emitter.emit(); } function declareExpectedEvents() internal { - cheats.expectEmit(true, false, false, true); + vm.expectEmit(true, false, false, true); emit B(2); - cheats.expectEmit(true, false, false, true); + vm.expectEmit(true, false, false, true); emit D(4); - cheats.expectEmit(true, false, false, true); + vm.expectEmit(true, false, false, true); emit E(5); } } From a2ac7b629206a8125ae022333e55e631ffa60fdc Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Tue, 4 Jul 2023 15:27:30 -0400 Subject: [PATCH 11/27] chore: add table of contents --- src/migrating-to-v1/breaking-changes.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 4c91a24ce..12481acbb 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -1,5 +1,13 @@ ## Breaking changes +### Table of contents +1. [Introduction](#introduction) +2. [`expectEmit`](#expectemit) +3. [`expectCall`](#expectcall) +4. [`expectRevert`](#expectrevert) + +### Introduction + When it comes to breaking changes, there has been a slight rework for the assertion (`expect*`) cheatcodes that removes previously allowed unintended or confusing behavior. The biggest change is that these cheatcodes will now work only for the next call, not at the "test" level anymore. The philosophy behind this is that by making these cheatcodes more strict through this requirement, tests will be more accurate as the cheatcodes will now only succeed under more strict conditions. From 58b8e73aed651e21192ab2453d875ae6c849acf4 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Tue, 4 Jul 2023 15:32:31 -0400 Subject: [PATCH 12/27] chore: add setup for protocol --- src/migrating-to-v1/breaking-changes.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 12481acbb..0d2ce4ba3 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -142,6 +142,10 @@ contract Protocol { contract Caller { Protocol public protocol; + function setUp() public { + protocol = new Protocol(); + } + function foo() public { // do something... } From 34218f67b36ddfa48bd4900b1a0e10c856a1b719 Mon Sep 17 00:00:00 2001 From: evalir Date: Tue, 4 Jul 2023 15:37:21 -0400 Subject: [PATCH 13/27] Update src/migrating-to-v1/breaking-changes.md Co-authored-by: Matt Solomon --- src/migrating-to-v1/breaking-changes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 0d2ce4ba3..c0f61ab47 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -172,7 +172,7 @@ contract ExpectCallTest is Test { /// call `protocol.doSomething()` internally. /// `doSomething()` is nested inside `bar`, so this passes. function testExpectCall() public { - vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething)); + vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething, ())); // This will call bar internally, so this is valid. caller.bar(); } From 0cadc0493794909cbb2d54bf1677b78370291249 Mon Sep 17 00:00:00 2001 From: evalir Date: Tue, 4 Jul 2023 15:46:34 -0400 Subject: [PATCH 14/27] Apply suggestions from code review Co-authored-by: Matt Solomon --- src/migrating-to-v1/breaking-changes.md | 13 ++++++------- src/migrating-to-v1/deprecated-features.md | 18 +++++++++--------- src/migrating-to-v1/migrating-to-v1.md | 6 ++++-- src/migrating-to-v1/non-breaking-changes.md | 2 +- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index c0f61ab47..a12f32862 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -209,11 +209,11 @@ contract ExpectCallTest is Test { #### `expectRevert` -`expectRevert` currently allows to expect for reverts at the test level. This means that a revert can be declared at the top of the test, and as long as a function in the test reverts, the test will pass. The new behavior is the following: +`expectRevert` currently allows to expect for reverts at the test-depth level. This means that a revert can be declared at the top of the test, and as long as a function in the test reverts, the test will pass. The new behavior is the following: - `expectRevert` CANNOT be used with other `expect` cheatcodes. It will fail if it detects they're being used at the same time. - - The reasoning for this is that calls that revert are rolled back, and events and calls therefore never happen. This restrictions helps model real execution behavior. -- `expectRevert` now works only for the next call. If the next call does not revert, the cheatcode will fail and therefore the test will also fail. The depth of the revert does not matter: as long as the next call reverts, either inmediately or deeper into its call subtree, the cheatcode will succeed. + - The reasoning for this is that calls that revert are rolled back, and events and calls therefore never happen. This restriction helps model real execution behavior. +- `expectRevert` now works only for the next call. If the next call does not revert, the cheatcode will fail and therefore the test will also fail. The depth of the revert does not matter: as long as the next call reverts, either immediately or deeper into its call subtree, the cheatcode will succeed. To illustrate, see the following examples: @@ -239,7 +239,7 @@ contract ExpectRevertTest is Test { /// CORRECT BEHAVIOR: We expect `revertWithMessage` to revert, /// so we declare a expectRevert and then call the function. function testExpectRevertString() public { - vm.expectRevert("revert"); + vm.expectRevert(bytes("revert")); reverter.revertWithMessage("revert"); } @@ -248,8 +248,7 @@ contract ExpectRevertTest is Test { /// `doNotRevert()`, which does not revert. The cheatcode therefore fails. function testFailRevertNotOnImmediateNextCall() public { // expectRevert should only work for the next call. However, - // we do not inmediately revert, so, - // we fail. + // we do not immediately revert, so this test fails. vm.expectRevert("revert"); reverter.doNotRevert(); reverter.revertWithMessage("revert"); @@ -292,7 +291,7 @@ contract MathLibTest is Test { /// "dangling" expectRevert. function testRevertOnMathLibWithNoMock() public { vm.expectRevert(); - MathLib.add(2 ** 128 - 1, 2 ** 255 - 1); + MathLib.add(type(uint256).max, 1); } /// CORRECT BEHAVIOR: mock.add will revert due to arithmetic errors, diff --git a/src/migrating-to-v1/deprecated-features.md b/src/migrating-to-v1/deprecated-features.md index 5a840bca5..ecb7c6f51 100644 --- a/src/migrating-to-v1/deprecated-features.md +++ b/src/migrating-to-v1/deprecated-features.md @@ -1,10 +1,10 @@ -## Deprecated features: +## Deprecated features -With foundry V1, we've decided to go ahead and start deprecating features that are considered anti-practices or might be misleading for developers. Deprecated means that while you can continue using these features, it is now a discouraged practice and you should plan on incrementally migrating away from using these. +With foundry V1, we've started deprecating features that are considered anti-patterns or might be misleading and confusing for developers. Deprecated means that while you can continue using these features, it is now a discouraged practice and you should plan on migrating away from using these. ### Removing the Invariant keyword -The invariant keyword has now been deprecated, and the new keyword is now `statefulFuzz`. This means that now writing tests in this manner is valid: +The `invariant` test prefix has now been deprecated, and the new expected prefix `statefulFuzz`. This means that now writing tests in this manner is valid: ```solidity /// Old, deprecated way of declaring an invariant test @@ -14,15 +14,15 @@ function invariantTestEq() public { /// New function statefulFuzzTestEq() public { - // Assert your invariants + // Assert your invariants... } ``` -### Deprecating testFail +### `testFail` -Using `testFail` to write failing tests is now discouraged. While it's usage is extremely common, we've decided to deprecate it because it can introduce footguns in a test. +Using `testFail` to write failing tests is now discouraged, because it can introduce footguns in a test. Specifically, it cannot distinguish between revert reasons, and therefore tests may inadvertently pass and this is hard to detect. -The now-recommended pattern is to refactor the actions that will make your test revert and subsequently fail in an utility function, and use `expectRevert` to ensure that this function call failed. This way, the test failure is expected explicitly, instead of introducing the possibility that the test fails in an unintended way silently. +A better pattern is to use `expectRevert` to ensure that a function call reverted in a specific way. This way, the test failure is expected explicitly, removing the possibility that the test fails in an unintended, undetectable way. An example: @@ -46,10 +46,10 @@ contract TestFailDeprecated is Test { mock.revert_(); } - /// New way, without using testFail. + // Better way, without using testFail. /// The call to revert_ has been refactored and is now expected to fail /// with expectRevert() - function testReverterReverts() public { + function test_RevertIf_AlwaysReverts() public { vm.expectRevert(); // Call using `this` to increase depth this.exposed_call_Reverter(); diff --git a/src/migrating-to-v1/migrating-to-v1.md b/src/migrating-to-v1/migrating-to-v1.md index f90f6a15b..bce6bddb4 100644 --- a/src/migrating-to-v1/migrating-to-v1.md +++ b/src/migrating-to-v1/migrating-to-v1.md @@ -1,12 +1,14 @@ ## Migrating to Foundry V1 -Foundry V1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting semver. It's the culmination of over a year of work and more than 200 contributors that chimed in to make it happen. +Foundry V1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting [semver](https://semver.org/). It's the culmination of over a year of work and more than 200 contributors that made it happen. With the V1 release, very few breaking changes have been included, meaning migration should be mostly painless. This guide will help you understand what these changes are and how to handle them properly. ### What to expect -V1 mainly brings more strictness to several cheatcodes, which will be the main thing to migrate early on. While we recommend migrating as soon as possible to V1, It's possible to upgrade your local installation and migrate the tests, while maintaining nightly usage on CI to avoid any failures that might disrupt your workflow. +V1 mainly increases the strictness of several cheatcodes, and accommodating this is the primary change required to migrate to V1. We recommend migrating to V1 as soon as possible, as the stricter cheats may reveal bugs or issues in your code and test suite. + +If migration is time consuming, you can consider upgrading your local foundry installation while migrating tests, but maintaining nightly usage on CI to avoid any failures that might disrupt your workflow. Aside from this, V1 is still the same Foundry you know. diff --git a/src/migrating-to-v1/non-breaking-changes.md b/src/migrating-to-v1/non-breaking-changes.md index bb93a7aa2..30b689a01 100644 --- a/src/migrating-to-v1/non-breaking-changes.md +++ b/src/migrating-to-v1/non-breaking-changes.md @@ -1,6 +1,6 @@ ## Non-breaking but important changes - Snapshot failures between fuzz runs are now persisted, so workarounds are not needed anymore. -- Sensitive broadcast logs are now logged into the `.gitignore`d `cache` directory, instead of written to logs. +- Sensitive broadcast logs are now logged into the `.gitignore`d `cache` directory, instead of being included in the broadcast files that are intended to be committed. - The base fee calculation on Anvil is now entirely skipped if the fee is set to 0. - `vm.startPrank` now overwrites the existing prank instead of just erroring. However, the already set prank must be used first. \ No newline at end of file From 85cf2cc7d98310173f64159e709071416bb8d299 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Wed, 5 Jul 2023 14:36:54 -0400 Subject: [PATCH 15/27] chore: remove natspec comments, add links to best practice --- src/migrating-to-v1/breaking-changes.md | 70 +++++++++++++------------ 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index a12f32862..32e022e2a 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -87,9 +87,9 @@ contract EmitTest is Test { emitter.emit(); } - /// INCORRECT BEHAVIOR: Declare all your expectEmits in the wrong order. - /// emit() emits [A, B, C, D, E], and we're expecting [D, B, E]. - /// This fails, as D is emitted after B, not before. + // INCORRECT BEHAVIOR: Declare all your expectEmits in the wrong order. + // emit() emits [A, B, C, D, E], and we're expecting [D, B, E]. + // This fails, as D is emitted after B, not before. function testExpectEmitWindowFailure() public { vm.expectEmit(); emit D(bytes32("gm"), 4, address(0xc4f3)); @@ -100,11 +100,11 @@ contract EmitTest is Test { emitter.emit(); } - /// CORRECT BEHAVIOR: Declare all your expectEmits in an internal function. - /// Calling a contract function internally is a JUMP, not a call, - /// therefore it's a valid pattern, useful if you have a lot of events to expect. - /// emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. - /// This passes. + // CORRECT BEHAVIOR: Declare all your expectEmits in an internal function. + // Calling a contract function internally is a JUMP, not a call, + // therefore it's a valid pattern, useful if you have a lot of events to expect. + // emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. + // This passes. function testExpectEmitWithInternalFunction() public { declareExpectedEvents(); emitter.emit(); @@ -167,35 +167,37 @@ contract ExpectCallTest is Test { caller = new Caller(); } - /// CORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, - /// So we declare our expected call, and then call `caller.bar()`, which will - /// call `protocol.doSomething()` internally. - /// `doSomething()` is nested inside `bar`, so this passes. + // CORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, + // So we declare our expected call, and then call `caller.bar()`, which will + // call `protocol.doSomething()` internally. + // `doSomething()` is nested inside `bar`, so this passes. function testExpectCall() public { vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething, ())); // This will call bar internally, so this is valid. caller.bar(); } - /// INCORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, - /// So we declare our expected call, and then call `protocol.doSomething()` directly. - /// `doSomething()` is not nested, but rather called at the test level. - /// This doesn't satisfy the depth requirement described above, - /// so this fails. + // INCORRECT BEHAVIOR: We expect a call to `doSomething()` in the next call, + // So we declare our expected call, and then call `protocol.doSomething()` directly. + // `doSomething()` is not nested, but rather called at the test level. + // This doesn't satisfy the depth requirement described above, + // so this fails. function testExpectCallFailure() public { - vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething)); + vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething, ())); // We're calling doSomething() directly here. // This is not inside the next call's subcall tree, but rather a test-level // call. This fails. caller.protocol.doSomething(); } - /// CORRECT BEHAVIOR: Sometimes, we want to directly expect the call we're gonna - /// call next, but we need to satisfy the depth requirement. In those cases, - /// this pattern can be used. - /// Calling exposed_callFoo using `this` will create a new call context which - /// will increase depth. In its subcall tree, we'll detect `exposed_callFoo`, - /// which will make the test pass. + // CORRECT BEHAVIOR: Sometimes, we want to directly expect the call we're gonna + // call next, but we need to satisfy the depth requirement. In those cases, + // this pattern can be used. + // Calling exposed_callFoo using `this` will create a new call context which + // will increase depth. In its subcall tree, we'll detect `exposed_callFoo`, + // which will make the test pass. + // See more info on the best practices section: + // https://book.getfoundry.sh/tutorials/best-practices#internal-functions function testExpectCallWithNest() public { vm.expectCall(address(caller), abi.encodeWithSelector(caller.foo.selector)); this.exposed_callFoo(); @@ -243,9 +245,9 @@ contract ExpectRevertTest is Test { reverter.revertWithMessage("revert"); } - /// INCORRECT BEHAVIOR: We correctly expect that a function will revert in this - /// test, but we declare the expected revert before the wrong function, - /// `doNotRevert()`, which does not revert. The cheatcode therefore fails. + // INCORRECT BEHAVIOR: We correctly expect that a function will revert in this + // test, but we declare the expected revert before the wrong function, + // `doNotRevert()`, which does not revert. The cheatcode therefore fails. function testFailRevertNotOnImmediateNextCall() public { // expectRevert should only work for the next call. However, // we do not immediately revert, so this test fails. @@ -284,18 +286,18 @@ contract MathLibTest is Test { mock = new MathLibMock(); } - /// INCORRECT BEHAVIOR: MathLib.add will revert due to arithmetic errors, - /// but as the function is marked as internal, it'll be a test level revert - /// instead of a call revert that expectRevert is supposed to catch. - /// The test will fail, and the error will let you know that you have a - /// "dangling" expectRevert. + // INCORRECT BEHAVIOR: MathLib.add will revert due to arithmetic errors, + // but as the function is marked as internal, it'll be a test level revert + // instead of a call revert that expectRevert is supposed to catch. + // The test will fail, and the error will let you know that you have a + // "dangling" expectRevert. function testRevertOnMathLibWithNoMock() public { vm.expectRevert(); MathLib.add(type(uint256).max, 1); } - /// CORRECT BEHAVIOR: mock.add will revert due to arithmetic errors, - /// and it will be successfully detected by the `expectRevert` cheatcode. + // CORRECT BEHAVIOR: mock.add will revert due to arithmetic errors, + // and it will be successfully detected by the `expectRevert` cheatcode. function testRevertOnMathLibWithMock() public { vm.expectRevert(); mock.add(2 ** 128 - 1, 2 ** 255 - 1); From f1a8b2bd11f48c3c112b528ddf583ef3ff4bc303 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 07:55:23 -0400 Subject: [PATCH 16/27] feat: fix several naming issues --- src/migrating-to-v1/breaking-changes.md | 8 ++++---- src/migrating-to-v1/deprecated-features.md | 2 +- src/migrating-to-v1/migrating-to-v1.md | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 32e022e2a..2ba550b64 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -14,7 +14,7 @@ The biggest change is that these cheatcodes will now work only for the next call #### `expectEmit` -`expectEmit` previously allowed you to assert that a log was emitted during the execution of the test. With V1, the following changes have been made: +`expectEmit` previously allowed you to assert that a log was emitted during the execution of the test. With v1, the following changes have been made: - It now only works for the _next call_. This means if you used to declare all your expected emits at the top of the test, you may now have to move them to just before the next call you perform. Cheatcode calls are ignored. As long as the events are emitted during the next call's execution, they will be matched. - The events have to be _ordered_. That means, if you're trying to match events [A, B, C], you must declare them in this order. @@ -266,7 +266,7 @@ See the following example for testing a revert on a library. ```solidity library MathLib { - function add(uint a, uint b) internal pure { + function add(uint a, uint b) internal pure returns (uint256) { return a + b; } } @@ -274,7 +274,7 @@ library MathLib { // intermediate "mock" contract that calls the library function // and returns the result. contract MathLibMock { - function add(uint a, uint b) external { + function add(uint a, uint b) external returns (uint256) { return MathLib.add(a, b); } } @@ -300,7 +300,7 @@ contract MathLibTest is Test { // and it will be successfully detected by the `expectRevert` cheatcode. function testRevertOnMathLibWithMock() public { vm.expectRevert(); - mock.add(2 ** 128 - 1, 2 ** 255 - 1); + mock.add(type(uint256).max, 1); } } ``` \ No newline at end of file diff --git a/src/migrating-to-v1/deprecated-features.md b/src/migrating-to-v1/deprecated-features.md index ecb7c6f51..2c6f343ba 100644 --- a/src/migrating-to-v1/deprecated-features.md +++ b/src/migrating-to-v1/deprecated-features.md @@ -1,6 +1,6 @@ ## Deprecated features -With foundry V1, we've started deprecating features that are considered anti-patterns or might be misleading and confusing for developers. Deprecated means that while you can continue using these features, it is now a discouraged practice and you should plan on migrating away from using these. +With foundry v1, we've started deprecating features that are considered anti-patterns or might be misleading and confusing for developers. Deprecated means that while you can continue using these features, it is now a discouraged practice and you should plan on migrating away from using these. ### Removing the Invariant keyword diff --git a/src/migrating-to-v1/migrating-to-v1.md b/src/migrating-to-v1/migrating-to-v1.md index bce6bddb4..7b9b2b42e 100644 --- a/src/migrating-to-v1/migrating-to-v1.md +++ b/src/migrating-to-v1/migrating-to-v1.md @@ -1,12 +1,12 @@ -## Migrating to Foundry V1 +## Migrating to Foundry v1 -Foundry V1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting [semver](https://semver.org/). It's the culmination of over a year of work and more than 200 contributors that made it happen. +Foundry v1 is the blazing-fast smart contract testing framework you already know, but finally marked as stable and respecting [semver](https://semver.org/). It's the culmination of over a year of work and more than 250 contributors that made it happen. -With the V1 release, very few breaking changes have been included, meaning migration should be mostly painless. This guide will help you understand what these changes are and how to handle them properly. +With the v1 release, very few breaking changes have been included, meaning migration should be mostly painless. This guide will help you understand what these changes are and how to handle them properly. ### What to expect -V1 mainly increases the strictness of several cheatcodes, and accommodating this is the primary change required to migrate to V1. We recommend migrating to V1 as soon as possible, as the stricter cheats may reveal bugs or issues in your code and test suite. +v1 mainly increases the strictness of several cheatcodes, and accommodating this is the primary change required to migrate to v1. We recommend migrating to v1 as soon as possible, as the stricter cheats may reveal bugs or issues in your code and test suite. If migration is time consuming, you can consider upgrading your local foundry installation while migrating tests, but maintaining nightly usage on CI to avoid any failures that might disrupt your workflow. From b2bbe4f20753cfb74b32723dd9f8de64656464e3 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 08:01:32 -0400 Subject: [PATCH 17/27] chore: link to support channel as well --- src/migrating-to-v1/migrating-to-v1.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrating-to-v1/migrating-to-v1.md b/src/migrating-to-v1/migrating-to-v1.md index 7b9b2b42e..c86fcc28e 100644 --- a/src/migrating-to-v1/migrating-to-v1.md +++ b/src/migrating-to-v1/migrating-to-v1.md @@ -14,4 +14,4 @@ Aside from this, V1 is still the same Foundry you know. ### I need support. Where do I go? -The [dev Telegram](https://t.me/foundry_rs) is available for any concerns you may have that are not covered in this guide. \ No newline at end of file +The [dev Telegram](https://t.me/foundry_rs) is available for general conversation around Foundry and its features, while the [support Telegram](https://t.me/foundry_support) is available for any concerns you may have that are not covered in this guide. \ No newline at end of file From 09a4d1fe264e804aea2f17837f81797910115871 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 09:40:13 -0400 Subject: [PATCH 18/27] chore: add precompile restriction rationale --- src/migrating-to-v1/deprecated-features.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrating-to-v1/deprecated-features.md b/src/migrating-to-v1/deprecated-features.md index 2c6f343ba..9a7fb07c3 100644 --- a/src/migrating-to-v1/deprecated-features.md +++ b/src/migrating-to-v1/deprecated-features.md @@ -64,4 +64,4 @@ contract TestFailDeprecated is Test { ### Removing cheatcodes support on some precompiles -It is now impossible to use the following cheatcodes on precompiles: `etch`, `load`, `store`, `deal`. \ No newline at end of file +It is now impossible to use the following cheatcodes on precompiles: `etch`, `load`, `store`, `deal`. The rationale is that utilizing cheatcodes on precompile addresses (0 < n < 9) can cause unexpected behavior and test failures. Prefer using the `makeAddr` cheatcode for creating addresses to use in tests. \ No newline at end of file From e8805681a68cdcfbd109672ebcda698252710894 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 09:48:10 -0400 Subject: [PATCH 19/27] chore: expect arithmetic error --- src/migrating-to-v1/breaking-changes.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 2ba550b64..7e711ca34 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -292,14 +292,14 @@ contract MathLibTest is Test { // The test will fail, and the error will let you know that you have a // "dangling" expectRevert. function testRevertOnMathLibWithNoMock() public { - vm.expectRevert(); + vm.expectRevert(stdError.arithmeticError); MathLib.add(type(uint256).max, 1); } // CORRECT BEHAVIOR: mock.add will revert due to arithmetic errors, // and it will be successfully detected by the `expectRevert` cheatcode. function testRevertOnMathLibWithMock() public { - vm.expectRevert(); + vm.expectRevert(stdError.arithmeticError); mock.add(type(uint256).max, 1); } } From b814ccf460c5f31d58cab6d7d2ee3421602b999e Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 09:57:56 -0400 Subject: [PATCH 20/27] chore: add more examples --- src/migrating-to-v1/breaking-changes.md | 32 +++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 7e711ca34..2077f6dc2 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -258,7 +258,7 @@ contract ExpectRevertTest is Test { } ``` -### Testing internal calls or libraries with `expectCall` and `expectRevert` +### Testing internal calls or libraries with `expectCall`, `expectRevert` and `expectEmit` With the changes to `expectCall` and `expectRevert`, library maintainers might need to modify tests that expect calls or reverts on library functions that might be marked as internal. As an external call needs to be forced to satisfy the depth requirement, an intermediate contract that exposes an identical API to the function that needs to be called can be used. @@ -271,6 +271,13 @@ library MathLib { } } +library EmitLib { + event Log(); + function emit() internal { + emit Log(); + } +} + // intermediate "mock" contract that calls the library function // and returns the result. contract MathLibMock { @@ -279,11 +286,21 @@ contract MathLibMock { } } +contract EmitLibMock { + function log() external { + EmitLib.emit(); + } +} + contract MathLibTest is Test { - MathLibMock public mock; + MathLibMock public mathMock; + EmitLibMock public emitMock; + + event Log(); function setUp() public { - mock = new MathLibMock(); + mathMock = new MathLibMock(); + emitMock = new EmitLibMock(); } // INCORRECT BEHAVIOR: MathLib.add will revert due to arithmetic errors, @@ -300,7 +317,14 @@ contract MathLibTest is Test { // and it will be successfully detected by the `expectRevert` cheatcode. function testRevertOnMathLibWithMock() public { vm.expectRevert(stdError.arithmeticError); - mock.add(type(uint256).max, 1); + mathMock.add(type(uint256).max, 1); + } + + // CORRECT BEHAVIOR: The same pattern applies to `expectRevert`. + function testLibraryEmitWithMock() public { + vm.expectEmit(); + emit Log(); + emitMock.log(); } } ``` \ No newline at end of file From f7c1e1057304b9b98d878e458b5be333bbfbfdeb Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 10:23:00 -0400 Subject: [PATCH 21/27] chore: add invariant naming change rationale --- src/migrating-to-v1/deprecated-features.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/migrating-to-v1/deprecated-features.md b/src/migrating-to-v1/deprecated-features.md index 9a7fb07c3..d550261ad 100644 --- a/src/migrating-to-v1/deprecated-features.md +++ b/src/migrating-to-v1/deprecated-features.md @@ -4,17 +4,17 @@ With foundry v1, we've started deprecating features that are considered anti-pat ### Removing the Invariant keyword -The `invariant` test prefix has now been deprecated, and the new expected prefix `statefulFuzz`. This means that now writing tests in this manner is valid: +The `invariant` test prefix has now been deprecated, and the new expected prefix is `statefulFuzz`. This is mainly to have more correctness on naming: Invariants can be tested with regular fuzz tests. The difference between Foundry's fuzz and invariant tests is that fuzz tests are *stateless* while invariant tests are *stateful*. This means that now writing tests in this manner is valid: ```solidity /// Old, deprecated way of declaring an invariant test function invariantTestEq() public { - // Assert your invariants... + // Assert your invariants } /// New function statefulFuzzTestEq() public { - // Assert your invariants... + // Assert your invariants } ``` @@ -50,7 +50,7 @@ contract TestFailDeprecated is Test { /// The call to revert_ has been refactored and is now expected to fail /// with expectRevert() function test_RevertIf_AlwaysReverts() public { - vm.expectRevert(); + vm.expectRevert("This reverts); // Call using `this` to increase depth this.exposed_call_Reverter(); } From fb27b8c532eee25802a2b61070eeb27443746512 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 10:25:02 -0400 Subject: [PATCH 22/27] chore: add example for expectCall --- src/migrating-to-v1/breaking-changes.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 2077f6dc2..9eb079018 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -320,6 +320,12 @@ contract MathLibTest is Test { mathMock.add(type(uint256).max, 1); } + // CORRECT BEHAVIOR: Use the mock contract to expect a call on a lib + function testCanCallMathLib() public { + vm.expectCall(address(mathMock), abi.encodeWithSelector(mathMock.add.selector, 1, 2)); + mathMock.add(1, 2); + } + // CORRECT BEHAVIOR: The same pattern applies to `expectRevert`. function testLibraryEmitWithMock() public { vm.expectEmit(); From 539c7050e02146c9b7c684d987e9d6204c5565a3 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 10:25:59 -0400 Subject: [PATCH 23/27] feat: add to table of contents --- src/migrating-to-v1/breaking-changes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 9eb079018..28f579217 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -5,6 +5,7 @@ 2. [`expectEmit`](#expectemit) 3. [`expectCall`](#expectcall) 4. [`expectRevert`](#expectrevert) +5. [Testing internal or library calls with the new changes](#testing-internal-calls-or-libraries-with-expectcall-expectrevert-and-expectemit) ### Introduction From 386abddd8bbe258b9d58f84b7dcca7e08d6e1421 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 11:11:45 -0400 Subject: [PATCH 24/27] chore: link to snapshot failure pr --- src/migrating-to-v1/non-breaking-changes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrating-to-v1/non-breaking-changes.md b/src/migrating-to-v1/non-breaking-changes.md index 30b689a01..768308f4e 100644 --- a/src/migrating-to-v1/non-breaking-changes.md +++ b/src/migrating-to-v1/non-breaking-changes.md @@ -1,6 +1,6 @@ ## Non-breaking but important changes -- Snapshot failures between fuzz runs are now persisted, so workarounds are not needed anymore. +- Snapshot failures between fuzz runs [are now persisted](https://github.com/foundry-rs/foundry/pull/4974), so workarounds are not needed anymore when testing fuzz runs that use snapshots. - Sensitive broadcast logs are now logged into the `.gitignore`d `cache` directory, instead of being included in the broadcast files that are intended to be committed. - The base fee calculation on Anvil is now entirely skipped if the fee is set to 0. - `vm.startPrank` now overwrites the existing prank instead of just erroring. However, the already set prank must be used first. \ No newline at end of file From 51ce22b76ef18e80d941f9687ea18167bb2c9339 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 6 Jul 2023 13:01:26 -0400 Subject: [PATCH 25/27] chore: fix some compilation errors --- src/migrating-to-v1/breaking-changes.md | 26 ++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 28f579217..47c258b8d 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -33,7 +33,7 @@ contract Emitter { event E(uint256 indexed e); /// emit() emits events [A, B, C, D, E] - function emit() external { + function emitEvent() external { emit A(1); emit B(2); emit C(3, 3); @@ -57,7 +57,7 @@ contract EmitTest is Test { // CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, // on the test. - // emit() emits [A, B, C, D, E], and we're expecting [A, B, C, D, E]. + // emitEvent() emits [A, B, C, D, E], and we're expecting [A, B, C, D, E]. // This passes. function testExpectEmit() public { vm.expectEmit(true, false, false, true); @@ -71,12 +71,12 @@ contract EmitTest is Test { emit D(bytes32("gm"), 4, address(0xc4f3)); vm.expectEmit(); emit E(5); - emitter.emit(); + emitter.emitEvent(); } // CORRECT BEHAVIOR: Declare all your expectEmits just before the next call, // on the test. - // emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. + // emitEvent() emits [A, B, C, D, E], and we're expecting [B, D, E]. // This passes. function testExpectEmitWindow() public { vm.expectEmit(); @@ -85,11 +85,11 @@ contract EmitTest is Test { emit D(bytes32("gm"), 4, address(0xc4f3)); vm.expectEmit(); emit E(5); - emitter.emit(); + emitter.emitEvent(); } // INCORRECT BEHAVIOR: Declare all your expectEmits in the wrong order. - // emit() emits [A, B, C, D, E], and we're expecting [D, B, E]. + // emitEvent() emits [A, B, C, D, E], and we're expecting [D, B, E]. // This fails, as D is emitted after B, not before. function testExpectEmitWindowFailure() public { vm.expectEmit(); @@ -98,24 +98,24 @@ contract EmitTest is Test { emit B(2); vm.expectEmit(); emit E(5); - emitter.emit(); + emitter.emitEvent(); } // CORRECT BEHAVIOR: Declare all your expectEmits in an internal function. // Calling a contract function internally is a JUMP, not a call, // therefore it's a valid pattern, useful if you have a lot of events to expect. - // emit() emits [A, B, C, D, E], and we're expecting [B, D, E]. + // emitEvent() emits [A, B, C, D, E], and we're expecting [B, D, E]. // This passes. function testExpectEmitWithInternalFunction() public { declareExpectedEvents(); - emitter.emit(); + emitter.emitEvent(); } function declareExpectedEvents() internal { vm.expectEmit(true, false, false, true); emit B(2); vm.expectEmit(true, false, false, true); - emit D(4); + emit D(bytes32("gm"), 4, address(0xc4f3)); vm.expectEmit(true, false, false, true); emit E(5); } @@ -173,7 +173,7 @@ contract ExpectCallTest is Test { // call `protocol.doSomething()` internally. // `doSomething()` is nested inside `bar`, so this passes. function testExpectCall() public { - vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething, ())); + vm.expectCall(caller.protocol.address, abi.encodeCall(Protocol.doSomething, ())); // This will call bar internally, so this is valid. caller.bar(); } @@ -184,11 +184,11 @@ contract ExpectCallTest is Test { // This doesn't satisfy the depth requirement described above, // so this fails. function testExpectCallFailure() public { - vm.expectCall(address(caller.protocol), abi.encodeCall(caller.protocol.doSomething, ())); + vm.expectCall(caller.protocol.address, abi.encodeCall(Protocol.doSomething, ())); // We're calling doSomething() directly here. // This is not inside the next call's subcall tree, but rather a test-level // call. This fails. - caller.protocol.doSomething(); + caller.protocol().doSomething(); } // CORRECT BEHAVIOR: Sometimes, we want to directly expect the call we're gonna From 0622765622b80670ca2b838eff1b368440516b65 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Fri, 7 Jul 2023 17:06:03 -0400 Subject: [PATCH 26/27] chore: push doc changes from lucas --- src/migrating-to-v1/breaking-changes.md | 2 +- src/migrating-to-v1/deprecated-features.md | 8 ++++---- src/migrating-to-v1/migrating-to-v1.md | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index 47c258b8d..f6abe279b 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -279,7 +279,7 @@ library EmitLib { } } -// intermediate "mock" contract that calls the library function +// Intermediate "mock" contract that calls the library function // and returns the result. contract MathLibMock { function add(uint a, uint b) external returns (uint256) { diff --git a/src/migrating-to-v1/deprecated-features.md b/src/migrating-to-v1/deprecated-features.md index d550261ad..ac2bbc64e 100644 --- a/src/migrating-to-v1/deprecated-features.md +++ b/src/migrating-to-v1/deprecated-features.md @@ -1,8 +1,8 @@ ## Deprecated features -With foundry v1, we've started deprecating features that are considered anti-patterns or might be misleading and confusing for developers. Deprecated means that while you can continue using these features, it is now a discouraged practice and you should plan on migrating away from using these. +With Foundry v1, we've started deprecating features that are considered anti-patterns or might be misleading and confusing for developers. Deprecated means that while you can continue using these features, it is now a discouraged practice and you should plan on migrating away from using these. -### Removing the Invariant keyword +### Removing the `invariant` prefix The `invariant` test prefix has now been deprecated, and the new expected prefix is `statefulFuzz`. This is mainly to have more correctness on naming: Invariants can be tested with regular fuzz tests. The difference between Foundry's fuzz and invariant tests is that fuzz tests are *stateless* while invariant tests are *stateful*. This means that now writing tests in this manner is valid: @@ -49,8 +49,8 @@ contract TestFailDeprecated is Test { // Better way, without using testFail. /// The call to revert_ has been refactored and is now expected to fail /// with expectRevert() - function test_RevertIf_AlwaysReverts() public { - vm.expectRevert("This reverts); + function test_revertIf_alwaysReverts() public { + vm.expectRevert("This reverts"); // Call using `this` to increase depth this.exposed_call_Reverter(); } diff --git a/src/migrating-to-v1/migrating-to-v1.md b/src/migrating-to-v1/migrating-to-v1.md index c86fcc28e..2db136b0d 100644 --- a/src/migrating-to-v1/migrating-to-v1.md +++ b/src/migrating-to-v1/migrating-to-v1.md @@ -10,7 +10,7 @@ v1 mainly increases the strictness of several cheatcodes, and accommodating this If migration is time consuming, you can consider upgrading your local foundry installation while migrating tests, but maintaining nightly usage on CI to avoid any failures that might disrupt your workflow. -Aside from this, V1 is still the same Foundry you know. +Aside from this, v1 is still the same Foundry you know. ### I need support. Where do I go? From 9f67b28d479691c83c76fd0832173d196dcfb109 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Fri, 7 Jul 2023 17:20:51 -0400 Subject: [PATCH 27/27] chore: add expectemit refresher --- src/migrating-to-v1/breaking-changes.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/migrating-to-v1/breaking-changes.md b/src/migrating-to-v1/breaking-changes.md index f6abe279b..392d669aa 100644 --- a/src/migrating-to-v1/breaking-changes.md +++ b/src/migrating-to-v1/breaking-changes.md @@ -60,6 +60,11 @@ contract EmitTest is Test { // emitEvent() emits [A, B, C, D, E], and we're expecting [A, B, C, D, E]. // This passes. function testExpectEmit() public { + // A quick refresher on `vm.expectEmit` params: + // The first three booleans indicate if you want to check `topic1`, `topic2` and `topic3`. + // `topic0` is always checked. + // The last boolean indicates if you want to check the data. + // If you want to read further, you can go to the `vm.expectEmit` reference: https://book.getfoundry.sh/cheatcodes/expect-emit?highlight=expectEmit#expectemit vm.expectEmit(true, false, false, true); emit A(1); vm.expectEmit(false, false, false, true);