Skip to content

Commit

Permalink
fix(tm2): load config defaults if not specified in existing config (g…
Browse files Browse the repository at this point in the history
…nolang#1544)

## Description

This PR fixes the flow of loading Gno configuration files. 
Previously, configuration files were loaded without applying default
values, causing to funky behavior.

Additionally, this PR drops the config TOML template, instead relying on
the TOML package itself to marshal.

Issue spotted in gnolang#1238 

Note:
I've tried to keep the original functionality of
`LoadOrMakeConfigWithOptions`, because if this is changed or broken up
in its current state, things start breaking across dimensions.

Funky behavior that's fixed:
The `EventStore` fields were not being saved in the original config
(since we utilized a template for it, and this was not present in the
template). Now, config files should properly save on disk the event
store configuration.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
  • Loading branch information
zivkovicmilos authored and leohhhn committed Jan 26, 2024
1 parent 5fecaa0 commit 5c431df
Show file tree
Hide file tree
Showing 19 changed files with 310 additions and 352 deletions.
1 change: 1 addition & 0 deletions contribs/gnodev/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
)

require (
dario.cat/mergo v1.0.0 // indirect
github.com/btcsuite/btcd/btcec/v2 v2.3.2 // indirect
github.com/btcsuite/btcd/btcutil v1.1.3 // indirect
github.com/cespare/xxhash v1.1.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions contribs/gnodev/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions contribs/gnokeykc/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ require (
)

require (
dario.cat/mergo v1.0.0 // indirect
github.com/alessio/shellescape v1.4.1 // indirect
github.com/btcsuite/btcd/btcec/v2 v2.3.2 // indirect
github.com/btcsuite/btcd/btcutil v1.1.3 // indirect
Expand Down
3 changes: 3 additions & 0 deletions contribs/gnokeykc/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion gno.land/cmd/gnoland/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func execStart(c *startCfg, io commands.IO) error {
cfg, loadCfgErr = config.LoadConfigFile(c.nodeConfigPath)
} else {
// Load the default node configuration
cfg, loadCfgErr = config.LoadOrMakeConfigWithOptions(dataDir, nil)
cfg, loadCfgErr = config.LoadOrMakeConfigWithOptions(dataDir)
}

if loadCfgErr != nil {
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ require (
gopkg.in/yaml.v3 v3.0.1
)

require dario.cat/mergo v1.0.0

require (
github.com/btcsuite/btcd/btcec/v2 v2.3.2
github.com/btcsuite/btcd/chaincfg/chainhash v1.0.2 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions misc/loop/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
)

require (
dario.cat/mergo v1.0.0 // indirect
github.com/btcsuite/btcd/btcec/v2 v2.3.2 // indirect
github.com/btcsuite/btcd/btcutil v1.1.3 // indirect
github.com/cespare/xxhash v1.1.0 // indirect
Expand Down
3 changes: 3 additions & 0 deletions misc/loop/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions tm2/pkg/bft/blockchain/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"

abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types"
cfg "github.com/gnolang/gno/tm2/pkg/bft/config"
"github.com/gnolang/gno/tm2/pkg/bft/mempool/mock"
Expand All @@ -21,6 +19,7 @@ import (
"github.com/gnolang/gno/tm2/pkg/log"
"github.com/gnolang/gno/tm2/pkg/p2p"
"github.com/gnolang/gno/tm2/pkg/testutils"
"github.com/stretchr/testify/assert"
)

var config *cfg.Config
Expand Down
133 changes: 84 additions & 49 deletions tm2/pkg/bft/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"path/filepath"

"dario.cat/mergo"
abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types"
cns "github.com/gnolang/gno/tm2/pkg/bft/consensus/config"
mem "github.com/gnolang/gno/tm2/pkg/bft/mempool/config"
Expand All @@ -21,11 +22,11 @@ type Config struct {
BaseConfig `toml:",squash"`

// Options for services
RPC *rpc.RPCConfig `toml:"rpc"`
P2P *p2p.P2PConfig `toml:"p2p"`
Mempool *mem.MempoolConfig `toml:"mempool"`
Consensus *cns.ConsensusConfig `toml:"consensus"`
TxEventStore *eventstore.Config `toml:"tx_event_store"`
RPC *rpc.RPCConfig `toml:"rpc" comment:"##### rpc server configuration options #####"`
P2P *p2p.P2PConfig `toml:"p2p" comment:"##### peer to peer configuration options #####"`
Mempool *mem.MempoolConfig `toml:"mempool" comment:"##### mempool configuration options #####"`
Consensus *cns.ConsensusConfig `toml:"consensus" comment:"##### consensus configuration options #####"`
TxEventStore *eventstore.Config `toml:"tx_event_store" comment:"##### event store #####"`
}

// DefaultConfig returns a default configuration for a Tendermint node
Expand All @@ -40,37 +41,65 @@ func DefaultConfig() *Config {
}
}

type ConfigOptions func(cfg *Config)

// LoadOrMakeConfigWithOptions loads configuration or saves one
// made by modifying the default config with override options
func LoadOrMakeConfigWithOptions(root string, options ConfigOptions) (*Config, error) {
var cfg *Config
type Option func(cfg *Config)

// LoadOrMakeConfigWithOptions loads the configuration located in the given
// root directory, at [defaultConfigFilePath].
//
// If the config does not exist, it is created, starting from the values in
// `DefaultConfig` and applying the defaults in opts.
func LoadOrMakeConfigWithOptions(root string, opts ...Option) (*Config, error) {
// Initialize the config as default
var (
cfg = DefaultConfig()
configPath = join(root, defaultConfigFilePath)
)

// Config doesn't exist, create it
// from the default one
for _, opt := range opts {
opt(cfg)
}

configPath := join(root, defaultConfigFilePath)
// Check if the config exists
if osm.FileExists(configPath) {
var loadErr error

// Load the configuration
if cfg, loadErr = LoadConfigFile(configPath); loadErr != nil {
loadedCfg, loadErr := LoadConfigFile(configPath)
if loadErr != nil {
return nil, loadErr
}

cfg.SetRootDir(root)
cfg.EnsureDirs()
} else {
cfg = DefaultConfig()
if options != nil {
options(cfg)
// Merge the loaded config with the default values
if err := mergo.Merge(loadedCfg, cfg); err != nil {
return nil, err
}
cfg.SetRootDir(root)
cfg.EnsureDirs()
WriteConfigFile(configPath, cfg)

// Validate the configuration
if validateErr := cfg.ValidateBasic(); validateErr != nil {
return nil, fmt.Errorf("unable to validate config, %w", validateErr)
// Set the root directory
loadedCfg.SetRootDir(root)

// Make sure the directories are initialized
if err := loadedCfg.EnsureDirs(); err != nil {
return nil, err
}

return loadedCfg, nil
}

cfg.SetRootDir(root)

// Make sure the directories are initialized
if err := cfg.EnsureDirs(); err != nil {
return nil, err
}

// Validate the configuration
if validateErr := cfg.ValidateBasic(); validateErr != nil {
return nil, fmt.Errorf("unable to validate config, %w", validateErr)
}

// Save the config
if err := WriteConfigFile(configPath, cfg); err != nil {
return nil, err
}

return cfg, nil
Expand All @@ -79,7 +108,7 @@ func LoadOrMakeConfigWithOptions(root string, options ConfigOptions) (*Config, e
// TestConfig returns a configuration that can be used for testing
func TestConfig() *Config {
return &Config{
BaseConfig: TestBaseConfig(),
BaseConfig: testBaseConfig(),
RPC: rpc.TestRPCConfig(),
P2P: p2p.TestP2PConfig(),
Mempool: mem.TestMempoolConfig(),
Expand All @@ -95,21 +124,27 @@ func (cfg *Config) SetRootDir(root string) *Config {
cfg.P2P.RootDir = root
cfg.Mempool.RootDir = root
cfg.Consensus.RootDir = root

return cfg
}

// EnsureDirs ensures default directories in root dir (and root dir).
func (cfg *Config) EnsureDirs() {
func (cfg *Config) EnsureDirs() error {
rootDir := cfg.BaseConfig.RootDir

if err := osm.EnsureDir(rootDir, DefaultDirPerm); err != nil {
panic(err.Error())
return fmt.Errorf("no root directory, %w", err)
}

if err := osm.EnsureDir(filepath.Join(rootDir, defaultConfigDir), DefaultDirPerm); err != nil {
panic(err.Error())
return fmt.Errorf("no config directory, %w", err)
}

if err := osm.EnsureDir(filepath.Join(rootDir, defaultDataDir), DefaultDirPerm); err != nil {
panic(err.Error())
return fmt.Errorf("no data directory, %w", err)
}

return nil
}

// ValidateBasic performs basic validation (checking param bounds, etc.) and
Expand Down Expand Up @@ -172,18 +207,18 @@ type BaseConfig struct {
// TCP or UNIX socket address of the ABCI application,
// or the name of an ABCI application compiled in with the Tendermint binary,
// or empty if local application instance.
ProxyApp string `toml:"proxy_app"`
ProxyApp string `toml:"proxy_app" comment:"TCP or UNIX socket address of the ABCI application, \n or the name of an ABCI application compiled in with the Tendermint binary"`

// Local application instance in lieu of remote app.
LocalApp abci.Application

// A custom human readable name for this node
Moniker string `toml:"moniker"`
Moniker string `toml:"moniker" comment:"A custom human readable name for this node"`

// If this node is many blocks behind the tip of the chain, FastSync
// allows them to catchup quickly by downloading blocks in parallel
// and verifying their commits
FastSyncMode bool `toml:"fast_sync"`
FastSyncMode bool `toml:"fast_sync" comment:"If this node is many blocks behind the tip of the chain, FastSync\n allows them to catchup quickly by downloading blocks in parallel\n and verifying their commits"`

// Database backend: goleveldb | cleveldb | boltdb
// * goleveldb (github.com/gnolang/goleveldb - most popular implementation)
Expand All @@ -197,42 +232,42 @@ type BaseConfig struct {
// - EXPERIMENTAL
// - may be faster is some use-cases (random reads - indexer)
// - use boltdb build tag (go build -tags boltdb)
DBBackend string `toml:"db_backend"`
DBBackend string `toml:"db_backend" comment:"Database backend: goleveldb | cleveldb | boltdb\n * goleveldb (github.com/gnolang/goleveldb - most popular implementation)\n - pure go\n - stable\n * cleveldb (uses levigo wrapper)\n - fast\n - requires gcc\n - use cleveldb build tag (go build -tags cleveldb)\n * boltdb (uses etcd's fork of bolt - go.etcd.io/bbolt)\n - EXPERIMENTAL\n - may be faster is some use-cases (random reads - indexer)\n - use boltdb build tag (go build -tags boltdb)"`

// Database directory
DBPath string `toml:"db_dir"`
DBPath string `toml:"db_dir" comment:"Database directory"`

// Output level for logging
LogLevel string `toml:"log_level"`
LogLevel string `toml:"log_level" comment:"Output level for logging, including package level options"`

// Output format: 'plain' (colored text) or 'json'
LogFormat string `toml:"log_format"`
LogFormat string `toml:"log_format" comment:"Output format: 'plain' (colored text) or 'json'"`

// Path to the JSON file containing the initial validator set and other meta data
Genesis string `toml:"genesis_file"`
Genesis string `toml:"genesis_file" comment:"Path to the JSON file containing the initial validator set and other meta data"`

// Path to the JSON file containing the private key to use as a validator in the consensus protocol
PrivValidatorKey string `toml:"priv_validator_key_file"`
PrivValidatorKey string `toml:"priv_validator_key_file" comment:"Path to the JSON file containing the private key to use as a validator in the consensus protocol"`

// Path to the JSON file containing the last sign state of a validator
PrivValidatorState string `toml:"priv_validator_state_file"`
PrivValidatorState string `toml:"priv_validator_state_file" comment:"Path to the JSON file containing the last sign state of a validator"`

// TCP or UNIX socket address for Tendermint to listen on for
// connections from an external PrivValidator process
PrivValidatorListenAddr string `toml:"priv_validator_laddr"`
PrivValidatorListenAddr string `toml:"priv_validator_laddr" comment:"TCP or UNIX socket address for Tendermint to listen on for\n connections from an external PrivValidator process"`

// A JSON file containing the private key to use for p2p authenticated encryption
NodeKey string `toml:"node_key_file"`
NodeKey string `toml:"node_key_file" comment:"Path to the JSON file containing the private key to use for node authentication in the p2p protocol"`

// Mechanism to connect to the ABCI application: local | socket
ABCI string `toml:"abci"`
ABCI string `toml:"abci" comment:"Mechanism to connect to the ABCI application: socket | grpc"`

// TCP or UNIX socket address for the profiling server to listen on
ProfListenAddress string `toml:"prof_laddr"`
ProfListenAddress string `toml:"prof_laddr" comment:"TCP or UNIX socket address for the profiling server to listen on"`

// If true, query the ABCI app on connecting to a new peer
// so the app can decide if we should keep the connection or not
FilterPeers bool `toml:"filter_peers"` // false
FilterPeers bool `toml:"filter_peers" comment:"If true, query the ABCI app on connecting to a new peer\n so the app can decide if we should keep the connection or not"` // false
}

// DefaultBaseConfig returns a default base configuration for a Tendermint node
Expand All @@ -255,8 +290,8 @@ func DefaultBaseConfig() BaseConfig {
}
}

// TestBaseConfig returns a base configuration for testing a Tendermint node
func TestBaseConfig() BaseConfig {
// testBaseConfig returns a base configuration for testing a Tendermint node
func testBaseConfig() BaseConfig {
cfg := DefaultBaseConfig()
cfg.chainID = "tendermint_test"
cfg.ProxyApp = "mock://kvstore"
Expand Down
Loading

0 comments on commit 5c431df

Please sign in to comment.