Skip to content

Conversation

@gustavo-grieco
Copy link
Contributor

No description provided.

@gustavo-grieco gustavo-grieco requested a review from 0xPhaze March 24, 2023 14:00
Compound comp = Compound(0x4Ddc2D193948926D02f9B1fE9e1daa0718270ED5);
constructor() {
hevm.roll(16771449); // sets the correct block number
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that by writing hevm.roll(16771449);, you will fork the blockchain state from block 16771449? This behavior would then be different to the way foundry works. There an initial block number parameter is passed in and if you skip to a new block, it's all just simulated locally, but your state is still loaded from the initially set block height.

This difference is not necessarily bad, but it could break some tests that rely on relative time/block jumps, i.e. the contract requires 10 blocks to have passed and you write hevm.roll(block.number + 10), but now the actual state has changed and your assumptions on the current state might be different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Echidna only sets a particular state as the initial one, but after that, everything else can be changed. I will clarify this in the article.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a note about this.


## Example

One of the most anticipated features of Echidna 2.1.0 is the state network forking. This means that Echidna can run starting with an existing blockchain state provided by an external RPC service (Infura, Alchemy, local node, etc).
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should reword this, and remove the "most anticipated features" part

Copy link
Contributor

Choose a reason for hiding this comment

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

We should also have the introduction in an introduction section, and not in the example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reworked this as introduction and example, please take a look

```

While the source code for the [cETH contract is available](https://etherscan.io/address/0x4ddc2d193948926d02f9b1fe9e1daa0718270ed5#code), their source maps are NOT.
In order to generate the coverage report for a fetched contract, **both** source code and source mapping should be available. In that case, there will be a new directory inside the corpus directory to show coverage for each contract that was fetched. In any case, the coverage report will be always available for the user-provided contracts, such as this one:
Copy link
Contributor

Choose a reason for hiding this comment

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

mhh, not related to this article, but why not relying on crytic-compile for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, crytic-compile cannot produce the same bytecode as the one deployed, so it is unclear if that will be useful, but @arcz has the detail.

@gustavo-grieco
Copy link
Contributor Author

This PR is ready for another round of review or to merge it. As usual, the failed test are unrelated link. @montyly

@montyly montyly added this pull request to the merge queue Jul 11, 2023
Merged via the queue into master with commit 0aa2581 Jul 11, 2023
@montyly montyly deleted the dev-state-network-forking branch July 11, 2023 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants