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.

Sign up for GitHub

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

Jump to bottom

fix(server/v2/cometbft): wire app closer #22240

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+36 −13
Open

fix(server/v2/cometbft): wire app closer #22240

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9c2af67
fix(server/v2/cometbft): close app
julienrbrt Oct 11, 2024
2afd304
close main db from store/v2
julienrbrt Oct 11, 2024
029d19f
typo
julienrbrt Oct 11, 2024
6e737e6
updates
julienrbrt Oct 11, 2024
9b6e888
updates
julienrbrt Oct 12, 2024
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
3 changes: 3 additions & 0 deletions server/v2/cometbft/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type Consensus[T transaction.Tx] struct {
logger log.Logger
appName, version string
app *appmanager.AppManager[T]
appCloser func() error
txCodec transaction.Codec[T]
store types.Store
streaming streaming.Manager
Expand Down Expand Up @@ -77,6 +78,7 @@ func NewConsensus[T transaction.Tx](
logger log.Logger,
appName string,
app *appmanager.AppManager[T],
appCloser func() error,
mp mempool.Mempool[T],
indexedEvents map[string]struct{},
queryHandlersMap map[string]appmodulev2.Handler,
Expand All @@ -89,6 +91,7 @@ func NewConsensus[T transaction.Tx](
appName: appName,
version: getCometBFTServerVersion(),
app: app,
appCloser: appCloser,
cfg: cfg,
store: store,
logger: logger,
Expand Down
6 changes: 3 additions & 3 deletions server/v2/cometbft/abci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"cosmossdk.io/core/store"
"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
am "cosmossdk.io/server/v2/appmanager"
"cosmossdk.io/server/v2/appmanager"
"cosmossdk.io/server/v2/cometbft/handlers"
cometmock "cosmossdk.io/server/v2/cometbft/internal/mock"
"cosmossdk.io/server/v2/cometbft/mempool"
Expand Down Expand Up @@ -672,7 +672,7 @@ func setUpConsensus(t *testing.T, gasLimit uint64, mempool mempool.Mempool[mock.
sc := cometmock.NewMockCommiter(log.NewNopLogger(), string(actorName), "stf")
mockStore := cometmock.NewMockStore(ss, sc)

b := am.Builder[mock.Tx]{
b := appmanager.Builder[mock.Tx]{
STF: s,
DB: mockStore,
ValidateTxGasLimit: gasLimit,
Expand All @@ -688,7 +688,7 @@ func setUpConsensus(t *testing.T, gasLimit uint64, mempool mempool.Mempool[mock.
am, err := b.Build()
require.NoError(t, err)

return NewConsensus[mock.Tx](log.NewNopLogger(), "testing-app", am, mempool, map[string]struct{}{}, nil, mockStore, Config{AppTomlConfig: DefaultAppTomlConfig()}, mock.TxCodec{}, "test")
return NewConsensus[mock.Tx](log.NewNopLogger(), "testing-app", am, func() error { return nil }, mempool, map[string]struct{}{}, nil, mockStore, Config{AppTomlConfig: DefaultAppTomlConfig()}, mock.TxCodec{}, "test")
}

// Check target version same with store's latest version
Expand Down
1 change: 1 addition & 0 deletions server/v2/cometbft/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func (s *CometBFTServer[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logg
s.logger,
appI.Name(),
appI.GetAppManager(),
appI.Close,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Inconsistency Detected in NewConsensus Function Signature

The NewConsensus function is being called with an additional appI.Close parameter, but its signature does not reflect this change.

  • File: server/v2/cometbft/server.go
  • Issue: NewConsensus function signature missing the appI.Close parameter.
🔗 Analysis chain

LGTM. Verify consistency of appI.Close usage.

The addition of appI.Close to the NewConsensus function call enhances the application's lifecycle management. This change allows for graceful shutdown of the application.

Please ensure that this change is consistent with the NewConsensus function signature and its usage across the codebase. Run the following script to verify:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of NewConsensus function and appI.Close method

# Test 1: Check the NewConsensus function signature
echo "Checking NewConsensus function signature:"
ast-grep --lang go --pattern 'func NewConsensus($_) $_'

# Test 2: Check for other occurrences of NewConsensus function calls
echo "Checking other NewConsensus function calls:"
rg --type go 'NewConsensus\(' -A 10 -B 2

# Test 3: Check for the definition and usage of Close method in AppI interface
echo "Checking AppI interface for Close method:"
ast-grep --lang go --pattern 'type AppI[T $_] interface {
  $$$
  Close() $_
  $$$
}'

Length of output: 1613

s.serverOptions.Mempool(cfg),
indexEvents,
appI.GetQueryHandlers(),
Expand Down
1 change: 1 addition & 0 deletions server/v2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ type AppI[T transaction.Tx] interface {
GetQueryHandlers() map[string]appmodulev2.Handler
GetStore() store.RootStore
GetSchemaDecoderResolver() decoding.DecoderResolver
Close() error
}
10 changes: 10 additions & 0 deletions simapp/v2/app_di.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,16 @@ func (app *SimApp[T]) TxConfig() client.TxConfig {
return app.txConfig
}

// GetStore returns the root store.
func (app *SimApp[T]) GetStore() store.RootStore {
return app.store
}

// Close overwrites the base Close method to close the stores.
func (app *SimApp[T]) Close() error {
if err := app.store.Close(); err != nil {
return err
}

return app.App.Close()
}
Loading