Skip to content

Commit

Permalink
[CORE-29] Migrate from stoppable to BaseApp#Stop.
Browse files Browse the repository at this point in the history
  • Loading branch information
lcwik committed Oct 18, 2023
1 parent c2be0ad commit 77155ba
Show file tree
Hide file tree
Showing 27 changed files with 36 additions and 201 deletions.
19 changes: 12 additions & 7 deletions protocol/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
reflectionv1 "cosmossdk.io/api/cosmos/reflection/v1"
sdklog "cosmossdk.io/log"

dbm "github.com/cometbft/cometbft-db"
abci "github.com/cometbft/cometbft/abci/types"
tmjson "github.com/cometbft/cometbft/libs/json"
Expand Down Expand Up @@ -95,8 +94,7 @@ import (
"github.com/dydxprotocol/v4-chain/protocol/app/middleware"
"github.com/dydxprotocol/v4-chain/protocol/app/prepare"
"github.com/dydxprotocol/v4-chain/protocol/app/process"
// Lib
"github.com/dydxprotocol/v4-chain/protocol/app/stoppable"

"github.com/dydxprotocol/v4-chain/protocol/lib"
"github.com/dydxprotocol/v4-chain/protocol/lib/metrics"
timelib "github.com/dydxprotocol/v4-chain/protocol/lib/time"
Expand Down Expand Up @@ -284,6 +282,8 @@ type App struct {
// delayed until after the gRPC service is initialized so that the gRPC service will be available and the daemons
// can correctly operate.
startDaemons func()

PriceFeedClient *pricefeedclient.Client
}

// assertAppPreconditions assert invariants required for an application to start.
Expand Down Expand Up @@ -546,7 +546,6 @@ func New(
grpc.NewServer(),
&daemontypes.FileHandlerImpl{},
daemonFlags.Shared.SocketAddress,
appFlags.GrpcAddress,
)
// Setup server for pricefeed messages. The server will wait for gRPC messages containing price
// updates and then encode them into an in-memory cache shared by the prices module.
Expand Down Expand Up @@ -602,7 +601,7 @@ func New(
// Start pricefeed client for sending prices for the pricefeed server to consume. These prices
// are retrieved via third-party APIs like Binance and then are encoded in-memory and
// periodically sent via gRPC to a shared socket with the server.
client := pricefeedclient.StartNewClient(
app.PriceFeedClient = pricefeedclient.StartNewClient(
// The client will use `context.Background` so that it can have a different context from
// the main application.
context.Background(),
Expand All @@ -614,7 +613,6 @@ func New(
constants.StaticExchangeDetails,
&pricefeedclient.SubTaskRunnerImpl{},
)
stoppable.RegisterServiceForTestCleanup(appFlags.GrpcAddress, client)
}

// Start Bridge Daemon.
Expand Down Expand Up @@ -1418,8 +1416,15 @@ func (app *App) setAnteHandler(txConfig client.TxConfig) {
app.SetAnteHandler(anteHandler)
}

// Close invokes an ordered shutdown of routines.
func (app *App) Close() error {
return app.BaseApp.Close()
if app.PriceFeedClient != nil {
app.PriceFeedClient.Stop()
}
if app.Server != nil {
app.Server.Stop()
}
return nil
}

// RegisterSwaggerAPI registers swagger route with API Server
Expand Down
9 changes: 9 additions & 0 deletions protocol/app/app_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package app_test

import (
"gopkg.in/typ.v4/slices"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -94,6 +95,14 @@ func TestAppIsFullyInitialized(t *testing.T) {
t.Run(name, func(t *testing.T) {
dydxApp := testapp.DefaultTestApp(tc.customFlags)
uninitializedFields := getUninitializedStructFields(reflect.ValueOf(*dydxApp))

// Note that the PriceFeedClient is currently hard coded as disabled in GetDefaultTestAppOptions.
// Normally it would be only disabled for non-validating full nodes or for nodes where the
// price feed client is explicitly disabled.
if idx := slices.Index(uninitializedFields, "PriceFeedClient"); idx >= 0 {
slices.Remove(&uninitializedFields, idx)
}

require.Len(
t,
uninitializedFields,
Expand Down
46 changes: 0 additions & 46 deletions protocol/app/stoppable/stoppable.go

This file was deleted.

42 changes: 0 additions & 42 deletions protocol/app/stoppable/stoppable_test.go

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ func (s *PriceDaemonIntegrationTestSuite) SetupTest() {
grpc.NewServer(),
&daemontypes.FileHandlerImpl{},
s.daemonFlags.Shared.SocketAddress,
"test",
)
s.daemonServer.ExpectPricefeedDaemon(servertypes.MaximumAcceptableUpdateDelay(s.daemonFlags.Price.LoopDelayMs))
s.exchangePriceCache = pricefeedserver_types.NewMarketToExchangePrices(pricefeed_types.MaxPriceAge)
Expand Down
1 change: 0 additions & 1 deletion protocol/daemons/pricefeed/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ func TestStop(t *testing.T) {
grpc.NewServer(),
&daemontypes.FileHandlerImpl{},
daemonFlags.Shared.SocketAddress,
"test",
)
daemonServer.WithPriceFeedMarketToExchangePrices(
pricefeed_types.NewMarketToExchangePrices(5 * time.Second),
Expand Down
2 changes: 2 additions & 0 deletions protocol/daemons/server/liquidation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func TestLiquidateSubaccounts_Empty_Update(t *testing.T) {
liquidatableSubaccountIds := liquidationtypes.NewLiquidatableSubaccountIds()

s := createServerWithMocks(
t,
mockGrpcServer,
mockFileHandler,
).WithLiquidatableSubaccountIds(
Expand All @@ -36,6 +37,7 @@ func TestLiquidateSubaccounts_Multiple_Subaccount_Ids(t *testing.T) {
liquidatableSubaccountIds := liquidationtypes.NewLiquidatableSubaccountIds()

s := createServerWithMocks(
t,
mockGrpcServer,
mockFileHandler,
).WithLiquidatableSubaccountIds(
Expand Down
4 changes: 4 additions & 0 deletions protocol/daemons/server/pricefeed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func TestUpdateMarketPrices_Valid(t *testing.T) {
mockFileHandler := &mocks.FileHandler{}

s := createServerWithMocks(
t,
mockGrpcServer,
mockFileHandler,
).WithPriceFeedMarketToExchangePrices(
Expand All @@ -43,6 +44,7 @@ func TestUpdateMarketPrices_NotInitialized(t *testing.T) {

// Create a new server without initializing `MarketToExchange` field.
s := createServerWithMocks(
t,
mockGrpcServer,
mockFileHandler,
)
Expand All @@ -69,6 +71,7 @@ func TestUpdateMarketPrices_InvalidEmptyRequest(t *testing.T) {
mockFileHandler := &mocks.FileHandler{}

s := createServerWithMocks(
t,
mockGrpcServer,
mockFileHandler,
).WithPriceFeedMarketToExchangePrices(
Expand Down Expand Up @@ -145,6 +148,7 @@ func TestUpdateMarketPrices_InvalidExchangePrices(t *testing.T) {
mockFileHandler := &mocks.FileHandler{}

s := createServerWithMocks(
t,
mockGrpcServer,
mockFileHandler,
).WithPriceFeedMarketToExchangePrices(
Expand Down
6 changes: 1 addition & 5 deletions protocol/daemons/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
gometrics "github.com/armon/go-metrics"
"github.com/cometbft/cometbft/libs/log"
"github.com/cosmos/cosmos-sdk/telemetry"
"github.com/dydxprotocol/v4-chain/protocol/app/stoppable"
bridgeapi "github.com/dydxprotocol/v4-chain/protocol/daemons/bridge/api"
"github.com/dydxprotocol/v4-chain/protocol/daemons/constants"
liquidationapi "github.com/dydxprotocol/v4-chain/protocol/daemons/liquidation/api"
Expand Down Expand Up @@ -42,17 +41,14 @@ func NewServer(
grpcServer daemontypes.GrpcServer,
fileHandler daemontypes.FileHandler,
socketAddress string,
uniqueTestIdentifier string,
) *Server {
srv := &Server{
return &Server{
logger: logger,
gsrv: grpcServer,
fileHandler: fileHandler,
socketAddress: socketAddress,
updateMonitor: types.NewUpdateFrequencyMonitor(types.DaemonStartupGracePeriod, logger),
}
stoppable.RegisterServiceForTestCleanup(uniqueTestIdentifier, srv)
return srv
}

// Stop stops the daemon server's gRPC service.
Expand Down
15 changes: 8 additions & 7 deletions protocol/daemons/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/cometbft/cometbft/libs/log"
pricefeedconstants "github.com/dydxprotocol/v4-chain/protocol/daemons/constants"
"github.com/dydxprotocol/v4-chain/protocol/daemons/server"
daemontypes "github.com/dydxprotocol/v4-chain/protocol/daemons/types"
"github.com/dydxprotocol/v4-chain/protocol/mocks"
"github.com/dydxprotocol/v4-chain/protocol/testutil/grpc"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -41,7 +40,7 @@ func TestStartServer_ListenFailsWhenInUse(t *testing.T) {
return nil
})

s := createServerWithMocks(mockGrpcServer, mockFileHandler)
s := createServerWithMocks(t, mockGrpcServer, mockFileHandler)

errorString := fmt.Sprintf(
"listen %s %s: bind: address already in use",
Expand All @@ -67,6 +66,7 @@ func TestStart_Valid(t *testing.T) {
mockFileHandler := &mocks.FileHandler{}

s := createServerWithMocks(
t,
mockGrpcServer,
mockFileHandler,
)
Expand Down Expand Up @@ -115,6 +115,7 @@ func TestStart_MixedInvalid(t *testing.T) {
mockFileHandler := &mocks.FileHandler{}

s := createServerWithMocks(
t,
mockGrpcServer,
mockFileHandler,
)
Expand Down Expand Up @@ -166,7 +167,6 @@ func TestRegisterDaemon_DoesNotPanic(t *testing.T) {
grpcServer,
&mocks.FileHandler{},
grpc.SocketPath,
"test",
)
defer server.Stop()

Expand All @@ -183,7 +183,6 @@ func TestRegisterDaemon_DoubleRegistrationPanics(t *testing.T) {
grpcServer,
&mocks.FileHandler{},
grpc.SocketPath,
"test",
)
defer server.Stop()

Expand All @@ -203,16 +202,18 @@ func TestRegisterDaemon_DoubleRegistrationPanics(t *testing.T) {
}

func createServerWithMocks(
mockGrpcServer daemontypes.GrpcServer,
mockFileHandler daemontypes.FileHandler,
t testing.TB,
mockGrpcServer *mocks.GrpcServer,
mockFileHandler *mocks.FileHandler,
) *server.Server {
server := server.NewServer(
log.NewNopLogger(),
mockGrpcServer,
mockFileHandler,
grpc.SocketPath,
"test",
)
mockGrpcServer.On("Stop").Return().Once()
t.Cleanup(server.Stop)
server.DisableUpdateMonitoringForTesting()
return server
}
Expand Down
5 changes: 0 additions & 5 deletions protocol/testutil/prices/cli/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cli

import (
"fmt"
"github.com/dydxprotocol/v4-chain/protocol/app/stoppable"
"testing"

"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
Expand Down Expand Up @@ -46,9 +45,5 @@ func NetworkWithMarketObjects(t *testing.T, n int) (*network.Network, []types.Ma
require.NoError(t, err)
cfg.GenesisState[types.ModuleName] = buf

t.Cleanup(func() {
stoppable.StopServices(t, cfg.GRPCAddress)
})

return network.New(t, cfg), state.MarketParams, state.MarketPrices
}
5 changes: 0 additions & 5 deletions protocol/x/blocktime/client/cli/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
package cli_test

import (
"github.com/dydxprotocol/v4-chain/protocol/app/stoppable"
"strconv"
"testing"

Expand Down Expand Up @@ -40,10 +39,6 @@ func setupNetwork(
net := network.New(t, cfg)
ctx := net.Validators[0].ClientCtx

t.Cleanup(func() {
stoppable.StopServices(t, cfg.GRPCAddress)
})

return net, ctx
}

Expand Down
Loading

0 comments on commit 77155ba

Please sign in to comment.