Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Merged by Bors] - Replace ganache-cli with anvil #3555

Closed
wants to merge 20 commits into from
Closed

[Merged by Bors] - Replace ganache-cli with anvil #3555

wants to merge 20 commits into from

Conversation

pawanjay176
Copy link
Member

@pawanjay176 pawanjay176 commented Sep 8, 2022 •
edited
Loading

Issue Addressed

N/A

Proposed Changes

Replace ganache-cli with anvil https://github.com/foundry-rs/foundry/blob/master/anvil/README.md
We can lose all js dependencies in CI as a consequence.

Additional info

Also changes the ethers-rs version used in the execution layer (for the transaction reconstruction) to a newer one. This was necessary to get use the ethers utils for anvil. The fixed execution engine integration tests should catch any potential issues with the payload reconstruction after #3592

@michaelsproul
Copy link
Member

Just came across this again and I'm very keen to adopt it because it seems like it would allow us to remove the dependency on the web3 crate in favour of ethers. The web3 crate is the source of many outdated and unmaintained deps in our Cargo.lock.

@pawanjay176
Copy link
Member Author

@michaelsproul Fixed this up. I had to use the downgraded versions of ethers in the eth1_test_rig to satisfy msrv requirements.
Using cargo install for anvil in CI but we could also potentially look into https://github.com/foundry-rs/foundry-toolchain to speed up anvil installation a bit.

@pawanjay176 pawanjay176 added the ready-for-review The code is ready for review label Feb 13, 2023
@michaelsproul
Copy link
Member

michaelsproul commented Feb 13, 2023 •
edited
Loading

@pawanjay176 we're about to bump our MSRV to 1.65 for Capella anyway, so I think we may as well bump it in this PR and stick to one version of ethers?

I love seeing the Cargo.lock with no web3 crate 😍. Will be awesome if we can make it a net reduction in number of deps by consolidating the ethers version too!

@pawanjay176
Copy link
Member Author

pawanjay176 commented Feb 14, 2023 •
edited
Loading

Bumped up the ethers version. As discussed, we could merge it when we merge capella where msrv is already 1.65.
Blocked until then.

@pawanjay176
Copy link
Member Author

Unblocking this guy. cc @michaelsproul

michaelsproul
michaelsproul reviewed Mar 28, 2023
View reviewed changes
Cargo.lock Outdated Show resolved Hide resolved
@michaelsproul
Copy link
Member

Sorry I pushed another commit to mess with arbitrary, try merging unstable again and then we can merge 🙏

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. v4.1.0 Post-Capella minor release and removed ready-for-review The code is ready for review labels Mar 29, 2023
@pawanjay176
Copy link
Member Author

Merged unstable @michaelsproul

@pawanjay176 pawanjay176 added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Mar 29, 2023
michaelsproul
michaelsproul approved these changes Mar 29, 2023
View reviewed changes
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Approved modulo one small error fix

testing/eth1_test_rig/src/lib.rs Outdated Show resolved Hide resolved
@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Mar 29, 2023
@paulhauner paulhauner added v4.2.0 Q2 2023 and removed v4.1.0 Post-Capella minor release labels Apr 13, 2023
@paulhauner paulhauner added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 15, 2023
@paulhauner
Copy link
Member

I flagged this as ready-for-review since that seems appropriate given the last few comments :)

michaelsproul
michaelsproul reviewed May 15, 2023
View reviewed changes
Cargo.lock
@@ -9718,13 +9768,12 @@ dependencies = [

[[package]]
name = "x25519-dalek"
version = "2.0.0-rc.2"
version = "2.0.0-pre.1"
Copy link
Member

Choose a reason for hiding this comment

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

this looks really weird, but it's because of snow:

https://github.com/mcginty/snow/releases/tag/v0.9.1

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels May 15, 2023
@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 15, 2023
@pawanjay176 @michaelsproul
Replace ganache-cli with anvil (#3555)
8a3eb4d 
## Issue Addressed

N/A

## Proposed Changes

Replace ganache-cli with anvil https://github.com/foundry-rs/foundry/blob/master/anvil/README.md
We can lose all js dependencies in CI as a consequence.

## Additional info
Also changes the ethers-rs version used in the execution layer (for the transaction reconstruction) to a newer one. This was necessary to get use the ethers utils for anvil. The fixed execution engine integration tests should catch any potential issues with the payload reconstruction after #3592 


Co-authored-by: Michael Sproul <michael@sigmaprime.io>
@bors
Copy link

bors bot commented May 15, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Replace ganache-cli with anvil [Merged by Bors] - Replace ganache-cli with anvil May 15, 2023
@bors bors bot closed this May 15, 2023
@AgeManning AgeManning mentioned this pull request May 18, 2023
Closed
ghost pushed a commit to oone-world/lighthouse that referenced this pull request Jul 13, 2023
@pawanjay176 @michaelsproul
Replace ganache-cli with anvil (sigp#3555)
b939894 
## Issue Addressed

N/A

## Proposed Changes

Replace ganache-cli with anvil https://github.com/foundry-rs/foundry/blob/master/anvil/README.md
We can lose all js dependencies in CI as a consequence.

## Additional info
Also changes the ethers-rs version used in the execution layer (for the transaction reconstruction) to a newer one. This was necessary to get use the ethers utils for anvil. The fixed execution engine integration tests should catch any potential issues with the payload reconstruction after sigp#3592 


Co-authored-by: Michael Sproul <michael@sigmaprime.io>
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
@pawanjay176 @michaelsproul @Woodpile37
Replace ganache-cli with anvil (sigp#3555)
cf9d780 
N/A

Replace ganache-cli with anvil https://github.com/foundry-rs/foundry/blob/master/anvil/README.md
We can lose all js dependencies in CI as a consequence.

Also changes the ethers-rs version used in the execution layer (for the transaction reconstruction) to a newer one. This was necessary to get use the ethers utils for anvil. The fixed execution engine integration tests should catch any potential issues with the payload reconstruction after sigp#3592

Co-authored-by: Michael Sproul <michael@sigmaprime.io>
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
@pawanjay176 @michaelsproul @Woodpile37
Replace ganache-cli with anvil (sigp#3555)
d89d16e 
N/A

Replace ganache-cli with anvil https://github.com/foundry-rs/foundry/blob/master/anvil/README.md
We can lose all js dependencies in CI as a consequence.

Also changes the ethers-rs version used in the execution layer (for the transaction reconstruction) to a newer one. This was necessary to get use the ethers utils for anvil. The fixed execution engine integration tests should catch any potential issues with the payload reconstruction after sigp#3592

Co-authored-by: Michael Sproul <michael@sigmaprime.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelsproul michaelsproul michaelsproul approved these changes

Assignees
No one assigned
Labels
ready-for-merge This PR is ready to merge. v4.2.0 Q2 2023
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pawanjay176 @michaelsproul @paulhauner