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

cmd(chain/serve): support serving a multi-validator architecture #2473

Closed
ilgooz opened this issue May 11, 2022 · 16 comments
Closed

cmd(chain/serve): support serving a multi-validator architecture #2473

ilgooz opened this issue May 11, 2022 · 16 comments
Assignees
Labels
bounty type:feat To implement new feature.

Comments

@ilgooz
Copy link
Member

ilgooz commented May 11, 2022

Learn more about the bounty on Ignite's Discord.

Currently, ignite chain serve command will only initialize and start a single validator during development. The information about the chain and configuration about the validator is derived from chain's config file (AKA config.yml).

We would like to optionally support a multi-validator architecture to simulate a much more realistic scenario by allowing developers to test their blockchain in a multi-validator environment.

To support running multiple validators:

  • We need to introduce breaking changes to the config file (later to be migrated by introduce a migration system for the config file #2470) so, we can allow configuring multiple chains individually.
  • Make changes in the following packages:
    • ignite/chainconfig
    • ignite/services/chain
    • ignite/cmd
    • and some other packages as needed, the ones above are the major entrypoints for this feature.

1- the config file

Please check the current version for the underlying struct for the config file from here. We need to introduce some breaking changes to this struct.

The new format of the config file should be like this:

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.
    # yml representation of ~/.marsd/config/app.toml
    app:
      grpc:
        address: ""
      grpc-web:
        address: ""
      api:
        address: ""
        
    # validators[].config was `init.config` before and moved under validator info.
    # yml representation of ~/.marsd/config/config.toml
    config: 
      rpc:
        laddr: ""
      p2p:
        laddr: ""
      pprof_laddr: ""

    # validators[].client was `init.client` before and moved under validator info.
    # yml representation of ~/.marsd/config/client.toml
    client: 

    # 'gentx' is a new field to introduce under the validator info,
    # when this is provided we should use it to overwrite defaults while creating a gentx.
    # we need to create a struct for gentx.
    gentx:
      amount: 1000stake # this should overwrite if `validators[].bonded` was also set.
      moniker: mymoniker # if this isn't provided we can default to `moniker-alice`,
                        # `alice` stands for whatever the name of the validator is: `validators[].name`
      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.

Note that, as we're deprecating the host top level field, we should examine the contents of validators[].app and validators[].config to get the all listen addresses (e.g.: rpc, api, grpc, etc.) for each validator.

When these addresses aren't set, we should automatically pick a port number (with an incremental order) for each validator.

Note that following should overwrite/merge (which is the current behavior already) their corresponding files/structs:

  • validators[].app < renamed
  • validators[].config < renamed
  • validators[].client < renamed
  • validators[].gentx < new
  • genesis < exists

2- visit chain serve cmd and service/chain package

We need to do some changes and refactoring in these two to support initializing and starting multiple validators. Please take a good look of their source code and package API design.

Home dir of the nodes:

We used to create a home dir for the chain by its name e.g.: ~/.mars.

Now, we need to create a separate home dir for each validator. Use the name of the chain as the root and use the account name associated with the validator to create a home dir for each validator. E.g.: ~/.mars/alice, ~/.mars/bob.

By default, chain's binary will use ~/.mars as the home dir name. For this to still work, we should create symbolic link as ~/.mars and make it point to the first validator's (from the validators array in the config file) home dir.

Since we have multiple home dirs, make sure to import/replicate accounts (defined in config file) and other similar configuration to all validators (home dirs).

Please note that mars name is being as a sample here and it represents the name of the underlying chain.

--validator flag

Currently ignite chain serve command and services/chain package is designed in a way to print logs of a single validator (logs that we collect from node's process). When you're using the ignite chain serve command with --verbose flag it prints the logs from the validator running under the hood.

Since we need to support multiple validators and spin up multiple nodes, we'll be having multiple log sources. It's not ideal to print logs for all the validators. We should only print logs from a single validator. To make it possible, add this new --validator flag to chain serve to allow user to pick which validator's logs to see.

Value of the --validator flag can be the account names that associated with validators (validators[].name). E.g.: --validator alice.

If the provided value does not match with any validators, serve command should fail with an error explaining the situation.

When this flag isn't set, we can default to the first validator appears in the validators array.

serve command also prints general information about a validator like which servers (e.g. rpc, api) are started and their addresses. Since there would be multiple validators and addresses, we should print these information for the underlying validator.

handle failing nodes

If any of the node fails (process exits), serve command should also fail (exit 1) and show the logs from the first failing validator. We should also print the information (name) about which validator has failed.

3- changes in the config file template

Keep in mind that when you use ignite scaffold chain, we scaffold a chain with a default config.yml file. While we're introducing changes to the Config struct, we should also update the config.yml template to match with the new format.

To do that check the config.yml template.

Also, update the config file to serve two validators by default(alice and bob).

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.

@jsimnz
Copy link

jsimnz commented May 28, 2022

Hi @ilgooz I expressed interest on this bounty issue on the discord (cc'd you there in a dedicated thread) a couple of days ago. Just wanted to checkin directly on this thread for visibility.

@ilgooz
Copy link
Member Author

ilgooz commented May 29, 2022

Hey John @jsimnz! Happy to see you being interested on this one! This is such an essential and widely requested feature. Can't wait to see your solution!

@ilgooz
Copy link
Member Author

ilgooz commented May 29, 2022

By the way, we should also update the config.yml reference.

@jsimnz
Copy link

jsimnz commented May 29, 2022

By the way, we should also update the config.yml reference.

Will do!

As for the config stuff, should I incorporate the changes from #2470 before the related PR is merged, or should I wait till the final review is ready.

@jsimnz
Copy link

jsimnz commented Jun 6, 2022

There is an overload of terms in the config struct and in the requirements for this PR.

Originally there is a config.Client which handles client code generation (OpenAPI, Vuex, and Dart), additionally there is init.Client which handles config/client.toml values. This may lead others (def not me :) ) to confusion.

Should this be addressed in this issue/PR or left as is?

@ilgooz
Copy link
Member Author

ilgooz commented Jun 6, 2022

Hey, I believe they should be independent for now because we need to make a refactoring in the config migration PR.

I propose that we just update the Config struct in the multi-validator support PR in the way how it has to be for the feature.

We can do the wiring later to benefit from config file migrations, it could be done later once we merge both PRs.

@jsimnz
Copy link

jsimnz commented Jun 15, 2022

hey @ilgooz.

Work is progressing well, have most of the multi process stuff working for each validator, but there is a design constraint that I'm trying to work through.

The issue description talks about the changes to the HOME directories, where each validator has their own home folder under the main home folder. IE: alice validator's home lives in ~/.mars/alice and contains its respective config files, data files, keyring, etc. This is replicated for all defined validators in the config.yaml. Which is clear, but you then mentioning creating a symbolic link for ~/.mars pointing to the first validators home dir, which as mentioned above is ~/.mars/alice.

Unfortuently this creates a cricular link that I don't believe any OS supports. IE: ~/.mars => ~/.mars/alice (assuming alice is the first validator in the config).

At the moment I have the multi home system working without the root ~/.mars folder link, and instead ensure that the --home CLI flag for the app binary is always present, and points to the respective validator folder. IE: marsd --home ~/.mars/alice status.

One design question here:
The ignite cli command also has a --home flag which gets persisted into the app daemon --home flag (ignite chain serve --home ~/.my-other-home will eventually call marsd --home ~/.my-other-home). But since we want to support multiple validators, should we just assume that the ignite cli --home flag actually points to the "root" home directory which itself will contain the various validator sub folders (ie: ~/.my-other-home/alice exists)?

The current implementation is working using the above design, but wanted to check in and see your thoughts.

@ilgooz
Copy link
Member Author

ilgooz commented Jun 17, 2022

Yep, all sounds good!

We don't need a symlink then and if you're directly using chain's binary to do things manually, you need to specifically define the home dir through the home flag. e.g: ~/.mars/alice

And ignite chain serve's --home can by default point to the root dir: ~/.mars

@hammadtq
Copy link

hammadtq commented Aug 1, 2022

hey @ilgooz.

Work is progressing well, have most of the multi process stuff working for each validator, but there is a design constraint that I'm trying to work through.

The issue description talks about the changes to the HOME directories, where each validator has their own home folder under the main home folder. IE: alice validator's home lives in ~/.mars/alice and contains its respective config files, data files, keyring, etc. This is replicated for all defined validators in the config.yaml. Which is clear, but you then mentioning creating a symbolic link for ~/.mars pointing to the first validators home dir, which as mentioned above is ~/.mars/alice.

Unfortuently this creates a cricular link that I don't believe any OS supports. IE: ~/.mars => ~/.mars/alice (assuming alice is the first validator in the config).

At the moment I have the multi home system working without the root ~/.mars folder link, and instead ensure that the --home CLI flag for the app binary is always present, and points to the respective validator folder. IE: marsd --home ~/.mars/alice status.

One design question here: The ignite cli command also has a --home flag which gets persisted into the app daemon --home flag (ignite chain serve --home ~/.my-other-home will eventually call marsd --home ~/.my-other-home). But since we want to support multiple validators, should we just assume that the ignite cli --home flag actually points to the "root" home directory which itself will contain the various validator sub folders (ie: ~/.my-other-home/alice exists)?

The current implementation is working using the above design, but wanted to check in and see your thoughts.

Sorry not adding anything constructive here but just wanted to check if there is anything that I can use now to support multi-validator architecture?

@ilgooz
Copy link
Member Author

ilgooz commented Aug 1, 2022

Hey, I think @jsimnz is working in this, I should defer to them for updates.

John can you please let us know about the status of this feature?

@jsimnz
Copy link

jsimnz commented Aug 6, 2022

Happy to open up a PR in the coming days to get the feedback going 👍

@jsimnz
Copy link

jsimnz commented Aug 22, 2022

#2772
cc @ilgooz @hammadtq

@aljo242
Copy link
Contributor

aljo242 commented Dec 13, 2022

@jsimnz pinging for visibility:

The PR was closed because the develop branch was deleted. Could you re-open and target main?

@jsimnz
Copy link

jsimnz commented Dec 16, 2022

@jsimnz pinging for visibility

👍

@Pantani
Copy link
Collaborator

Pantani commented Jun 15, 2023

@jsimnz, any updates on it?

@julienrbrt
Copy link
Member

Superseded by #4374.
@jsimnz if you are willing to take it, the bounty has been updated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty type:feat To implement new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants