Create pre-upgrade command. Fix minimum-gas-prices repacking unset. #1594
Conversation
…it also set the commit timeout if needed. Make it so that stuff that uses the global viper also gets everything that's needed.
…fo about what cosmovisor expects.
…ding it for the unit tests.
…ill be indicate the desired exit code.
…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.
…that it happens before the config files are loaded or any defaults are requested.
…ght need to can do so), and use a temp directory instead of the default.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1594 +/- ##
==========================================
+ Coverage 63.52% 63.55% +0.03%
==========================================
Files 222 223 +1
Lines 28243 28362 +119
==========================================
+ Hits 17940 18026 +86
- Misses 9122 9149 +27
- Partials 1181 1187 +6
|
…tput a message stating that it should be set to 5s.
… so that make run cuts blocks faster.
|
General comments re: 5s values and floors... While 5 is what the SDK has used ... Tendermint recommends 1s ... and we have had some success with this lower value. I wouldn't want to enforce something higher unless we have to. There is a strong possibility of networks running on a private intranet which might want to leverage the lower value to improve the network performance for low latency workloads. With this in mind it seems we should take a middle ground here and boost the value to 5 seconds for our public main net ... but preserve the ability to run the lower values (perhaps with a warning (edit: info? given the SDK has removed warn from the logger). |
# Conflicts: # CHANGELOG.md
… half the default.
…hat. 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.
…instead of setting it using the config command at the end.
# Conflicts: # CHANGELOG.md
…ix the debug pubkey-raw command that had a conflicting shorthand flag being added.
I tweaked the PR a bit. It now only shows the error/warning if it's a mainnet node. It also only shows it if it's less than 2500ms. Also, during pre-upgrade, it'll only get updated under the same circumstances. I put the log message at the During Basically, all our local chains should still use 1s, but a fresh mainnet or testnet node will get 5s. And only Unrelated to this PR: The default timeout commit used for the integration test networks (e.g. CLI tests) is |
…'t used before this PR and it wasn't really worthwhile even after setting it due to the package it's in.
…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.
…1594) (#1599) * 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.
Description
This PR:
provenanced pre-upgradecommand as outlined by cosmovisor documentation. It will update the config files to the new formats and setconsensus.timeout_committo 5 seconds.consensus.timeout_commitis too low.minimum-gas-pricesvalue was being set to "" when either rewriting a packed config or callingprovenanced config packon an already packed config.provenanced debug pubkey-rawcommand that was trying to add a flag with a-tshorthand that conflicted with the--testnet/-tpersistent flag on the root command.Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes