Skip to content

Commit

Permalink
chore: default TimeoutCommit to 3s (allow opt out of default override…
Browse files Browse the repository at this point in the history
…s) (backport #7769) (#7782)

* chore: default TimeoutCommit to 3s (allow opt out of default overrides) (#7769)

* overwrite with defaults flag

* add logic gate for overriding

* add log line

* make verbiage clearer

* add changelog

* dont check for specific value

* unused var

(cherry picked from commit bf01cf0)

* reject conflig defaults

* reject defaults

---------

Co-authored-by: Adam Tucker <[email protected]>
Co-authored-by: Adam Tucker <[email protected]>
  • Loading branch information
3 people authored Mar 20, 2024
1 parent ec352ee commit 24a4965
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 28 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#7665](https://github.com/osmosis-labs/osmosis/pull/7665) feat(x/protorev): Use Transient store to store swap backruns.
* [#7503](https://github.com/osmosis-labs/osmosis/pull/7503) Add IBC wasm light clients module

## v23.0.8-iavl-v1 & v23.0.8

* [#7769](https://github.com/osmosis-labs/osmosis/pull/7769) Set and default timeout commit to 3s. Add flag to prevent custom overrides if not desired.

## v23.0.7-iavl-v1

* [#7750](https://github.com/osmosis-labs/osmosis/pull/7750) IAVL bump to improve pruning
Expand Down
7 changes: 5 additions & 2 deletions cmd/osmosisd/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ const (

// FlagSetEnv defines a flag to create environment file & save current home directory into it.
FlagSetEnv = "set-env"

// FlagRejectConfigDefaults defines a flag to reject some select defaults that override what is in the config file.
FlagRejectConfigDefaults = "reject-config-defaults"
)

type printInfo struct {
Expand Down Expand Up @@ -109,8 +112,8 @@ func InitCmd(mbm module.BasicManager, defaultNodeHome string) *cobra.Command {
config.StateSync.TrustPeriod = 112 * time.Hour

// The original default is 5s and is set in Cosmos SDK.
// We lower it to 4s for faster block times.
config.Consensus.TimeoutCommit = 4 * time.Second
// We lower it to 3s for faster block times.
config.Consensus.TimeoutCommit = 3 * time.Second

config.SetRoot(clientCtx.HomeDir)

Expand Down
41 changes: 23 additions & 18 deletions cmd/osmosisd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,9 @@ const (

var (
//go:embed "osmosis-1-assetlist.json" "osmo-test-5-assetlist.json"
assetFS embed.FS
mainnetId = "osmosis-1"
testnetId = "osmo-test-5"
fiveSecondsString = (5 * time.Second).String()
assetFS embed.FS
mainnetId = "osmosis-1"
testnetId = "osmo-test-5"
)

func loadAssetList(initClientCtx client.Context, cmd *cobra.Command, basedenomToIBC, IBCtoBasedenom bool) (map[string]DenomUnitMap, map[string]string) {
Expand Down Expand Up @@ -440,7 +439,7 @@ func overwriteConfigTomlValues(serverCtx *server.Context) error {
// It does not exist, so we update the default config.toml to update
// We modify the default config.toml to have faster block times
// It will be written by server.InterceptConfigsPreRunHandler
tmcConfig.Consensus.TimeoutCommit = 4 * time.Second
tmcConfig.Consensus.TimeoutCommit = 3 * time.Second
} else {
// config.toml exists

Expand All @@ -466,10 +465,8 @@ func overwriteConfigTomlValues(serverCtx *server.Context) error {
}

// The original default is 5s and is set in Cosmos SDK.
// We lower it to 4s for faster block times.
if timeoutCommitValue == fiveSecondsString {
serverCtx.Config.Consensus.TimeoutCommit = 4 * time.Second
}
// We lower it to 3s for faster block times.
serverCtx.Config.Consensus.TimeoutCommit = 3 * time.Second

defer func() {
if err := recover(); err != nil {
Expand Down Expand Up @@ -700,16 +697,23 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
cmd.RunE = func(cmd *cobra.Command, args []string) error {
serverCtx := server.GetServerContextFromCmd(cmd)

// overwrite config.toml values
err := overwriteConfigTomlValues(serverCtx)
if err != nil {
return err
}
// Get flag value for rejecting config defaults
rejectConfigDefaults := serverCtx.Viper.GetBool(FlagRejectConfigDefaults)

// overwrite app.toml values
err = overwriteAppTomlValues(serverCtx)
if err != nil {
return err
// overwrite config.toml and app.toml values, if rejectConfigDefaults is false
if !rejectConfigDefaults {
// Add ctx logger line to indicate that config.toml and app.toml values are being overwritten
serverCtx.Logger.Info("Overwriting config.toml and app.toml values with some recommended defaults. To prevent this, set the --reject-config-defaults flag to true.")

err := overwriteConfigTomlValues(serverCtx)
if err != nil {
return err
}

err = overwriteAppTomlValues(serverCtx)
if err != nil {
return err
}
}

return startRunE(cmd, args)
Expand All @@ -734,6 +738,7 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
func addModuleInitFlags(startCmd *cobra.Command) {
crisis.AddModuleInitFlags(startCmd)
wasm.AddModuleInitFlags(startCmd)
startCmd.Flags().Bool(FlagRejectConfigDefaults, false, "Reject some select recommended default values from being automatically set in the config.toml and app.toml")
}

// queryCommand adds transaction and account querying commands.
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/configurer/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ func (bc *baseConfigurer) runValidators(chainConfig *chain.Config) error {
// Iterate over each node
for _, node := range chainConfig.NodeConfigs {
go func(n *chain.NodeConfig) {
defer wg.Done() // Decrement the WaitGroup counter when the goroutine is done
errCh <- n.Run() // Run the node and send any error to the channel
defer wg.Done() // Decrement the WaitGroup counter when the goroutine is done
errCh <- n.Run(false) // Run the node and send any error to the channel
}(node)
}

Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/configurer/chain/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ func NewNodeConfig(t *testing.T, initNode *initialization.Node, initConfig *init
// Run runs a node container for the given nodeIndex.
// The node configuration must be already added to the chain config prior to calling this
// method.
func (n *NodeConfig) Run() error {
func (n *NodeConfig) Run(rejectConfigDefaults bool) error {
maxRetries := 3
currentRetry := 0

for currentRetry < maxRetries {
n.t.Logf("starting node container: %s", n.Name)
resource, err := n.containerManager.RunNodeResource(n.chainId, n.Name, n.ConfigDir)
resource, err := n.containerManager.RunNodeResource(n.chainId, n.Name, n.ConfigDir, rejectConfigDefaults)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/configurer/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ func (uc *UpgradeConfigurer) upgradeContainers(chainConfig *chain.Config, propHe
uc.containerManager.OsmosisTag = containers.CurrentBranchOsmoTag

for _, node := range chainConfig.NodeConfigs {
if err := node.Run(); err != nil {
if err := node.Run(true); err != nil {
return err
}
}
Expand Down
9 changes: 7 additions & 2 deletions tests/e2e/containers/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,19 +472,24 @@ func (m *Manager) RunHermesResource(chainAID, osmoARelayerNodeName, osmoAValMnem

// RunNodeResource runs a node container. Assigns containerName to the container.
// Mounts the container on valConfigDir volume on the running host. Returns the container resource and error if any.
func (m *Manager) RunNodeResource(chainId string, containerName, valCondifDir string) (*dockertest.Resource, error) {
func (m *Manager) RunNodeResource(chainId string, containerName, valCondifDir string, rejectConfigDefaults bool) (*dockertest.Resource, error) {
pwd, err := os.Getwd()
if err != nil {
return nil, err
}

cmd := []string{"start"}
if rejectConfigDefaults {
cmd = append(cmd, "--reject-config-defaults=true")
}

runOpts := &dockertest.RunOptions{
Name: containerName,
Repository: m.OsmosisRepository,
Tag: m.OsmosisTag,
NetworkID: m.network.Network.ID,
User: "root:root",
Cmd: []string{"start"},
Cmd: cmd,
Mounts: []string{
fmt.Sprintf("%s/:/osmosis/.osmosisd", valCondifDir),
fmt.Sprintf("%s/scripts:/osmosis", pwd),
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (s *IntegrationTestSuite) StateSync() {
chainANode.WaitUntil(hasSnapshotsAvailable)

// start the state synchin node.
err = stateSynchingNode.Run()
err = stateSynchingNode.Run(true)
s.Require().NoError(err)

// ensure that the state syncing node cathes up to the running node.
Expand Down

0 comments on commit 24a4965

Please sign in to comment.