-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(forge): support multiple forks #1715
Conversation
Copying this over from Discord. Basically local storage, memory and stack should be persistent and external calls use latest state of a particular defined fork. Here is my intuition for how this would work:
For a more concrete implementation we are currently simulating xchain integration tests like this: https://github.com/makerdao/xdomain/blob/ed0435803597ccb4a2328713c0ef8fc0093b2093/packages/dss-bridge/src/tests/Integration.t.sol#L129
We would like to be able to swap between forks like in the example above and call into |
Per @hexonaut's feedback / user testing I vote we get this merged and iterate on any bugs discovered, wdyt? |
function selectFork(uint256) external; | ||
// Updates the currently active fork to given block number | ||
// This is similar to `roll` but for the fork | ||
function rollFork(uint256) external; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a smol question, how does this work if you do not use any of the fork cheatcodes, but just use the CLI args? Still as if you had used one of the cheatcodes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exactly, initializing with CLI args will be the same as creating and selecting during setup
// Creates a new fork with the given endpoint and the latest block and returns the identifier of the fork | ||
function createFork(string calldata) external returns(uint256); | ||
// takes a fork identifier created by `createFork` and changes the state | ||
function selectFork(uint256) external; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another question here actually: if you pass a fork URL using the CLI, then do createFork
etc., how would you revert back to the initial fork passed via CLI? Is it possible? If not, should we discourage passing --rpc-url
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we launch in fork mode (via CLI) this fork will always have id 0
, I added another cheatcode activeFork()(uint256)
that returns the currently active fork, so the id of the CLI fork can be retrieved as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small questions but LGTM and I think we should merge and iterate. No need to make any changes to the PR to address the questions I posed, just important to keep in mind re: documentation and so on
+1 on that, writing docs for the book now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - some nits/qs, beyond that feel free to merge whenever.
// sending and receiving data from the remote client(s) | ||
let _ = std::thread::Builder::new() | ||
.name("multi-fork-backend-thread".to_string()) | ||
.spawn(move || { | ||
let rt = tokio::runtime::Builder::new_current_thread() | ||
.enable_all() | ||
.build() | ||
.expect("failed to create multi-fork-backend-thread tokio runtime"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the difference between this and a tokio::spawn
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the runs handler in a standalone runtime on a separate thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya i guess what's the benefit? spawning a tokio future in a multithreaded runtime vs spawning a thread and launching a new runtime (is it just better? cuz you launch a new multithreaded runtime in a new thread?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a dedicated thread in which the runtime's driving the single future. this rt doesn't spawn any threads but uses the current thread it's in.
tokio::spawn
requires an existing runtime and we don't actually have any other async stuff when tests are executed.
// modify state | ||
bytes32 value = bytes32(uint(1)); | ||
// "0x3617319a054d772f909f7c479a2cebe5066e836a939412e32403c99029b92eff" is the slot storing the balance of zero address for the weth contract | ||
// `cast index address uint 0x0000000000000000000000000000000000000000 3` | ||
bytes32 zero_address_balance_slot = 0x3617319a054d772f909f7c479a2cebe5066e836a939412e32403c99029b92eff; | ||
cheats.store(WETH_TOKEN_ADDR, zero_address_balance_slot, value); | ||
assertEq(WETH.balanceOf(0x0000000000000000000000000000000000000000), 1, "Cheatcode did not change value at the storage slot."); | ||
|
||
// switch forks and ensure local modified state is persistent | ||
cheats.selectFork(forkB); | ||
assertEq(WETH.balanceOf(0x0000000000000000000000000000000000000000), 1, "Cheatcode did not change value at the storage slot."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confused a bit here, why is the WETH
balance the same in both forks? shouldn't it be 1
in just forkA
and still be neq 1 in forkB
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
writes are persistent across fork swaps, only the remote half is swapped.
I tried to explain this here https://github.com/foundry-rs/book/pull/442/files#diff-ef0012936dece3a24d02ea8d8e27bf208801f8be2e0689cb74a053c36351c05aR35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @hexonaut
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any chance of changing the behaviour here? would be great if it's possible to have separate states for writes as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at this currently, should be possible.
lfg |
Motivation
support multiple forks during tests
Ref #1649 #834 #1162 #748 #939
Closes #834
Closes #1162
Solution
This introduces new cheatcodes:
foundry/testdata/cheats/Cheats.sol
Lines 106 to 118 in 9088be5
forkBlockVariable
basically addresses this #748atm it's expected that
fork
related cheat codes are called in setup and snapshots from within the test.another idea would be to add support for storing multiple RPC endpoints along with their aliases so that RPC endpoints can be used by an alias. Probably best to add this in the configuration