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.

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: Optimism builder & miner #534

Merged
merged 26 commits into from
Aug 9, 2024
Merged

Conversation

Wodann
Copy link
Member

@Wodann Wodann commented Jun 28, 2024

No description provided.

@Wodann Wodann self-assigned this Jun 28, 2024
Copy link

changeset-bot bot commented Jun 28, 2024

🦋 Changeset detected

Latest commit: 3132383

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@nomicfoundation/edr Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Wodann Wodann changed the base branch from feat/multichain to refactor/multichain-blockchain June 28, 2024 23:12
@Wodann Wodann changed the base branch from refactor/multichain-blockchain to feat/multichain June 28, 2024 23:13
@Wodann Wodann force-pushed the refactor/optimism-builder branch from 846bd81 to 4d73b60 Compare June 28, 2024 23:15
github-actions bot and others added 7 commits July 1, 2024 18:12
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix: allow missing nonce in remote blocks

* misc: add changeset
* chore: Explicitly depend on semver and fs-extra in hardhat-tests

Appeases ESLint. These are de facto pulled by other dependencies but
it's implicit.

* refactor: Unify ts-node and use the one currently used by Hardhat

Deduping the hardhat package makes it easier to patch it while working
on the stack trace porting feature branch.

* chore: Use the newest Hardhat 2.22.6

This will make patching the changes easier to review as we will sync
with the upstream as it has some changes already related to the stack
traces that we port.
@Wodann Wodann force-pushed the refactor/optimism-builder branch from 5576b41 to 6bebd72 Compare July 8, 2024 22:57
fvictorio and others added 4 commits July 9, 2024 15:57
* feat: upgrade revm dependencies

* Create wild-phones-drum.md
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@Wodann Wodann had a problem deploying to github-action-benchmark July 11, 2024 19:36 — with GitHub Actions Failure
fvictorio and others added 3 commits July 16, 2024 10:10
* feat: add support for RIP-7212

* misc: add changelog

* fix: updated index.d.ts

* fix: revert rename of InvalidFEOpcode

* fix: set enableRip7212 in ProviderConfig

* test: validate that disabling RIP-7212 works

* refactor: use runtime variable instead of const

* Path Hardhat dev dep to work with latest EDR changes

EDR uses Hardhat as a dev dependency to run some javascript tests. The
way this works is that the tests are run using Hardhat, but we use pnpm
to override the EDR dependency in Hardhat with the local one. This works
fine as long as there are no breaking changes in EDR. When there are, we
have a circular dependency problem: we can't publish a new version of
EDR until the tests pass, but for the tests to pass we need a version of
Hardhat that works with the new version of EDR.

A temporary workaround for this is to use `pnpm patch` to temporarily
modify the Hardhat code in a way that works with the breaking change.
In this case, this just means adding the new field when constructing
the provider.

---------

Co-authored-by: Franco Victorio <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@Wodann Wodann linked an issue Jul 17, 2024 that may be closed by this pull request
* build: upgrade revm to v12

* test: remove invalid test

* misc: add changeset
@Wodann Wodann had a problem deploying to github-action-benchmark July 23, 2024 17:23 — with GitHub Actions Error
@Wodann Wodann had a problem deploying to github-action-benchmark July 23, 2024 17:37 — with GitHub Actions Error
@Wodann Wodann had a problem deploying to github-action-benchmark July 23, 2024 17:42 — with GitHub Actions Error
@Wodann Wodann had a problem deploying to github-action-benchmark July 23, 2024 18:02 — with GitHub Actions Error
@Wodann Wodann had a problem deploying to github-action-benchmark July 23, 2024 18:07 — with GitHub Actions Error
@Wodann Wodann temporarily deployed to github-action-benchmark July 23, 2024 18:19 — with GitHub Actions Inactive
@Wodann Wodann temporarily deployed to github-action-benchmark July 24, 2024 17:21 — with GitHub Actions Inactive
@Wodann Wodann temporarily deployed to github-action-benchmark July 25, 2024 23:50 — with GitHub Actions Inactive
@Wodann Wodann had a problem deploying to github-action-benchmark July 31, 2024 18:40 — with GitHub Actions Error
@Wodann Wodann had a problem deploying to github-action-benchmark July 31, 2024 18:48 — with GitHub Actions Error
@Wodann Wodann had a problem deploying to github-action-benchmark July 31, 2024 19:06 — with GitHub Actions Error
@Wodann Wodann force-pushed the refactor/optimism-builder branch from efe4ee8 to e496ce4 Compare July 31, 2024 19:07
@Wodann Wodann temporarily deployed to github-action-benchmark July 31, 2024 19:08 — with GitHub Actions Inactive
@Wodann Wodann marked this pull request as ready for review July 31, 2024 21:16
@fvictorio
Copy link
Member

This PR seems to have unrelated changes. Maybe you accidentally merged main into the refactor/optimism-builder branch?

@Wodann
Copy link
Member Author

Wodann commented Aug 8, 2024

This PR seems to have unrelated changes. Maybe you accidentally merged main into the refactor/optimism-builder branch?

That's correct.

I considered doing a separate PR to merge main into the feature branch, but it's quite complicated as main upgraded to a new version of REVM, whereas my feature branch is using a fork. I upgraded to the latest REVM in my fork as well, but as a result the merge to main is intermingled with the changes of this PR and not easily separable.

If you'd like I can still do the effort to do a separate PR that merges main into the HEAD of the feature branch while incorporating the upgraded fork.

@fvictorio
Copy link
Member

I'm not sure. If that's too much work, it might not be worth it. But this PR as it is is really hard to review.

Perhaps you can list the commits or commit ranges that need reviewing?

In any case, all the changes in areas that I usually review seem to come from main, so I'll leave it up to @agostbiro to decide how he prefers to review this.

Copy link
Member

@agostbiro agostbiro left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @Wodann, it's awesome work!

Re reviewing: it's too much to go over line by line, but after @Wodann's walkthrough I did a scan + played around with it a bit locally and this + knowing @Wodann's work gives me high confidence to approve.

Comment on lines 9 to 16
// > execution aborted (timeout = 10s)
//
// Potentially the block is too old?
// mainnet_pre_bedrock => OptimismChainSpec {
// block_number: 98_235_064,
// chain_id: 10,
// url: get_alchemy_url().replace("eth-", "opt-"),
// },
Copy link
Member

Choose a reason for hiding this comment

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

Should this be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed by cbdb02b

@@ -0,0 +1,32 @@
[package]
name = "edr_opt"
Copy link
Member

Choose a reason for hiding this comment

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

It took me a moment to parse the name, I'd prefer edr_optimism if possible

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed by 597c4b8

Comment on lines 63 to 71
/// Type representing an error that occurs when converting an RPC block.
type RpcBlockConversionError: Debug + std::error::Error;

impl<ChainSpecT> SyncChainSpec for ChainSpecT where
ChainSpecT: ChainSpec<Transaction: Send + Sync> + Send + Sync + 'static
{
}
/// Type representing an error that occurs when converting an RPC receipt.
type RpcReceiptConversionError: Debug + std::error::Error;

/// Type representing an error that occurs when converting an RPC
/// transaction.
type RpcTransactionConversionError: Debug + std::error::Error;
Copy link
Member

Choose a reason for hiding this comment

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

std::error::Error already requires Debug, so I think the explicit Debug bound could be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed by cbdb02b

@Wodann Wodann had a problem deploying to github-action-benchmark August 9, 2024 15:06 — with GitHub Actions Error
@Wodann Wodann had a problem deploying to github-action-benchmark August 9, 2024 15:11 — with GitHub Actions Error
@Wodann Wodann temporarily deployed to github-action-benchmark August 9, 2024 15:26 — with GitHub Actions Inactive
@Wodann Wodann merged commit cd9ebb9 into feat/multichain Aug 9, 2024
36 checks passed
@Wodann Wodann deleted the refactor/optimism-builder branch August 9, 2024 21:57
@Wodann Wodann restored the refactor/optimism-builder branch August 9, 2024 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants