diff --git a/node/cmd/guardiand/node.go b/node/cmd/guardiand/node.go index 3900b4a71a..f7a3ed180d 100644 --- a/node/cmd/guardiand/node.go +++ b/node/cmd/guardiand/node.go @@ -233,6 +233,7 @@ var ( chainGovernorEnabled *bool governorFlowCancelEnabled *bool + coinGeckoApiKey *string ccqEnabled *bool ccqAllowedRequesters *string @@ -439,6 +440,7 @@ func init() { chainGovernorEnabled = NodeCmd.Flags().Bool("chainGovernorEnabled", false, "Run the chain governor") governorFlowCancelEnabled = NodeCmd.Flags().Bool("governorFlowCancelEnabled", false, "Enable flow cancel on the governor") + coinGeckoApiKey = NodeCmd.Flags().String("coinGeckoApiKey", "", "CoinGecko Pro API key. If no API key is provided, CoinGecko requests may be throttled or blocked.") ccqEnabled = NodeCmd.Flags().Bool("ccqEnabled", false, "Enable cross chain query support") ccqAllowedRequesters = NodeCmd.Flags().String("ccqAllowedRequesters", "", "Comma separated list of signers allowed to submit cross chain queries") @@ -828,6 +830,10 @@ func runNode(cmd *cobra.Command, args []string) { logger.Fatal("Either --gatewayContract, --gatewayWS and --gatewayLCD must all be set or all unset") } + if !*chainGovernorEnabled && *coinGeckoApiKey != "" { + logger.Fatal("If coinGeckoApiKey is set, then chainGovernorEnabled must be set") + } + var publicRpcLogDetail common.GrpcLogDetail switch *publicRpcLogDetailStr { case "none": @@ -1583,7 +1589,7 @@ func runNode(cmd *cobra.Command, args []string) { node.GuardianOptionDatabase(db), node.GuardianOptionWatchers(watcherConfigs, ibcWatcherConfig), node.GuardianOptionAccountant(*accountantWS, *accountantContract, *accountantCheckEnabled, accountantWormchainConn, *accountantNttContract, accountantNttWormchainConn), - node.GuardianOptionGovernor(*chainGovernorEnabled, *governorFlowCancelEnabled), + node.GuardianOptionGovernor(*chainGovernorEnabled, *governorFlowCancelEnabled, *coinGeckoApiKey), node.GuardianOptionGatewayRelayer(*gatewayRelayerContract, gatewayRelayerWormchainConn), node.GuardianOptionQueryHandler(*ccqEnabled, *ccqAllowedRequesters), node.GuardianOptionAdminService(*adminSocketPath, ethRPC, ethContract, rpcMap), diff --git a/node/pkg/adminrpc/adminserver_test.go b/node/pkg/adminrpc/adminserver_test.go index f1167f71c1..a7e8742177 100644 --- a/node/pkg/adminrpc/adminserver_test.go +++ b/node/pkg/adminrpc/adminserver_test.go @@ -322,7 +322,7 @@ func Test_adminCommands(t *testing.T) { } func newNodePrivilegedServiceForGovernorTests() *nodePrivilegedService { - gov := governor.NewChainGovernor(zap.NewNop(), &db.MockGovernorDB{}, wh_common.GoTest, false) + gov := governor.NewChainGovernor(zap.NewNop(), &db.MockGovernorDB{}, wh_common.GoTest, false, "") return &nodePrivilegedService{ db: nil, diff --git a/node/pkg/governor/governor.go b/node/pkg/governor/governor.go index fbea430def..8f6137b1c8 100644 --- a/node/pkg/governor/governor.go +++ b/node/pkg/governor/governor.go @@ -201,6 +201,7 @@ type ChainGovernor struct { statusPublishCounter int64 configPublishCounter int64 flowCancelEnabled bool + coinGeckoApiKey string } func NewChainGovernor( @@ -208,6 +209,7 @@ func NewChainGovernor( db db.GovernorDB, env common.Environment, flowCancelEnabled bool, + coinGeckoApiKey string, ) *ChainGovernor { return &ChainGovernor{ db: db, @@ -218,6 +220,7 @@ func NewChainGovernor( msgsSeen: make(map[string]bool), env: env, flowCancelEnabled: flowCancelEnabled, + coinGeckoApiKey: coinGeckoApiKey, } } diff --git a/node/pkg/governor/governor_monitoring_test.go b/node/pkg/governor/governor_monitoring_test.go index 5683e17429..2cbd08c815 100644 --- a/node/pkg/governor/governor_monitoring_test.go +++ b/node/pkg/governor/governor_monitoring_test.go @@ -11,7 +11,7 @@ import ( func TestIsVAAEnqueuedNilMessageID(t *testing.T) { logger, _ := zap.NewProduction() - gov := NewChainGovernor(logger, nil, common.GoTest, true) + gov := NewChainGovernor(logger, nil, common.GoTest, true, "") enqueued, err := gov.IsVAAEnqueued(nil) require.EqualError(t, err, "no message ID specified") assert.Equal(t, false, enqueued) diff --git a/node/pkg/governor/governor_prices.go b/node/pkg/governor/governor_prices.go index f3ebe8fded..88e02417a4 100644 --- a/node/pkg/governor/governor_prices.go +++ b/node/pkg/governor/governor_prices.go @@ -44,7 +44,7 @@ func (gov *ChainGovernor) initCoinGecko(ctx context.Context, run bool) error { } // Create the set of queries, breaking the IDs into the appropriate size chunks. - gov.coinGeckoQueries = createCoinGeckoQueries(ids, tokensPerCoinGeckoQuery) + gov.coinGeckoQueries = createCoinGeckoQueries(ids, tokensPerCoinGeckoQuery, gov.coinGeckoApiKey) for queryIdx, query := range gov.coinGeckoQueries { gov.logger.Info("coingecko query: ", zap.Int("queryIdx", queryIdx), zap.String("query", query)) } @@ -64,7 +64,7 @@ func (gov *ChainGovernor) initCoinGecko(ctx context.Context, run bool) error { } // createCoinGeckoQueries creates the set of CoinGecko queries, breaking the set of IDs into the appropriate size chunks. -func createCoinGeckoQueries(idList []string, tokensPerQuery int) []string { +func createCoinGeckoQueries(idList []string, tokensPerQuery int, coinGeckoApiKey string) []string { var queries []string queryIdx := 0 tokenIdx := 0 @@ -72,7 +72,7 @@ func createCoinGeckoQueries(idList []string, tokensPerQuery int) []string { first := true for _, coinGeckoId := range idList { if tokenIdx%tokensPerQuery == 0 && tokenIdx != 0 { - queries = append(queries, createCoinGeckoQuery(ids)) + queries = append(queries, createCoinGeckoQuery(ids, coinGeckoApiKey)) ids = "" first = true queryIdx += 1 @@ -88,19 +88,29 @@ func createCoinGeckoQueries(idList []string, tokensPerQuery int) []string { } if ids != "" { - queries = append(queries, createCoinGeckoQuery(ids)) + queries = append(queries, createCoinGeckoQuery(ids, coinGeckoApiKey)) } return queries } // createCoinGeckoQuery creates a CoinGecko query for the specified set of IDs. -func createCoinGeckoQuery(ids string) string { +func createCoinGeckoQuery(ids string, coinGeckoApiKey string) string { params := url.Values{} params.Add("ids", ids) params.Add("vs_currencies", "usd") - query := "https://api.coingecko.com/api/v3/simple/price?" + params.Encode() + // If modifying this code, ensure that the test 'TestCoinGeckoPriceChecks' passes when adding a pro API key to it. + // Since the code requires an API key (which we don't want to publish to git), this + // part of the test is normally skipped but mods to sensitive places should still be checked + query := "" + if coinGeckoApiKey == "" { + query = "https://api.coingecko.com/api/v3/simple/price?" + params.Encode() + } else { // Pro version API key path + params.Add("x_cg_pro_api_key", coinGeckoApiKey) + query = "https://pro-api.coingecko.com/api/v3/simple/price?" + params.Encode() + } + return query } @@ -160,7 +170,7 @@ func (gov *ChainGovernor) queryCoinGecko(ctx context.Context) error { query := query + "&" + params.Encode() thisResult, err := gov.queryCoinGeckoChunk(query) if err != nil { - gov.logger.Error("CoinGecko query failed", zap.Int("queryIdx", queryIdx), zap.String("query", query), zap.Error(err)) + gov.logger.Error("CoinGecko query failed", zap.Error(err), zap.Int("queryIdx", queryIdx), zap.String("query", query)) gov.revertAllPrices() return err } @@ -309,7 +319,7 @@ func CheckQuery(logger *zap.Logger) error { logger.Info("Instantiating governor.") ctx := context.Background() var db db.MockGovernorDB - gov := NewChainGovernor(logger, &db, common.MainNet, true) + gov := NewChainGovernor(logger, &db, common.MainNet, true, "") if err := gov.initConfig(); err != nil { return err diff --git a/node/pkg/governor/governor_test.go b/node/pkg/governor/governor_test.go index 388af53d37..d44168c5d7 100644 --- a/node/pkg/governor/governor_test.go +++ b/node/pkg/governor/governor_test.go @@ -302,7 +302,7 @@ func TestFlowCancelFeatureFlag(t *testing.T) { ctx := context.Background() var db db.MockGovernorDB - gov := NewChainGovernor(zap.NewNop(), &db, common.GoTest, true) + gov := NewChainGovernor(zap.NewNop(), &db, common.GoTest, true, "") // Trigger the evaluation of the flow cancelling config err := gov.Run(ctx) @@ -322,7 +322,7 @@ func TestFlowCancelFeatureFlag(t *testing.T) { assert.NotZero(t, numFlowCancelling) // Disable flow cancelling - gov = NewChainGovernor(zap.NewNop(), &db, common.GoTest, false) + gov = NewChainGovernor(zap.NewNop(), &db, common.GoTest, false, "") // Trigger the evaluation of the flow cancelling config err = gov.Run(ctx) @@ -666,7 +666,7 @@ func newChainGovernorForTestWithLogger(ctx context.Context, logger *zap.Logger) } var db db.MockGovernorDB - gov := NewChainGovernor(logger, &db, common.GoTest, true) + gov := NewChainGovernor(logger, &db, common.GoTest, true, "") err := gov.Run(ctx) if err != nil { @@ -2183,7 +2183,7 @@ func TestSmallerPendingTransfersAfterBigOneShouldGetReleased(t *testing.T) { func TestMainnetConfigIsValid(t *testing.T) { logger := zap.NewNop() var db db.MockGovernorDB - gov := NewChainGovernor(logger, &db, common.GoTest, true) + gov := NewChainGovernor(logger, &db, common.GoTest, true, "") gov.env = common.TestNet err := gov.initConfig() @@ -2193,7 +2193,7 @@ func TestMainnetConfigIsValid(t *testing.T) { func TestTestnetConfigIsValid(t *testing.T) { logger := zap.NewNop() var db db.MockGovernorDB - gov := NewChainGovernor(logger, &db, common.GoTest, true) + gov := NewChainGovernor(logger, &db, common.GoTest, true, "") gov.env = common.TestNet err := gov.initConfig() @@ -3187,7 +3187,7 @@ func TestCoinGeckoQueries(t *testing.T) { ids[idx] = fmt.Sprintf("id%d", idx) } - queries := createCoinGeckoQueries(ids, tc.chunkSize) + queries := createCoinGeckoQueries(ids, tc.chunkSize, "") require.Equal(t, tc.expectedQueries, len(queries)) results := make(map[string]string) @@ -3216,6 +3216,50 @@ func TestCoinGeckoQueries(t *testing.T) { } } +// Test the URL of CoinGecko queries to be correct +func TestCoinGeckoQueryFormat(t *testing.T) { + id_amount := 10 + ids := make([]string, id_amount) + for idx := 0; idx < id_amount; idx++ { + ids[idx] = fmt.Sprintf("id%d", idx) + } + + // Create and parse the query + queries := createCoinGeckoQueries(ids, 100, "") // No API key + require.Equal(t, len(queries), 1) + query_url, err := url.Parse(queries[0]) + require.Equal(t, err, nil) + params, err := url.ParseQuery(query_url.RawQuery) + require.Equal(t, err, nil) + + // Test the portions of the URL for the non-pro version of the API + require.Equal(t, query_url.Scheme, "https") + require.Equal(t, query_url.Host, "api.coingecko.com") + require.Equal(t, query_url.Path, "/api/v3/simple/price") + require.Equal(t, params.Has("x_cg_pro_api_key"), false) + require.Equal(t, params.Has("vs_currencies"), true) + require.Equal(t, params["vs_currencies"][0], "usd") + require.Equal(t, params.Has("ids"), true) + + // Create and parse the query with an API key + queries = createCoinGeckoQueries(ids, 100, "FAKE_KEY") // With API key + require.Equal(t, len(queries), 1) + query_url, err = url.Parse(queries[0]) + require.Equal(t, err, nil) + params, err = url.ParseQuery(query_url.RawQuery) + require.Equal(t, err, nil) + + // Test the portions of the URL actually provided + require.Equal(t, query_url.Scheme, "https") + require.Equal(t, query_url.Host, "pro-api.coingecko.com") + require.Equal(t, query_url.Path, "/api/v3/simple/price") + require.Equal(t, params.Has("x_cg_pro_api_key"), true) + require.Equal(t, params["x_cg_pro_api_key"][0], "FAKE_KEY") + require.Equal(t, params.Has("vs_currencies"), true) + require.Equal(t, params["vs_currencies"][0], "usd") + require.Equal(t, params.Has("ids"), true) +} + // setupLogsCapture is a helper function for making a zap logger/observer combination for testing that certain logs have been made func setupLogsCapture(t testing.TB, options ...zap.Option) (*zap.Logger, *observer.ObservedLogs) { t.Helper() diff --git a/node/pkg/node/adminServiceRunnable.go b/node/pkg/node/adminServiceRunnable.go index 44e1e79bc8..d892333238 100644 --- a/node/pkg/node/adminServiceRunnable.go +++ b/node/pkg/node/adminServiceRunnable.go @@ -77,7 +77,7 @@ func adminServiceRunnable( contract := ethcommon.HexToAddress(*ethContract) evmConnector, err = connectors.NewEthereumBaseConnector(ctx, "eth", *ethRpc, contract, logger) if err != nil { - return nil, fmt.Errorf("failed to connecto to ethereum") + return nil, fmt.Errorf("failed to connect to ethereum") } } diff --git a/node/pkg/node/node_test.go b/node/pkg/node/node_test.go index 9c74be7af4..38929c3209 100644 --- a/node/pkg/node/node_test.go +++ b/node/pkg/node/node_test.go @@ -188,7 +188,7 @@ func mockGuardianRunnable(t testing.TB, gs []*mockGuardian, mockGuardianIndex ui GuardianOptionDatabase(db), GuardianOptionWatchers(watcherConfigs, nil), GuardianOptionNoAccountant(), // disable accountant - GuardianOptionGovernor(true, false), + GuardianOptionGovernor(true, false, ""), GuardianOptionGatewayRelayer("", nil), // disable gateway relayer GuardianOptionP2P(gs[mockGuardianIndex].p2pKey, networkID, bootstrapPeers, nodeName, false, false, cfg.p2pPort, "", 0, "", "", func() string { return "" }), GuardianOptionPublicRpcSocket(cfg.publicSocket, publicRpcLogDetail), diff --git a/node/pkg/node/options.go b/node/pkg/node/options.go index 48f2e8c5af..5103148c69 100644 --- a/node/pkg/node/options.go +++ b/node/pkg/node/options.go @@ -218,7 +218,7 @@ func GuardianOptionAccountant( // GuardianOptionGovernor enables or disables the governor. // Dependencies: db -func GuardianOptionGovernor(governorEnabled bool, flowCancelEnabled bool) *GuardianOption { +func GuardianOptionGovernor(governorEnabled bool, flowCancelEnabled bool, coinGeckoApiKey string) *GuardianOption { return &GuardianOption{ name: "governor", dependencies: []string{"db"}, @@ -227,9 +227,13 @@ func GuardianOptionGovernor(governorEnabled bool, flowCancelEnabled bool) *Guard if flowCancelEnabled { logger.Info("chain governor is enabled with flow cancel enabled") } else { + logger.Info("chain governor is enabled without flow cancel") } - g.gov = governor.NewChainGovernor(logger, g.db, g.env, flowCancelEnabled) + if coinGeckoApiKey != "" { + logger.Info("coingecko pro API key in use") + } + g.gov = governor.NewChainGovernor(logger, g.db, g.env, flowCancelEnabled, coinGeckoApiKey) } else { logger.Info("chain governor is disabled") } diff --git a/node/pkg/publicrpc/publicrpcserver_test.go b/node/pkg/publicrpc/publicrpcserver_test.go index a6b9dad416..8e2446df0c 100644 --- a/node/pkg/publicrpc/publicrpcserver_test.go +++ b/node/pkg/publicrpc/publicrpcserver_test.go @@ -69,7 +69,7 @@ func TestGetSignedVAABadAddress(t *testing.T) { func TestGovernorIsVAAEnqueuedNoMessage(t *testing.T) { ctx := context.Background() logger, _ := zap.NewProduction() - gov := governor.NewChainGovernor(logger, nil, common.GoTest, false) + gov := governor.NewChainGovernor(logger, nil, common.GoTest, false, "") server := &PublicrpcServer{logger: logger, gov: gov} // A message without the messageId set should not panic but return an error instead.