Configuration Design

[Olshansk]

Module Configuration Design

Goal

The goal of this discussion is to make a decision on how configurations should be designed and passed into their respective modules.

Why now?

The foundation for config management will likely spread throughout the entire codebase and set the foundation for all future work, with a very high cost being modified/refactored at a later point in time.

V0

In V0, the config was structured like so:

type Config struct {
	TendermintConfig config.Config `json:"tendermint_config"`
	PocketConfig     PocketConfig  `json:"pocket_config"`
}

With TendermintConfig designed like so:

type Config struct {
	// Top level options use an anonymous struct
	BaseConfig `mapstructure:",squash"`

	// Options for services
	RPC             *RPCConfig             `mapstructure:"rpc"`
	P2P             *P2PConfig             `mapstructure:"p2p"`
	Mempool         *MempoolConfig         `mapstructure:"mempool"`
	FastSync        *FastSyncConfig        `mapstructure:"fastsync"`
	Consensus       *ConsensusConfig       `mapstructure:"consensus"`
	TxIndex         *TxIndexConfig         `mapstructure:"tx_index"`
	Instrumentation *InstrumentationConfig `mapstructure:"instrumentation"`
}

Though every config was a separate independent structure, their existence in the same types package led to an implicit interdependency which made it extremely difficult, sometimes non-feasible, to make certain changes. Examples of this are outside the scope of this discussion.

NOTE: We're using consensus as an example but the same applies to other modules as well

V1 - Initial Proof-Of-Concept

The first implementation of V1 added a factory function so:

func Create(cfg *genesis.Config, genesis *genesis.GenesisState) (modules.ConsensusModule, error) { ... }

That leveraged a shared config protobuf in shared/types/genesis/proto/config.proto

message Config {
  BaseConfig base = 1;
  ConsensusConfig consensus = 2;
  UtilityConfig utility = 3;
  PersistenceConfig persistence = 4;
  P2PConfig p2p = 5;
  TelemetryConfig telemetry = 6;
}

This was just an initial approach that made sense to get things running but was always planned to be revisited in the future.

V1 - Today

In PR #178, we tended to issue #163, which aimed at taking a step forward in removing unnecessarily shared dependencies. The new factory function now looks like so:

func Create(configPath, genesisPath string, useRandomPK bool) (modules.ConsensusModule, error) { ... }

Moving the onus of configuration validation and parsing to each individual module.

V1 - Today - Temp

Some "shortcuts" were done to iterate faster on main. Even though the consensus config protobuf is encapsualted in consensus/types/proto:

message ConsensusConfig { ... }

A shared interface still exists in shared/modules/types that is intended to be deleted in the future:

type ConsensusConfig interface {
	GetMaxMempoolBytes() uint64
	GetPaceMakerConfig() PacemakerConfig
}

---

Some issues have been seen with the new approach, so the tradeoffs of both needs to be compared before moving forward

---

Config Tradeoffs

Legend

• ❌ : Not practical / feasible / possible
• ➖ : Inconvenient / difficult / not-enforced
• ✅ : Convenient / easy / enforced

Consideration	Why	V0	V1 - Strings	V1 - Structs
1. Config cross-contamination	Individual module swap	❌	✅	➖ (possible but not enforceable)
2. Config injection / mocking	Testing	➖ (possible but not great)	➖ (possible but not great)	✅
3. Semantic configs	Understanding & clarity	✅	➖ (constrained to module)	✅
4. Code clarity	Code health	❌	➖ (okay but not great)	✅
5. Config ownership	Module independence	➖	✅	➖ (possible but not enforceable)

1. Config cross-contamination

As discussed in this GH comment, we want to have the optionality of being able to freely swap one consensus module for another without risking cross-module dependency.

It is easy to start creating cross-module dependencies, and it will practically not be possible to refactor this out once the project is mature (i.e. see Tendermint)

2. Config injection/mocking

With the string approach, injecting custom/testing configs requires:

1. Generate configs / initialize structs
2. Marshall config to string
3. Store string somewhere on disk/file
4. Pass path to string to mocked/tested module

With the semantic configs approach, we can:

1. Generate configs / initialize struct
2. Pass initialized struct to module

Or Altneratively:

1. Mock config (e.g. consensusConfig.EXPECT().GetTimeout().Return(10))
2. Pass mocked struct to module

3. Semantic configs

This point is a matter of developer preference, including but not limited to the following questions:

• Is it easier to understand the module-specific configs right next to the module?
• Is it easier to view all the configs in one place?
• Is it a big value-add to be able to cmd-click on structure in the Create function header to view the configs?

4. Code clarity

There is nothing Pocket specific about code clarity, code quality or code health and is also a function of developer preference.

For example, there are a handful of places where there is a lot of copy-pasted code with this approach.

    cm := new(ConsensusModule)
    c, err := cm.InitConfig(configPath)
    if err != nil {
    	return nil, err
    }
    g, err := cm.InitGenesis(genesisPath)
    if err != nil {
    	return nil, err
    }
    cfg := c.(*typesCons.ConsensusConfig)

Generally, code clarity is a focus because:

• Developer speed (less code -> less writing)
• Review speed (less code -> less reading)
• Cognitive load (easier to understand/learn)
• Onboarding (less onboarding time)
• Reduce code duplication (less code -> less area for bugs)

5. Config ownership

This is a tradeoff of having one place deserialize and validate all the configs and having each module expect functional/validated configs being provided to them, versus moving that responsibility to each individual module.

Tradeoff Summary

Though everything can be done with each approach, a summary of the tradeoffs is:

1. Module independence (for module swapping)
2. Config ownership (for lack of config centralization)

In favour of:

1. Mockable/injectable configs (for testing)
2. Shorter & more de-duped code (for code clarity)

---

EDIT 1:

We updated the poll, which cancelled previous votes after the team had a discussion with this output: #234 (comment)

[Olshansk]

Disclaimer: I tried to provide the tradeoffs above in an unbiased way though my personal preference is for having semantic configurations. I personally believe it will help the long-term vision health of the project as the team and external contributors grow, and make future features easier to ship.

[deblasis]

Thank you for this!

I think we all agreed that config paths are not the way to go, at the same time I feel the need for using interfaces while we figure out our testing framework.

I have put together PR #235 just to show what I think we could do, I would love if you (@andrewnguyen22 , @Olshansk and really anyone who wants to jump in and comment) could give it a quick scan to at least agree if this is going on the right path or it's a complete no-go.

[Olshansk]

Wanted to share some more details inline for other readers of the thread.

The main idea here is factory Create function with the same interface for every module:

func Create(runtime modules.Runtime) (modules.Module, error) {
	var m ConsensusModule
	return m.Create(runtime)
}

With a module-specific genesis and config initialization:

func (*ConsensusModule) Create(runtime modules.Runtime) (modules.Module, error) {
	cfg := runtime.GetConfig()
	genesis := runtime.GetGenesis()

	moduleCfg := cfg.Consensus.(*typesCons.ConsensusConfig)
	moduleGenesis := genesis.ConsensusGenesisState.(*typesCons.ConsensusGenesisState)

	...
}

That enables:

1. Implementation specific configs (e.g. typesCons.ConsensusConfig) and genesis (e.g. typesCons.ConsensusGenesisState)
2. Repository shared interfaces (e.g. func (c *Config) GetConsensusPort() uint32 { ... } in runtime/config.go)
3. Environment variable injection using viper

[andrewnguyen22]

Reviewed here #235 (review)

I added several comments to underscore this point:

It's arguable whether or not V1 structs provides more code clarity than V1 strings due to:

• loosening of interface and patterns
• enabling cross module access of module specific configuration
• requiring an additional global runtime package which arguably increases cognitive load

I would have structured the tradeoff table more like this:

Config Tradeoffs

Legend

• ❌ : Not practical / feasible / possible
• ➖ : Inconvenient / difficult / not-enforced
• ✅ : Convenient / easy / enforced

Consideration	Why	V0	V1 - Strings	V1 - Structs
1. Config cross-contamination	Individual module swap	❌	✅	➖ (possible but not enforceable)
2. Config injection / mocking	Testing	➖ (possible but not great)	➖ (possible but not great)	✅
3. Code clarity	Code health	❌	➖ (more difficult creation signatures)	➖ (introduces a new centralized pkg, loosens interfaces/patterns)
4. Config ownership	Module independence	➖	✅	➖ (possible but not enforceable)

I removed Semantic Configs because:

This point is a matter of developer preference

[andrewnguyen22]

Shorter & more de-duped code (for code clarity)

I believe this can be done with either approach by sharing json.Unmarshal configuration utils

[Olshansk]

Yup, definitely a fair way to structure it as well.

[Olshansk]

The core team had an internal discussion & meeting about this, so I just wanted to highlight a few of the major takeaways.

In addition to the original considerations listed above, we discussed whether the Pocket Node should be treated as a monolith or a collection of microservices.

After reviewing #235 together, the conclusions we came to are:

1. Module-specific configs are good because:
   • They enable different implementations
2. Shared interfaces (i.e. a subset of the configs) are good because:
   • It's a contract and expectation on how the module should behave (e.g. common to microservices)
   • Implementation that doesn't follow the shared interface SHOULD break things (i.e. feature, and not a bug)
   • Pocket is an application-specific blockchain with an application-specific bus, and this is an extension of that
3. A shared runtime/config package enables the modularity/readability/injectability/testability we need (i.e see considerations above)
4. There is likely no technical solution that can replace social construct for long-term code health other than:
   • Trying to enforce best practices through automation
   • Extensive documentation
   • Code reviews & processes
   • Developer/community education

---

A couple of action items (just for thought but outside the scope of this discussion) that are enabled by shared interfaces are:

• Interface for node orchestration
• Interface for node telemetry

These would be swappable based on implementation, but a shared contract will still exist.