From 6138bef06bc9a4c997a36b4fc0e46a4dedd59d07 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:50:38 -0400 Subject: [PATCH 1/6] enforce sidecar versions --- protocol/app/app.go | 1 + protocol/daemons/slinky/client/client.go | 53 +++++++++++-- protocol/daemons/slinky/client/client_test.go | 2 +- protocol/daemons/slinky/client/constants.go | 8 +- .../slinky/client/sidecar_version_checker.go | 76 +++++++++++++++++++ 5 files changed, 128 insertions(+), 12 deletions(-) create mode 100644 protocol/daemons/slinky/client/sidecar_version_checker.go diff --git a/protocol/app/app.go b/protocol/app/app.go index 5b4e38955a..ea0950e6e3 100644 --- a/protocol/app/app.go +++ b/protocol/app/app.go @@ -888,6 +888,7 @@ func New( ) app.RegisterDaemonWithHealthMonitor(app.SlinkyClient.GetMarketPairHC(), maxDaemonUnhealthyDuration) app.RegisterDaemonWithHealthMonitor(app.SlinkyClient.GetPriceHC(), maxDaemonUnhealthyDuration) + app.RegisterDaemonWithHealthMonitor(app.SlinkyClient.GetSidecarVersionHC(), maxDaemonUnhealthyDuration) } } diff --git a/protocol/daemons/slinky/client/client.go b/protocol/daemons/slinky/client/client.go index 1d02337002..8c80844faa 100644 --- a/protocol/daemons/slinky/client/client.go +++ b/protocol/daemons/slinky/client/client.go @@ -20,14 +20,16 @@ import ( // Client is the daemon implementation for pulling price data from the slinky sidecar. type Client struct { - ctx context.Context - cf context.CancelFunc - marketPairFetcher MarketPairFetcher - marketPairHC daemontypes.HealthCheckable - priceFetcher PriceFetcher - priceHC daemontypes.HealthCheckable - wg sync.WaitGroup - logger log.Logger + ctx context.Context + cf context.CancelFunc + marketPairFetcher MarketPairFetcher + marketPairHC daemontypes.HealthCheckable + priceFetcher PriceFetcher + priceHC daemontypes.HealthCheckable + sidecarVersionChecker SidecarVersionChecker + sidecarVersionHC daemontypes.HealthCheckable + wg sync.WaitGroup + logger log.Logger } func newClient(ctx context.Context, logger log.Logger) *Client { @@ -43,6 +45,11 @@ func newClient(ctx context.Context, logger log.Logger) *Client { &libtime.TimeProviderImpl{}, logger, ), + sidecarVersionHC: daemontypes.NewTimeBoundedHealthCheckable( + SlinkyClientSidecarVersionFetcherDaemonModuleName, + &libtime.TimeProviderImpl{}, + logger, + ), logger: logger, } client.ctx, client.cf = context.WithCancel(ctx) @@ -57,6 +64,10 @@ func (c *Client) GetPriceHC() daemontypes.HealthCheckable { return c.priceHC } +func (c *Client) GetSidecarVersionHC() daemontypes.HealthCheckable { + return c.sidecarVersionHC +} + // start creates the main goroutines of the Client. func (c *Client) start( slinky oracleclient.OracleClient, @@ -146,6 +157,32 @@ func (c *Client) RunMarketPairFetcher(ctx context.Context, appFlags appflags.Fla } } +// RunSidecarVersionChecker periodically calls the sidecarVersionChecker to check if the running sidecar version +// is at least a minimum acceptable version +func (c *Client) RunSidecarVersionChecker(ctx context.Context) { + err := c.sidecarVersionChecker.Start(ctx) + if err != nil { + c.logger.Error("Error initializing sidecarVersionChecker in slinky daemon: %w", err) + panic(err) + } + ticker := time.NewTicker(SlinkySidecarCheckDelay) + defer ticker.Stop() + for { + select { + case <-ticker.C: + err = c.sidecarVersionChecker.CheckSidecarVersion(ctx) + if err != nil { + c.logger.Error("Sidecar version check failed", "error", err) + c.sidecarVersionHC.ReportFailure(errors.Wrap(err, "Sidecar version check failed for slinky daemon")) + } else { + c.sidecarVersionHC.ReportSuccess() + } + case <-ctx.Done(): + return + } + } +} + // StartNewClient creates and runs a Client. // The client creates the MarketPairFetcher and PriceFetcher, // connects to the required grpc services, and launches them in goroutines. diff --git a/protocol/daemons/slinky/client/client_test.go b/protocol/daemons/slinky/client/client_test.go index a256619b98..1196e22d7e 100644 --- a/protocol/daemons/slinky/client/client_test.go +++ b/protocol/daemons/slinky/client/client_test.go @@ -73,7 +73,7 @@ func TestClient(t *testing.T) { ) waitTime := time.Second * 5 require.Eventually(t, func() bool { - return cli.GetMarketPairHC().HealthCheck() == nil && cli.GetPriceHC().HealthCheck() == nil + return cli.GetMarketPairHC().HealthCheck() == nil && cli.GetPriceHC().HealthCheck() == nil && cli.GetSidecarVersionHC().HealthCheck() == nil }, waitTime, time.Millisecond*500, "Slinky daemon failed to become healthy within %s", waitTime) cli.Stop() } diff --git a/protocol/daemons/slinky/client/constants.go b/protocol/daemons/slinky/client/constants.go index aa711e69b9..83bfbba4fc 100644 --- a/protocol/daemons/slinky/client/constants.go +++ b/protocol/daemons/slinky/client/constants.go @@ -12,11 +12,13 @@ var ( // SlinkyMarketParamFetchDelay is the frequency at which we query the x/price module to refresh mappings from // currency pair to x/price ID. SlinkyMarketParamFetchDelay = time.Millisecond * 1900 + SlinkySidecarCheckDelay = time.Minute * 10 ) const ( // SlinkyClientDaemonModuleName is the module name used in logging. - SlinkyClientDaemonModuleName = "slinky-client-daemon" - SlinkyClientPriceFetcherDaemonModuleName = "slinky-client-price-fetcher-daemon" - SlinkyClientMarketPairFetcherDaemonModuleName = "slinky-client-market-pair-fetcher-daemon" + SlinkyClientDaemonModuleName = "slinky-client-daemon" + SlinkyClientPriceFetcherDaemonModuleName = "slinky-client-price-fetcher-daemon" + SlinkyClientMarketPairFetcherDaemonModuleName = "slinky-client-market-pair-fetcher-daemon" + SlinkyClientSidecarVersionFetcherDaemonModuleName = "slinky-client-sidecar-version-fetcher-daemon" ) diff --git a/protocol/daemons/slinky/client/sidecar_version_checker.go b/protocol/daemons/slinky/client/sidecar_version_checker.go new file mode 100644 index 0000000000..093bcc87ad --- /dev/null +++ b/protocol/daemons/slinky/client/sidecar_version_checker.go @@ -0,0 +1,76 @@ +package client + +import ( + "context" + "fmt" + + "cosmossdk.io/log" + "github.com/hashicorp/go-version" + + oracleclient "github.com/skip-mev/connect/v2/service/clients/oracle" + "github.com/skip-mev/connect/v2/service/servers/oracle/types" +) + +const ( + MinSidecarVersion = "v1.0.12" +) + +// SidecarVersionChecker is a lightweight process run in a goroutine by the slinky client. +// Its purpose is to query the running sidecar version and check if it is at least a minimum +// acceptable version. +type SidecarVersionChecker interface { + Start(ctx context.Context) error + Stop() + CheckSidecarVersion(context.Context) error +} + +// SidecarVersionCheckerImpl implements the SidecarVersionChecker interface. +type SidecarVersionCheckerImpl struct { + Logger log.Logger + slinky oracleclient.OracleClient +} + +func NewSidecarVersionChecker(logger log.Logger, slinky oracleclient.OracleClient) SidecarVersionChecker { + return &SidecarVersionCheckerImpl{ + Logger: logger, + slinky: slinky, + } +} + +// Start initializes the underlying connections of the SidecarVersionChecker. +func (s *SidecarVersionCheckerImpl) Start( + ctx context.Context) error { + return s.slinky.Start(ctx) +} + +// Stop closes all existing connections. +func (s *SidecarVersionCheckerImpl) Stop() { + _ = s.slinky.Stop() +} + +func (p *SidecarVersionCheckerImpl) CheckSidecarVersion(ctx context.Context) error { + // get prices from slinky sidecar via GRPC + slinkyResponse, err := p.slinky.Version(ctx, &types.QueryVersionRequest{}) + if err != nil { + return err + } + current, err := version.NewVersion(slinkyResponse.Version) + if err != nil { + return fmt.Errorf("failed to parse current version: %w", err) + } + + minimum, err := version.NewVersion(MinSidecarVersion) + if err != nil { + return fmt.Errorf("failed to parse minimum version: %w", err) + } + + // Compare versions + if current.LessThan(minimum) { + return fmt.Errorf("sidecar version %s is less than minimum required version %s", current, minimum) + } + + // Version is acceptable + p.Logger.Info("Sidecar version check passed", "version", current) + return nil + +} From 72905a87cb7675850b89ddfa573bbf89bc841b1d Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Thu, 3 Oct 2024 21:37:29 -0400 Subject: [PATCH 2/6] fixes --- protocol/daemons/slinky/client/client.go | 14 ++++++++++++++ protocol/daemons/slinky/client/constants.go | 2 +- .../slinky/client/sidecar_version_checker.go | 11 ++++++----- protocol/docker-compose.yml | 2 +- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/protocol/daemons/slinky/client/client.go b/protocol/daemons/slinky/client/client.go index 8c80844faa..577212832f 100644 --- a/protocol/daemons/slinky/client/client.go +++ b/protocol/daemons/slinky/client/client.go @@ -2,6 +2,7 @@ package client import ( "context" + "fmt" "sync" "time" @@ -82,6 +83,7 @@ func (c *Client) start( defer c.wg.Done() c.RunMarketPairFetcher(c.ctx, appFlags, grpcClient) }() + // 2. Start the PriceFetcher c.priceFetcher = NewPriceFetcher( c.marketPairFetcher, @@ -94,6 +96,17 @@ func (c *Client) start( defer c.wg.Done() c.RunPriceFetcher(c.ctx) }() + + // 3. Start the SidecarVersionChecker + c.sidecarVersionChecker = NewSidecarVersionChecker( + slinky, + c.logger, + ) + c.wg.Add(1) + go func() { + defer c.wg.Done() + c.RunSidecarVersionChecker(c.ctx) + }() return nil } @@ -160,6 +173,7 @@ func (c *Client) RunMarketPairFetcher(ctx context.Context, appFlags appflags.Fla // RunSidecarVersionChecker periodically calls the sidecarVersionChecker to check if the running sidecar version // is at least a minimum acceptable version func (c *Client) RunSidecarVersionChecker(ctx context.Context) { + fmt.Println("Running SidecarVersionChecker") err := c.sidecarVersionChecker.Start(ctx) if err != nil { c.logger.Error("Error initializing sidecarVersionChecker in slinky daemon: %w", err) diff --git a/protocol/daemons/slinky/client/constants.go b/protocol/daemons/slinky/client/constants.go index 83bfbba4fc..865289b150 100644 --- a/protocol/daemons/slinky/client/constants.go +++ b/protocol/daemons/slinky/client/constants.go @@ -12,7 +12,7 @@ var ( // SlinkyMarketParamFetchDelay is the frequency at which we query the x/price module to refresh mappings from // currency pair to x/price ID. SlinkyMarketParamFetchDelay = time.Millisecond * 1900 - SlinkySidecarCheckDelay = time.Minute * 10 + SlinkySidecarCheckDelay = time.Second * 10 ) const ( diff --git a/protocol/daemons/slinky/client/sidecar_version_checker.go b/protocol/daemons/slinky/client/sidecar_version_checker.go index 093bcc87ad..b8e0d66e5b 100644 --- a/protocol/daemons/slinky/client/sidecar_version_checker.go +++ b/protocol/daemons/slinky/client/sidecar_version_checker.go @@ -12,7 +12,7 @@ import ( ) const ( - MinSidecarVersion = "v1.0.12" + MinSidecarVersion = "v2.1.0" ) // SidecarVersionChecker is a lightweight process run in a goroutine by the slinky client. @@ -26,14 +26,14 @@ type SidecarVersionChecker interface { // SidecarVersionCheckerImpl implements the SidecarVersionChecker interface. type SidecarVersionCheckerImpl struct { - Logger log.Logger slinky oracleclient.OracleClient + logger log.Logger } -func NewSidecarVersionChecker(logger log.Logger, slinky oracleclient.OracleClient) SidecarVersionChecker { +func NewSidecarVersionChecker(slinky oracleclient.OracleClient, logger log.Logger) SidecarVersionChecker { return &SidecarVersionCheckerImpl{ - Logger: logger, slinky: slinky, + logger: logger, } } @@ -55,6 +55,7 @@ func (p *SidecarVersionCheckerImpl) CheckSidecarVersion(ctx context.Context) err return err } current, err := version.NewVersion(slinkyResponse.Version) + fmt.Println("Sidecar version", current) if err != nil { return fmt.Errorf("failed to parse current version: %w", err) } @@ -70,7 +71,7 @@ func (p *SidecarVersionCheckerImpl) CheckSidecarVersion(ctx context.Context) err } // Version is acceptable - p.Logger.Info("Sidecar version check passed", "version", current) + p.logger.Info("Sidecar version check passed", "version", current) return nil } diff --git a/protocol/docker-compose.yml b/protocol/docker-compose.yml index 7314dee56b..d4dfaf55e1 100644 --- a/protocol/docker-compose.yml +++ b/protocol/docker-compose.yml @@ -116,7 +116,7 @@ services: volumes: - ./localnet/dydxprotocol3:/dydxprotocol/chain/.dave/data connect0: - image: ghcr.io/skip-mev/connect-sidecar:v2.1.0 + image: ghcr.io/skip-mev/connect-sidecar:v2.0.1 entrypoint: > sh -c "connect --marketmap-provider dydx_migration_api --oracle-config /etc/connect/oracle.json --log-std-out-level error" environment: From 137f3ab342c56256dc31ccd7822f93bf58b4b42d Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Tue, 15 Oct 2024 10:31:00 -0400 Subject: [PATCH 3/6] push to dev --- .github/workflows/indexer-build-and-push-dev-staging.yml | 1 + .github/workflows/protocol-build-and-push-snapshot.yml | 1 + .github/workflows/protocol-build-and-push.yml | 1 + 3 files changed, 3 insertions(+) diff --git a/.github/workflows/indexer-build-and-push-dev-staging.yml b/.github/workflows/indexer-build-and-push-dev-staging.yml index 5a98b72552..3adf567168 100644 --- a/.github/workflows/indexer-build-and-push-dev-staging.yml +++ b/.github/workflows/indexer-build-and-push-dev-staging.yml @@ -6,6 +6,7 @@ on: # yamllint disable-line rule:truthy - main - 'release/indexer/v[0-9]+.[0-9]+.x' # e.g. release/indexer/v0.1.x - 'release/indexer/v[0-9]+.x' # e.g. release/indexer/v1.x + - 'chenyao/enforce-sidecar-versions' # TODO(DEC-837): Customize github build and push to ECR by service with paths jobs: diff --git a/.github/workflows/protocol-build-and-push-snapshot.yml b/.github/workflows/protocol-build-and-push-snapshot.yml index e5ff2c8b05..3ea30813f2 100644 --- a/.github/workflows/protocol-build-and-push-snapshot.yml +++ b/.github/workflows/protocol-build-and-push-snapshot.yml @@ -6,6 +6,7 @@ on: # yamllint disable-line rule:truthy - main - 'release/protocol/v[0-9]+.[0-9]+.x' # e.g. release/protocol/v0.1.x - 'release/protocol/v[0-9]+.x' # e.g. release/protocol/v1.x + - 'chenyao/enforce-sidecar-versions' jobs: build-and-push-snapshot-dev: diff --git a/.github/workflows/protocol-build-and-push.yml b/.github/workflows/protocol-build-and-push.yml index ade5341369..7b8668a2df 100644 --- a/.github/workflows/protocol-build-and-push.yml +++ b/.github/workflows/protocol-build-and-push.yml @@ -6,6 +6,7 @@ on: # yamllint disable-line rule:truthy - main - 'release/protocol/v[0-9]+.[0-9]+.x' # e.g. release/protocol/v0.1.x - 'release/protocol/v[0-9]+.x' # e.g. release/protocol/v1.x + - 'chenyao/enforce-sidecar-versions' jobs: build-and-push-dev: From e1b5fc4df5c936ea1a87be92fa8868a673d74d5a Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Tue, 15 Oct 2024 11:05:33 -0400 Subject: [PATCH 4/6] cleanup --- .github/workflows/indexer-build-and-push-dev-staging.yml | 1 - .github/workflows/protocol-build-and-push-snapshot.yml | 1 - .github/workflows/protocol-build-and-push.yml | 1 - protocol/daemons/slinky/client/client.go | 2 -- protocol/daemons/slinky/client/constants.go | 2 +- protocol/daemons/slinky/client/sidecar_version_checker.go | 2 +- protocol/docker-compose.yml | 2 +- 7 files changed, 3 insertions(+), 8 deletions(-) diff --git a/.github/workflows/indexer-build-and-push-dev-staging.yml b/.github/workflows/indexer-build-and-push-dev-staging.yml index 3adf567168..5a98b72552 100644 --- a/.github/workflows/indexer-build-and-push-dev-staging.yml +++ b/.github/workflows/indexer-build-and-push-dev-staging.yml @@ -6,7 +6,6 @@ on: # yamllint disable-line rule:truthy - main - 'release/indexer/v[0-9]+.[0-9]+.x' # e.g. release/indexer/v0.1.x - 'release/indexer/v[0-9]+.x' # e.g. release/indexer/v1.x - - 'chenyao/enforce-sidecar-versions' # TODO(DEC-837): Customize github build and push to ECR by service with paths jobs: diff --git a/.github/workflows/protocol-build-and-push-snapshot.yml b/.github/workflows/protocol-build-and-push-snapshot.yml index 3ea30813f2..e5ff2c8b05 100644 --- a/.github/workflows/protocol-build-and-push-snapshot.yml +++ b/.github/workflows/protocol-build-and-push-snapshot.yml @@ -6,7 +6,6 @@ on: # yamllint disable-line rule:truthy - main - 'release/protocol/v[0-9]+.[0-9]+.x' # e.g. release/protocol/v0.1.x - 'release/protocol/v[0-9]+.x' # e.g. release/protocol/v1.x - - 'chenyao/enforce-sidecar-versions' jobs: build-and-push-snapshot-dev: diff --git a/.github/workflows/protocol-build-and-push.yml b/.github/workflows/protocol-build-and-push.yml index 7b8668a2df..ade5341369 100644 --- a/.github/workflows/protocol-build-and-push.yml +++ b/.github/workflows/protocol-build-and-push.yml @@ -6,7 +6,6 @@ on: # yamllint disable-line rule:truthy - main - 'release/protocol/v[0-9]+.[0-9]+.x' # e.g. release/protocol/v0.1.x - 'release/protocol/v[0-9]+.x' # e.g. release/protocol/v1.x - - 'chenyao/enforce-sidecar-versions' jobs: build-and-push-dev: diff --git a/protocol/daemons/slinky/client/client.go b/protocol/daemons/slinky/client/client.go index 577212832f..6e3c76db2d 100644 --- a/protocol/daemons/slinky/client/client.go +++ b/protocol/daemons/slinky/client/client.go @@ -2,7 +2,6 @@ package client import ( "context" - "fmt" "sync" "time" @@ -173,7 +172,6 @@ func (c *Client) RunMarketPairFetcher(ctx context.Context, appFlags appflags.Fla // RunSidecarVersionChecker periodically calls the sidecarVersionChecker to check if the running sidecar version // is at least a minimum acceptable version func (c *Client) RunSidecarVersionChecker(ctx context.Context) { - fmt.Println("Running SidecarVersionChecker") err := c.sidecarVersionChecker.Start(ctx) if err != nil { c.logger.Error("Error initializing sidecarVersionChecker in slinky daemon: %w", err) diff --git a/protocol/daemons/slinky/client/constants.go b/protocol/daemons/slinky/client/constants.go index 865289b150..4cfdb3e218 100644 --- a/protocol/daemons/slinky/client/constants.go +++ b/protocol/daemons/slinky/client/constants.go @@ -12,7 +12,7 @@ var ( // SlinkyMarketParamFetchDelay is the frequency at which we query the x/price module to refresh mappings from // currency pair to x/price ID. SlinkyMarketParamFetchDelay = time.Millisecond * 1900 - SlinkySidecarCheckDelay = time.Second * 10 + SlinkySidecarCheckDelay = time.Second * 60 ) const ( diff --git a/protocol/daemons/slinky/client/sidecar_version_checker.go b/protocol/daemons/slinky/client/sidecar_version_checker.go index b8e0d66e5b..dcd07fc9b9 100644 --- a/protocol/daemons/slinky/client/sidecar_version_checker.go +++ b/protocol/daemons/slinky/client/sidecar_version_checker.go @@ -12,7 +12,7 @@ import ( ) const ( - MinSidecarVersion = "v2.1.0" + MinSidecarVersion = "v1.0.12" ) // SidecarVersionChecker is a lightweight process run in a goroutine by the slinky client. diff --git a/protocol/docker-compose.yml b/protocol/docker-compose.yml index d4dfaf55e1..7314dee56b 100644 --- a/protocol/docker-compose.yml +++ b/protocol/docker-compose.yml @@ -116,7 +116,7 @@ services: volumes: - ./localnet/dydxprotocol3:/dydxprotocol/chain/.dave/data connect0: - image: ghcr.io/skip-mev/connect-sidecar:v2.0.1 + image: ghcr.io/skip-mev/connect-sidecar:v2.1.0 entrypoint: > sh -c "connect --marketmap-provider dydx_migration_api --oracle-config /etc/connect/oracle.json --log-std-out-level error" environment: From 95e6f40e2ddf976ee0cacbd18357b6e639b09c2a Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Tue, 15 Oct 2024 16:05:31 -0400 Subject: [PATCH 5/6] add tests --- protocol/daemons/slinky/client/client.go | 2 +- protocol/daemons/slinky/client/client_test.go | 12 +++- protocol/daemons/slinky/client/constants.go | 1 + .../slinky/client/sidecar_version_checker.go | 9 +-- .../client/sidecar_version_checker_test.go | 62 +++++++++++++++++ protocol/go.mod | 2 +- protocol/mocks/Makefile | 3 +- protocol/mocks/SidecarVersionChecker.go | 69 +++++++++++++++++++ 8 files changed, 148 insertions(+), 12 deletions(-) create mode 100644 protocol/daemons/slinky/client/sidecar_version_checker_test.go create mode 100644 protocol/mocks/SidecarVersionChecker.go diff --git a/protocol/daemons/slinky/client/client.go b/protocol/daemons/slinky/client/client.go index 6e3c76db2d..9023600f75 100644 --- a/protocol/daemons/slinky/client/client.go +++ b/protocol/daemons/slinky/client/client.go @@ -196,7 +196,7 @@ func (c *Client) RunSidecarVersionChecker(ctx context.Context) { } // StartNewClient creates and runs a Client. -// The client creates the MarketPairFetcher and PriceFetcher, +// The client creates the MarketPairFetcher, PriceFetcher, an SidecarVersionChecker, // connects to the required grpc services, and launches them in goroutines. // It is non-blocking and returns on successful startup. // If it hits a critical error in startup it panics. diff --git a/protocol/daemons/slinky/client/client_test.go b/protocol/daemons/slinky/client/client_test.go index 1196e22d7e..8c6d439321 100644 --- a/protocol/daemons/slinky/client/client_test.go +++ b/protocol/daemons/slinky/client/client_test.go @@ -52,7 +52,7 @@ func TestClient(t *testing.T) { }() slinky.On("Stop").Return(nil) - slinky.On("Start", mock.Anything).Return(nil).Once() + slinky.On("Start", mock.Anything).Return(nil).Twice() slinky.On("Prices", mock.Anything, mock.Anything). Return(&types.QueryPricesResponse{ Prices: map[string]string{ @@ -60,8 +60,14 @@ func TestClient(t *testing.T) { }, Timestamp: time.Now(), }, nil) + slinky.On("Version", mock.Anything, mock.Anything). + Return(&types.QueryVersionResponse{ + Version: client.MinSidecarVersion, + }, nil) + client.SlinkyPriceFetchDelay = time.Millisecond client.SlinkyMarketParamFetchDelay = time.Millisecond + client.SlinkySidecarCheckDelay = time.Millisecond cli = client.StartNewClient( context.Background(), slinky, @@ -73,7 +79,9 @@ func TestClient(t *testing.T) { ) waitTime := time.Second * 5 require.Eventually(t, func() bool { - return cli.GetMarketPairHC().HealthCheck() == nil && cli.GetPriceHC().HealthCheck() == nil && cli.GetSidecarVersionHC().HealthCheck() == nil + return cli.GetMarketPairHC().HealthCheck() == nil && + cli.GetPriceHC().HealthCheck() == nil && + cli.GetSidecarVersionHC().HealthCheck() == nil }, waitTime, time.Millisecond*500, "Slinky daemon failed to become healthy within %s", waitTime) cli.Stop() } diff --git a/protocol/daemons/slinky/client/constants.go b/protocol/daemons/slinky/client/constants.go index 4cfdb3e218..e563b66197 100644 --- a/protocol/daemons/slinky/client/constants.go +++ b/protocol/daemons/slinky/client/constants.go @@ -21,4 +21,5 @@ const ( SlinkyClientPriceFetcherDaemonModuleName = "slinky-client-price-fetcher-daemon" SlinkyClientMarketPairFetcherDaemonModuleName = "slinky-client-market-pair-fetcher-daemon" SlinkyClientSidecarVersionFetcherDaemonModuleName = "slinky-client-sidecar-version-fetcher-daemon" + MinSidecarVersion = "v1.0.12" ) diff --git a/protocol/daemons/slinky/client/sidecar_version_checker.go b/protocol/daemons/slinky/client/sidecar_version_checker.go index dcd07fc9b9..6e6309ae86 100644 --- a/protocol/daemons/slinky/client/sidecar_version_checker.go +++ b/protocol/daemons/slinky/client/sidecar_version_checker.go @@ -11,10 +11,6 @@ import ( "github.com/skip-mev/connect/v2/service/servers/oracle/types" ) -const ( - MinSidecarVersion = "v1.0.12" -) - // SidecarVersionChecker is a lightweight process run in a goroutine by the slinky client. // Its purpose is to query the running sidecar version and check if it is at least a minimum // acceptable version. @@ -55,7 +51,6 @@ func (p *SidecarVersionCheckerImpl) CheckSidecarVersion(ctx context.Context) err return err } current, err := version.NewVersion(slinkyResponse.Version) - fmt.Println("Sidecar version", current) if err != nil { return fmt.Errorf("failed to parse current version: %w", err) } @@ -67,11 +62,11 @@ func (p *SidecarVersionCheckerImpl) CheckSidecarVersion(ctx context.Context) err // Compare versions if current.LessThan(minimum) { - return fmt.Errorf("sidecar version %s is less than minimum required version %s", current, minimum) + return fmt.Errorf("Sidecar version %s is less than minimum required version %s. "+ + "The node will shut down soon", current, minimum) } // Version is acceptable p.logger.Info("Sidecar version check passed", "version", current) return nil - } diff --git a/protocol/daemons/slinky/client/sidecar_version_checker_test.go b/protocol/daemons/slinky/client/sidecar_version_checker_test.go new file mode 100644 index 0000000000..97dd6b6839 --- /dev/null +++ b/protocol/daemons/slinky/client/sidecar_version_checker_test.go @@ -0,0 +1,62 @@ +package client_test + +import ( + "context" + "testing" + + "cosmossdk.io/log" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/dydxprotocol/v4-chain/protocol/daemons/slinky/client" + "github.com/dydxprotocol/v4-chain/protocol/mocks" + "github.com/skip-mev/connect/v2/service/servers/oracle/types" +) + +func TestSidecarVersionChecker(t *testing.T) { + logger := log.NewTestLogger(t) + var fetcher client.SidecarVersionChecker + + t.Run("Checks sidecar version passes", func(t *testing.T) { + slinky := mocks.NewOracleClient(t) + slinky.On("Stop").Return(nil) + slinky.On("Start", mock.Anything).Return(nil).Once() + slinky.On("Version", mock.Anything, mock.Anything). + Return(&types.QueryVersionResponse{ + Version: client.MinSidecarVersion, + }, nil) + fetcher = client.NewSidecarVersionChecker(slinky, logger) + require.NoError(t, fetcher.Start(context.Background())) + require.NoError(t, fetcher.CheckSidecarVersion(context.Background())) + fetcher.Stop() + }) + + t.Run("Checks sidecar version less than minimum version", func(t *testing.T) { + slinky := mocks.NewOracleClient(t) + slinky.On("Stop").Return(nil) + slinky.On("Start", mock.Anything).Return(nil).Once() + slinky.On("Version", mock.Anything, mock.Anything). + Return(&types.QueryVersionResponse{ + Version: "v0.0.0", + }, nil) + fetcher = client.NewSidecarVersionChecker(slinky, logger) + require.NoError(t, fetcher.Start(context.Background())) + require.ErrorContains(t, fetcher.CheckSidecarVersion(context.Background()), + "Sidecar version 0.0.0 is less than minimum required version") + fetcher.Stop() + }) + + t.Run("Checks sidecar version incorrectly formatted", func(t *testing.T) { + slinky := mocks.NewOracleClient(t) + slinky.On("Stop").Return(nil) + slinky.On("Start", mock.Anything).Return(nil).Once() + slinky.On("Version", mock.Anything, mock.Anything). + Return(&types.QueryVersionResponse{ + Version: "a.b.c", + }, nil) + fetcher = client.NewSidecarVersionChecker(slinky, logger) + require.NoError(t, fetcher.Start(context.Background())) + require.ErrorContains(t, fetcher.CheckSidecarVersion(context.Background()), "Malformed version: a.b.c") + fetcher.Stop() + }) +} diff --git a/protocol/go.mod b/protocol/go.mod index 2af0fc5601..1d940dd105 100644 --- a/protocol/go.mod +++ b/protocol/go.mod @@ -62,6 +62,7 @@ require ( github.com/go-kit/log v0.2.1 github.com/gorilla/websocket v1.5.3 github.com/hashicorp/go-metrics v0.5.3 + github.com/hashicorp/go-version v1.7.0 github.com/ory/dockertest/v3 v3.10.0 github.com/pelletier/go-toml v1.9.5 github.com/rs/zerolog v1.33.0 @@ -257,7 +258,6 @@ require ( github.com/hashicorp/go-plugin v1.6.0 // indirect github.com/hashicorp/go-safetemp v1.0.0 // indirect github.com/hashicorp/go-uuid v1.0.3 // indirect - github.com/hashicorp/go-version v1.7.0 // indirect github.com/hashicorp/golang-lru v1.0.2 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/hashicorp/yamux v0.1.1 // indirect diff --git a/protocol/mocks/Makefile b/protocol/mocks/Makefile index dc0fda979d..33ef2f48fa 100644 --- a/protocol/mocks/Makefile +++ b/protocol/mocks/Makefile @@ -60,7 +60,8 @@ mock-gen: @go run github.com/vektra/mockery/v2 --name=PriceUpdateGenerator --dir=./app/prepare/prices --recursive --output=./mocks @go run github.com/vektra/mockery/v2 --name=PriceFetcher --dir=./daemons/slinky/client --recursive --output=./mocks @go run github.com/vektra/mockery/v2 --name=MarketPairFetcher --dir=./daemons/slinky/client --recursive --output=./mocks + @go run github.com/vektra/mockery/v2 --name=SidecarVersionChecker --dir=./daemons/slinky/client --recursive --output=./mocks @go run github.com/vektra/mockery/v2 --name=OracleClient --dir=$(GOPATH)/pkg/mod/github.com/skip-mev/connect/v2@$(CONNECT_VERSION)/service/clients/oracle --recursive --output=./mocks @go run github.com/vektra/mockery/v2 --name=ExtendVoteHandler --dir=$(GOPATH)/pkg/mod/github.com/dydxprotocol/cosmos-sdk@$(COSMOS_VERSION)/types --recursive --output=./mocks @go run github.com/vektra/mockery/v2 --name=UpdateMarketPriceTxDecoder --dir=./app/process --recursive --output=./mocks - @go run github.com/vektra/mockery/v2 --name=AssetsKeeper --dir=./x//types --recursive --output=./mocks + @go run github.com/vektra/mockery/v2 --name=AssetsKeeper --dir=./x/assets/types --recursive --output=./mocks diff --git a/protocol/mocks/SidecarVersionChecker.go b/protocol/mocks/SidecarVersionChecker.go new file mode 100644 index 0000000000..2719f588fe --- /dev/null +++ b/protocol/mocks/SidecarVersionChecker.go @@ -0,0 +1,69 @@ +// Code generated by mockery v2.46.0. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// SidecarVersionChecker is an autogenerated mock type for the SidecarVersionChecker type +type SidecarVersionChecker struct { + mock.Mock +} + +// CheckSidecarVersion provides a mock function with given fields: _a0 +func (_m *SidecarVersionChecker) CheckSidecarVersion(_a0 context.Context) error { + ret := _m.Called(_a0) + + if len(ret) == 0 { + panic("no return value specified for CheckSidecarVersion") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context) error); ok { + r0 = rf(_a0) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Start provides a mock function with given fields: ctx +func (_m *SidecarVersionChecker) Start(ctx context.Context) error { + ret := _m.Called(ctx) + + if len(ret) == 0 { + panic("no return value specified for Start") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context) error); ok { + r0 = rf(ctx) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Stop provides a mock function with given fields: +func (_m *SidecarVersionChecker) Stop() { + _m.Called() +} + +// NewSidecarVersionChecker creates a new instance of SidecarVersionChecker. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewSidecarVersionChecker(t interface { + mock.TestingT + Cleanup(func()) +}) *SidecarVersionChecker { + mock := &SidecarVersionChecker{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From 5a1b2de283cb1abff4b0e6e7e212fd7993763e45 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Wed, 16 Oct 2024 13:49:20 -0400 Subject: [PATCH 6/6] nits --- protocol/daemons/slinky/client/client.go | 6 +++--- protocol/daemons/slinky/client/sidecar_version_checker.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/protocol/daemons/slinky/client/client.go b/protocol/daemons/slinky/client/client.go index 9023600f75..08aa04c467 100644 --- a/protocol/daemons/slinky/client/client.go +++ b/protocol/daemons/slinky/client/client.go @@ -170,11 +170,11 @@ func (c *Client) RunMarketPairFetcher(ctx context.Context, appFlags appflags.Fla } // RunSidecarVersionChecker periodically calls the sidecarVersionChecker to check if the running sidecar version -// is at least a minimum acceptable version +// is at least a minimum acceptable version. func (c *Client) RunSidecarVersionChecker(ctx context.Context) { err := c.sidecarVersionChecker.Start(ctx) if err != nil { - c.logger.Error("Error initializing sidecarVersionChecker in slinky daemon: %w", err) + c.logger.Error("Error initializing sidecarVersionChecker in slinky daemon", "error", err) panic(err) } ticker := time.NewTicker(SlinkySidecarCheckDelay) @@ -196,7 +196,7 @@ func (c *Client) RunSidecarVersionChecker(ctx context.Context) { } // StartNewClient creates and runs a Client. -// The client creates the MarketPairFetcher, PriceFetcher, an SidecarVersionChecker, +// The client creates the MarketPairFetcher, PriceFetcher, and SidecarVersionChecker, // connects to the required grpc services, and launches them in goroutines. // It is non-blocking and returns on successful startup. // If it hits a critical error in startup it panics. diff --git a/protocol/daemons/slinky/client/sidecar_version_checker.go b/protocol/daemons/slinky/client/sidecar_version_checker.go index 6e6309ae86..af7ec97199 100644 --- a/protocol/daemons/slinky/client/sidecar_version_checker.go +++ b/protocol/daemons/slinky/client/sidecar_version_checker.go @@ -44,9 +44,9 @@ func (s *SidecarVersionCheckerImpl) Stop() { _ = s.slinky.Stop() } -func (p *SidecarVersionCheckerImpl) CheckSidecarVersion(ctx context.Context) error { - // get prices from slinky sidecar via GRPC - slinkyResponse, err := p.slinky.Version(ctx, &types.QueryVersionRequest{}) +func (s *SidecarVersionCheckerImpl) CheckSidecarVersion(ctx context.Context) error { + // Retrieve sidecar version via gRPC + slinkyResponse, err := s.slinky.Version(ctx, &types.QueryVersionRequest{}) if err != nil { return err } @@ -67,6 +67,6 @@ func (p *SidecarVersionCheckerImpl) CheckSidecarVersion(ctx context.Context) err } // Version is acceptable - p.logger.Info("Sidecar version check passed", "version", current) + s.logger.Info("Sidecar version check passed", "version", current) return nil }