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

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. ```
celestiaorg · Dec 12, 2024 · d20916c · d20916c
1 parent ad3a7fa
commit d20916c
Show file tree

Hide file tree

Showing 17 changed files with 68 additions and 19 deletions.
diff --git a/Makefile b/Makefile
@@ -48,6 +48,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
@@ -130,6 +130,15 @@ celestia-appd tx blob pay-for-blob 0x00010203040506070809 0x48656c6c6f2c20576f72
 
 If you 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])
@@ -481,7 +488,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
 }
@@ -539,8 +546,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
 }
@@ -565,7 +572,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
 }
@@ -849,3 +856,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
@@ -253,7 +253,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().
 		WithChainID("test").
 		WithValidators(genesis.NewDefaultValidator(testnode.DefaultValidatorAccountName)).

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
@@ -132,6 +132,10 @@ is performed. Note, when enabled, gRPC will also be automatically enabled.
 				serverCtx.Logger.Info(fmt.Sprintf("No default value exists for the v2 upgrade height when the chainID is %v", clientCtx.ChainID))
 			}
 
+			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/pkg/appconsts/chain_ids.go b/pkg/appconsts/chain_ids.go
@@ -5,3 +5,5 @@ const (
 	MochaChainID   = "mocha-4"
 	MainnetChainID = "celestia"
 )
+
+var PublicNetworks = []string{ArabicaChainID, MochaChainID, MainnetChainID}
diff --git a/scripts/single-node.sh b/scripts/single-node.sh
@@ -102,6 +102,7 @@ startCelestiaApp() {
     --api.enable \
     --grpc.enable \
     --grpc-web.enable \
+    --timeout-commit 1s \
     --force-no-bbr # no need to require BBR usage on a local node
 }
 

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
@@ -176,6 +176,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)),
 		)
@@ -198,6 +199,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)),
 	)