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

Organize Configuration #538

Closed
majecty opened this issue Oct 12, 2020 · 14 comments
Closed

Organize Configuration #538

majecty opened this issue Oct 12, 2020 · 14 comments

Comments

@majecty
Copy link
Contributor

majecty commented Oct 12, 2020

Configuration Files

TOML configuration file

All configurations described in the TOML configuration file can be customized in CLI arguments.
It has configurations for a node. Each node can change the values in the TOML configuration file freely. It does not affect network behavior.

For example, you can change the port number or data directory in the TOML configuration file.

Scheme file

Contains configurations for a network. It contains Tendermint parameters and Genesis block information. All nodes in a network should have the same scheme file.

App descriptor

Contains configurations for modules. It contains information that which modules are used in a network. It contains configurations for modules.

Some configuration in the app descriptor must be the same in a network. For example, which modules are used. How they are connected must be the same in a network. Some configurations like the number of threads do not affect network behavior.

Splitting App descriptor to App descriptor and Link descriptor

We have a plan to split the app descriptor file: Foundry configuration (genesis config, transaction services) and general linking instructions (export, import, init config). Let's say the latter as a 'link descriptor'.

See #512

Removing Scheme file

We can think of the content of the scheme file is configurations for the host in the app descriptor. The app descriptor contains the genesis configuration of modules. The scheme file contains the genesis header's information. We don't need to have two configuration files for a genesis block.

Configuration for network or for node

The current app descriptor has two types of configurations. As I said before, some configurations are for the network, some configurations are for a node.

We didn't discuss yet whether should we split them or how do we split them.

@majecty
Copy link
Contributor Author

majecty commented Oct 12, 2020

A new configuration draft

Goal

  • Splitting node configuration file and network configuration file.
  • Allow users to set module's node configuration files using command-line arguments.

Node configuration files

config.yml will contain a node's configuration for host and modules.

For modules' configurations, we will use parameter names used in param-default section in link descriptor.

Example

host:
  mining:
    mem_pool_mem_limit: 512
apps:
  thread-pool-size: 16
  account-check-tx: "full"

CLI args

CLI args can change configurations set by the "Node configuration file".
For host configurations, it will share the same configuration struct.

For modules' configurations, it will use the names described in param-default section in the lik descriptor.

Example

$ foundry --mem-pool-mem-limit 512 --thread-pool-size 16 --account-check-tx full

Expected problems

Merging CLI argument config files looks difficult.
We are using clap to parse CLI arguments and YML file to define CLI arguments.
CLAP seems not to provide a way to merge configuration files.

foundry.yml that contains CLI arguments information:
https://github.com/CodeChain-io/foundry/blob/master/foundry/foundry.yml

App descriptor

The app descriptor will have the same configuration except for imports and exports.

Example

modules:
  module-account:
    genesis-config:
      - 0a6902  
    tags:
      previliged: true
...

host:
  tendermint:
    "timeoutPropose": 10000,
    "timeoutProposeDelta": 5000,
    "timeoutPrevote": 10000,
    "timeoutPrevoteDelta": 5000,
    "timeoutPrecommit": 10000,
    "timeoutPrecommitDelta": 5000,
    "timeoutCommit": 10000
  genesis:
    "seal":
      "tendermint":
        "prev_view": "0x0",
        "cur_view": "0x0",
        "precommits": [ ... ]
    author: "xxdd",
    "timestamp": "0x00",
    "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "extraData": "0x"

transactions:
  account: module-account
  stamp: module-stamp
  token: module-token
[modules.module-account]
genesis-config = ["0a6902"]
tags = { privileged = true }

[host.tendermint]
timeoutPropose = 10000
timeoutProposeDelta = 5000
timeoutPrevote = 10000
timeoutPrevoteDelta = 5000
timeoutPrecommit = 10000
timeoutPrecommitDelta = 5000
timeoutCommit = 10000

[host.genesis]
author = "xxdd"
timestamp = "0x00"
parentHash = "0x0000000000000000000000000000000000000000000000000000000000000000"
extraData = "0x"

[host.genesis.seal.tendermint]
prev_view = "0x0"
cur_view = "0x0"
precommits = []

[transactions]
account = "module-account"
stamp = "module-stamp"
token = "module-token"

Concerns

tag is used to give some authorization to modules. Having tag in app descriptor is subtle. Since we are not using tags, we will revisit it later.

Link descriptor

The link descriptor contains imports and exports of modules. The link descriptor does not contain any blockchain concepts. It can be used outisde of blockchain context.

Param alias and default value

A module receives init-config parameters to customize it's behavior. To make config file and CLI argument be able to override the values, app descriptor file should have the parameter in param-aliases and param-defaults.

Let's asume that the module "module-account" has "some-configuration" parameter. We can alias it to "hello" using the config below:

modules:
  module-account:
    init-config: ["some-configuration"]
    
param-aliases:
  module-account:
    some-configuration: hello
[modules.module-account]
init-config = ["some-configuration"]

[param-aliases.module-account]
some-configuration = "hello"

Now a user can use configuration file or CLI argument "--hello" to set module-account's "some-configuration".

Example

default-sandboxer: single-process

modules:
  module-account:
    hash: a010000000012345678901234567890123456789012345678901234567890123
    init-config: [
      "some-configuration"
    ]
    exports:
      stateful:
        stateful: {}
      tx-owner:
        tx-owner: {}
      account-manager:
        account-manager: {}
      get-account-and-seq:
        get-account-and-seq: {}
      handle-graphql-request:
        handle-graphql-request: {}
    imports:
      token-manager: module-token/token-manager
      
param-aliases:
  module-accout:
    some-configuration: hello

param-defaults:
  hello: Annyeong Haseyo
default-sandboxer = "single-process"

[modules.module-account]
hash = "a010000000012345678901234567890123456789012345678901234567890123"
init-config = ["some-configuration"]
exports.stateful.stateful = {}
exports.tx-owner.tx-owner = {}
exports.account-manager.account-manager = {}
exports.get-account-and-seq.get-account-and-seq = {}
exports.handle-graphql-request.handle-graphql-request = {}
imports.token-manager = "module-token/token-manager"

[param-aliases.module-accout]
some-configuration = "hello"

[param-defaults]
hello = "Annyeong Haseyo"

Plan

  • Change Yaml to Toml (need a review)
  • Remove scheme file
  • Change Foundry CLI and configuration file to use a common source of truth
  • Expose modules' init-config in CLI and config
  • Split app descritor and link descriptor

@junha1
Copy link
Contributor

junha1 commented Oct 12, 2020

@majecty I have some comments.

  1. init-config and default-sandboxer: single-process should belong to the link descriptor
  2. I think CLI is ok to be only for the node config. (not module config)
  3. And app-descriptor should specify genesis-config, not init-config
  4. I don't understand why you put module-staking: in node config. node config must be application-agnostic.

@dynaxis
Copy link
Member

dynaxis commented Oct 12, 2020

In my opinion, the config may also contain overrides for app descriptor under a section designated so.

Also I agree with @majecty on the complexity of the code merging CLI options into the config. By sacrificing a bit of type safety in merging them and postponing type checking to a later stage (for instance, when actually stuffing out structs representing the config with serde), I think it is more or less possible to reduce the complexity.

@dynaxis
Copy link
Member

dynaxis commented Oct 12, 2020

Another comment. If we have module hashes in link descriptor, then the corresponding app descriptor may use names assigned in the link descriptor.

@majecty
Copy link
Contributor Author

majecty commented Oct 12, 2020

@junha1

  1. and 3. Thanks. I've updated the draft as you said. I do not still understand what is the bar to split the app descriptor and link descriptor. Can you explain it to me?
  2. and 4. Modules can have configurations for a node not a network. I think the number of threads is the option for a node. Do you think "the number of thread" is a network configuration?

@dynaxis
Copy link
Member

dynaxis commented Oct 12, 2020

I think I need to think a bit more about where tags will be specified. Tags are conceived as metadata for some parties (definitely the coordinator) to check as prerequisites for assigning previleges to some modules (that's the "express-that-you-know-what-you-do" thing).

Since tags are not actually used yet, I will need to think a bit more. Let me come back with details soon.

@junha1
Copy link
Contributor

junha1 commented Oct 12, 2020

@majecty number of threads can be different for each module, but it is module-specific configuration. node-config should be responsible for only the consensus engine. And as I said before, I think link descriptor can be different in a same network, so each node can specify any value for number of threads.

@majecty
Copy link
Contributor Author

majecty commented Oct 12, 2020

@dynaxis

In my opinion, the config may also contain overrides for app descriptor under a section designated so.

Do you mean the the node configuration can override values in app descriptor? I thought CLI arguments are only for node configuration. Changing a network's configuration using CLI args makes user easy to make mistakes. Could you give me an example parameter that described in the app descriptor and configurable in CLI?
I agree with you. We can make it work first and enhance it step by step.

@majecty
Copy link
Contributor Author

majecty commented Oct 12, 2020

@dynaxis

Also I agree with @majecty on the complexity of the code merging CLI options into the config. By sacrificing a bit of type safety in merging them and postponing type checking to a later stage (for instance, when actually stuffing out structs representing the config with serde), I think it is more or less possible to reduce the complexity.

I agree with you. We can make it work first and enhance it later. Merging configs perfectly need too much work than it's benefit.

@majecty
Copy link
Contributor Author

majecty commented Oct 12, 2020

@dynaxis

Another comment. If we have module hashes in link descriptor, then the corresponding app descriptor may use names assigned in the link descriptor.

Looks good idea. I've updated it.

@majecty
Copy link
Contributor Author

majecty commented Oct 12, 2020

@junha1
It seems that we are saying different things.

There are two properties in configurations.

  1. Host or Module
    Host configurations are not related modules.
    Module configurations are related modules.

  2. Node or Network
    Network configurations are related a network. Changing a network configuration of a node will make the node fail to communicate other nodes.
    Node configurations are related to a node. Changing them only affect a node.

The goal of this draft is to split configurations of node and network.
I want to collect node configurations in a node configuration file.

If you don't agree with the goal, let's make a consensus on the goal first.

@junha1
Copy link
Contributor

junha1 commented Oct 12, 2020

Let's discuss on meeting.

@dynaxis
Copy link
Member

dynaxis commented Oct 14, 2020

The TOML versions look good to me.

@majecty
Copy link
Contributor Author

majecty commented Nov 24, 2020

DONE

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

No branches or pull requests

3 participants