Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: set defaults for --v2-upgrade-height #4091

Merged
merged 10 commits into from
Dec 9, 2024
Merged

feat: set defaults for --v2-upgrade-height #4091

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ad04239
feat: include v2 upgrade height in binary
rootulp Dec 6, 2024
885e18f
debug: try another option of inferring chain ID
rootulp Dec 6, 2024
7ca80f3
debug: clientCtx has the chainID
rootulp Dec 6, 2024
a67461f
fix: found a place to set default upgrade height
rootulp Dec 6, 2024
989e97d
refactor: the fix
rootulp Dec 6, 2024
4d5a837
testing
rootulp Dec 6, 2024
2f37ee0
hacky test
rootulp Dec 6, 2024
5f978b2
R2f37ee054825d5226f383b3f87b0c400d916f881evert "hacky test"
rootulp Dec 6, 2024
7b12947
revert: unneeded changes
rootulp Dec 6, 2024
8d02c3f
fix: log
rootulp Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions cmd/celestia-appd/cmd/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"strings"
"time"

"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/codec"
Expand Down Expand Up @@ -117,6 +118,20 @@ is performed. Note, when enabled, gRPC will also be automatically enabled.
return err
}

switch clientCtx.ChainID {
rootulp marked this conversation as resolved.
Show resolved Hide resolved
case appconsts.ArabicaChainID:
serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.ArabicaUpgradeHeightV2, appconsts.ArabicaChainID))
serverCtx.Viper.SetDefault(UpgradeHeightFlag, appconsts.ArabicaUpgradeHeightV2)
case appconsts.MochaChainID:
serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.MochaUpgradeHeightV2, appconsts.MochaChainID))
serverCtx.Viper.SetDefault(UpgradeHeightFlag, appconsts.MochaUpgradeHeightV2)
case appconsts.MainnetChainID:
serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.MainnetChainID, appconsts.MainnetChainID))
serverCtx.Viper.SetDefault(UpgradeHeightFlag, appconsts.MainnetUpgradeHeightV2)
default:
serverCtx.Logger.Info(fmt.Sprintf("No default value exists for the v2 upgrade height when the chainID is %v", clientCtx.ChainID))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incorrect log messages and improve code structure

There are several issues in the chain ID handling logic:

  1. In the MochaChainID case, the log message swaps the upgrade height and chain ID values
  2. In the MainnetChainID case, the log message uses MainnetChainID twice instead of MainnetUpgradeHeightV2
  3. Consider extracting the upgrade height flag string to a constant

Apply these fixes:

 switch clientCtx.ChainID {
 case appconsts.ArabicaChainID:
     serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.ArabicaChainID, appconsts.ArabicaUpgradeHeightV2))
     serverCtx.Viper.SetDefault(UpgradeHeightFlag, appconsts.ArabicaUpgradeHeightV2)
 case appconsts.MochaChainID:
-    serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.MochaUpgradeHeightV2, appconsts.MochaChainID))
+    serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.MochaChainID, appconsts.MochaUpgradeHeightV2))
     serverCtx.Viper.SetDefault(UpgradeHeightFlag, appconsts.MochaUpgradeHeightV2)
 case appconsts.MainnetChainID:
-    serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.MainnetChainID, appconsts.MainnetChainID))
+    serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.MainnetChainID, appconsts.MainnetUpgradeHeightV2))
     serverCtx.Viper.SetDefault(UpgradeHeightFlag, appconsts.MainnetUpgradeHeightV2)

Consider extracting the upgrade height flag to a constant:

+const upgradeHeightFlagName = "v2-upgrade-height"
+
 switch clientCtx.ChainID {
 case appconsts.ArabicaChainID:
     serverCtx.Logger.Info(fmt.Sprintf("Since the chainID is %v, configuring the default v2 upgrade height to %v", appconsts.ArabicaChainID, appconsts.ArabicaUpgradeHeightV2))
-    serverCtx.Viper.SetDefault(UpgradeHeightFlag, appconsts.ArabicaUpgradeHeightV2)
+    serverCtx.Viper.SetDefault(upgradeHeightFlagName, appconsts.ArabicaUpgradeHeightV2)

Committable suggestion skipped: line range outside the PR's diff.


withTM, _ := cmd.Flags().GetBool(flagWithTendermint)
if !withTM {
serverCtx.Logger.Info("starting ABCI without Tendermint")
Expand Down
2 changes: 2 additions & 0 deletions pkg/appconsts/chain_ids.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ package appconsts

const (
ArabicaChainID = "arabica-11"
MochaChainID = "mocha-4"
MainnetChainID = "celestia"
)
13 changes: 13 additions & 0 deletions pkg/appconsts/upgrade_heights.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package appconsts

const (
// ArabicaUpgradeHeightV2 is the block height at which the arabica-11
// upgraded from app version 1 to 2.
ArabicaUpgradeHeightV2 = 1751707
// MochaUpgradeHeightV2 is the block height at which the mocha-4 upgraded
// from app version 1 to 2.
MochaUpgradeHeightV2 = 2585031
// MainnetUpgradeHeightV2 is the block height at which the celestia upgraded
// from app version 1 to 2.
MainnetUpgradeHeightV2 = 2371495
)
Loading