feat!: version UpgradeHeightDelay (#3980)

## Overview

closes #3978 #3979

this is just a nice to have for v3. If we're worried about this causing
issues at all, we should just postpone this imo.

app/app.go

@@ -280,7 +280,7 @@ func New(
 		),
 	)
 
-	app.SignalKeeper = signal.NewKeeper(appCodec, keys[signaltypes.StoreKey], app.StakingKeeper, appconsts.UpgradeHeightDelay())
+	app.SignalKeeper = signal.NewKeeper(appCodec, keys[signaltypes.StoreKey], app.StakingKeeper)
 
 	app.IBCKeeper = ibckeeper.NewKeeper(
 		appCodec,

app/module/configurator_test.go

@@ -37,7 +37,7 @@ func TestConfigurator(t *testing.T) {
 		stateStore.MountStoreWithDB(storeKey, storetypes.StoreTypeIAVL, db)
 		require.NoError(t, stateStore.LoadLatestVersion())
 
-		keeper := signal.NewKeeper(config.Codec, storeKey, nil, 0)
+		keeper := signal.NewKeeper(config.Codec, storeKey, nil)
 		require.NotNil(t, keeper)
 		upgradeModule := signal.NewAppModule(keeper)
 		manager, err := module.NewManager([]module.VersionedModule{

app/test/upgrade_test.go

@@ -37,6 +37,10 @@ func TestAppUpgradeV3(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping TestAppUpgradeV3 in short mode")
 	}
+
+	appconsts.OverrideUpgradeHeightDelayStr = "1"
+	defer func() { appconsts.OverrideUpgradeHeightDelayStr = "" }()
+
 	testApp, genesis := SetupTestAppWithUpgradeHeight(t, 3)
 	upgradeFromV1ToV2(t, testApp)
 
@@ -102,7 +106,7 @@ func TestAppUpgradeV3(t *testing.T) {
 
 	// brace yourselfs, this part may take a while
 	initialHeight := int64(4)
-	for height := initialHeight; height < initialHeight+appconsts.DefaultUpgradeHeightDelay; height++ {
+	for height := initialHeight; height < initialHeight+appconsts.UpgradeHeightDelay(v2.Version); height++ {
 		appVersion := v2.Version
 		_ = testApp.BeginBlock(abci.RequestBeginBlock{
 			Header: tmproto.Header{
@@ -112,7 +116,7 @@ func TestAppUpgradeV3(t *testing.T) {
 		})
 
 		endBlockResp = testApp.EndBlock(abci.RequestEndBlock{
-			Height: 3 + appconsts.DefaultUpgradeHeightDelay,
+			Height: 3 + appconsts.UpgradeHeightDelay(v2.Version),
 		})
 
 		require.Equal(t, appconsts.GetTimeoutCommit(appVersion), endBlockResp.Timeouts.TimeoutCommit)
@@ -137,7 +141,7 @@ func TestAppUpgradeV3(t *testing.T) {
 	_ = testApp.BeginBlock(abci.RequestBeginBlock{
 		Header: tmproto.Header{
 			ChainID: genesis.ChainID,
-			Height:  initialHeight + appconsts.DefaultUpgradeHeightDelay,
+			Height:  initialHeight + appconsts.UpgradeHeightDelay(v3.Version),
 			Version: tmversion.Consensus{App: 3},
 		},
 	})
@@ -148,7 +152,7 @@ func TestAppUpgradeV3(t *testing.T) {
 	require.Equal(t, abci.CodeTypeOK, deliverTxResp.Code, deliverTxResp.Log)
 
 	respEndBlock := testApp.EndBlock(abci.
-		RequestEndBlock{Height: initialHeight + appconsts.DefaultUpgradeHeightDelay})
+		RequestEndBlock{Height: initialHeight + appconsts.UpgradeHeightDelay(v3.Version)})
 	require.Equal(t, appconsts.GetTimeoutCommit(v3.Version), respEndBlock.Timeouts.TimeoutCommit)
 	require.Equal(t, appconsts.GetTimeoutPropose(v3.Version), respEndBlock.Timeouts.TimeoutPropose)
 }

pkg/appconsts/global_consts.go

@@ -1,8 +1,6 @@
 package appconsts
 
 import (
-	"strconv"
-
 	"github.com/celestiaorg/go-square/v2/share"
 	"github.com/celestiaorg/rsmt2d"
 	"github.com/tendermint/tendermint/pkg/consts"
@@ -26,11 +24,6 @@ const (
 
 	// BondDenom defines the native staking denomination
 	BondDenom = "utia"
-
-	// DefaultUpgradeHeightDelay is the number of blocks after a quorum has been
-	// reached that the chain should upgrade to the new version. Assuming a block
-	// interval of 12 seconds, this is 7 days.
-	DefaultUpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks.
 )
 
 var (
@@ -51,18 +44,6 @@ var (
 	SupportedShareVersions = share.SupportedShareVersions
 )
 
-// UpgradeHeightDelay returns the delay in blocks after a quorum has been reached that the chain should upgrade to the new version.
-func UpgradeHeightDelay() int64 {
-	if OverrideUpgradeHeightDelayStr != "" {
-		parsedValue, err := strconv.ParseInt(OverrideUpgradeHeightDelayStr, 10, 64)
-		if err != nil {
-			panic("Invalid OverrideUpgradeHeightDelayStr value")
-		}
-		return parsedValue
-	}
-	return DefaultUpgradeHeightDelay
-}
-
 // HashLength returns the length of a hash in bytes.
 func HashLength() int {
 	return hashLength

pkg/appconsts/v1/app_consts.go

@@ -8,4 +8,8 @@ const (
 	SubtreeRootThreshold int    = 64
 	TimeoutPropose              = time.Second * 10
 	TimeoutCommit               = time.Second * 11
+	// UpgradeHeightDelay is the number of blocks after a quorum has been
+	// reached that the chain should upgrade to the new version. Assuming a block
+	// interval of 12 seconds, this is 7 days.
+	UpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks.
 )

pkg/appconsts/v2/app_consts.go

@@ -8,4 +8,8 @@ const (
 	SubtreeRootThreshold int    = 64
 	TimeoutPropose              = time.Second * 10
 	TimeoutCommit               = time.Second * 11
+	// UpgradeHeightDelay is the number of blocks after a quorum has been
+	// reached that the chain should upgrade to the new version. Assuming a block
+	// interval of 12 seconds, this is 7 days.
+	UpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks.
 )

pkg/appconsts/v3/app_consts.go

@@ -11,4 +11,8 @@ const (
 	MaxTxSize            int    = 2097152 // 2 MiB in bytes
 	TimeoutPropose              = time.Millisecond * 3500
 	TimeoutCommit               = time.Millisecond * 4200
+	// UpgradeHeightDelay is the number of blocks after a quorum has been
+	// reached that the chain should upgrade to the new version. Assuming a block
+	// interval of 12 seconds, this is 7 days.
+	UpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 6) // 7 days * 24 hours * 60 minutes * 60 seconds / 6 seconds per block = 100,800 blocks.
 )

pkg/appconsts/versioned_consts.go

@@ -77,3 +77,23 @@ func GetTimeoutCommit(v uint64) time.Duration {
 		return v3.TimeoutCommit
 	}
 }
+
+// UpgradeHeightDelay returns the delay in blocks after a quorum has been reached that the chain should upgrade to the new version.
+func UpgradeHeightDelay(v uint64) int64 {
+	if OverrideUpgradeHeightDelayStr != "" {
+		parsedValue, err := strconv.ParseInt(OverrideUpgradeHeightDelayStr, 10, 64)
+		if err != nil {
+			panic("Invalid OverrideUpgradeHeightDelayStr value")
+		}
+		return parsedValue
+	}
+	switch v {
+	case v1.Version:
+		return v1.UpgradeHeightDelay
+	case v2.Version:
+		return v2.UpgradeHeightDelay
+	default:
+		return v3.UpgradeHeightDelay
+
+	}
+}

x/signal/integration_test.go

@@ -77,7 +77,7 @@ func TestUpgradeIntegration(t *testing.T) {
 	require.False(t, shouldUpgrade)
 	require.EqualValues(t, 0, version)
 
-	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.DefaultUpgradeHeightDelay)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.UpgradeHeightDelay(version))
 
 	shouldUpgrade, version = app.SignalKeeper.ShouldUpgrade(ctx)
 	require.True(t, shouldUpgrade)

x/signal/keeper.go

@@ -5,6 +5,7 @@ import (
 	"encoding/binary"
 
 	sdkmath "cosmossdk.io/math"
+	"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
 	"github.com/celestiaorg/celestia-app/v3/x/signal/types"
 	"github.com/cosmos/cosmos-sdk/codec"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
@@ -41,24 +42,18 @@ type Keeper struct {
 	// stakingKeeper is used to fetch validators to calculate the total power
 	// signalled to a version.
 	stakingKeeper StakingKeeper
-
-	// upgradeHeightDelayBlocks is the number of blocks after a quorum has been
-	// reached that the chain should upgrade to the new version
-	upgradeHeightDelayBlocks int64
 }
 
 // NewKeeper returns a signal keeper.
 func NewKeeper(
 	binaryCodec codec.BinaryCodec,
 	storeKey storetypes.StoreKey,
 	stakingKeeper StakingKeeper,
-	upgradeHeightDelayBlocks int64,
 ) Keeper {
 	return Keeper{
-		binaryCodec:              binaryCodec,
-		storeKey:                 storeKey,
-		stakingKeeper:            stakingKeeper,
-		upgradeHeightDelayBlocks: upgradeHeightDelayBlocks,
+		binaryCodec:   binaryCodec,
+		storeKey:      storeKey,
+		stakingKeeper: stakingKeeper,
 	}
 }
 
@@ -109,7 +104,7 @@ func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types
 		}
 		upgrade := types.Upgrade{
 			AppVersion:    version,
-			UpgradeHeight: sdkCtx.BlockHeader().Height + k.upgradeHeightDelayBlocks,
+			UpgradeHeight: sdkCtx.BlockHeader().Height + appconsts.UpgradeHeightDelay(version),
 		}
 		k.setUpgrade(sdkCtx, upgrade)
 	}

x/signal/keeper_test.go

@@ -69,7 +69,7 @@ func TestGetVotingPowerThreshold(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			config := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 			stakingKeeper := newMockStakingKeeper(tc.validators)
-			k := signal.NewKeeper(config.Codec, nil, stakingKeeper, appconsts.DefaultUpgradeHeightDelay)
+			k := signal.NewKeeper(config.Codec, nil, stakingKeeper)
 			got := k.GetVotingPowerThreshold(sdk.Context{})
 			assert.Equal(t, tc.want, got, fmt.Sprintf("want %v, got %v", tc.want.String(), got.String()))
 		})
@@ -183,7 +183,7 @@ func TestTallyingLogic(t *testing.T) {
 	require.False(t, shouldUpgrade) // should be false because upgrade height hasn't been reached.
 	require.Equal(t, uint64(0), version)
 
-	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.DefaultUpgradeHeightDelay)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.UpgradeHeightDelay(version))
 
 	shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx)
 	require.True(t, shouldUpgrade) // should be true because upgrade height has been reached.
@@ -426,7 +426,7 @@ func TestGetUpgrade(t *testing.T) {
 		got, err := upgradeKeeper.GetUpgrade(ctx, &types.QueryGetUpgradeRequest{})
 		require.NoError(t, err)
 		assert.Equal(t, v2.Version, got.Upgrade.AppVersion)
-		assert.Equal(t, appconsts.DefaultUpgradeHeightDelay, got.Upgrade.UpgradeHeight)
+		assert.Equal(t, appconsts.UpgradeHeightDelay(v2.Version), got.Upgrade.UpgradeHeight)
 	})
 }
 
@@ -452,7 +452,7 @@ func setup(t *testing.T) (signal.Keeper, sdk.Context, *mockStakingKeeper) {
 	)
 
 	config := encoding.MakeConfig(app.ModuleEncodingRegisters...)
-	upgradeKeeper := signal.NewKeeper(config.Codec, signalStore, mockStakingKeeper, appconsts.DefaultUpgradeHeightDelay)
+	upgradeKeeper := signal.NewKeeper(config.Codec, signalStore, mockStakingKeeper)
 	return upgradeKeeper, mockCtx, mockStakingKeeper
 }