feat!: versioned timeouts (#3882)

Closes #3859

manually tested in
#3882 (comment)

also tested in knuu

---------

Co-authored-by: evan-forbes <[email protected]>
Co-authored-by: Evan Forbes <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Rootul P <[email protected]>

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,
@@ -458,7 +458,7 @@ func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.R
 func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
 	res := app.manager.EndBlock(ctx, req)
 	currentVersion := app.AppVersion()
-	// For v1 only we upgrade using a agreed upon height known ahead of time
+	// For v1 only we upgrade using an agreed upon height known ahead of time
 	if currentVersion == v1 {
 		// check that we are at the height before the upgrade
 		if req.Height == app.upgradeHeightV2-1 {
@@ -480,6 +480,8 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo
 			app.SignalKeeper.ResetTally(ctx)
 		}
 	}
+	res.Timeouts.TimeoutCommit = appconsts.GetTimeoutCommit(currentVersion)
+	res.Timeouts.TimeoutPropose = appconsts.GetTimeoutPropose(currentVersion)
 	return res
 }
 
@@ -535,11 +537,15 @@ func (app *App) Info(req abci.RequestInfo) abci.ResponseInfo {
 	if resp.AppVersion > 0 && !app.IsSealed() {
 		app.mountKeysAndInit(resp.AppVersion)
 	}
+
+	resp.Timeouts.TimeoutPropose = appconsts.GetTimeoutPropose(resp.AppVersion)
+	resp.Timeouts.TimeoutCommit = appconsts.GetTimeoutCommit(resp.AppVersion)
+
 	return resp
 }
 
 // InitChain implements the ABCI interface. This method is a wrapper around
-// baseapp's InitChain so we can take the app version and setup the multicommit
+// baseapp's InitChain so that we can take the app version and setup the multicommit
 // store.
 //
 // Side-effect: calls baseapp.Init()
@@ -558,6 +564,8 @@ func (app *App) InitChain(req abci.RequestInitChain) (res abci.ResponseInitChain
 		app.SetInitialAppVersionInConsensusParams(ctx, appVersion)
 		app.SetAppVersion(ctx, appVersion)
 	}
+	res.Timeouts.TimeoutCommit = appconsts.GetTimeoutCommit(appVersion)
+	res.Timeouts.TimeoutPropose = appconsts.GetTimeoutPropose(appVersion)
 	return res
 }
 

app/default_overrides.go

@@ -267,8 +267,8 @@ func DefaultConsensusConfig() *tmcfg.Config {
 	cfg.Mempool.MaxTxsBytes = 39_485_440
 	cfg.Mempool.Version = "v1" // prioritized mempool
 
-	cfg.Consensus.TimeoutPropose = appconsts.TimeoutPropose
-	cfg.Consensus.TimeoutCommit = appconsts.TimeoutCommit
+	cfg.Consensus.TimeoutPropose = appconsts.GetTimeoutPropose(appconsts.LatestVersion)
+	cfg.Consensus.TimeoutCommit = appconsts.GetTimeoutCommit(appconsts.LatestVersion)
 	cfg.Consensus.SkipTimeoutCommit = false
 
 	cfg.TxIndex.Indexer = "null"

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)
 
@@ -88,6 +92,10 @@ func TestAppUpgradeV3(t *testing.T) {
 		Height: 3,
 	})
 	require.Equal(t, v2.Version, endBlockResp.ConsensusParamUpdates.Version.AppVersion)
+	require.Equal(t, appconsts.GetTimeoutCommit(v2.Version),
+		endBlockResp.Timeouts.TimeoutCommit)
+	require.Equal(t, appconsts.GetTimeoutPropose(v2.Version),
+		endBlockResp.Timeouts.TimeoutPropose)
 	testApp.Commit()
 	require.NoError(t, signer.IncrementSequence(testnode.DefaultValidatorAccountName))
 
@@ -98,18 +106,22 @@ 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{
 				Height:  height,
-				Version: tmversion.Consensus{App: 2},
+				Version: tmversion.Consensus{App: appVersion},
 			},
 		})
 
 		endBlockResp = testApp.EndBlock(abci.RequestEndBlock{
-			Height: 3 + appconsts.DefaultUpgradeHeightDelay,
+			Height: 3 + appconsts.UpgradeHeightDelay(v2.Version),
 		})
 
+		require.Equal(t, appconsts.GetTimeoutCommit(appVersion), endBlockResp.Timeouts.TimeoutCommit)
+		require.Equal(t, appconsts.GetTimeoutPropose(appVersion), endBlockResp.Timeouts.TimeoutPropose)
+
 		_ = testApp.Commit()
 	}
 	require.Equal(t, v3.Version, endBlockResp.ConsensusParamUpdates.Version.AppVersion)
@@ -129,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},
 		},
 	})
@@ -139,7 +151,10 @@ func TestAppUpgradeV3(t *testing.T) {
 	})
 	require.Equal(t, abci.CodeTypeOK, deliverTxResp.Code, deliverTxResp.Log)
 
-	_ = testApp.EndBlock(abci.RequestEndBlock{})
+	respEndBlock := testApp.EndBlock(abci.
+		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)
 }
 
 // TestAppUpgradeV2 verifies that the all module's params are overridden during an
@@ -271,7 +286,10 @@ func SetupTestAppWithUpgradeHeight(t *testing.T, upgradeHeight int64) (*app.App,
 
 	// assert that the chain starts with version provided in genesis
 	infoResp := testApp.Info(abci.RequestInfo{})
-	require.EqualValues(t, app.DefaultInitialConsensusParams().Version.AppVersion, infoResp.AppVersion)
+	appVersion := app.DefaultInitialConsensusParams().Version.AppVersion
+	require.EqualValues(t, appVersion, infoResp.AppVersion)
+	require.EqualValues(t, appconsts.GetTimeoutCommit(appVersion), infoResp.Timeouts.TimeoutCommit)
+	require.EqualValues(t, appconsts.GetTimeoutPropose(appVersion), infoResp.Timeouts.TimeoutPropose)
 
 	supportedVersions := []uint64{v1.Version, v2.Version, v3.Version}
 	require.Equal(t, supportedVersions, testApp.SupportedVersions())
@@ -286,7 +304,11 @@ func upgradeFromV1ToV2(t *testing.T, testApp *app.App) {
 		Height:  2,
 		Version: tmversion.Consensus{App: 1},
 	}})
-	testApp.EndBlock(abci.RequestEndBlock{Height: 2})
+	endBlockResp := testApp.EndBlock(abci.RequestEndBlock{Height: 2})
+	require.Equal(t, appconsts.GetTimeoutCommit(v1.Version),
+		endBlockResp.Timeouts.TimeoutCommit)
+	require.Equal(t, appconsts.GetTimeoutPropose(v1.Version),
+		endBlockResp.Timeouts.TimeoutPropose)
 	testApp.Commit()
 	require.EqualValues(t, 2, testApp.AppVersion())
 }

go.mod

@@ -260,5 +260,5 @@ replace (
 	github.com/cosmos/ledger-cosmos-go => github.com/cosmos/ledger-cosmos-go v0.12.4
 	github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1
 	github.com/syndtr/goleveldb => github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7
-	github.com/tendermint/tendermint => github.com/celestiaorg/celestia-core v1.42.0-tm-v0.34.35
+	github.com/tendermint/tendermint => github.com/celestiaorg/celestia-core v1.43.0-tm-v0.34.35
 )

go.sum

@@ -317,8 +317,8 @@ github.com/celestiaorg/bittwister v0.0.0-20231213180407-65cdbaf5b8c7 h1:nxplQi8w
 github.com/celestiaorg/bittwister v0.0.0-20231213180407-65cdbaf5b8c7/go.mod h1:1EF5MfOxVf0WC51Gb7pJ6bcZxnXKNAf9pqWtjgPBAYc=
 github.com/celestiaorg/blobstream-contracts/v3 v3.1.0 h1:h1Y4V3EMQ2mFmNtWt2sIhZIuyASInj1a9ExI8xOsTOw=
 github.com/celestiaorg/blobstream-contracts/v3 v3.1.0/go.mod h1:x4DKyfKOSv1ZJM9NwV+Pw01kH2CD7N5zTFclXIVJ6GQ=
-github.com/celestiaorg/celestia-core v1.42.0-tm-v0.34.35 h1:bWy5XOgeuuSLe0Lc/htL9/QLEURBjA1JTvEko6bEBhg=
-github.com/celestiaorg/celestia-core v1.42.0-tm-v0.34.35/go.mod h1:/fK0n3ps09t5uErBQe1QZbrE81L81MNUzWpFyWQLDT0=
+github.com/celestiaorg/celestia-core v1.43.0-tm-v0.34.35 h1:L4GTm+JUXhB0a/nGPMq6jEqqe6THuYSQ8m2kUCtZYqw=
+github.com/celestiaorg/celestia-core v1.43.0-tm-v0.34.35/go.mod h1:bFr0lAGwaJ0mOHSBmib5/ca5pbBf1yKWGPs93Td0HPw=
 github.com/celestiaorg/cosmos-sdk v1.25.0-sdk-v0.46.16 h1:f+fTe7GGk0/qgdzyqB8kk8EcDf9d6MC22khBTQiDXsU=
 github.com/celestiaorg/cosmos-sdk v1.25.0-sdk-v0.46.16/go.mod h1:07Z8HJqS8Rw4XlZ+ok3D3NM/X/in8mvcGLvl0Zb5wrA=
 github.com/celestiaorg/go-square v1.1.1 h1:Cy3p8WVspVcyOqHM8BWFuuYPwMitO1pYGe+ImILFZRA=

local_devnet/docker-compose.yml

@@ -6,6 +6,7 @@ services:
     container_name: core0
     build:
       context: ..
+      dockerfile: ./docker/Dockerfile
     expose:
       - "26660" # for prometheus
     ports:
@@ -31,6 +32,7 @@ services:
     container_name: core1
     build:
       context: ..
+      dockerfile: ./docker/Dockerfile
     expose:
       - "26660" # for prometheus
     depends_on:
@@ -59,6 +61,7 @@ services:
     container_name: core2
     build:
       context: ..
+      dockerfile: ./docker/Dockerfile
     expose:
       - "26660" # for prometheus
     depends_on:
@@ -87,6 +90,7 @@ services:
     container_name: core3
     build:
       context: ..
+      dockerfile: ./docker/Dockerfile
     expose:
       - "26660" # for prometheus
     depends_on:

pkg/appconsts/consensus_consts.go

@@ -3,8 +3,6 @@ package appconsts
 import "time"
 
 const (
-	TimeoutPropose = time.Second * 10
-	TimeoutCommit  = time.Second * 11
 	// GoalBlockTime is the target time interval between blocks. Since the block
 	// interval isn't enforced at consensus, the real block interval isn't
 	// guaranteed to exactly match GoalBlockTime. GoalBlockTime is currently targeted

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

@@ -1,7 +1,15 @@
 package v1
 
+import "time"
+
 const (
 	Version              uint64 = 1
 	SquareSizeUpperBound int    = 128
 	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

@@ -1,7 +1,15 @@
 package v2
 
+import "time"
+
 const (
 	Version              uint64 = 2
 	SquareSizeUpperBound int    = 128
 	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

@@ -1,10 +1,18 @@
 package v3
 
+import "time"
+
 const (
 	Version              uint64 = 3
 	SquareSizeUpperBound int    = 128
 	SubtreeRootThreshold int    = 64
 	TxSizeCostPerByte    uint64 = 10
 	GasPerBlobByte       uint32 = 8
 	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

@@ -2,7 +2,10 @@ package appconsts
 
 import (
 	"strconv"
+	"time"
 
+	v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1"
+	v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2"
 	v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3"
 )
 
@@ -52,3 +55,45 @@ var (
 	DefaultTxSizeCostPerByte    = TxSizeCostPerByte(LatestVersion)
 	DefaultGasPerBlobByte       = GasPerBlobByte(LatestVersion)
 )
+
+func GetTimeoutPropose(v uint64) time.Duration {
+	switch v {
+	case v1.Version:
+		return v1.TimeoutPropose
+	case v2.Version:
+		return v2.TimeoutPropose
+	default:
+		return v3.TimeoutPropose
+	}
+}
+
+func GetTimeoutCommit(v uint64) time.Duration {
+	switch v {
+	case v1.Version:
+		return v1.TimeoutCommit
+	case v2.Version:
+		return v2.TimeoutCommit
+	default:
+		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
+
+	}
+}

test/e2e/benchmark/benchmark.go

@@ -22,6 +22,8 @@ type BenchmarkTest struct {
 	manifest *Manifest
 }
 
+// NewBenchmarkTest wraps around testnet.New to create a new benchmark test.
+// It may modify genesis consensus parameters based on manifest.
 func NewBenchmarkTest(name string, manifest *Manifest) (*BenchmarkTest, error) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -52,7 +54,7 @@ func NewBenchmarkTest(name string, manifest *Manifest) (*BenchmarkTest, error) {
 }
 
 // SetupNodes creates genesis nodes and tx clients based on the manifest.
-// There will be manifest.Validators validators and manifest.TxClients tx clients.
+// There will be manifest.Validators many validators and manifest.TxClients many tx clients.
 // Each tx client connects to one validator. If TxClients are fewer than Validators, some validators will not have a tx client.
 func (b *BenchmarkTest) SetupNodes() error {
 	ctx := context.Background()