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.

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: override timeout commit via --timeout-commit (backport #4103) #4114

Merged
merged 3 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 20 additions & 3 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -188,6 +193,7 @@ func New(
invCheckPeriod uint,
encodingConfig encoding.Config,
upgradeHeightV2 int64,
timeoutCommit time.Duration,
appOpts servertypes.AppOptions,
baseAppOptions ...func(*baseapp.BaseApp),
) *App {
Expand All @@ -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])
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
12 changes: 8 additions & 4 deletions app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/test/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion cmd/celestia-appd/cmd/app_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion cmd/celestia-appd/cmd/app_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
Expand Down
9 changes: 7 additions & 2 deletions cmd/celestia-appd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions cmd/celestia-appd/cmd/start.go
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,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")
Expand Down
2 changes: 1 addition & 1 deletion docs/maintainers/release-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
6 changes: 3 additions & 3 deletions docs/release-notes/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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:

Expand Down
4 changes: 4 additions & 0 deletions pkg/appconsts/chain_ids.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@ package appconsts

const (
ArabicaChainID = "arabica-11"
MochaChainID = "mocha-4"
MainnetChainID = "celestia"
)

var PublicNetworks = []string{ArabicaChainID, MochaChainID, MainnetChainID}
4 changes: 2 additions & 2 deletions scripts/single-node.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions test/tokenfilter/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/util/malicious/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/util/test_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func NewTestApp() *app.App {
cast.ToUint(emptyOpts.Get(server.FlagInvCheckPeriod)),
encCfg,
0,
0,
emptyOpts,
)
}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions test/util/testnode/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
)
Expand All @@ -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),
)
Expand Down
3 changes: 2 additions & 1 deletion tools/chainbuilder/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
)
Expand Down
3 changes: 2 additions & 1 deletion tools/chainbuilder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
)
Expand Down
Loading