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

docs: document the config file #194

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

vcastellm
Copy link
Contributor

@vcastellm vcastellm commented Nov 19, 2024

Add documentation to config file

  • Text is copied from the original Go comment in config.go when existing

@Stefan-Ethernal Stefan-Ethernal changed the title doc: document part of the config file docs: document part of the config file Nov 19, 2024
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 suggestion.

Files not reviewed (1)
  • crates/cdk/src/main.rs: Evaluated as low risk
Comments skipped due to low confidence (1)

config/default.go:3

  • The comment contains spelling errors. 'This' should be 'These' and 'doesnt' should be 'doesn't'.
// This values doesnt have a default value because depend on the environment / deployment

config/default.go Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Nov 21, 2024

@vcastellm vcastellm marked this pull request as ready for review November 21, 2024 15:06
@vcastellm vcastellm changed the title docs: document part of the config file docs: document the config file Nov 21, 2024
config/default.go Outdated Show resolved Hide resolved
Copy link
Contributor

@rachit77 rachit77 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@Stefan-Ethernal Stefan-Ethernal left a comment

Choose a reason for hiding this comment

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

Generally LGTM

config/default.go Outdated Show resolved Hide resolved
config/default.go Outdated Show resolved Hide resolved
config/default.go Outdated Show resolved Hide resolved
URL = "{{L1URL}}"

# ----------------------------------------------------------------------------
# allow that L1 gas price calculation use multiple sources
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# allow that L1 gas price calculation use multiple sources
# MultiGasProvider allows that L1 gas price calculation use multiple sources

NumRequests = 1000
Interval = "1s"
# ------------------------------------------------------------------------------
# GRPC server host
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# GRPC server host
# Host denotes GRPC server IP address

Host = "0.0.0.0"

# ------------------------------------------------------------------------------
# GRPC server port
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# GRPC server port
# Port denotes the GRPC server port

Comment on lines +458 to +468
# ------------------------------------------------------------------------------
# TxProfitabilityCheckerType type for checking is it profitable for aggregator to validate batch
# possible values: base/acceptall
# ------------------------------------------------------------------------------
TxProfitabilityCheckerType = "acceptall"

# ------------------------------------------------------------------------------
# TxProfitabilityMinReward min reward for base tx profitability checker when aggregator will validate batch
# this parameter is used for the base tx profitability checker
# ------------------------------------------------------------------------------
TxProfitabilityMinReward = "1.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Double check, but I believe this was removed through #189.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why this was removed @ToniRamirezM?

AggLayerTxTimeout = "5m"

# ------------------------------------------------------------------------------
# AggLayerURL url of the agglayer service
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# AggLayerURL url of the agglayer service
# AggLayerURL is the url of the agglayer service

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.

3 participants