cleanup for initial feedback review

smartcontractkit · Sep 22, 2023 · 1526b81 · 1526b81
1 parent a01e0a8
commit 1526b81
Show file tree

Hide file tree

Showing 8 changed files with 143 additions and 131 deletions.
diff --git a/core/chains/evm/chain.go b/core/chains/evm/chain.go
@@ -169,7 +169,7 @@ type RelayerConfig struct {
 	GenGasEstimator   func(*big.Int) gas.EvmFeeEstimator
 }
 
-func (r RelayerConfig) validate() error {
+func (r RelayerConfig) Validate() error {
 	var err error
 	if r.AppConfig == nil {
 		err = errors.Join(err, fmt.Errorf("nil AppConfig"))

diff --git a/core/cmd/shell_local_test.go b/core/cmd/shell_local_test.go
@@ -41,8 +41,7 @@ import (
 
 func genTestEVMRelayers(t *testing.T, opts evm.ChainRelayExtenderConfig, ks evmrelayer.CSAETHKeystore) *chainlink.CoreRelayerChainInteroperators {
 	f := chainlink.RelayerFactory{
-		Logger: opts.Logger,
-		//DB:           opts.DB,
+		Logger:       opts.Logger,
 		QConfig:      opts.AppConfig.Database(),
 		LoopRegistry: plugins.NewLoopRegistry(opts.Logger),
 	}

diff --git a/core/cmd/shell_test.go b/core/cmd/shell_test.go
@@ -369,8 +369,7 @@ func TestSetupSolanaRelayer(t *testing.T) {
 	})
 
 	rf := chainlink.RelayerFactory{
-		Logger: lggr,
-		//	DB:           pgtest.NewSqlxDB(t),
+		Logger:       lggr,
 		QConfig:      tConfig.Database(),
 		LoopRegistry: reg,
 	}
@@ -456,8 +455,7 @@ func TestSetupStarkNetRelayer(t *testing.T) {
 		}
 	})
 	rf := chainlink.RelayerFactory{
-		Logger: lggr,
-		//DB:           pgtest.NewSqlxDB(t),
+		Logger:       lggr,
 		QConfig:      tConfig.Database(),
 		LoopRegistry: reg,
 	}

diff --git a/core/internal/cltest/cltest.go b/core/internal/cltest/cltest.go
@@ -386,8 +386,7 @@ func NewApplicationWithConfig(t testing.TB, cfg chainlink.GeneralConfig, flagsAn
 	loopRegistry := plugins.NewLoopRegistry(lggr)
 
 	relayerFactory := chainlink.RelayerFactory{
-		Logger: lggr,
-		//	DB:           db,
+		Logger:       lggr,
 		QConfig:      cfg.Database(),
 		LoopRegistry: loopRegistry,
 		GRPCOpts:     loop.GRPCOpts{},

diff --git a/core/internal/testutils/pgtest/pgtest.go b/core/internal/testutils/pgtest/pgtest.go
@@ -3,7 +3,6 @@ package pgtest
 import (
 	"database/sql"
 	"encoding/json"
-	"fmt"
 	"testing"
 
 	"github.com/google/uuid"
@@ -32,15 +31,11 @@ func NewSqlDB(t *testing.T) *sql.DB {
 }
 
 func NewEVMScopedDB(t testing.TB) *sqlx.DB {
-	// hack to scope to evm schema
-	url := withSchema(defaultDBURL, "evm")
+	// hack to scope to evm schema. the value "evm" will need to be dynamic to support multiple relayers
+	url := pg.SchemaScopedConnection(defaultDBURL, "evm")
 	return NewSqlxDB(t, WithURL(url))
 }
 
-func withSchema(conn, schema string) string {
-	return fmt.Sprintf("%s&options=-csearch_path=%s", conn, schema)
-}
-
 func NewSqlxDB(t testing.TB, opts ...ConnectionOpt) *sqlx.DB {
 	testutils.SkipShortDB(t)
 	conn := &pg.ConnectionScope{

diff --git a/core/services/chainlink/relayer_chain_interoperators_test.go b/core/services/chainlink/relayer_chain_interoperators_test.go
@@ -173,8 +173,7 @@ func TestCoreRelayerChainInteroperators(t *testing.T) {
 	lggr := logger.TestLogger(t)
 
 	factory := chainlink.RelayerFactory{
-		Logger: lggr,
-		//DB:           db,
+		Logger:       lggr,
 		QConfig:      cfg.Database(),
 		LoopRegistry: plugins.NewLoopRegistry(lggr),
 		GRPCOpts:     loop.GRPCOpts{},
@@ -224,109 +223,109 @@ func TestCoreRelayerChainInteroperators(t *testing.T) {
 			},
 			expectedRelayerNetworks: map[relay.Network]struct{}{relay.EVM: {}},
 		},
-		/*
-			{name: "2 solana chain with 2 node",
 
-				initFuncs: []chainlink.CoreRelayerChainInitFunc{
-					chainlink.InitSolana(testctx, factory, chainlink.SolanaFactoryConfig{
-						Keystore:      keyStore.Solana(),
-						SolanaConfigs: cfg.SolanaConfigs()}),
-				},
-				expectedSolanaChainCnt: 2,
-				expectedSolanaNodeCnt:  2,
-				expectedSolanaRelayerIds: []relay.ID{
-					{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID1)},
-					{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID2)},
-				},
-				expectedRelayerNetworks: map[relay.Network]struct{}{relay.Solana: {}},
+		{name: "2 solana chain with 2 node",
+
+			initFuncs: []chainlink.CoreRelayerChainInitFunc{
+				chainlink.InitSolana(testctx, factory, chainlink.SolanaFactoryConfig{
+					Keystore:      keyStore.Solana(),
+					SolanaConfigs: cfg.SolanaConfigs()}),
 			},
+			expectedSolanaChainCnt: 2,
+			expectedSolanaNodeCnt:  2,
+			expectedSolanaRelayerIds: []relay.ID{
+				{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID1)},
+				{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID2)},
+			},
+			expectedRelayerNetworks: map[relay.Network]struct{}{relay.Solana: {}},
+		},
 
-			{name: "2 starknet chain with 4 nodes",
+		{name: "2 starknet chain with 4 nodes",
 
-				initFuncs: []chainlink.CoreRelayerChainInitFunc{
-					chainlink.InitStarknet(testctx, factory, chainlink.StarkNetFactoryConfig{
-						Keystore:        keyStore.StarkNet(),
-						StarknetConfigs: cfg.StarknetConfigs()}),
-				},
-				expectedStarknetChainCnt: 2,
-				expectedStarknetNodeCnt:  4,
-				expectedStarknetRelayerIds: []relay.ID{
-					{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID1)},
-					{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID2)},
-				},
-				expectedRelayerNetworks: map[relay.Network]struct{}{relay.StarkNet: {}},
+			initFuncs: []chainlink.CoreRelayerChainInitFunc{
+				chainlink.InitStarknet(testctx, factory, chainlink.StarkNetFactoryConfig{
+					Keystore:        keyStore.StarkNet(),
+					StarknetConfigs: cfg.StarknetConfigs()}),
+			},
+			expectedStarknetChainCnt: 2,
+			expectedStarknetNodeCnt:  4,
+			expectedStarknetRelayerIds: []relay.ID{
+				{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID1)},
+				{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID2)},
 			},
+			expectedRelayerNetworks: map[relay.Network]struct{}{relay.StarkNet: {}},
+		},
 
-			{
-				name: "2 cosmos chains with 2 nodes",
-				initFuncs: []chainlink.CoreRelayerChainInitFunc{
-					chainlink.InitCosmos(testctx, factory, chainlink.CosmosFactoryConfig{
-						Keystore:         keyStore.Cosmos(),
-						CosmosConfigs:    cfg.CosmosConfigs(),
-						EventBroadcaster: pg.NewNullEventBroadcaster()}),
-				},
-				expectedCosmosChainCnt: 2,
-				expectedCosmosNodeCnt:  2,
-				expectedCosmosRelayerIds: []relay.ID{
-					{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID1)},
-					{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID2)},
-				},
-				expectedRelayerNetworks: map[relay.Network]struct{}{relay.Cosmos: {}},
+		{
+			name: "2 cosmos chains with 2 nodes",
+			initFuncs: []chainlink.CoreRelayerChainInitFunc{
+				chainlink.InitCosmos(testctx, factory, chainlink.CosmosFactoryConfig{
+					Keystore:         keyStore.Cosmos(),
+					CosmosConfigs:    cfg.CosmosConfigs(),
+					EventBroadcaster: pg.NewNullEventBroadcaster()}),
+			},
+			expectedCosmosChainCnt: 2,
+			expectedCosmosNodeCnt:  2,
+			expectedCosmosRelayerIds: []relay.ID{
+				{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID1)},
+				{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID2)},
 			},
+			expectedRelayerNetworks: map[relay.Network]struct{}{relay.Cosmos: {}},
+		},
 
-			{name: "all chains",
+		{name: "all chains",
 
-				initFuncs: []chainlink.CoreRelayerChainInitFunc{chainlink.InitSolana(testctx, factory, chainlink.SolanaFactoryConfig{
-					Keystore:      keyStore.Solana(),
-					SolanaConfigs: cfg.SolanaConfigs()}),
-					chainlink.InitEVM(testctx, factory, chainlink.EVMFactoryConfig{
-						RelayerConfig: &evm.RelayerConfig{
-							AppConfig:        cfg,
-							EventBroadcaster: pg.NewNullEventBroadcaster(),
-							MailMon:          &utils.MailboxMonitor{},
-						},
-						CSAETHKeystore: keyStore,
-					}),
-					chainlink.InitStarknet(testctx, factory, chainlink.StarkNetFactoryConfig{
-						Keystore:        keyStore.StarkNet(),
-						StarknetConfigs: cfg.StarknetConfigs()}),
-					chainlink.InitCosmos(testctx, factory, chainlink.CosmosFactoryConfig{
-						Keystore:         keyStore.Cosmos(),
-						CosmosConfigs:    cfg.CosmosConfigs(),
+			initFuncs: []chainlink.CoreRelayerChainInitFunc{chainlink.InitSolana(testctx, factory, chainlink.SolanaFactoryConfig{
+				Keystore:      keyStore.Solana(),
+				SolanaConfigs: cfg.SolanaConfigs()}),
+				chainlink.InitEVM(testctx, factory, chainlink.EVMFactoryConfig{
+					RelayerConfig: &evm.RelayerConfig{
+						AppConfig:        cfg,
 						EventBroadcaster: pg.NewNullEventBroadcaster(),
-					}),
-				},
-				expectedEVMChainCnt: 2,
-				expectedEVMNodeCnt:  3,
-				expectedEVMRelayerIds: []relay.ID{
-					{Network: relay.EVM, ChainID: relay.ChainID(evmChainID1.String())},
-					{Network: relay.EVM, ChainID: relay.ChainID(evmChainID2.String())},
-				},
-
-				expectedSolanaChainCnt: 2,
-				expectedSolanaNodeCnt:  2,
-				expectedSolanaRelayerIds: []relay.ID{
-					{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID1)},
-					{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID2)},
-				},
+						MailMon:          &utils.MailboxMonitor{},
+						DB:               pgtest.NewEVMScopedDB(t),
+					},
+					CSAETHKeystore: keyStore,
+				}),
+				chainlink.InitStarknet(testctx, factory, chainlink.StarkNetFactoryConfig{
+					Keystore:        keyStore.StarkNet(),
+					StarknetConfigs: cfg.StarknetConfigs()}),
+				chainlink.InitCosmos(testctx, factory, chainlink.CosmosFactoryConfig{
+					Keystore:         keyStore.Cosmos(),
+					CosmosConfigs:    cfg.CosmosConfigs(),
+					EventBroadcaster: pg.NewNullEventBroadcaster(),
+				}),
+			},
+			expectedEVMChainCnt: 2,
+			expectedEVMNodeCnt:  3,
+			expectedEVMRelayerIds: []relay.ID{
+				{Network: relay.EVM, ChainID: relay.ChainID(evmChainID1.String())},
+				{Network: relay.EVM, ChainID: relay.ChainID(evmChainID2.String())},
+			},
 
-				expectedStarknetChainCnt: 2,
-				expectedStarknetNodeCnt:  4,
-				expectedStarknetRelayerIds: []relay.ID{
-					{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID1)},
-					{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID2)},
-				},
+			expectedSolanaChainCnt: 2,
+			expectedSolanaNodeCnt:  2,
+			expectedSolanaRelayerIds: []relay.ID{
+				{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID1)},
+				{Network: relay.Solana, ChainID: relay.ChainID(solanaChainID2)},
+			},
 
-				expectedCosmosChainCnt: 2,
-				expectedCosmosNodeCnt:  2,
-				expectedCosmosRelayerIds: []relay.ID{
-					{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID1)},
-					{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID2)},
-				},
+			expectedStarknetChainCnt: 2,
+			expectedStarknetNodeCnt:  4,
+			expectedStarknetRelayerIds: []relay.ID{
+				{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID1)},
+				{Network: relay.StarkNet, ChainID: relay.ChainID(starknetChainID2)},
+			},
 
-				expectedRelayerNetworks: map[relay.Network]struct{}{relay.EVM: {}, relay.Cosmos: {}, relay.Solana: {}, relay.StarkNet: {}},
+			expectedCosmosChainCnt: 2,
+			expectedCosmosNodeCnt:  2,
+			expectedCosmosRelayerIds: []relay.ID{
+				{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID1)},
+				{Network: relay.Cosmos, ChainID: relay.ChainID(cosmosChainID2)},
 			},
-		*/
+
+			expectedRelayerNetworks: map[relay.Network]struct{}{relay.EVM: {}, relay.Cosmos: {}, relay.Solana: {}, relay.StarkNet: {}},
+		},
 	}
 	for _, tt := range tests {
 		tt := tt

diff --git a/core/services/chainlink/relayer_factory.go b/core/services/chainlink/relayer_factory.go
@@ -2,6 +2,7 @@ package chainlink
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/pelletier/go-toml/v2"
@@ -37,18 +38,30 @@ type EVMFactoryConfig struct {
 	evmrelay.CSAETHKeystore
 }
 
+// TODO plumb the generic validator here maybe
+func (c EVMFactoryConfig) Validate() error {
+	var err error
+	if c.RelayerConfig == nil {
+		err = errors.Join(err, fmt.Errorf("nil RelayerConfig"))
+	} else {
+		err = errors.Join(err, c.RelayerConfig.Validate())
+	}
+	if c.CSAETHKeystore == nil {
+		err = errors.Join(err, fmt.Errorf("nil CSAETH Keystore"))
+	}
+	if err != nil {
+		err = fmt.Errorf("invalid EVMFactoryConfig: %w", err)
+	}
+	return err
+}
+
 func (r *RelayerFactory) NewEVM(ctx context.Context, config EVMFactoryConfig) (map[relay.ID]evmrelay.LoopRelayAdapter, error) {
 	// TODO impl EVM loop. For now always 'fallback' to an adapter and embedded chain
-	/*
-		if config.DBConn == "" {
-			u := config.AppConfig.Database().URL()
-			config.DBConn = withSchema((&u).String(), "evm")
-		}
-		db, err := sqlx.Open(string(dialects.Postgres), config.DBConn)
-		if err != nil {
-			return nil, err
-		}
-	*/
+
+	err := config.Validate()
+	if err != nil {
+		return nil, err
+	}
 	relayers := make(map[relay.ID]evmrelay.LoopRelayAdapter)
 
 	// override some common opts with the factory values. this seems weird... maybe other signatures should change, or this should take a different type...
@@ -220,12 +233,6 @@ type CosmosFactoryConfig struct {
 }
 
 func (r *RelayerFactory) NewCosmos(ctx context.Context, config CosmosFactoryConfig) (map[relay.ID]cosmos.LoopRelayerChainer, error) {
-	/*
-		db, err := sqlx.Open(string(dialects.Postgres), config.DBConn)
-		if err != nil {
-			return nil, err
-		}
-	*/
 	relayers := make(map[relay.ID]cosmos.LoopRelayerChainer)
 
 	var (
@@ -255,7 +262,3 @@ func (r *RelayerFactory) NewCosmos(ctx context.Context, config CosmosFactoryConf
 	return relayers, nil
 
 }
-
-func withSchema(conn, schema string) string {
-	return fmt.Sprintf("%s&options=-csearch_path=%s", conn, schema)
-}
diff --git a/core/services/pg/locked_db.go b/core/services/pg/locked_db.go
@@ -2,8 +2,8 @@ package pg
 
 import (
 	"context"
-	"fmt"
 	"net/url"
+	"strings"
 	"time"
 
 	"github.com/google/uuid"
@@ -154,13 +154,32 @@ func openDB(appID uuid.UUID, cfg LockedDBConfig, opts ...ConnectionOpt) (db *sql
 
 type ConnectionOpt func(*string)
 
+// WithSchema scopes the current url connection string to the given schema
 func WithSchema(schema string) ConnectionOpt {
 	return func(url *string) {
-		u := withSchema(*url, schema)
+		u := SchemaScopedConnection(*url, schema)
 		url = &u
 	}
 }
 
-func withSchema(conn, schema string) string {
-	return fmt.Sprintf("%s&options=-csearch_path=%s", conn, schema)
+// WithURL sets the url connection string
+func WithURL(override string) ConnectionOpt {
+	return func(url *string) {
+		url = &override
+	}
+}
+
+func SchemaScopedConnection(conn, schema string) string {
+	// documentation on this options is limited; find by searching the official postgres docs
+	// https://www.postgresql.org/docs/16/app-psql.html
+
+	//TODO real parsing for connection query string
+	opt := "options=-csearch_path=" + schema
+	// assume the conn has a query string that we can append to
+	seperator := "&"
+	if !strings.Contains(conn, "?") {
+		// no query string, so make one
+		seperator = "?"
+	}
+	return conn + seperator + opt
 }