Skip to content

Commit

Permalink
Create pre-upgrade command. Fix minimum-gas-prices repacking unset. (#…
Browse files Browse the repository at this point in the history 
…1594)

* Create a new pre_upgrade command that updates the config files. Make it also set the commit timeout if needed. Make it so that stuff that uses the global viper also gets everything that's needed.

* Force the timeout commit to 5s in all cases. Add some comments and info about what cosmovisor expects.

* Undo the PersistentPreRunE extraction change since I ended up not needing it for the unit tests.

* Fix the existing LogIfError functions.

* Tweak errors returned from pre-upgrade to contain more context and still be indicate the desired exit code.

* Allow creation of the root command that won't seal the app config so we can run it multiple times, e.g. from unit tests. Also, add extra handling of ErrorCodes to main since it's sometimes a pointer and sometimes not.

* Add unit tests on the pre-upgrade command.

* Add a unit test for when the config can't be written.

* Add newline to end of success message.

* Move the global min-gas-prices setting stuff into the interceptor so that it happens before the config files are loaded or any defaults are requested.

* For the init cmd test, don't seal the config (so other things that might need to can do so), and use a temp directory instead of the default.

* Update some tests that started failing because the default timeout_commit changed.

* Just pass sealConfig into the SetConfig function instead of optionally calling it.

* For the pre-unpgrade tests, just add the home dir as a flag for the command.

* Add temporary changelog entries.

* Remove some lines added just for debugging.

* When starting a node with a timeout_commit of less than 4 seconds, output a message stating that it should be set to 5s.

* Update the initialization script to put the timeout_commit back to 1s so that make run cuts blocks faster.

* Only issue low timeout_commit warning on mainnet.

* Replace some deprecated stuff and fix some receivers.

* During pre-upgrae Only update the configured value if it's lower than half the default.

* Only log the warning on mainnet and only if it's less than half the default.

* Add the --timeout-commit flag to the init command. If provided, use that. Otherwise, if something other than mainnet or testnet, use 1s. Otherwise, use whatever was previously in their config or the default if it's brand new.

* Update the initialize script to provide the --timeout-commit to init instead of setting it using the config command at the end.

* Upper-case the first letter of all commmand and flag usage strings. Fix the debug pubkey-raw command that had a conflicting shorthand flag being added.

* Fix the --help flag capitalization.

* Undo the usage capitalization stuff. That's a bit too off-topic for this PR.

* In the pre-upgrade command only change the timeout_commit for mainnet nodes.

* Get rid of the global ChainID variable (in cmd/root.go) since it wasn't used before this PR and it wasn't really worthwhile even after setting it due to the package it's in.
  • Loading branch information
@SpicyLemon
SpicyLemon committed Jun 20, 2023
1 parent fd3c877 commit 968f15b
Show file tree
Hide file tree
Showing 15 changed files with 852 additions and 109 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,14 @@ Ref: https://keepachangelog.com/en/1.0.0/

## Unreleased

### Features

* Add pre-upgrade command that updates config files to newest format and sets `consensus.timeout_commit` to `5s` [PR 1594](https://github.com/provenance-io/provenance/pull/1594)

### Bug Fixes

* Add `NewUpdateAccountAttributeExpirationCmd` to the CLI [#1592](https://github.com/provenance-io/provenance/issues/1592).
* Fix `minimum-gas-prices` from sometimes getting unset in the configs [PR 1594](https://github.com/provenance-io/provenance/pull/1594)

---

Expand Down
3 changes: 1 addition & 2 deletions app/upgrades_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ func (s *UpgradeTestSuite) GetLogOutput(msg string, args ...interface{}) string
// Use this if there's a possible error that we probably don't care about (but might).
func (s *UpgradeTestSuite) LogIfError(err error, format string, args ...interface{}) {
if err != nil {
args = append(args, err)
s.T().Logf(format+": %v", args)
s.T().Logf(format+" error: %v", append(args, err)...)
}
}

Expand Down
4 changes: 3 additions & 1 deletion cmd/provenanced/cmd/cmd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ import (
)

func TestInitCmd(t *testing.T) {
rootCmd, _ := cmd.NewRootCmd()
home := t.TempDir()
rootCmd, _ := cmd.NewRootCmd(false)
rootCmd.SetArgs([]string{
"--home", home,
"init", // Test the init cmd
"simapp-test", // Moniker
fmt.Sprintf("--%s=%s", cli.FlagOverwrite, "true"), // Overwrite genesis.json, in case it already exists
Expand Down
81 changes: 40 additions & 41 deletions cmd/provenanced/cmd/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"io"
"os"
"regexp"
"strings"
Expand All @@ -17,16 +17,15 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"github.com/tendermint/tendermint/libs/log"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/server"
sdksim "github.com/cosmos/cosmos-sdk/simapp"

"github.com/provenance-io/provenance/cmd/provenanced/cmd"
provconfig "github.com/provenance-io/provenance/cmd/provenanced/config"
"github.com/provenance-io/provenance/internal/pioconfig"

tmconfig "github.com/tendermint/tendermint/config"
"github.com/tendermint/tendermint/libs/log"
)

type ConfigTestSuite struct {
Expand Down Expand Up @@ -57,7 +56,7 @@ func (s *ConfigTestSuite) SetupTest() {
WithCodec(encodingConfig.Codec).
WithHomeDir(s.Home)
clientCtx.Viper = viper.New()
serverCtx := server.NewContext(clientCtx.Viper, tmconfig.DefaultConfig(), log.NewNopLogger())
serverCtx := server.NewContext(clientCtx.Viper, provconfig.DefaultTmConfig(), log.NewNopLogger())

ctx := context.Background()
ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx)
Expand Down Expand Up @@ -86,7 +85,7 @@ func TestConfigTestSuite(t *testing.T) {
// Test setup above. Test helpers below.
//

func (s ConfigTestSuite) getConfigCmd() *cobra.Command {
func (s *ConfigTestSuite) getConfigCmd() *cobra.Command {
// What I really need here is a cobra.Command
// that already has a context.
// So I'm going to call --help on the config command
Expand All @@ -104,7 +103,7 @@ func (s ConfigTestSuite) getConfigCmd() *cobra.Command {
return configCmd
}

func (s ConfigTestSuite) ensureConfigFiles() {
func (s *ConfigTestSuite) ensureConfigFiles() {
configCmd := s.getConfigCmd()
// Extract the individual config objects.
appConfig, aerr := provconfig.ExtractAppConfig(configCmd)
Expand All @@ -118,66 +117,66 @@ func (s ConfigTestSuite) ensureConfigFiles() {
provconfig.SaveConfigs(configCmd, appConfig, tmConfig, clientConfig, false)
}

func (s ConfigTestSuite) makeConfigHeaderLine(t, fn string) string {
func (s *ConfigTestSuite) makeConfigHeaderLine(t, fn string) string {
return fmt.Sprintf("%s Config: %s/config/%s (or env)", t, s.Home, fn)
}

func (s ConfigTestSuite) makeAppConfigHeaderLines() string {
func (s *ConfigTestSuite) makeAppConfigHeaderLines() string {
return s.makeConfigHeaderLine(s.HeaderStrApp, s.BaseFNApp) + "\n----------------"
}

func (s ConfigTestSuite) makeTMConfigHeaderLines() string {
func (s *ConfigTestSuite) makeTMConfigHeaderLines() string {
return s.makeConfigHeaderLine(s.HeaderStrTM, s.BaseFNTM) + "\n-----------------------"
}

func (s ConfigTestSuite) makeClientConfigHeaderLines() string {
func (s *ConfigTestSuite) makeClientConfigHeaderLines() string {
return s.makeConfigHeaderLine(s.HeaderStrClient, s.baseFNClient) + "\n-------------------"
}

func (s ConfigTestSuite) makeConfigUpdatedLine(t, fn string) string {
func (s *ConfigTestSuite) makeConfigUpdatedLine(t, fn string) string {
return fmt.Sprintf("%s Config Updated: %s/config/%s", t, s.Home, fn)
}

func (s ConfigTestSuite) makeAppConfigUpdateLines() string {
func (s *ConfigTestSuite) makeAppConfigUpdateLines() string {
return s.makeConfigUpdatedLine(s.HeaderStrApp, s.BaseFNApp) + "\n------------------------"
}

func (s ConfigTestSuite) makeTMConfigUpdateLines() string {
func (s *ConfigTestSuite) makeTMConfigUpdateLines() string {
return s.makeConfigUpdatedLine(s.HeaderStrTM, s.BaseFNTM) + "\n-------------------------------"
}

func (s ConfigTestSuite) makeClientConfigUpdateLines() string {
func (s *ConfigTestSuite) makeClientConfigUpdateLines() string {
return s.makeConfigUpdatedLine(s.HeaderStrClient, s.baseFNClient) + "\n---------------------------"
}

func (s ConfigTestSuite) makeConfigDiffHeaderLine(t, fn string) string {
func (s *ConfigTestSuite) makeConfigDiffHeaderLine(t, fn string) string {
return fmt.Sprintf("%s Config Differences from Defaults: %s/config/%s (or env)", t, s.Home, fn)
}

func (s ConfigTestSuite) makeAppDiffHeaderLines() string {
func (s *ConfigTestSuite) makeAppDiffHeaderLines() string {
return s.makeConfigDiffHeaderLine(s.HeaderStrApp, s.BaseFNApp) + "\n------------------------------------------"
}

func (s ConfigTestSuite) makeTMDiffHeaderLines() string {
func (s *ConfigTestSuite) makeTMDiffHeaderLines() string {
return s.makeConfigDiffHeaderLine(s.HeaderStrTM, s.BaseFNTM) + "\n-------------------------------------------------"
}

func (s ConfigTestSuite) makeClientDiffHeaderLines() string {
func (s *ConfigTestSuite) makeClientDiffHeaderLines() string {
return s.makeConfigDiffHeaderLine(s.HeaderStrClient, s.baseFNClient) + "\n---------------------------------------------"
}

func (s ConfigTestSuite) makeMultiLine(lines ...string) string {
func (s *ConfigTestSuite) makeMultiLine(lines ...string) string {
return strings.Join(lines, "\n") + "\n"
}

func (s ConfigTestSuite) makeKeyUpdatedLine(key, oldVal, newVal string) string {
func (s *ConfigTestSuite) makeKeyUpdatedLine(key, oldVal, newVal string) string {
return fmt.Sprintf("%s Was: %s, Is Now: %s", key, oldVal, newVal)
}

func applyMockIODiscardOutErr(c *cobra.Command) *bytes.Buffer {
b := bytes.NewBufferString("")
c.SetOut(ioutil.Discard)
c.SetErr(ioutil.Discard)
c.SetOut(io.Discard)
c.SetErr(io.Discard)
return b
}

Expand Down Expand Up @@ -227,7 +226,7 @@ func (s *ConfigTestSuite) TestConfigCmdGet() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
s.Require().NoError(err, "%s - unexpected error executing configCmd", configCmd.Name())
out, err := ioutil.ReadAll(b)
out, err := io.ReadAll(b)
s.Require().NoError(err, "%s - unexpected error reading configCmd output", configCmd.Name())
outStr := string(out)

Expand All @@ -248,7 +247,7 @@ func (s *ConfigTestSuite) TestConfigCmdGet() {
// Tendermint header and a few entries.
{"tendermint header", regexp.MustCompile(`(?m)^Tendermint Config: .*/config/` + s.BaseFNTM + ` \(or env\)$`)},
{"tendermint fast_sync", regexp.MustCompile(`(?m)^fast_sync=true$`)},
{"tendermint consensus.timeout_commit", regexp.MustCompile(`(?m)^consensus.timeout_commit="1s"$`)},
{"tendermint consensus.timeout_commit", regexp.MustCompile(`(?m)^consensus.timeout_commit="5s"$`)},
{"tendermint mempool.size", regexp.MustCompile(`(?m)^mempool.size=5000$`)},
{"tendermint statesync.trust_period", regexp.MustCompile(`(?m)^statesync.trust_period="168h0m0s"$`)},
{"tendermint p2p.recv_rate", regexp.MustCompile(`(?m)^p2p.recv_rate=5120000$`)},
Expand Down Expand Up @@ -276,7 +275,7 @@ func (s *ConfigTestSuite) TestConfigCmdGet() {
b2 := applyMockIOOutErr(configCmd2)
err2 := configCmd2.Execute()
require.NoError(t, err2, "%s - unexpected error executing configCmd", configCmd2.Name())
out2, err2 := ioutil.ReadAll(b2)
out2, err2 := io.ReadAll(b2)
require.NoError(t, err2, "%s - unexpected error reading configCmd output", configCmd2.Name())
out2Str := string(out2)
require.Equal(t, outStr, out2Str, "output of no-args vs output of %s", args)
Expand Down Expand Up @@ -319,7 +318,7 @@ func (s *ConfigTestSuite) TestConfigCmdGet() {
b3 := applyMockIOOutErr(configCmd3)
err3 := configCmd3.Execute()
require.NoError(t, err3, "%s - unexpected error executing configCmd", configCmd3.Name())
out3, rerr3 := ioutil.ReadAll(b3)
out3, rerr3 := io.ReadAll(b3)
require.NoError(t, rerr3, "%s - unexpected error reading configCmd output", configCmd3.Name())
out3Str := string(out3)
require.Contains(t, outStr, out3Str, "output of no-args vs output of %s", tc.args)
Expand Down Expand Up @@ -392,7 +391,7 @@ func (s *ConfigTestSuite) TestConfigGetMulti() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s - unexpected error executing configCmd", configCmd.Name(), args)
out, err := ioutil.ReadAll(b)
out, err := io.ReadAll(b)
require.NoError(t, err, "%s %s - unexpected error reading configCmd output", configCmd.Name(), args)
outStr := string(out)
assert.Equal(t, tc.expected, outStr, "%s %s - output", configCmd.Name(), args)
Expand All @@ -417,7 +416,7 @@ func (s *ConfigTestSuite) TestConfigGetMulti() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s - expected error executing configCmd", configCmd.Name(), args)
out, err := ioutil.ReadAll(b)
out, err := io.ReadAll(b)
require.NoError(t, err, "%s %s - unexpected error reading configCmd output", configCmd.Name(), args)
outStr := string(out)
assert.Equal(t, expected, outStr, "%s %s - output", configCmd.Name(), args)
Expand All @@ -440,7 +439,7 @@ func (s *ConfigTestSuite) TestConfigGetMulti() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s - expected error executing configCmd", configCmd.Name(), args)
out, err := ioutil.ReadAll(b)
out, err := io.ReadAll(b)
require.NoError(t, err, "%s %s - unexpected error reading configCmd output", configCmd.Name(), args)
outStr := string(out)
assert.Equal(t, expected, outStr, "%s %s - output", configCmd.Name(), args)
Expand All @@ -466,7 +465,7 @@ func (s *ConfigTestSuite) TestConfigChanged() {
allEqual("client"),
"",
}
expectedAllOutLines := []string{}
var expectedAllOutLines []string
expectedAllOutLines = append(expectedAllOutLines, expectedAppOutLines...)
expectedAllOutLines = append(expectedAllOutLines, expectedTMOutLines...)
expectedAllOutLines = append(expectedAllOutLines, expectedClientOutLines...)
Expand Down Expand Up @@ -505,7 +504,7 @@ func (s *ConfigTestSuite) TestConfigChanged() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s - unexpected error executing configCmd", configCmd.Name(), tc.args)
out, err := ioutil.ReadAll(b)
out, err := io.ReadAll(b)
require.NoError(t, err, "%s %s - unexpected error reading configCmd output", configCmd.Name(), tc.args)
outStr := string(out)
assert.Equal(t, tc.out, outStr, "%s %s - output", configCmd.Name(), tc.args)
Expand Down Expand Up @@ -544,7 +543,7 @@ func (s *ConfigTestSuite) TestConfigSetValidation() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s unexpected error executing configCmd", configCmd.Name(), tc.args)
out, rerr := ioutil.ReadAll(b)
out, rerr := io.ReadAll(b)
require.NoError(t, rerr, "%s %s unexpected error reading output", configCmd.Name(), tc.args)
outStr := string(out)
assert.True(t, strings.Contains(outStr, expected), "%s %s output", configCmd.Name(), tc.args)
Expand Down Expand Up @@ -610,7 +609,7 @@ func (s *ConfigTestSuite) TestConfigCmdSet() {
},
{
name: "consensus.timeout_commit",
oldVal: `"1s"`,
oldVal: `"5s"`,
newVal: `"2s"`,
toMatch: []*regexp.Regexp{reTMConfigUpdated},
},
Expand Down Expand Up @@ -669,7 +668,7 @@ func (s *ConfigTestSuite) TestConfigCmdSet() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s - unexpected error in execution", configCmd.Name(), args)
out, rerr := ioutil.ReadAll(b)
out, rerr := io.ReadAll(b)
require.NoError(t, rerr, "%s %s - unexpected error in reading", configCmd.Name(), args)
outStr := string(out)
for _, re := range tc.toMatch {
Expand Down Expand Up @@ -702,7 +701,7 @@ func (s *ConfigTestSuite) TestConfigSetMulti() {
out: s.makeMultiLine(
s.makeTMConfigUpdateLines(),
s.makeKeyUpdatedLine("log_format", `"plain"`, `"json"`),
s.makeKeyUpdatedLine("consensus.timeout_commit", `"1s"`, `"950ms"`),
s.makeKeyUpdatedLine("consensus.timeout_commit", `"5s"`, `"950ms"`),
""),
},
{
Expand Down Expand Up @@ -746,7 +745,7 @@ func (s *ConfigTestSuite) TestConfigSetMulti() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s - unexpected error in execution", configCmd.Name(), tc.args)
out, rerr := ioutil.ReadAll(b)
out, rerr := io.ReadAll(b)
require.NoError(t, rerr, "%s %s - unexpected error in reading", configCmd.Name(), tc.args)
outStr := string(out)
assert.Equal(t, tc.out, outStr, "%s %s output", configCmd.Name(), tc.args)
Expand All @@ -766,7 +765,7 @@ func (s *ConfigTestSuite) TestPackUnpack() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s - unexpected error in execution", configCmd.Name(), args)
out, rerr := ioutil.ReadAll(b)
out, rerr := io.ReadAll(b)
require.NoError(t, rerr, "%s %s - unexpected error in reading", configCmd.Name(), args)
outStr := string(out)

Expand All @@ -780,7 +779,7 @@ func (s *ConfigTestSuite) TestPackUnpack() {
assert.False(t, provconfig.FileExists(appFile), "file exists: app")
tmFile := provconfig.GetFullPathToAppConf(configCmd)
assert.Contains(t, outStr, tmFile, "tendermint filename")
assert.False(t, provconfig.FileExists(tmFile), "file exists: tenderming")
assert.False(t, provconfig.FileExists(tmFile), "file exists: tendermint")
clientFile := provconfig.GetFullPathToAppConf(configCmd)
assert.Contains(t, outStr, clientFile, "client filename")
assert.False(t, provconfig.FileExists(clientFile), "file exists: client")
Expand All @@ -793,7 +792,7 @@ func (s *ConfigTestSuite) TestPackUnpack() {
b := applyMockIOOutErr(configCmd)
err := configCmd.Execute()
require.NoError(t, err, "%s %s - unexpected error in execution", configCmd.Name(), args)
out, rerr := ioutil.ReadAll(b)
out, rerr := io.ReadAll(b)
require.NoError(t, rerr, "%s %s - unexpected error in reading", configCmd.Name(), args)
outStr := string(out)

Expand All @@ -805,7 +804,7 @@ func (s *ConfigTestSuite) TestPackUnpack() {
assert.True(t, provconfig.FileExists(appFile), "file exists: app")
tmFile := provconfig.GetFullPathToAppConf(configCmd)
assert.Contains(t, outStr, tmFile, "tendermint filename")
assert.True(t, provconfig.FileExists(tmFile), "file exists: tenderming")
assert.True(t, provconfig.FileExists(tmFile), "file exists: tendermint")
clientFile := provconfig.GetFullPathToAppConf(configCmd)
assert.Contains(t, outStr, clientFile, "client filename")
assert.True(t, provconfig.FileExists(clientFile), "file exists: client")
Expand Down
Loading

0 comments on commit 968f15b

Please sign in to comment.