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

Add babylon app config #105

Merged
merged 2 commits into from
Aug 30, 2022
Merged

Add babylon app config #105

merged 2 commits into from
Aug 30, 2022

Conversation

KonradStaniec
Copy link
Contributor

@KonradStaniec KonradStaniec commented Aug 27, 2022

Add additional section to app.toml with configuration related to bitcoin.

Why app.toml, not genesis params ?

  1. Some things are shared between btc-checkpoint and btc-lightclient modules like for example (powLimit). So it makes sense to take it from common config.
  2. We are using powLimit to validate proof of work in checkTx which is stateless. The only way to pass configured value there is through global object which can be initialised at start from some global config. (The same thing applies to magic string used in checkpoints i.e we have BBN and BBT)

For now only putting powLimit and "checkpointTag" in config as those are globals used in checkTx validation, rest of params can be decided/added in subsequent pr-s, or per need basis.

@aakoshh
Copy link
Contributor

aakoshh commented Aug 29, 2022

Just regarding genesis.json vs app.toml - I thought the first would be stuff that we need consensus over, while the latter is for things that are at the discretion of the node operator.

app/app.go Show resolved Hide resolved
customAppConfig := CustomAppConfig{
Config: *srvCfg,
}
babylonConfig.MinGasPrices = "0stake"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not "0bbn"?

Copy link
Contributor

@aakoshh aakoshh left a comment

Choose a reason for hiding this comment

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

Awesome!

@KonradStaniec KonradStaniec merged commit 536af49 into main Aug 30, 2022
@KonradStaniec KonradStaniec deleted the custom-babylon-config branch August 30, 2022 16:11
@vitsalis
Copy link
Member

I’m not entirely sure whether putting the BTC network that is being used and the checkpoint tag in the configuration file is the correct way. My main thinking is this:
Looking at app.toml , it only contains configuration related to how a node is operated (e.g. ports), but not anything related to consensus.
If anyone puts in a wrong combination of configuration (e.g. BTC mainnet, but BBT tag) then this would not lead to consensus.
On the other hand, if we put such stuff in the genesis (or params which are part of genesis) then a node that just runs our code (using the default genesis) is not going to have any issues. However, this would mean that we would have to add genesis configuration for both the btclightclient and btccheckpoint module which would have to be in sync.

I have thought a little bit about this due to the base header of the btclightclient module. Initially, I was inclined to put this in a configuration file, but then decided against it, since this is an important part of consensus and should be the same for everybody. However, with the base header being in the genesis, it is not as easy to setup different types of test networks. For example, if I need to test babylon using a BTC simnet or testnet, I need to create a custom genesis.json file that contains the base header that I want to use. This is not as simple as modifying a configuration file, but I think it is the better way to go.

vitsalis pushed a commit that referenced this pull request Jan 21, 2024
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