feat: override timeout commit via --timeout-commit (backport #4103) (…

…#4114) Closes #4080 ## Testing 1. `./scripts/single-node.sh` now makes blocks faster 🎊 2. `./scripts/arabica.sh` throws an error if you try to add `--timeout-commit 1s` ``` Error: the --timeout-commit flag was used on arabica-11 but it is unsupported on public networks: arabica-11, mocha-4, celestia. The --timeout-commit flag should only be used on private testnets. ``` --------- Co-authored-by: Rootul P <[email protected]>
celestiaorg · Dec 16, 2024 · 29afcf6 · 29afcf6
1 parent c456d75
commit 29afcf6
Show file tree

Hide file tree

Showing 19 changed files with 76 additions and 25 deletions.
diff --git a/Makefile b/Makefile
@@ -51,6 +51,7 @@ help: Makefile
 build: mod
 	@cd ./cmd/celestia-appd
 	@mkdir -p build/
+	@echo "--> Building build/celestia-appd"
 	@go build $(BUILD_FLAGS) -o build/ ./cmd/celestia-appd
 .PHONY: build
 

diff --git a/README.md b/README.md
@@ -126,6 +126,15 @@ celestia-appd tx blob pay-for-blob 0x00010203040506070809 0x48656c6c6f2c20576f72
 
 If import celestia-app as a Go module, you may need to add some Go module `replace` directives to avoid type incompatibilities. Please see the `replace` directive in [go.mod](./go.mod) for inspiration.
 
+### Usage in tests
+
+If you are running celestia-app in tests, you may want to override the `timeout_commit` to produce blocks faster. By default, a celestia-app chain with app version >= 3 will produce blocks every ~6 seconds. To produce blocks faster, you can override the `timeout_commit` with the `--timeout-commit` flag.
+
+```shell
+# Start celestia-appd with a one second timeout commit.
+celestia-appd start --timeout-commit 1s
+```
+
 ## Contributing
 
 This repo attempts to conform to [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/) so PR titles should ideally start with `fix:`, `feat:`, `build:`, `chore:`, `ci:`, `docs:`, `style:`, `refactor:`, `perf:`, or `test:` because this helps with semantic versioning and changelog generation. It is especially important to include an `!` (e.g. `feat!:`) if the PR includes a breaking change.

diff --git a/app/app.go b/app/app.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"slices"
+	"time"
 
 	"github.com/celestiaorg/celestia-app/v3/app/ante"
 	"github.com/celestiaorg/celestia-app/v3/app/encoding"
@@ -170,6 +171,10 @@ type App struct {
 	// upgradeHeightV2 is used as a coordination mechanism for the height-based
 	// upgrade from v1 to v2.
 	upgradeHeightV2 int64
+	// timeoutCommit is used to override the default timeoutCommit. This is
+	// useful for testing purposes and should not be used on public networks
+	// (Arabica, Mocha, or Mainnet Beta).
+	timeoutCommit time.Duration
 	// MsgGateKeeper is used to define which messages are accepted for a given
 	// app version.
 	MsgGateKeeper *ante.MsgVersioningGateKeeper
@@ -188,6 +193,7 @@ func New(
 	invCheckPeriod uint,
 	encodingConfig encoding.Config,
 	upgradeHeightV2 int64,
+	timeoutCommit time.Duration,
 	appOpts servertypes.AppOptions,
 	baseAppOptions ...func(*baseapp.BaseApp),
 ) *App {
@@ -214,6 +220,7 @@ func New(
 		tkeys:             tkeys,
 		memKeys:           memKeys,
 		upgradeHeightV2:   upgradeHeightV2,
+		timeoutCommit:     timeoutCommit,
 	}
 
 	app.ParamsKeeper = initParamsKeeper(appCodec, encodingConfig.Amino, keys[paramstypes.StoreKey], tkeys[paramstypes.TStoreKey])
@@ -480,7 +487,7 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo
 			app.SignalKeeper.ResetTally(ctx)
 		}
 	}
-	res.Timeouts.TimeoutCommit = appconsts.GetTimeoutCommit(currentVersion)
+	res.Timeouts.TimeoutCommit = app.getTimeoutCommit(currentVersion)
 	res.Timeouts.TimeoutPropose = appconsts.GetTimeoutPropose(currentVersion)
 	return res
 }
@@ -538,8 +545,8 @@ func (app *App) Info(req abci.RequestInfo) abci.ResponseInfo {
 		app.mountKeysAndInit(resp.AppVersion)
 	}
 
+	resp.Timeouts.TimeoutCommit = app.getTimeoutCommit(resp.AppVersion)
 	resp.Timeouts.TimeoutPropose = appconsts.GetTimeoutPropose(resp.AppVersion)
-	resp.Timeouts.TimeoutCommit = appconsts.GetTimeoutCommit(resp.AppVersion)
 
 	return resp
 }
@@ -564,7 +571,7 @@ 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.TimeoutCommit = app.getTimeoutCommit(appVersion)
 	res.Timeouts.TimeoutPropose = appconsts.GetTimeoutPropose(appVersion)
 	return res
 }
@@ -848,3 +855,13 @@ func (app *App) OfferSnapshot(req abci.RequestOfferSnapshot) abci.ResponseOfferS
 func isSupportedAppVersion(appVersion uint64) bool {
 	return appVersion == v1 || appVersion == v2 || appVersion == v3
 }
+
+// getTimeoutCommit returns the timeoutCommit if a user has overridden it via the
+// --timeout-commit flag. Otherwise, it returns the default timeout commit based
+// on the app version.
+func (app *App) getTimeoutCommit(appVersion uint64) time.Duration {
+	if app.timeoutCommit != 0 {
+		return app.timeoutCommit
+	}
+	return appconsts.GetTimeoutCommit(appVersion)
+}
diff --git a/app/app_test.go b/app/app_test.go
@@ -5,6 +5,7 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/celestiaorg/celestia-app/v3/app"
 	"github.com/celestiaorg/celestia-app/v3/app/encoding"
@@ -29,9 +30,10 @@ func TestNew(t *testing.T) {
 	invCheckPeriod := uint(1)
 	encodingConfig := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 	upgradeHeight := int64(0)
+	timeoutCommit := time.Second
 	appOptions := NoopAppOptions{}
 
-	got := app.New(logger, db, traceStore, invCheckPeriod, encodingConfig, upgradeHeight, appOptions)
+	got := app.New(logger, db, traceStore, invCheckPeriod, encodingConfig, upgradeHeight, timeoutCommit, appOptions)
 
 	t.Run("initializes ICAHostKeeper", func(t *testing.T) {
 		assert.NotNil(t, got.ICAHostKeeper)
@@ -65,8 +67,9 @@ func TestInitChain(t *testing.T) {
 	invCheckPeriod := uint(1)
 	encodingConfig := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 	upgradeHeight := int64(0)
+	timeoutCommit := time.Second
 	appOptions := NoopAppOptions{}
-	testApp := app.New(logger, db, traceStore, invCheckPeriod, encodingConfig, upgradeHeight, appOptions)
+	testApp := app.New(logger, db, traceStore, invCheckPeriod, encodingConfig, upgradeHeight, timeoutCommit, appOptions)
 	genesisState, _, _ := util.GenesisStateWithSingleValidator(testApp, "account")
 	appStateBytes, err := json.MarshalIndent(genesisState, "", " ")
 	require.NoError(t, err)
@@ -103,7 +106,7 @@ func TestInitChain(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			application := app.New(logger, db, traceStore, invCheckPeriod, encodingConfig, upgradeHeight, appOptions)
+			application := app.New(logger, db, traceStore, invCheckPeriod, encodingConfig, upgradeHeight, timeoutCommit, appOptions)
 			if tc.wantPanic {
 				assert.Panics(t, func() { application.InitChain(tc.request) })
 			} else {
@@ -160,6 +163,7 @@ func createTestApp(t *testing.T) *app.App {
 	db := tmdb.NewMemDB()
 	config := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 	upgradeHeight := int64(3)
+	timeoutCommit := time.Second
 	snapshotDir := filepath.Join(t.TempDir(), "data", "snapshots")
 	t.Cleanup(func() {
 		err := os.RemoveAll(snapshotDir)
@@ -174,7 +178,7 @@ func createTestApp(t *testing.T) *app.App {
 	snapshotStore, err := snapshots.NewStore(snapshotDB, snapshotDir)
 	require.NoError(t, err)
 	baseAppOption := baseapp.SetSnapshot(snapshotStore, snapshottypes.NewSnapshotOptions(10, 10))
-	testApp := app.New(log.NewNopLogger(), db, nil, 0, config, upgradeHeight, util.EmptyAppOptions{}, baseAppOption)
+	testApp := app.New(log.NewNopLogger(), db, nil, 0, config, upgradeHeight, timeoutCommit, util.EmptyAppOptions{}, baseAppOption)
 	require.NoError(t, err)
 	response := testApp.Info(abci.RequestInfo{})
 	require.Equal(t, uint64(0), response.AppVersion)

diff --git a/app/test/upgrade_test.go b/app/test/upgrade_test.go
@@ -257,7 +257,7 @@ func SetupTestAppWithUpgradeHeight(t *testing.T, upgradeHeight int64) (*app.App,
 
 	db := dbm.NewMemDB()
 	encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)
-	testApp := app.New(log.NewNopLogger(), db, nil, 0, encCfg, upgradeHeight, util.EmptyAppOptions{})
+	testApp := app.New(log.NewNopLogger(), db, nil, 0, encCfg, upgradeHeight, 0, util.EmptyAppOptions{})
 	genesis := genesis.NewDefaultGenesis().
 		WithValidators(genesis.NewDefaultValidator(testnode.DefaultValidatorAccountName)).
 		WithConsensusParams(app.DefaultInitialConsensusParams())

diff --git a/cmd/celestia-appd/cmd/app_exporter.go b/cmd/celestia-appd/cmd/app_exporter.go
@@ -20,7 +20,7 @@ func appExporter(
 	appOptions servertypes.AppOptions,
 ) (servertypes.ExportedApp, error) {
 	config := encoding.MakeConfig(app.ModuleEncodingRegisters...)
-	application := app.New(logger, db, traceStore, uint(1), config, 0, appOptions)
+	application := app.New(logger, db, traceStore, uint(1), config, 0, 0, appOptions)
 	if height != -1 {
 		if err := application.LoadHeight(height); err != nil {
 			return servertypes.ExportedApp{}, err

diff --git a/cmd/celestia-appd/cmd/app_server.go b/cmd/celestia-appd/cmd/app_server.go
@@ -44,10 +44,13 @@ func NewAppServer(logger log.Logger, db dbm.DB, traceStore io.Writer, appOptions
 	}
 
 	return app.New(
-		logger, db, traceStore,
+		logger,
+		db,
+		traceStore,
 		cast.ToUint(appOptions.Get(server.FlagInvCheckPeriod)),
 		encoding.MakeConfig(app.ModuleEncodingRegisters...),
 		cast.ToInt64(appOptions.Get(UpgradeHeightFlag)),
+		cast.ToDuration(appOptions.Get(TimeoutCommitFlag)),
 		appOptions,
 		baseapp.SetPruning(pruningOpts),
 		baseapp.SetMinGasPrices(cast.ToString(appOptions.Get(server.FlagMinGasPrices))),

diff --git a/cmd/celestia-appd/cmd/root.go b/cmd/celestia-appd/cmd/root.go
@@ -37,6 +37,9 @@ const (
 	// UpgradeHeightFlag is the flag to specify the upgrade height for v1 to v2
 	// application upgrade.
 	UpgradeHeightFlag = "v2-upgrade-height"
+
+	// TimeoutCommit is a flag that can be used to override the timeout_commit.
+	TimeoutCommitFlag = "timeout-commit"
 )
 
 // NewRootCmd creates a new root command for celestia-appd.
@@ -125,7 +128,7 @@ func initRootCommand(rootCommand *cobra.Command, encodingConfig encoding.Config)
 	)
 
 	// Add the following commands to the rootCommand: start, tendermint, export, version, and rollback.
-	addCommands(rootCommand, app.DefaultNodeHome, NewAppServer, appExporter, addModuleInitFlags)
+	addCommands(rootCommand, app.DefaultNodeHome, NewAppServer, appExporter, addStartFlags)
 }
 
 // setDefaultConsensusParams sets the default consensus parameters for the
@@ -136,9 +139,11 @@ func setDefaultConsensusParams(command *cobra.Command) error {
 	return server.SetCmdServerContext(command, ctx)
 }
 
-func addModuleInitFlags(startCmd *cobra.Command) {
+// addStartFlags adds flags to the start command.
+func addStartFlags(startCmd *cobra.Command) {
 	crisis.AddModuleInitFlags(startCmd)
 	startCmd.Flags().Int64(UpgradeHeightFlag, 0, "Upgrade height to switch from v1 to v2. Must be coordinated amongst all validators")
+	startCmd.Flags().Duration(TimeoutCommitFlag, 0, "Override the application configured timeout_commit. Note: only for testing purposes.")
 }
 
 // replaceLogger optionally replaces the logger with a file logger if the flag

diff --git a/cmd/celestia-appd/cmd/start.go b/cmd/celestia-appd/cmd/start.go
@@ -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"
@@ -117,6 +118,10 @@ is performed. Note, when enabled, gRPC will also be automatically enabled.
 				return err
 			}
 
+			if contains(appconsts.PublicNetworks, clientCtx.ChainID) && serverCtx.Viper.GetDuration(TimeoutCommitFlag) != 0 {
+				return fmt.Errorf("the --timeout-commit flag was used on %v but it is unsupported on public networks: %v. The --timeout-commit flag should only be used on private testnets", clientCtx.ChainID, strings.Join(appconsts.PublicNetworks, ", "))
+			}
+
 			withTM, _ := cmd.Flags().GetBool(flagWithTendermint)
 			if !withTM {
 				serverCtx.Logger.Info("starting ABCI without Tendermint")

diff --git a/docs/maintainers/release-guide.md b/docs/maintainers/release-guide.md
@@ -32,7 +32,7 @@ Follow the [creating a release candidate](#creating-a-release-candidate) section
 - The version tag should not include the `-rc` suffix.
 - If the release targets a testnet, suffix the release with `-arabica` or `-mocha`.
 - The release notes should contain an **Upgrade Notice** section with notable changes for node operators or library consumers.
-- The release notes section should contain a link to https://github.com/celestiaorg/celestia-app/blob/main/docs/release-notes/release-notes.md where we capture breaking changes
+- The release notes section should contain a link to <https://github.com/celestiaorg/celestia-app/blob/main/docs/release-notes/release-notes.md> where we capture breaking changes
 
 After creating the release:
 

diff --git a/docs/release-notes/release-notes.md b/docs/release-notes/release-notes.md
@@ -9,10 +9,10 @@ This guide provides notes for major version releases. These notes may be helpful
 #### Enabling BBR and MCTCP
 
 Consensus node operators must enable the BBR (Bottleneck Bandwidth and Round-trip propagation time) congestion control algorithm. See [#3774](https://github.com/celestiaorg/celestia-app/pull/3774).
-if using linux in docker, kubernetes, a vm or baremetal, this can be done by calling 
+if using linux in docker, kubernetes, a vm or baremetal, this can be done by calling
 
 ```sh
-make enable-bbr 
+make enable-bbr
 ```
 
 command on the host machine.
@@ -54,7 +54,7 @@ You can track the tally of signalling by validators using the following query
 celestia-appd query signal tally 3
 ```
 
-Once 5/6+ of the voting power have signalled, the upgrade will be ready. There is a hard coded delay between confirmation of the upgrade and execution to the new state machine. 
+Once 5/6+ of the voting power have signalled, the upgrade will be ready. There is a hard coded delay between confirmation of the upgrade and execution to the new state machine.
 
 To view the upcoming upgrade height use the following query:
 

diff --git a/pkg/appconsts/chain_ids.go b/pkg/appconsts/chain_ids.go
@@ -2,4 +2,8 @@ package appconsts
 
 const (
 	ArabicaChainID = "arabica-11"
+	MochaChainID   = "mocha-4"
+	MainnetChainID = "celestia"
 )
+
+var PublicNetworks = []string{ArabicaChainID, MochaChainID, MainnetChainID}
diff --git a/scripts/single-node.sh b/scripts/single-node.sh
@@ -104,8 +104,8 @@ startCelestiaApp() {
     --api.enable \
     --grpc.enable \
     --grpc-web.enable \
-    --v2-upgrade-height 3 \
-    --force-no-bbr // no need to require BBR usage on a local node
+    --timeout-commit 1s \
+    --force-no-bbr # no need to require BBR usage on a local node
 }
 
 if [ -f $GENESIS_FILE ]; then

diff --git a/test/tokenfilter/setup.go b/test/tokenfilter/setup.go
@@ -146,9 +146,7 @@ func SetupWithGenesisValSet(t testing.TB, valSet *tmtypes.ValidatorSet, genAccs
 	db := dbm.NewMemDB()
 	encCdc := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 	genesisState := app.NewDefaultGenesisState(encCdc.Codec)
-	app := app.New(
-		log.NewNopLogger(), db, nil, 5, encCdc, 0, simapp.EmptyAppOptions{},
-	)
+	app := app.New(log.NewNopLogger(), db, nil, 5, encCdc, 0, 0, simapp.EmptyAppOptions{})
 
 	// set genesis accounts
 	authGenesis := authtypes.NewGenesisState(authtypes.DefaultParams(), genAccs)

diff --git a/test/util/malicious/app.go b/test/util/malicious/app.go
@@ -56,7 +56,7 @@ func New(
 	appOpts servertypes.AppOptions,
 	baseAppOptions ...func(*baseapp.BaseApp),
 ) *App {
-	goodApp := app.New(logger, db, traceStore, invCheckPeriod, encodingConfig, 0, appOpts, baseAppOptions...)
+	goodApp := app.New(logger, db, traceStore, invCheckPeriod, encodingConfig, 0, 0, appOpts, baseAppOptions...)
 	badApp := &App{App: goodApp}
 
 	// set the malicious prepare proposal handler if it is set in the app options

diff --git a/test/util/test_app.go b/test/util/test_app.go
@@ -107,6 +107,7 @@ func NewTestApp() *app.App {
 		cast.ToUint(emptyOpts.Get(server.FlagInvCheckPeriod)),
 		encCfg,
 		0,
+		0,
 		emptyOpts,
 	)
 }
@@ -467,7 +468,7 @@ func SetupTestAppWithUpgradeHeight(t *testing.T, upgradeHeight int64) (*app.App,
 	db := dbm.NewMemDB()
 	chainID := "test_chain"
 	encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)
-	testApp := app.New(log.NewNopLogger(), db, nil, 0, encCfg, upgradeHeight, EmptyAppOptions{})
+	testApp := app.New(log.NewNopLogger(), db, nil, 0, encCfg, upgradeHeight, 0, EmptyAppOptions{})
 	genesisState, _, kr := GenesisStateWithSingleValidator(testApp, "account")
 	stateBytes, err := json.MarshalIndent(genesisState, "", " ")
 	require.NoError(t, err)

diff --git a/test/util/testnode/config.go b/test/util/testnode/config.go
@@ -189,6 +189,7 @@ func DefaultAppCreator(opts ...AppCreationOptions) srvtypes.AppCreator {
 			0,   // invCheckPerid
 			encodingConfig,
 			0, // v2 upgrade height
+			0, // timeout commit
 			simapp.EmptyAppOptions{},
 			baseapp.SetMinGasPrices(fmt.Sprintf("%v%v", appconsts.DefaultMinGasPrice, app.BondDenom)),
 		)
@@ -211,6 +212,7 @@ func CustomAppCreator(minGasPrice string) srvtypes.AppCreator {
 			0,   // invCheckPerid
 			encodingConfig,
 			0, // v2 upgrade height
+			0, // timeout commit
 			simapp.EmptyAppOptions{},
 			baseapp.SetMinGasPrices(minGasPrice),
 		)

diff --git a/tools/chainbuilder/integration_test.go b/tools/chainbuilder/integration_test.go
@@ -65,7 +65,8 @@ func TestRun(t *testing.T) {
 		nil,
 		0,
 		encCfg,
-		0,
+		0, // upgrade height v2
+		0, // timeout commit
 		util.EmptyAppOptions{},
 		baseapp.SetMinGasPrices(fmt.Sprintf("%f%s", appconsts.DefaultMinGasPrice, appconsts.BondDenom)),
 	)

diff --git a/tools/chainbuilder/main.go b/tools/chainbuilder/main.go
@@ -202,7 +202,8 @@ func Run(ctx context.Context, cfg BuilderConfig, dir string) error {
 		nil,
 		0,
 		encCfg,
-		0,
+		0, // upgrade height v2
+		0, // timeout commit
 		util.EmptyAppOptions{},
 		baseapp.SetMinGasPrices(fmt.Sprintf("%f%s", appconsts.DefaultMinGasPrice, appconsts.BondDenom)),
 	)