Skip to content

Commit

Permalink
[TRA-721] Enforce sidecar versions (#2491)
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyaoy authored Oct 17, 2024
1 parent bf2035d commit f7249a8
Show file tree
Hide file tree
Showing 9 changed files with 281 additions and 16 deletions.
1 change: 1 addition & 0 deletions protocol/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -888,6 +888,7 @@ func New(
)
app.RegisterDaemonWithHealthMonitor(app.SlinkyClient.GetMarketPairHC(), maxDaemonUnhealthyDuration)
app.RegisterDaemonWithHealthMonitor(app.SlinkyClient.GetPriceHC(), maxDaemonUnhealthyDuration)
app.RegisterDaemonWithHealthMonitor(app.SlinkyClient.GetSidecarVersionHC(), maxDaemonUnhealthyDuration)
}
}

Expand Down
67 changes: 58 additions & 9 deletions protocol/daemons/slinky/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -71,6 +82,7 @@ func (c *Client) start(
defer c.wg.Done()
c.RunMarketPairFetcher(c.ctx, appFlags, grpcClient)
}()

// 2. Start the PriceFetcher
c.priceFetcher = NewPriceFetcher(
c.marketPairFetcher,
Expand All @@ -83,6 +95,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
}

Expand Down Expand Up @@ -146,8 +169,34 @@ 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", "error", 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,
// 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.
Expand Down
12 changes: 10 additions & 2 deletions protocol/daemons/slinky/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,22 @@ 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{
"FOO/BAR": "100000000000",
},
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,
Expand All @@ -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
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()
}
9 changes: 6 additions & 3 deletions protocol/daemons/slinky/client/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@ 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 * 60
)

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"
MinSidecarVersion = "v1.0.12"
)
72 changes: 72 additions & 0 deletions protocol/daemons/slinky/client/sidecar_version_checker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
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"
)

// 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 {
slinky oracleclient.OracleClient
logger log.Logger
}

func NewSidecarVersionChecker(slinky oracleclient.OracleClient, logger log.Logger) SidecarVersionChecker {
return &SidecarVersionCheckerImpl{
slinky: slinky,
logger: logger,
}
}

// 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 (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
}
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. "+
"The node will shut down soon", current, minimum)
}

// Version is acceptable
s.logger.Info("Sidecar version check passed", "version", current)
return nil
}
62 changes: 62 additions & 0 deletions protocol/daemons/slinky/client/sidecar_version_checker_test.go
Original file line number Diff line number Diff line change
@@ -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()
})
}
2 changes: 1 addition & 1 deletion protocol/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion protocol/mocks/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading

0 comments on commit f7249a8

Please sign in to comment.