Skip to content
This repository was archived by the owner on Oct 25, 2024. It is now read-only.

[trivial] CLI fixes #92

Merged
Wazzymandias merged 1 commit into from
Aug 1, 2023
Merged

[trivial] CLI fixes #92

Wazzymandias merged 1 commit into from
Aug 1, 2023

Conversation

@Wazzymandias
Copy link
Contributor

@Wazzymandias Wazzymandias commented Aug 1, 2023 •
edited
Loading

📝 Summary

  • When miner.blocklist was not specified, it still tried to load empty file with the CLI flag, because Value was set to ""
  • When builder.algotype was not specified, it still tried to load default value of mev-geth, overriding miner.algotype if specified
  • SetBuilderConfig had a bug - cfg.Enabled was being set to whether or not BuilderEnabled was set. This was leading to incorrect behavior when --builder=false was set
  • Fix BuilderBlockValidationBlacklistSourceFilePath referencing wrong flag for file load
  • We weren't checking error when setting extra data for miner
  • This PR fixes above issues

📚 References

@Wazzymandias Wazzymandias requested review from avalonche and dvush August 1, 2023 01:30
@Wazzymandias Wazzymandias marked this pull request as ready for review August 1, 2023 01:41
@Wazzymandias Wazzymandias requested a review from sukoneck August 1, 2023 01:50
@Wazzymandias Wazzymandias merged commit 5c5ee0f into main Aug 1, 2023
@Wazzymandias Wazzymandias deleted the quickfix-cli branch August 1, 2023 02:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dvush dvush Awaiting requested review from dvush dvush is a code owner

@sukoneck sukoneck Awaiting requested review from sukoneck

1 more reviewer

@avalonche avalonche avalonche approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Wazzymandias @avalonche