Skip to content

Update default cli config#323

Merged
manav2401 merged 6 commits into
v0.3.0-devfrom
default-cli-config
Apr 26, 2022
Merged

Update default cli config#323
manav2401 merged 6 commits into
v0.3.0-devfrom
default-cli-config

Conversation

@manav2401

@manav2401 manav2401 commented Feb 10, 2022

Copy link
Copy Markdown
Member

This PR does following things

  • Updates some default config parameters for the new cli.The parameters updated are
    • HTTP flag
    • HTTP Modules
    • TxPool.GlobalSlots
    • TxPool.GlobalQueue
  • Fixes a bug which arises while setting maxpeers flag to 0.
  • Adds a new no-snapshot flag, which will be used to disable the snapshot database mode.
  • Fixes a bug while checking snapshot enabling. It was earlier checking in a wrong way as compared to old cli. The old cli condition can be found here.

@manav2401 manav2401 changed the base branch from master to v0.3.0-dev February 10, 2022 05:45

@vcastellm vcastellm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@ferranbt

Copy link
Copy Markdown
Contributor

Is Http enabled by default?

@manav2401

Copy link
Copy Markdown
Member Author

@ferranbt No it wasn't, but last week we enabled it, thinking that we can always disable by passing the --http false flag. But, that didn't work, so just reverting that change for now, making it disabled by default.
Also, I'll get back with the test as this thing of not overriding the defaults might be something to look at.

@ferranbt

Copy link
Copy Markdown
Contributor

So, what is the end goal? Http enabled or not?

@0xsharma 0xsharma self-requested a review March 24, 2022 12:32
Comment thread internal/cli/server/config.go Outdated
@manav2401

Copy link
Copy Markdown
Member Author

Have updated the default flags with newly decided values.
An open question: If txpool.PriceLimit is 30 gwei, should sealer.GasPrice be increased from 1 gewi to 30 gewi?

@manav2401 manav2401 requested a review from 0xsharma March 25, 2022 08:40

@0xsharma 0xsharma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM !

Comment thread internal/cli/server/config.go Outdated
Comment thread internal/cli/server/config.go
@manav2401 manav2401 requested review from 0xsharma and ferranbt April 6, 2022 13:48
Comment thread internal/cli/server/config.go
@manav2401 manav2401 merged commit 657b38b into v0.3.0-dev Apr 26, 2022
@manav2401 manav2401 deleted the default-cli-config branch April 26, 2022 08:21
vcastellm pushed a commit that referenced this pull request May 9, 2022
* update the default cli config

* fix: handle maxpeers flag override, added tests

* add: no-snapshot flag, handle snapshot disable

* fix: update default flags

* add: update the sealer.GasPrice default to 30 gewi

* fix: remove snapshot flag, rely on no-snapshot only
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.

6 participants