-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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: support forking at any transaction #3262
Conversation
@CodeForcer what is the expected behavior here? inclusive or not? |
4cca644
to
1970493
Compare
let block = fork | ||
.db | ||
.db | ||
.get_full_block(BlockNumber::Latest)? | ||
.number |
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.
can you not do eth_blockNumber
here instead of asking for the full block?
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.
can do that by adding another request variant for eth_blockNumber
let fork = self.inner.get_fork_by_id_mut(id)?; | ||
let full_block = | ||
fork.db.db.get_full_block(BlockNumber::Number(env.block.number.as_u64().into()))?; | ||
|
||
for tx in full_block.transactions.into_iter() { | ||
if tx.hash().eq(&tx_hash) { | ||
// found the target transaction | ||
return Ok(Some(tx)) | ||
} |
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.
can we cache this somehow to disk? so that forking a txhash does not replay all txs each time, and can maybe save the pre-tx hash state (or including it)?
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.
would require adding another mapping to how forks are tracked rn, basically (endpoint, block, tx index).
would be possible but requires a bit of refactoring effort, I'd leave this for a follow up for perf
Merging and we can iterate with @CodeForcer feedback |
* feat: add get full block request * feat: add transaction request * feat: integrate cheatcodes * feat: support forking at any transaction
Thanks team! |
does anvil also support this? |
Nope I believe Anvil only forks by block height, but feel free to open an issue to potentially support it! |
Motivation
Ref #2191
this adds additional cheatcodes for
create(select)Fork
androllFork
that accept transaction hashes instead of blocks.this will fork off the block the transaction was mined in and replays all prior transaction in the block, not including the transaction. Perhaps this should be inclusive?
needs some tests
Solution