-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: add a --dev
option
#3866
feat: add a --dev
option
#3866
Conversation
9990ada
to
0a86659
Compare
Will take a look, currently having some issues with CI, so it will take a minute. Sorry for the delayed review |
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.
supportive, smol nit re docs.
otherwise this looks pretty good
block_max_transactions: Option<usize>, | ||
block_time: Option<Duration>, |
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.
these should be mutually exclusive, but I can see how this would be more convenient coming from CLI
so at the very least, this needs to be included in the docs
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.
I was not sure how to handle this. Throw if both are Some? Merge them to one enum parameter including both variants?
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.
also lgtm w mattse's review - please open up an issue about including this in the book (new flags, we should also document what accounts are funded)
a2eb34f
to
9072ab4
Compare
Codecov Report
... and 43 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -115,13 +115,13 @@ where | |||
pool: Pool, | |||
to_engine: UnboundedSender<BeaconEngineMessage>, | |||
canon_state_notification: CanonStateNotificationSender, | |||
mode: MiningMode, |
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.
this is the best solution imo, even if it makes calling it slightly more verbose
Implements features described in #3699, adds the following cli arguments:
This configuration enables developers to experiment with Reth's source code or develop new applications without having to sync to a pre-existing public network.