From 15086e8a3019f6b36a075f67aeb7438f2bc2d1e2 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Thu, 24 Oct 2024 07:55:37 -0500 Subject: [PATCH 1/9] fix!: use the correct version when upgrading --- app/test/priority_test.go | 2 +- app/test/square_size_test.go | 4 +++- app/test/upgrade_test.go | 8 ++++---- pkg/appconsts/versioned_consts.go | 12 ++++++++---- test/util/testnode/config.go | 12 +++++++----- x/signal/integration_test.go | 4 +++- x/signal/keeper.go | 3 ++- x/signal/keeper_test.go | 5 +++-- 8 files changed, 31 insertions(+), 19 deletions(-) diff --git a/app/test/priority_test.go b/app/test/priority_test.go index da7936dd0d..b615b8e9ac 100644 --- a/app/test/priority_test.go +++ b/app/test/priority_test.go @@ -52,7 +52,7 @@ func (s *PriorityTestSuite) SetupSuite() { cfg := testnode.DefaultConfig(). WithFundedAccounts(s.accountNames...). // use a long block time to guarantee that some transactions are included in the same block - WithTimeoutCommit(time.Second) + WithBlockTime(time.Second) cctx, _, _ := testnode.NewNetwork(t, cfg) s.cctx = cctx diff --git a/app/test/square_size_test.go b/app/test/square_size_test.go index d030b89093..754a5d0adf 100644 --- a/app/test/square_size_test.go +++ b/app/test/square_size_test.go @@ -46,8 +46,10 @@ func (s *SquareSizeIntegrationTest) SetupSuite() { t := s.T() t.Log("setting up square size integration test") s.ecfg = encoding.MakeConfig(app.ModuleEncodingRegisters...) + cfg := testnode.DefaultConfig(). - WithModifiers(genesis.ImmediateProposals(s.ecfg.Codec)) + WithModifiers(genesis.ImmediateProposals(s.ecfg.Codec)). + WithBlockTime(time.Second) cctx, rpcAddr, grpcAddr := testnode.NewNetwork(t, cfg) diff --git a/app/test/upgrade_test.go b/app/test/upgrade_test.go index 4c2677c8e0..e42449d747 100644 --- a/app/test/upgrade_test.go +++ b/app/test/upgrade_test.go @@ -106,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.UpgradeHeightDelay(v2.Version); height++ { + for height := initialHeight; height < initialHeight+appconsts.UpgradeHeightDelay(testApp.GetChainID(), v2.Version); height++ { appVersion := v2.Version _ = testApp.BeginBlock(abci.RequestBeginBlock{ Header: tmproto.Header{ @@ -116,7 +116,7 @@ func TestAppUpgradeV3(t *testing.T) { }) endBlockResp = testApp.EndBlock(abci.RequestEndBlock{ - Height: 3 + appconsts.UpgradeHeightDelay(v2.Version), + Height: 3 + appconsts.UpgradeHeightDelay(testApp.GetChainID(), v2.Version), }) require.Equal(t, appconsts.GetTimeoutCommit(appVersion), endBlockResp.Timeouts.TimeoutCommit) @@ -141,7 +141,7 @@ func TestAppUpgradeV3(t *testing.T) { _ = testApp.BeginBlock(abci.RequestBeginBlock{ Header: tmproto.Header{ ChainID: genesis.ChainID, - Height: initialHeight + appconsts.UpgradeHeightDelay(v3.Version), + Height: initialHeight + appconsts.UpgradeHeightDelay(testApp.GetChainID(), v3.Version), Version: tmversion.Consensus{App: 3}, }, }) @@ -152,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.UpgradeHeightDelay(v3.Version)}) + RequestEndBlock{Height: initialHeight + appconsts.UpgradeHeightDelay(testApp.GetChainID(), v3.Version)}) require.Equal(t, appconsts.GetTimeoutCommit(v3.Version), respEndBlock.Timeouts.TimeoutCommit) require.Equal(t, appconsts.GetTimeoutPropose(v3.Version), respEndBlock.Timeouts.TimeoutPropose) } diff --git a/pkg/appconsts/versioned_consts.go b/pkg/appconsts/versioned_consts.go index 2455e87791..8f99d8c3bb 100644 --- a/pkg/appconsts/versioned_consts.go +++ b/pkg/appconsts/versioned_consts.go @@ -2,6 +2,7 @@ package appconsts import ( "strconv" + "strings" "time" v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" @@ -79,7 +80,7 @@ func GetTimeoutCommit(v uint64) time.Duration { } // 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 { +func UpgradeHeightDelay(chainID string, v uint64) int64 { if OverrideUpgradeHeightDelayStr != "" { parsedValue, err := strconv.ParseInt(OverrideUpgradeHeightDelayStr, 10, 64) if err != nil { @@ -87,10 +88,13 @@ func UpgradeHeightDelay(v uint64) int64 { } return parsedValue } - switch v { - case v1.Version: + switch { + case v == v1.Version: return v1.UpgradeHeightDelay - case v2.Version: + // ONLY ON ARABICA: don't return the v2 value even when the app version is + // v2 on arabica. This is due to a bug that was shipped on arabica, where + // the next version was used. + case v == v2.Version && !strings.Contains(chainID, "arabica"): return v2.UpgradeHeightDelay default: return v3.UpgradeHeightDelay diff --git a/test/util/testnode/config.go b/test/util/testnode/config.go index 0ec6e7920e..0085ba565a 100644 --- a/test/util/testnode/config.go +++ b/test/util/testnode/config.go @@ -82,9 +82,11 @@ func (c *Config) WithSuppressLogs(sl bool) *Config { return c } -// WithTimeoutCommit sets the TimeoutCommit and returns the Config. -func (c *Config) WithTimeoutCommit(d time.Duration) *Config { +// WithBlockTime sets the block time and returns the Config. +func (c *Config) WithBlockTime(d time.Duration) *Config { c.TmConfig.Consensus.TimeoutCommit = d + creator := DefaultAppCreator(d) + c.WithAppCreator(creator) return c } @@ -132,7 +134,7 @@ func DefaultConfig() *Config { WithTendermintConfig(DefaultTendermintConfig()). WithAppConfig(DefaultAppConfig()). WithAppOptions(DefaultAppOptions()). - WithAppCreator(DefaultAppCreator()). + WithAppCreator(DefaultAppCreator(time.Millisecond * 30)). WithSuppressLogs(true) } @@ -171,7 +173,7 @@ func DefaultTendermintConfig() *tmconfig.Config { return tmCfg } -func DefaultAppCreator() srvtypes.AppCreator { +func DefaultAppCreator(blockTime time.Duration) srvtypes.AppCreator { return func(_ log.Logger, _ tmdb.DB, _ io.Writer, _ srvtypes.AppOptions) srvtypes.Application { encodingConfig := encoding.MakeConfig(app.ModuleEncodingRegisters...) app := app.New( @@ -184,7 +186,7 @@ func DefaultAppCreator() srvtypes.AppCreator { simapp.EmptyAppOptions{}, baseapp.SetMinGasPrices(fmt.Sprintf("%v%v", appconsts.DefaultMinGasPrice, app.BondDenom)), ) - app.SetEndBlocker(wrapEndBlocker(app, time.Millisecond*30)) + app.SetEndBlocker(wrapEndBlocker(app, blockTime)) return app } } diff --git a/x/signal/integration_test.go b/x/signal/integration_test.go index c3dd2419dd..829a2d0be9 100644 --- a/x/signal/integration_test.go +++ b/x/signal/integration_test.go @@ -23,10 +23,12 @@ func TestUpgradeIntegration(t *testing.T) { cp := app.DefaultConsensusParams() cp.Version.AppVersion = v2.Version app, _ := testutil.SetupTestAppWithGenesisValSet(cp) + chainID := "test" ctx := sdk.NewContext(app.CommitMultiStore(), tmtypes.Header{ Version: tmversion.Consensus{ App: v2.Version, }, + ChainID: chainID, }, false, tmlog.NewNopLogger()) goCtx := sdk.WrapSDKContext(ctx) ctx = sdk.UnwrapSDKContext(goCtx) @@ -77,7 +79,7 @@ func TestUpgradeIntegration(t *testing.T) { require.False(t, shouldUpgrade) require.EqualValues(t, 0, version) - ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.UpgradeHeightDelay(version)) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.UpgradeHeightDelay(chainID, version)) shouldUpgrade, version = app.SignalKeeper.ShouldUpgrade(ctx) require.True(t, shouldUpgrade) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 3ee13a9708..b25aa35bb0 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -102,9 +102,10 @@ func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types if version <= sdkCtx.BlockHeader().Version.App { return &types.MsgTryUpgradeResponse{}, types.ErrInvalidUpgradeVersion.Wrapf("can not upgrade to version %v because it is less than or equal to current version %v", version, sdkCtx.BlockHeader().Version.App) } + header := sdkCtx.BlockHeader() upgrade := types.Upgrade{ AppVersion: version, - UpgradeHeight: sdkCtx.BlockHeader().Height + appconsts.UpgradeHeightDelay(version), + UpgradeHeight: sdkCtx.BlockHeader().Height + appconsts.UpgradeHeightDelay(header.ChainID, header.Version.App), } k.setUpgrade(sdkCtx, upgrade) } diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index f79d1c3884..6c688a3328 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -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.UpgradeHeightDelay(version)) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.UpgradeHeightDelay("test", 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.UpgradeHeightDelay(v2.Version), got.Upgrade.UpgradeHeight) + assert.Equal(t, appconsts.UpgradeHeightDelay("test", v2.Version), got.Upgrade.UpgradeHeight) }) } @@ -441,6 +441,7 @@ func setup(t *testing.T) (signal.Keeper, sdk.Context, *mockStakingKeeper) { Block: 1, App: 1, }, + ChainID: "test", }, false, log.NewNopLogger()) mockStakingKeeper := newMockStakingKeeper( map[string]int64{ From fd4b10851b6cafa1784741c8f714efd183db3c5b Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Thu, 24 Oct 2024 08:03:17 -0500 Subject: [PATCH 2/9] fix: lingering api breaks --- test/txsim/run_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/txsim/run_test.go b/test/txsim/run_test.go index d4cbdf22ba..2dfa608574 100644 --- a/test/txsim/run_test.go +++ b/test/txsim/run_test.go @@ -151,7 +151,7 @@ func TestTxSimulator(t *testing.T) { func Setup(t testing.TB) (keyring.Keyring, string, string) { t.Helper() - cfg := testnode.DefaultConfig().WithTimeoutCommit(300 * time.Millisecond).WithFundedAccounts("txsim-master") + cfg := testnode.DefaultConfig().WithBlockTime(300 * time.Millisecond).WithFundedAccounts("txsim-master") cctx, rpcAddr, grpcAddr := testnode.NewNetwork(t, cfg) @@ -165,7 +165,7 @@ func TestTxSimUpgrade(t *testing.T) { cp := app.DefaultConsensusParams() cp.Version.AppVersion = v2.Version cfg := testnode.DefaultConfig(). - WithTimeoutCommit(300 * time.Millisecond). + WithBlockTime(300 * time.Millisecond). WithConsensusParams(cp). WithFundedAccounts("txsim-master") cctx, _, grpcAddr := testnode.NewNetwork(t, cfg) From c9b257ed59cf830d6293fe4147fc5f5d760f9333 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Thu, 24 Oct 2024 09:20:22 -0500 Subject: [PATCH 3/9] test: add unit test for versioned upgrade --- pkg/appconsts/versioned_consts_test.go | 47 ++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index f621c0199e..249dd99805 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -80,3 +80,50 @@ func TestVersionedConsts(t *testing.T) { }) } } + +func TestUpgradeHeightDelay(t *testing.T) { + tests := []struct { + name string + chainID string + version uint64 + expectedUpgradeHeightDelay int64 + }{ + { + name: "v1 upgrade delay", + chainID: "test-chain", + version: v1.Version, + expectedUpgradeHeightDelay: v1.UpgradeHeightDelay, + }, + { + name: "v1 arabica upgrade delay", + chainID: "arabica-11", + version: v1.Version, + expectedUpgradeHeightDelay: v1.UpgradeHeightDelay, + }, + { + name: "v2 upgrade delay on non-arabica chain", + chainID: "celestia", + version: v2.Version, + expectedUpgradeHeightDelay: v2.UpgradeHeightDelay, + }, + { + name: "v2 upgrade delay on arabica", + chainID: "arabica-11", + version: v2.Version, + expectedUpgradeHeightDelay: v3.UpgradeHeightDelay, // falls back to v3 because of arabica bug + }, + { + name: "v3 upgrade delay", + chainID: "mocha-4", + version: 3, + expectedUpgradeHeightDelay: v3.UpgradeHeightDelay, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + actual := appconsts.UpgradeHeightDelay(tc.chainID, tc.version) + require.Equal(t, tc.expectedUpgradeHeightDelay, actual) + }) + } +} From 07a127135c507e28444020638c4f087a47c55abb Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Thu, 24 Oct 2024 09:28:10 -0500 Subject: [PATCH 4/9] chore: deprecate WithTimeoutCommit --- test/util/testnode/config.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/util/testnode/config.go b/test/util/testnode/config.go index 0085ba565a..fff1053494 100644 --- a/test/util/testnode/config.go +++ b/test/util/testnode/config.go @@ -90,6 +90,14 @@ func (c *Config) WithBlockTime(d time.Duration) *Config { return c } +// Deprecated: use WithBlockTime instead. WithTimeoutCommit sets the timeout +// commit in the cometBFT config and returns the Config. Warning, this won't +// actually change the block time and is being deprecated. +func (c *Config) WithTimeoutCommit(d time.Duration) *Config { + c.TmConfig.Consensus.TimeoutCommit = d + return c +} + // WithFundedAccounts sets the genesis accounts and returns the Config. func (c *Config) WithFundedAccounts(accounts ...string) *Config { c.Genesis = c.Genesis.WithKeyringAccounts( From ba4ef7bd9d78a1efbb9a5258a3ee413e1ec70f6e Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Mon, 28 Oct 2024 16:24:14 -0500 Subject: [PATCH 5/9] refactor: use arabica-11 instead of all arabica networks --- pkg/appconsts/chain_ids.go | 5 +++++ pkg/appconsts/versioned_consts.go | 16 +++++++++------- pkg/appconsts/versioned_consts_test.go | 5 +++++ 3 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 pkg/appconsts/chain_ids.go diff --git a/pkg/appconsts/chain_ids.go b/pkg/appconsts/chain_ids.go new file mode 100644 index 0000000000..50c26932f9 --- /dev/null +++ b/pkg/appconsts/chain_ids.go @@ -0,0 +1,5 @@ +package appconsts + +const ( + ArabicaChainID = "arabica-11" +) diff --git a/pkg/appconsts/versioned_consts.go b/pkg/appconsts/versioned_consts.go index 8f99d8c3bb..6034453598 100644 --- a/pkg/appconsts/versioned_consts.go +++ b/pkg/appconsts/versioned_consts.go @@ -2,7 +2,6 @@ package appconsts import ( "strconv" - "strings" "time" v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" @@ -88,13 +87,16 @@ func UpgradeHeightDelay(chainID string, v uint64) int64 { } return parsedValue } - switch { - case v == v1.Version: + switch v { + case v1.Version: return v1.UpgradeHeightDelay - // ONLY ON ARABICA: don't return the v2 value even when the app version is - // v2 on arabica. This is due to a bug that was shipped on arabica, where - // the next version was used. - case v == v2.Version && !strings.Contains(chainID, "arabica"): + case v2.Version: + // ONLY ON ARABICA: don't return the v2 value even when the app version is + // v2 on arabica. This is due to a bug that was shipped on arabica, where + // the next version was used. + if chainID == ArabicaChainID { + return v3.UpgradeHeightDelay + } return v2.UpgradeHeightDelay default: return v3.UpgradeHeightDelay diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index 249dd99805..1fc6c75cb0 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -105,6 +105,11 @@ func TestUpgradeHeightDelay(t *testing.T) { chainID: "celestia", version: v2.Version, expectedUpgradeHeightDelay: v2.UpgradeHeightDelay, + }, { + name: "v2 upgrade delay on an arabica network other than arabica-11", + chainID: "arabica-11", + version: v2.Version, + expectedUpgradeHeightDelay: v3.UpgradeHeightDelay, // falls back to v3 because of arabica bug }, { name: "v2 upgrade delay on arabica", From bce247e84c6cebe3714906be125cce2a7d5d2f8d Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 28 Oct 2024 16:32:16 -0500 Subject: [PATCH 6/9] Update x/signal/keeper.go Co-authored-by: Callum Waters --- x/signal/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index b25aa35bb0..7e85e604d5 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -105,7 +105,7 @@ func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types header := sdkCtx.BlockHeader() upgrade := types.Upgrade{ AppVersion: version, - UpgradeHeight: sdkCtx.BlockHeader().Height + appconsts.UpgradeHeightDelay(header.ChainID, header.Version.App), + UpgradeHeight: header.Height + appconsts.UpgradeHeightDelay(header.ChainID, header.Version.App), } k.setUpgrade(sdkCtx, upgrade) } From 02b2a78a63ba87c5efadf6a366adcb13d2a407f0 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Mon, 28 Oct 2024 16:38:33 -0500 Subject: [PATCH 7/9] add upgradeHeightdelay to the specs --- specs/src/parameters_v2.md | 9 +++++---- specs/src/parameters_v3.md | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/specs/src/parameters_v2.md b/specs/src/parameters_v2.md index 9bd3e63f25..ec93c9acda 100644 --- a/specs/src/parameters_v2.md +++ b/specs/src/parameters_v2.md @@ -8,10 +8,11 @@ hardcoded in the application or they are blocked by the `x/paramfilter` module. ## Global parameters -| Parameter | Default | Summary | Changeable via Governance | -|-------------------|---------|------------------------------------------------------------------------------------------------------------------------|---------------------------| -| MaxBlockSizeBytes | 100MiB | Hardcoded value in CometBFT for the protobuf encoded block. | False | -| MaxSquareSize | 128 | Hardcoded maximum square size determined per shares per row or column for the original data square (not yet extended). | False | +| Parameter | Default | Summary | Changeable via Governance | +|--------------------|---------|------------------------------------------------------------------------------------------------------------------------|---------------------------| +| MaxBlockSizeBytes | 100MiB | Hardcoded value in CometBFT for the protobuf encoded block. | False | +| MaxSquareSize | 128 | Hardcoded maximum square size determined per shares per row or column for the original data square (not yet extended). | False | +| UpgradeHeightDelay | 50400 | Height based delay after a successful `MsgTryUpgrade` has been submitted. | False | ## Module parameters diff --git a/specs/src/parameters_v3.md b/specs/src/parameters_v3.md index 860cdb286f..6f8d84fccb 100644 --- a/specs/src/parameters_v3.md +++ b/specs/src/parameters_v3.md @@ -8,10 +8,11 @@ hardcoded in the application or they are blocked by the `x/paramfilter` module. ## Global parameters -| Parameter | Default | Summary | Changeable via Governance | -|-------------------|---------|------------------------------------------------------------------------------------------------------------------------|---------------------------| -| MaxBlockSizeBytes | 100MiB | Hardcoded value in CometBFT for the protobuf encoded block. | False | -| MaxSquareSize | 128 | Hardcoded maximum square size determined per shares per row or column for the original data square (not yet extended). | False | +| Parameter | Default | Summary | Changeable via Governance | +|--------------------|---------|------------------------------------------------------------------------------------------------------------------------|---------------------------| +| MaxBlockSizeBytes | 100MiB | Hardcoded value in CometBFT for the protobuf encoded block. | False | +| MaxSquareSize | 128 | Hardcoded maximum square size determined per shares per row or column for the original data square (not yet extended). | False | +| UpgradeHeightDelay | 100800 | Height based delay after a successful `MsgTryUpgrade` has been submitted. | False | ## Module parameters From 04c7c340946c602f8d56a7b92bf9e37fb3529629 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Mon, 28 Oct 2024 16:46:37 -0500 Subject: [PATCH 8/9] chore: linter --- pkg/appconsts/versioned_consts_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index 1fc6c75cb0..8f67d4cefd 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -105,7 +105,8 @@ func TestUpgradeHeightDelay(t *testing.T) { chainID: "celestia", version: v2.Version, expectedUpgradeHeightDelay: v2.UpgradeHeightDelay, - }, { + }, + { name: "v2 upgrade delay on an arabica network other than arabica-11", chainID: "arabica-11", version: v2.Version, From c53ee35822cafa8eef14a590198d2dc7a4f0d816 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Tue, 29 Oct 2024 09:21:13 -0500 Subject: [PATCH 9/9] refactor: timeout option --- app/test/priority_test.go | 2 +- app/test/square_size_test.go | 2 +- pkg/appconsts/versioned_consts_test.go | 6 ----- test/txsim/run_test.go | 4 +-- test/util/testnode/config.go | 34 ++++++++++++++------------ 5 files changed, 22 insertions(+), 26 deletions(-) diff --git a/app/test/priority_test.go b/app/test/priority_test.go index b615b8e9ac..da7936dd0d 100644 --- a/app/test/priority_test.go +++ b/app/test/priority_test.go @@ -52,7 +52,7 @@ func (s *PriorityTestSuite) SetupSuite() { cfg := testnode.DefaultConfig(). WithFundedAccounts(s.accountNames...). // use a long block time to guarantee that some transactions are included in the same block - WithBlockTime(time.Second) + WithTimeoutCommit(time.Second) cctx, _, _ := testnode.NewNetwork(t, cfg) s.cctx = cctx diff --git a/app/test/square_size_test.go b/app/test/square_size_test.go index 754a5d0adf..b7d147fe0d 100644 --- a/app/test/square_size_test.go +++ b/app/test/square_size_test.go @@ -49,7 +49,7 @@ func (s *SquareSizeIntegrationTest) SetupSuite() { cfg := testnode.DefaultConfig(). WithModifiers(genesis.ImmediateProposals(s.ecfg.Codec)). - WithBlockTime(time.Second) + WithTimeoutCommit(time.Second) cctx, rpcAddr, grpcAddr := testnode.NewNetwork(t, cfg) diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index 8f67d4cefd..249dd99805 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -106,12 +106,6 @@ func TestUpgradeHeightDelay(t *testing.T) { version: v2.Version, expectedUpgradeHeightDelay: v2.UpgradeHeightDelay, }, - { - name: "v2 upgrade delay on an arabica network other than arabica-11", - chainID: "arabica-11", - version: v2.Version, - expectedUpgradeHeightDelay: v3.UpgradeHeightDelay, // falls back to v3 because of arabica bug - }, { name: "v2 upgrade delay on arabica", chainID: "arabica-11", diff --git a/test/txsim/run_test.go b/test/txsim/run_test.go index 2dfa608574..d4cbdf22ba 100644 --- a/test/txsim/run_test.go +++ b/test/txsim/run_test.go @@ -151,7 +151,7 @@ func TestTxSimulator(t *testing.T) { func Setup(t testing.TB) (keyring.Keyring, string, string) { t.Helper() - cfg := testnode.DefaultConfig().WithBlockTime(300 * time.Millisecond).WithFundedAccounts("txsim-master") + cfg := testnode.DefaultConfig().WithTimeoutCommit(300 * time.Millisecond).WithFundedAccounts("txsim-master") cctx, rpcAddr, grpcAddr := testnode.NewNetwork(t, cfg) @@ -165,7 +165,7 @@ func TestTxSimUpgrade(t *testing.T) { cp := app.DefaultConsensusParams() cp.Version.AppVersion = v2.Version cfg := testnode.DefaultConfig(). - WithBlockTime(300 * time.Millisecond). + WithTimeoutCommit(300 * time.Millisecond). WithConsensusParams(cp). WithFundedAccounts("txsim-master") cctx, _, grpcAddr := testnode.NewNetwork(t, cfg) diff --git a/test/util/testnode/config.go b/test/util/testnode/config.go index fff1053494..76508f91d4 100644 --- a/test/util/testnode/config.go +++ b/test/util/testnode/config.go @@ -82,20 +82,10 @@ func (c *Config) WithSuppressLogs(sl bool) *Config { return c } -// WithBlockTime sets the block time and returns the Config. -func (c *Config) WithBlockTime(d time.Duration) *Config { - c.TmConfig.Consensus.TimeoutCommit = d - creator := DefaultAppCreator(d) - c.WithAppCreator(creator) - return c -} - -// Deprecated: use WithBlockTime instead. WithTimeoutCommit sets the timeout -// commit in the cometBFT config and returns the Config. Warning, this won't -// actually change the block time and is being deprecated. +// WithTimeoutCommit sets the timeout commit in the cometBFT config and returns +// the Config. func (c *Config) WithTimeoutCommit(d time.Duration) *Config { - c.TmConfig.Consensus.TimeoutCommit = d - return c + return c.WithAppCreator(DefaultAppCreator(WithTimeoutCommit(d))) } // WithFundedAccounts sets the genesis accounts and returns the Config. @@ -142,7 +132,7 @@ func DefaultConfig() *Config { WithTendermintConfig(DefaultTendermintConfig()). WithAppConfig(DefaultAppConfig()). WithAppOptions(DefaultAppOptions()). - WithAppCreator(DefaultAppCreator(time.Millisecond * 30)). + WithTimeoutCommit(time.Millisecond * 30). WithSuppressLogs(true) } @@ -181,7 +171,15 @@ func DefaultTendermintConfig() *tmconfig.Config { return tmCfg } -func DefaultAppCreator(blockTime time.Duration) srvtypes.AppCreator { +type AppCreationOptions func(app *app.App) + +func WithTimeoutCommit(d time.Duration) AppCreationOptions { + return func(app *app.App) { + app.SetEndBlocker(wrapEndBlocker(app, d)) + } +} + +func DefaultAppCreator(opts ...AppCreationOptions) srvtypes.AppCreator { return func(_ log.Logger, _ tmdb.DB, _ io.Writer, _ srvtypes.AppOptions) srvtypes.Application { encodingConfig := encoding.MakeConfig(app.ModuleEncodingRegisters...) app := app.New( @@ -194,7 +192,11 @@ func DefaultAppCreator(blockTime time.Duration) srvtypes.AppCreator { simapp.EmptyAppOptions{}, baseapp.SetMinGasPrices(fmt.Sprintf("%v%v", appconsts.DefaultMinGasPrice, app.BondDenom)), ) - app.SetEndBlocker(wrapEndBlocker(app, blockTime)) + + for _, opt := range opts { + opt(app) + } + return app } }