fix: config.toml and app.toml overwrites

CHANGELOG.md

@@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * [#8006](https://github.com/osmosis-labs/osmosis/pull/8006), [#8014](https://github.com/osmosis-labs/osmosis/pull/8014) Speedup many BigDec operations
 * [#8030](https://github.com/osmosis-labs/osmosis/pull/8030) Delete legacy behavior where lockups could not unbond at very small block heights on a testnet
+* [#8118](https://github.com/osmosis-labs/osmosis/pull/8118) Config.toml and app.toml overwrite improvements
 
 ## v24.0.1
 

cmd/osmosisd/cmd/root.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	// "fmt"
 
+	b "bytes"
 	"embed"
 	"encoding/json"
 	"fmt"
@@ -12,11 +13,10 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
-	"time"
+	"text/template"
 
 	rosettaCmd "cosmossdk.io/tools/rosetta/cmd"
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/spf13/viper"
 
 	cometbftdb "github.com/cometbft/cometbft-db"
 
@@ -99,7 +99,31 @@ type DenomUnitMap struct {
 	Exponent uint64 `json:"exponent"`
 }
 
+type OsmosisMempoolConfig struct {
+	MaxGasWantedPerTx         string `mapstructure:"max-gas-wanted-per-tx"`
+	MinGasPriceForArbitrageTx string `mapstructure:"arbitrage-min-gas-fee"`
+	MinGasPriceForHighGasTx   string `mapstructure:"min-gas-price-for-high-gas-tx"`
+	Mempool1559Enabled        string `mapstructure:"adaptive-fee-enabled"`
+}
+
+var DefaultOsmosisMempoolConfig = OsmosisMempoolConfig{
+	MaxGasWantedPerTx:         "60000000",
+	MinGasPriceForArbitrageTx: ".1",
+	MinGasPriceForHighGasTx:   ".0025",
+	Mempool1559Enabled:        "true",
+}
+
+type CustomAppConfig struct {
+	serverconfig.Config
+	OsmosisMempoolConfig     OsmosisMempoolConfig `mapstructure:"osmosis-mempool"`
+	SidecarQueryServerConfig sqs.Config           `mapstructure:"osmosis-sqs"`
+	WasmConfig               wasmtypes.WasmConfig `mapstructure:"wasm"`
+}
+
+var OsmosisAppTemplate string
+
 const (
+	// app.toml
 	mempoolConfigName = "osmosis-mempool"
 
 	arbitrageMinGasFeeConfigName          = "arbitrage-min-gas-fee"
@@ -108,8 +132,11 @@ const (
 	maxGasWantedPerTxName                = "max-gas-wanted-per-tx"
 	recommendedNewMaxGasWantedPerTxValue = "60000000"
 
-	consensusConfigName     = "consensus"
-	timeoutCommitConfigName = "timeout_commit"
+	// config.toml
+	consensusConfigName = "consensus"
+
+	timeoutCommitConfigName          = "timeout_commit"
+	recommendedNewTimeoutCommitValue = "2s"
 )
 
 var (
@@ -422,10 +449,19 @@ func NewRootCmd() (*cobra.Command, params.EncodingConfig) {
 	return rootCmd, encodingConfig
 }
 
-// overwrites config.toml values if it exists, otherwise it writes the default config.toml
+// overwriteConfigTomlValues overwrites config.toml values. Returns error if config.toml does not exist
+//
+// Currently, overwrites:
+// - timeout_commit value in config.toml to 2s.
+//
+// Also overwrites the respective viper config value.
+//
+// Silently handles and skips any error/panic due to write permission issues.
+// No-op otherwise.
 func overwriteConfigTomlValues(serverCtx *server.Context) error {
 	// Get paths to config.toml and config parent directory
 	rootDir := serverCtx.Viper.GetString(tmcli.HomeFlag)
+
 	configParentDirPath := filepath.Join(rootDir, "config")
 	configFilePath := filepath.Join(configParentDirPath, "config.toml")
 
@@ -438,30 +474,10 @@ func overwriteConfigTomlValues(serverCtx *server.Context) error {
 	} else {
 		// config.toml exists
 
-		// Create a copy since the original viper from serverCtx
-		// contains app.toml config values that would get overwritten
-		// by ReadInConfig below.
-		viperCopy := viper.New()
-
-		viperCopy.SetConfigType("toml")
-		viperCopy.SetConfigName("config")
-		viperCopy.AddConfigPath(configParentDirPath)
-
-		// We read it in and modify the consensus timeout commit
-		// and write it back.
-		if err := viperCopy.ReadInConfig(); err != nil {
-			return fmt.Errorf("failed to read in %s: %w", configFilePath, err)
-		}
-
-		consensusValues := viperCopy.GetStringMapString(consensusConfigName)
-		timeoutCommitValue, ok := consensusValues[timeoutCommitConfigName]
-		if !ok {
-			return fmt.Errorf("failed to get %s.%s from %s: %w", consensusConfigName, timeoutCommitValue, configFilePath, err)
-		}
-
-		// The original default is 5s and is set in Cosmos SDK.
-		// We lower it to 2s for faster block times.
-		serverCtx.Config.Consensus.TimeoutCommit = 2 * time.Second
+		// Set new values in viper
+		//
+		// Set timeout_commit to 2s
+		serverCtx.Viper.Set(consensusConfigName+"."+timeoutCommitConfigName, recommendedNewTimeoutCommitValue)
 
 		defer func() {
 			if err := recover(); err != nil {
@@ -474,75 +490,83 @@ func overwriteConfigTomlValues(serverCtx *server.Context) error {
 			// It will be re-read in server.InterceptConfigsPreRunHandler
 			// this may panic for permissions issues. So we catch the panic.
 			// Note that this exits with a non-zero exit code if fails to write the file.
+
+			// Write the new config.toml file
 			tmcfg.WriteConfigFile(configFilePath, serverCtx.Config)
+		} else {
+			fmt.Println("config.toml is not writable. Cannot apply update. Please consder manually changing timeout_commit to " + recommendedNewTimeoutCommitValue)
 		}
 	}
 	return nil
 }
 
-// overwrites app.toml values. Returns error if app.toml does not exist
+// overwriteAppTomlValues overwrites app.toml values. Returns error if app.toml does not exist
+//
+// Currently, overwrites:
+// - arbitrage-min-gas-fee value in app.toml to 0.1.
+// - max-gas-wanted-per-tx value in app.toml to 60000000.
+//
+// Also overwrites the respective viper config value.
 //
-// Currently, overwrites arbitrage-min-gas-fee value in app.toml if it is set to 0.005. Similarly,
-// overwrites the given viper config value.
 // Silently handles and skips any error/panic due to write permission issues.
 // No-op otherwise.
 func overwriteAppTomlValues(serverCtx *server.Context) error {
-	// Get paths to config.toml and config parent directory
+	// Get paths to app.toml and config parent directory
 	rootDir := serverCtx.Viper.GetString(tmcli.HomeFlag)
 
 	configParentDirPath := filepath.Join(rootDir, "config")
-	configFilePath := filepath.Join(configParentDirPath, "app.toml")
+	appFilePath := filepath.Join(configParentDirPath, "app.toml")
 
-	fileInfo, err := os.Stat(configFilePath)
+	fileInfo, err := os.Stat(appFilePath)
 	if err != nil {
-		return fmt.Errorf("failed to read in %s: %w", configFilePath, err)
+		// something besides a does not exist error
+		if !os.IsNotExist(err) {
+			return fmt.Errorf("failed to read in %s: %w", appFilePath, err)
+		}
 	} else {
 		// app.toml exists
 
 		// Set new values in viper
+		//
+		// Set max-gas-wanted-per-tx to 60000000
 		serverCtx.Viper.Set(mempoolConfigName+"."+maxGasWantedPerTxName, recommendedNewMaxGasWantedPerTxValue)
+		// Set arbitrage-min-gas-fee to 0.1
 		serverCtx.Viper.Set(mempoolConfigName+"."+arbitrageMinGasFeeConfigName, recommendedNewArbitrageMinGasFeeValue)
 
+		// Unmarshal viper baseConfig to get the appConfig struct
+		// This must be done separately from the other appConfig values
+		// because the base configs aren't prepended with any specific key
+		var appConfig serverconfig.Config
+		if err := serverCtx.Viper.Unmarshal(&appConfig); err != nil {
+			return fmt.Errorf("failed to unmarshal viper appConfig: %w", err)
+		}
+
+		// Unmarshal the remaining viper app configs to get the remainingAppConfigs struct
+		// This houses the osmosis mempool, sidecar query server, and wasm configs
+		var remainingAppConfigs CustomAppConfig
+		if err := serverCtx.Viper.Unmarshal(&remainingAppConfigs); err != nil {
+			return fmt.Errorf("failed to unmarshal viper config: %w", err)
+		}
+
+		// Create a custom app config struct that brings together the appConfig and the remainingAppConfigs
+		customAppConfig := CustomAppConfig{Config: appConfig, OsmosisMempoolConfig: remainingAppConfigs.OsmosisMempoolConfig, SidecarQueryServerConfig: remainingAppConfigs.SidecarQueryServerConfig, WasmConfig: remainingAppConfigs.WasmConfig}
+
 		defer func() {
 			if err := recover(); err != nil {
-				fmt.Printf("failed to write to %s: %s\n", configFilePath, err)
+				fmt.Printf("failed to write to %s: %s\n", appFilePath, err)
 			}
 		}()
 
 		// Check if the file is writable
 		if fileInfo.Mode()&os.FileMode(0200) != 0 {
-			// Read the entire content of the file
-			content, err := os.ReadFile(configFilePath)
-			if err != nil {
-				return err
-			}
-
-			// Convert the content to a string
-			fileContent := string(content)
-
-			// Find the index of the search line
-			index := strings.Index(fileContent, arbitrageMinGasFeeConfigName)
-			if index == -1 {
-				return fmt.Errorf("search line not found in the file")
-			}
-
-			// Find the opening and closing quotes
-			openQuoteIndex := strings.Index(fileContent[index:], "\"")
-			openQuoteIndex += index
-
-			closingQuoteIndex := strings.Index(fileContent[openQuoteIndex+1:], "\"")
-			closingQuoteIndex += openQuoteIndex + 1
-
-			// Replace the old value with the new value
-			newFileContent := fileContent[:openQuoteIndex+1] + recommendedNewArbitrageMinGasFeeValue + fileContent[closingQuoteIndex:]
+			// It will be re-read in server.InterceptConfigsPreRunHandler
+			// this may panic for permissions issues. So we catch the panic.
+			// Note that this exits with a non-zero exit code if fails to write the file.
 
-			// Write the modified content back to the file
-			err = os.WriteFile(configFilePath, []byte(newFileContent), 0644)
-			if err != nil {
-				return err
-			}
+			// Write the new app.toml file
+			WriteCustomAppConfigFile(appFilePath, customAppConfig)
 		} else {
-			fmt.Println("app.toml is not writable. Cannot apply update. Please consder manually changing arbitrage-min-gas-fee to " + recommendedNewArbitrageMinGasFeeValue + "and max-gas-wanted-per-tx to " + recommendedNewMaxGasWantedPerTxValue)
+			fmt.Println("app.toml is not writable. Cannot apply update. Please consder manually changing arbitrage-min-gas-fee to " + recommendedNewArbitrageMinGasFeeValue + " and max-gas-wanted-per-tx to " + recommendedNewMaxGasWantedPerTxValue)
 		}
 	}
 	return nil
@@ -565,20 +589,6 @@ func getHomeEnvironment() string {
 // initAppConfig helps to override default appConfig template and configs.
 // return "", nil if no custom configuration is required for the application.
 func initAppConfig() (string, interface{}) {
-	type OsmosisMempoolConfig struct {
-		ArbitrageMinGasPrice string `mapstructure:"arbitrage-min-gas-fee"`
-	}
-
-	type CustomAppConfig struct {
-		serverconfig.Config
-
-		OsmosisMempoolConfig OsmosisMempoolConfig `mapstructure:"osmosis-mempool"`
-
-		SidecarQueryServerConfig sqs.Config `mapstructure:"osmosis-sqs"`
-
-		Wasm wasmtypes.WasmConfig `mapstructure:"wasm"`
-	}
-
 	// Optionally allow the chain developer to overwrite the SDK's default
 	// server config.
 	srvCfg := serverconfig.DefaultConfig()
@@ -589,32 +599,34 @@ func initAppConfig() (string, interface{}) {
 	// 128MB IAVL cache
 	srvCfg.IAVLCacheSize = 781250
 
-	memCfg := OsmosisMempoolConfig{ArbitrageMinGasPrice: "0.1"}
+	memCfg := DefaultOsmosisMempoolConfig
+
+	sqsCfg := sqs.DefaultConfig
 
-	sqsConfig := sqs.DefaultConfig
+	wasmCfg := wasmtypes.DefaultWasmConfig()
 
-	OsmosisAppCfg := CustomAppConfig{Config: *srvCfg, OsmosisMempoolConfig: memCfg, SidecarQueryServerConfig: sqsConfig, Wasm: wasmtypes.DefaultWasmConfig()}
+	OsmosisAppCfg := CustomAppConfig{Config: *srvCfg, OsmosisMempoolConfig: memCfg, SidecarQueryServerConfig: sqsCfg, WasmConfig: wasmCfg}
 
-	OsmosisAppTemplate := serverconfig.DefaultConfigTemplate + `
+	OsmosisAppTemplate = serverconfig.DefaultConfigTemplate + `
 ###############################################################################
 ###                      Osmosis Mempool Configuration                      ###
 ###############################################################################
 
 [osmosis-mempool]
 # This is the max allowed gas any tx.
 # This is only for local mempool purposes, and thus	is only ran on check tx.
-max-gas-wanted-per-tx = "60000000"
+max-gas-wanted-per-tx = "{{ .OsmosisMempoolConfig.MaxGasWantedPerTx }}"
 
 # This is the minimum gas fee any arbitrage tx should have, denominated in uosmo per gas
 # Default value of ".1" then means that a tx with 1 million gas costs (.1 uosmo/gas) * 1_000_000 gas = .1 osmo
-arbitrage-min-gas-fee = ".1"
+arbitrage-min-gas-fee = "{{ .OsmosisMempoolConfig.MinGasPriceForArbitrageTx }}"
 
 # This is the minimum gas fee any tx with high gas demand should have, denominated in uosmo per gas
 # Default value of ".0025" then means that a tx with 1 million gas costs (.0025 uosmo/gas) * 1_000_000 gas = .0025 osmo
-min-gas-price-for-high-gas-tx = ".0025"
+min-gas-price-for-high-gas-tx = "{{ .OsmosisMempoolConfig.MinGasPriceForHighGasTx }}"
 
 # This parameter enables EIP-1559 like fee market logic in the mempool
-adaptive-fee-enabled = "true"
+adaptive-fee-enabled = "{{ .OsmosisMempoolConfig.Mempool1559Enabled }}"
 
 ###############################################################################
 ###              Osmosis Sidecar Query Server Configuration                 ###
@@ -623,17 +635,29 @@ adaptive-fee-enabled = "true"
 [osmosis-sqs]
 
 # SQS service is disabled by default.
-is-enabled = "false"
+is-enabled = "{{ .SidecarQueryServerConfig.IsEnabled }}"
 
 # The hostname of the GRPC sqs service
 grpc-ingest-address = "{{ .SidecarQueryServerConfig.GRPCIngestAddress }}"
 # The maximum size of the GRPC message that can be received by the sqs service in bytes.
 grpc-ingest-max-call-size-bytes = "{{ .SidecarQueryServerConfig.GRPCIngestMaxCallSizeBytes }}"
 
 ###############################################################################
-###              		       Wasm Configuration    					    ###
+###                            Wasm Configuration                           ###
 ###############################################################################
-` + wasmtypes.DefaultConfigTemplate()
+
+[wasm]
+# Smart query gas limit is the max gas to be used in a smart query contract call
+query_gas_limit = {{ .WasmConfig.SmartQueryGasLimit }}
+
+# in-memory cache for Wasm contracts. Set to 0 to disable.
+# The value is in MiB not bytes
+memory_cache_size = {{ .WasmConfig.MemoryCacheSize }}
+
+# Simulation gas limit is the max gas to be used in a tx simulation call.
+# When not set the consensus max block gas is used instead
+# simulation_gas_limit =
+`
 
 	return OsmosisAppTemplate, OsmosisAppCfg
 }
@@ -1034,3 +1058,37 @@ func transformCoinValueToBaseInt(coinValue, coinDenom string, assetMap map[strin
 	}
 	return "", fmt.Errorf("denom %s not found in asset map", coinDenom)
 }
+
+// WriteCustomAppConfigFile first checks the provided config for a special case with the wasm config.
+// This determines what the template should be. Based on this, it writes the new app.toml config file.
+func WriteCustomAppConfigFile(configFilePath string, config CustomAppConfig) {
+	// There is a single wasm config that requires special logic to handle
+	// For some reason, they base a value on whether a line is commented out or not...
+
+	// Check if the simulation_gas_limit line should be commented out (it is either nil or 0)
+	isCommentedOut := config.WasmConfig.SimulationGasLimit == nil || *config.WasmConfig.SimulationGasLimit == 0
+
+	// Modify the template by uncommenting the line if a value is provided for simulation_gas_limit
+	if !isCommentedOut {
+		OsmosisAppTemplate = strings.Replace(OsmosisAppTemplate, "# simulation_gas_limit =", "simulation_gas_limit = {{ .WasmConfig.SimulationGasLimit }}", 1)
+	}
+
+	// Create a template object via the app template string
+	var buffer b.Buffer
+	tmpl := template.New("appConfigFileTemplate")
+	configTemplate, err := tmpl.Parse(OsmosisAppTemplate)
+	if err != nil {
+		panic(err)
+	}
+
+	// Execute the template with the provided config
+	if err := configTemplate.Execute(&buffer, config); err != nil {
+		panic(err)
+	}
+
+	// Write the new app.toml file
+	if err := os.WriteFile(configFilePath, buffer.Bytes(), 0o644); err != nil {
+		fmt.Printf(fmt.Sprintf("failed to write file: %v", err) + "\n")
+		os.Exit(1)
+	}
+}