cli

cli copied to clipboard

Learn more about the bounty on Ignite's Discord.

The parser for config.yml can be found at ignite/chainconfig.

1- version field

There wasn't a version field in the config file (AKA config.yml). We should introduce a top level version field to define the version of the config file.

E.g.: version: 0, version: 1.

And assume that version is 0 if no version field presents.

2- migration system

Introduce a migration system into chainconfig package to upgrade a chain's config file version to the latest version and do the necessary migrations to apply to the latest format to chain's config file.

Migration system should define Config struct for each version in their respective packages.

E.g.:

chainconfig/
    v0/
        v0.go

    v1/
        v1.go
    ...

In the chainconfig/config.go:

• Define a Config interface or generic.

type Config interface {
    Version() int
    Convert() (Config, error) // Migrates to the next version
    // ... whatever else needed
}

• Define the list of versions and map their Config structs (implementations).

var migration = map[Version]{
    0: v0.Config{},
    1: v1.Config{},
}

• Define the latest version as a variable.

var LatestVersion = // key of migration[len(migration)]

• Add a ConvertNext(Config) (Config, error) function to convert a Config to the next version of Config.
• Add a ConvertLatest(Config) (Config, error) function to convert a Config to the latest version of Config.

3- automatic migrations

Hook to all sub commands (e.g.: addGitChangesVerifier see 1, see 2) under ignite chain command set. Use the config file path given by the flag or if it's empty use chainconfig.LocateDefault to determine the path of the config file.

Check the version of the config file and check if it can upgraded to latest. If so, prompt a confirmation to user to get confirmation on overwriting the existing config file with the newer format. Inform about the new version number.

Also prompt confirmation if there are any uncommitted changes to get confirmation. Same as how addGitChangesVerifier used for ignite scaffold command set.

Add a --yes, -y flag to ignite chain command set, If this flag is provided, no need for prompting confirmation both for migration and uncommitted changes.

4- define the migration rules for version 1

version 1:

validators: # note that we're deprecating the `validator` top level field and converting it to an array.
  - name: alice
    bonded: 1000stake # note that we're renaming `staked` as `bonded`.

    # we're now deprecating the `host` field from the top level,
    # and directly using 'app' and 'config' fields to configure addresses.

    # `validators[].app` was `init.app` before and moved under validator info.
    # note that fields inside validators[].app is the yml representation of ~/.marsd/config/app.toml
    # if there is a struct defination for this file compatible with yml in the cosmos/cosmos-sdk, we can use it
    # otherwise, we need to define the struct as well.
    app:
      grpc:
        address: ""
      grpc-web:
        address: ""
      api:
        address: ""
        
    # validators[].config was `init.config` before and moved under validator info.
    # note that fields inside validators[].config is the yml representation of ~/.marsd/config/config.toml
    # if there is a struct defination for this file compatible with yml in the cosmos/cosmos-sdk, we can use it
    # otherwise, we need to define the struct as well.
    config: 
      rpc:
        laddr: ""
      p2p:
        laddr: ""
      pprof_laddr: ""

    # validators[].client was `init.client` before and moved under validator info.
    # note that fields inside validators[].client is the yml representation of ~/.marsd/config/client.toml
    # if there is a struct defination for this file compatible with yml in the cosmos/cosmos-sdk, we can use it
    # otherwise, we need to define the struct as well.
    client: 

    # 'gentx' is a new field to introduce under the validator info,
    # when this is provided we should use it to overwrite deaults while creating a gentx.
    # we need to create a struct for gentx.
    gentx:
      amount: 1000stake # from the argument of `marsd gentx` command 
      moniker: mymoniker
      commission-max-change-rate:
      # ... and other fields needed. Run 'marsd gentx -h' and
      # check the Example section in the printed docs to see all available configuration

  - name: bob
    bonded: 500stake

genesis: # we need to create a struct for this as well, or use from cosmos/cosmos-sdk (better to use it from SDK).

To summarize the changes in the config file:

1- validator field is deprecated and converted to an array with validators name. 2- init field is deprecated but fields inside the init field moved to validator info (items inside validators). 3- introducing gentx field to validator info to make it possible customizing gentxs. 4- host field is deprecated with all its sub fields, now you need to use validators[].app and validators[].config to configure addresses. 5- previously we were using map[string]interface{} to represent genesis, app, client, config. Now we need to use the existing structs for these from cosmos/cosmos-sdk or define ourselves if they do not exists.

update templates:

Update the config.yml template to scaffold version: 1 of the config with the new format.

Refactor and add tests

Refactor the configchain pkg if it's needed.

Please add unit tests for your additions.

Also add tests for converting these real chain's config files to v1:

• https://github.com/tharsis/evmos/blob/main/config.yml

Let us know

Please review the design choices above and take them as suggestions and nothing as final. Let us know if it can be improved or you have other ideas.

@ilgooz I am interested in this issue(together with the issue #2473 as well). I am not sure if you have assigned it to anyone.

Hey, great to hear that!

Can you please check out the participation rules, and show interest on a thread by sharing your past experiences in Go and other work?

https://discord.com/channels/893126937067802685/946155736289910894/948148181114421318

Thank you. 🙏

Hey Vincent @vinbrucelu! Nice to see you around here!

You have been assigned to this issue, we're eager to see your solution! 🎊

I plan to implement this bounty in multiple steps. The Step 1 is to refactor the current package of chainconfig, to convert the current config struct into v0 version, create the interface for Config with the necessary methods, and replace all the existences of Config with the newly created interface.

@ilgooz The step 1 has been accomplished in the PR I opened. In the 2nd step, I plan to add the schema of Version 1 and implement the conversion from v0 to v1. Tests will be added.

• Make the parser more intelligent, so it will first get the version number. If it is empty, then default to v0.
• Based on the version number, the parser selects the correct instance of the interface Config to decode the yaml.
• Add the support of v1.

I am not sure how we are going to deal with the fields: Accounts, Faucet and Build in the v0 version. @ilgooz Could you give me more description on them?

I did not find any existing definitions for the following structs in cosmos/cosmos-sdk: genesis, app, client and config.

The only thing I found is there is a struct defined for GenesisDoc in tendermint/tendermint, which looks like:

type GenesisDoc struct {
	GenesisTime     time.Time          `json:"genesis_time"`
	ChainID         string             `json:"chain_id"`
	InitialHeight   int64              `json:"initial_height,string"`
	ConsensusParams *ConsensusParams   `json:"consensus_params,omitempty"`
	Validators      []GenesisValidator `json:"validators,omitempty"`
	AppHash         tmbytes.HexBytes   `json:"app_hash"`
	AppState        json.RawMessage    `json:"app_state,omitempty"`
}

However, they are used to parsed the json. We have to change it in order to parse them in yaml format. We probably need to define them in the CLI.

We can in-fact leave defining structs for Genesis and other similar ones to the last. This is a bit fragile and their structure may change between version.

If there are no reliable ways of handling them defined in the Tendermint Core, or SDK we may even give up on the idea of defining structs.

@ilgooz I have got a few questions regarding the default value for the Config. In the old v0 version, fields like RPC, P2P etc, are saved in the struct Host:

Host: Host{
		// when in Docker on MacOS, it only works with 0.0.0.0.
		RPC:     "0.0.0.0:26657",
		P2P:     "0.0.0.0:26656",
		Prof:    "0.0.0.0:6060",
		GRPC:    "0.0.0.0:9090",
		GRPCWeb: "0.0.0.0:9091",
		API:     "0.0.0.0:1317",
	},

In v1, they will be saved in the validator. However, we support a list of validators in the config, do we save the default information for RPC, P2P, etc in the first item of validator in the list? What if there are multiple validators? Do we fill in the default values of RPC, P2P, etc for each of them?

And, how do we merge the default values into the v1.config? I found out that the lib "github.com/imdario/mergo" is unable to merge the structs in the array. See the issue: https://github.com/imdario/mergo/issues/209 For example, a list of validators in the config is unable to merge to the list of validators in another config, which means the default values for RPC, P2P, etc, will not be able to merged into the config.

To resolve this issue, I propose to add a method called "fill_default()" method in the config, we can call it each time parsing the config yaml. It can fill in the values of faucet, build and the RPC, P2P... information.

OR...

We still leverage the the lib "github.com/imdario/mergo" to do the merge work, but merge the list of validator separately and set the list back to the config.

Please share me your thought. Thx.

In v1, they will be saved in the validator. However, we support a list of validators in the config, do we save the default information for RPC, P2P, etc in the first item of validator in the list? What if there are multiple validators? Do we fill in the default values of RPC, P2P, etc for each of them?

I think we should auto fill the these addresses in the struct only when they don't present in the config.yml. For each validator, we can add the port numbers in incremental fashion.

For ex. user might configure some of the addresses for the first validator and all addresses in the second validator. In this scenario, we only should fill the missing addresses in the first one. If there was other missing ones in the second validator, we also would have to fill them by incrementing the relevant port numbers by one (relative to the validator defined just before).

But as you see this is something that should be computed after parsing the config.yml. So we cannot just set a default config struct somewhere like we used to do for v0 with the host field.

And, how do we merge the default values into the v1.config? I found out that the lib "github.com/imdario/mergo" is unable to merge the structs in the array. See the issue: https://github.com/imdario/mergo/issues/209 For example, a list of validators in the config is unable to merge to the list of validators in another config, which means the default values for RPC, P2P, etc, will not be able to merged into the config.

I think this is irrelevant now because of the behaviour in the first answer.

To resolve this issue, I propose to add a method called "fill_default()" method in the config, we can call it each time parsing the config yaml. It can fill in the values of faucet, build and the RPC, P2P... information.

I guess this is pretty similar to what I was describing? If not, please elaborate. 👍

Thank you!

Thx, only one thing to clarify: For example, if there are two validators in the config.yaml, with both of them having no config or app defined. We assign 0.0.0.0:26657 to the rpc for validator[0]. By incrementally, do we assign 0.0.0.0:26658 to the rpc for validator[1]? In this case, it is possible that rpc port in validator[1] can equal to other port number in validator[0], though it is not equal to the rpc port in validator[0]

For example, if there are two validators in the config.yaml, with both of them having no config or app defined. We assign 0.0.0.0:26657 to the rpc for validator[0]. By incrementally, do we assign 0.0.0.0:26658 to the rpc for validator[1]?

Yes

We assign 0.0.0.0:26657 to the rpc for validator[0]. By incrementally, do we assign 0.0.0.0:26658 to the rpc for validator[1]? In this case, it is possible that rpc port in validator[1] can equal to other port number in validator[0], though it is not equal to the rpc port in validator[0]

You're right. This is specially the case for the grpc/grpc_web, rpc/p2p ones. In this case, what if we do not increment the last digit by one but increment the one that before that? For ex:

V1:
		RPC:     "0.0.0.0:26657",
		P2P:     "0.0.0.0:26656",
		GRPC:    "0.0.0.0:9090",
		GRPCWeb: "0.0.0.0:9091",
 ...

V2:
		RPC:     "0.0.0.0:26667",
		P2P:     "0.0.0.0:26666",
		GRPC:    "0.0.0.0:9100",
		GRPCWeb: "0.0.0.0:9101",
 ...

@ilgooz So far, the pkg chainconfig has been refactored into the structure, that implemented the migration system. Config yamls at lower versions are able to migrate to the next version or the latest version. Config interface is added. both v0 and v1 funstion at this time.

V1 is now defined and able to use. V1 config is defined as in the design. V0 is able to converted to v1.

Test cases are added to verify the migration. Will see if there are more needed, and clean up as well.

The migration from v0 to v1 is done automatically. Next I will add the support in the "ignite chain" commands and subcommands to interact with users.

It is pretty close to ending phase of this bounty.

Under the v0 version, we can call directly

config.GetInit().Home
config.GetInit().Client
config.GetInit().KeyringBackend

There is only one validator, and there is no issue.

In v1, we will have multiple validators, containing multiple clients, KeyringBackend, etc, across multiple validators. What are we going to change the existing calls of config.GetInit().Home, config.GetInit().Client and config.GetInit().KeyringBackend? Do we pick up the fields from the first validator? Or change the existing implementation into plurals?

The same question for config.GetHost().RPC.

Or what I described above is in the scope of https://github.com/ignite-hq/cli/issues/2473?

And, how can I find the structure for gentx? How to install the CLI to run "marsd gentx -h"?

I have defined the struct for gentx, and retrieve the fields from it in IssueGentx. @ilgooz Could you review the PR please? Let me know if there is anything incorrect or missing. I think I have got everything covered for this issue. So far, we support one validator in the config yaml. For multi-validator support, we can continue with https://github.com/ignite-hq/cli/issues/2473.