add gateway connector capability config #14238

DavidOrchard · 2024-08-26T22:46:30Z

CAPPL-19

toml/types.go
add

type ConnectorConfig struct {
type WorkflowConnectorConfig struct {

and add

WorkflowConnector WorkflowConnectorConfig `toml:", omitempty"`

to Capabilities.

core/config/capabilities_config.go
add

type ConnectorConfig interface {
type ConnectorGatewayConfig interface {
type WorkflowConnectorConfig interface {

services/chainlink/config_capabilities.go
add

type workflowConnectorConfig struct {

delete core/services/gateway/connector/config.go
core/services/gateway/connector/connector.go
import config
update ConnectorConfig to config.ConnectorConfig
update function var from config to configLocal
Now has a type error on configLocal.donId, see comment.

cl-sonarqube-production · 2024-08-26T23:03:30Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

bolekk · 2024-08-27T01:43:52Z

core/capabilities/gateway_connector/config.go

+	"github.com/smartcontractkit/chainlink/v2/core/services/gateway/handlers/common"
+)
+
+type WorkflowConnectorConfig struct {


As discussed in DMs, this need to be a part of node's TOML config.

cedric-cordenier · 2024-08-29T15:21:55Z

core/services/gateway/connector/connector.go

 	url      *url.URL
 	wsClient network.WebSocketClient
 }

-func NewGatewayConnector(config *ConnectorConfig, signer Signer, handler GatewayConnectorHandler, clock clockwork.Clock, lggr logger.Logger) (GatewayConnector, error) {
-	if config == nil || signer == nil || handler == nil || clock == nil || lggr == nil {
+func NewGatewayConnector(configLocal *config.ConnectorConfig, signer Signer, handler GatewayConnectorHandler, clock clockwork.Clock, lggr logger.Logger) (GatewayConnector, error) {


@DavidOrchard Your problem is that you're passing in a pointer to an interface and then expecting to use the interface itself.

Interfaces are a bit strange in Golang: they represent a container type over a concrete type and for that reason are typically always passed around by value. The container itself could contain a pointer type though.

You should be able to fix this if you replace -> *config.ConnectorConfig with config.ConnectorConfig

I see the original (connector's)ConnectorConfig is a struct. If I want to preserve using a struct, does that mean I should use toml.ConnectorConfig instead of config.ConnectorConfig? Is that appropriate? cc @justinkaseman

From @cedric-cordenier via slack For config we usually pass around interfaces t is a bit odd as a pattern so your instincts are right, but it's correct in this context

cedric-cordenier · 2024-08-29T15:23:13Z

core/services/gateway/connector/connector.go

+	// configLocal.DonId undefined
+	// (type *"github.com/smartcontractkit/chainlink/v2/core/config".ConnectorConfig
+	// is pointer to interface, not interface)compilerMissingFieldOrMethod
+	if len(configLocal.DonId) == 0 || len(configLocal.DonId) > network.HandshakeDonIdLen {


Additionally, interfaces define a collection of methods a type must have; so the only way to access the underlying type is to call the methods. In other words, you have to call methods to access any attributes.

configLocal.DonId -> configLocal.DonId()

cedric-cordenier · 2024-08-29T15:23:43Z

core/services/gateway/connector/connector.go

 		return nil, errors.New("invalid DON ID")
 	}
-	addressBytes, err := hex.DecodeString(config.NodeAddress)
+	addressBytes, err := hex.DecodeString(configLocal.NodeAddress)


configLocal.NodeAddress -> configLocal.NodeAddress()

bolekk · 2024-08-29T15:26:52Z

core/config/capabilities_config.go

+	NodeAddress() string
+	DonId() string
+	Gateways() []ConnectorGatewayConfig
+	// copied from wsclient.go as can't import


It's a circular dependency in services/gateway/network. @DavidOrchard I think you could make this cleaner by getting rid of that circular dependency in services/gateway/config and then keeping the types in there.

cedric-cordenier · 2024-08-30T07:26:32Z

core/services/chainlink/config_capabilities.go

+	// "github.com/smartcontractkit/chainlink/v2/core/config".WorkflowConnectorConfig value in return statement:
+	//  *workflowConnectorConfig does not implement "github.com/smartcontractkit/chainlink/v2/core/config".WorkflowConnectorConfig (wrong type for method GatewayConnectorConfig)
+	//	have GatewayConnectorConfig() "github.com/smartcontractkit/chainlink/v2/core/config/toml".GatewayConnectorConfig
+	//	want GatewayConnectorConfig() "github.com/smartcontractkit/chainlink/v2/core/config".GatewayConnectorConfigcompilerInvalidIfaceAssign


Your problem here is that Go isn't very smart when it comes to determining whether an interface is satisfied, particularly when there's nesting involved.

toml.GatewayConnectorConfig might on its own satisfy this portion

GatewayConnectorConfig GatewayConnectorConfig json:"gatewayConnectorConfig"``

of the WorkflowConnectorInterface, but your method signature for the implementation needs to match, so line 70

func (c *workflowConnectorConfig) GatewayConnectorConfig() toml.GatewayConnectorConfig

needs to become

func (c *workflowConnectorConfig) GatewayConnectorConfig() GatewayConnectorConfig

cedric-cordenier · 2024-08-30T07:27:59Z

core/config/toml/types.go

+	}
+}
+
+type ConnectorGatewayConfig struct {


Note: you have both ConnectorGatewayConfig AND GatewayConnectorConfig which is a bit confusing here. Can we be more specific about what each config is for?

I agree that it's confusing. Note that the gateway/connector used to contain

type ConnectorConfig struct { NodeAddress string DonId string Gateways []ConnectorGatewayConfig WsClientConfig network.WebSocketClientConfig AuthMinChallengeLen int AuthTimestampToleranceSec uint32 } type ConnectorGatewayConfig struct { Id string URL string }

And there's also a GatewayConfig outthere.

We can rename this one into something more meaningful, maybe "GatewayNodeAddress" or "GatewayNodeIdentifier" (neither is perfect, though).

cedric-cordenier · 2024-08-30T07:29:35Z

core/services/chainlink/config_capabilities.go

+	// invalid operation: cannot indirect
+	// c.c.GatewayConnectorConfig (variable of type "github.com/smartcontractkit/chainlink/v2/core/config/toml".ConnectorConfig)
+	// compilerInvalidIndirection
+	return c.c.GatewayConnectorConfig


The toml struct won't be its own satisfy GatewayConnectorConfig since it doesn't have any of the interface methods on it. You'll need to define a wrapper like this and do the mapping of method on the interface to the attribute on the underlying toml fragment yourself.

ty. Having read your comment, now the error seems perfectly clear.

justinkaseman · 2024-08-30T22:17:30Z

core/services/gateway/connector/connector_test.go

@@ -33,18 +34,21 @@ Id = "another_one"
 URL = "wss://example.com:8090/node_endpoint"
 `

-func parseTOMLConfig(t *testing.T, tomlConfig string) *connector.ConnectorConfig {
-	var cfg connector.ConnectorConfig
+func parseTOMLConfig(t *testing.T, tomlConfig string) *config.ConnectorConfig {


Suggested change

func parseTOMLConfig(t *testing.T, tomlConfig string) *config.ConnectorConfig {

func parseTOMLConfig(t *testing.T, tomlConfig string) *config.GatewayConnectorConfig {

justinkaseman · 2024-08-30T22:17:40Z

core/services/gateway/connector/connector_test.go

-func parseTOMLConfig(t *testing.T, tomlConfig string) *connector.ConnectorConfig {
-	var cfg connector.ConnectorConfig
+func parseTOMLConfig(t *testing.T, tomlConfig string) *config.ConnectorConfig {
+	var cfg config.ConnectorConfig


Suggested change

var cfg config.ConnectorConfig

var cfg config.GatewayConnectorConfig

justinkaseman · 2024-08-30T22:22:08Z

core/services/gateway/connector/connector.go

 	url      *url.URL
 	wsClient network.WebSocketClient
 }

-func NewGatewayConnector(config *ConnectorConfig, signer Signer, handler GatewayConnectorHandler, clock clockwork.Clock, lggr logger.Logger) (GatewayConnector, error) {
-	if config == nil || signer == nil || handler == nil || clock == nil || lggr == nil {
+func NewGatewayConnector(configLocal config.GatewayConnectorConfig, signer Signer, handler GatewayConnectorHandler, clock clockwork.Clock, lggr logger.Logger) (GatewayConnector, error) {


config *ConnectorConfig was a struct, now configLocal config.GatewayConnectorConfig is an interface, which is why you had to change things in this file. It's also causing some breaks in some other files.

justinkaseman · 2024-08-30T22:24:38Z

core/services/gateway/integration_tests/gateway_integration_test.go

@@ -152,6 +153,13 @@ func TestIntegration_Gateway_NoFullNodes_BasicConnectionAndMessage(t *testing.T)

 	// Launch Connector
 	client := &client{privateKey: nodeKeys.PrivateKey}
+	// cannot use parseConnectorConfig(t, nodeConfigTemplate, nodeKeys.Address, nodeUrl)


Breaking from comment on NewGatewayConnector

justinkaseman · 2024-08-30T22:29:49Z

core/services/functions/connector_handler.go

@@ -83,6 +83,7 @@ func NewFunctionsConnectorHandler(pluginConfig *config.PluginConfig, signerKey *
 		allowedHeartbeatInitiators[strings.ToLower(initiator)] = struct{}{}
 	}
 	return &functionsConnectorHandler{
+		// pluginConfig.GatewayConnectorConfig.NodeAddress undefined (type *invalid type has no field or method NodeAddress)


core/services/ocr2/plugins/functions/config/config.go needs to be updated too

justinkaseman · 2024-08-30T22:30:15Z

core/services/functions/connector_handler_test.go

@@ -80,6 +80,7 @@ func TestFunctionsConnectorHandler(t *testing.T) {
 	subscriptions.On("Start", mock.Anything).Return(nil)
 	subscriptions.On("Close", mock.Anything).Return(nil)
 	config := &config.PluginConfig{
+		// undefined: gwconnector.ConnectorConfigcompilerUndeclaredImportedName


It's looking for ConnectorGatewayConfig struct

cedric-cordenier · 2024-09-03T16:59:52Z

core/services/functions/connector_handler_test.go

-		// undefined: gwconnector.ConnectorConfigcompilerUndeclaredImportedName
-		GatewayConnectorConfig: &gwconnector.ConnectorConfig{
+	config := config.PluginConfig{
+		// invalid composite literal type "github.com/smartcontractkit/chainlink/v2/core/config".GatewayConnectorConfigcompilerInvalidLit


You need to pass in a concrete implementation of the interface

cedric-cordenier · 2024-09-03T17:57:32Z

core/services/gateway/connector/connector.go

@@ -46,7 +47,7 @@ type GatewayConnectorHandler interface {
 type gatewayConnector struct {
 	services.StateMachine

-	config      *ConnectorConfig
+	config      config.GatewayConnectorConfig


This shouldn't need to be changed; keep using the struct (obviously you need the struct)

cedric-cordenier · 2024-09-04T08:53:52Z

core/config/toml/types.go

+	}
+
+	// TODO: verify this copy by reference is ok, or does array need to be copied by value
+	if f.Gateways != nil {


It's fine to just copy by reference here, but when returning the array (i.e. in the interface implementation) you probably do want to copy it

DavidOrchard · 2024-09-05T22:02:21Z

Closed as replaced by newer PR.

DavidOrchard added poc Proof of concept DO_NOT_MERGE labels Aug 26, 2024

DavidOrchard requested a review from a team as a code owner August 26, 2024 22:46

DavidOrchard requested review from krehermann and removed request for a team August 26, 2024 22:46

bolekk requested changes Aug 27, 2024

View reviewed changes

DavidOrchard force-pushed the feature/CAPPL-19-connector-service branch 2 times, most recently from 2167011 to 29f6e46 Compare August 29, 2024 00:57

cedric-cordenier reviewed Aug 29, 2024

View reviewed changes

bolekk reviewed Aug 29, 2024

View reviewed changes

DavidOrchard force-pushed the feature/CAPPL-19-connector-service branch from 29f6e46 to c639695 Compare August 29, 2024 22:27

DavidOrchard marked this pull request as draft August 29, 2024 22:31

cedric-cordenier reviewed Aug 30, 2024

View reviewed changes

DavidOrchard force-pushed the feature/CAPPL-19-connector-service branch from c639695 to e9cf6a8 Compare August 30, 2024 17:21

add gateway connector capability config

44de5da

DavidOrchard force-pushed the feature/CAPPL-19-connector-service branch from e9cf6a8 to 44de5da Compare August 30, 2024 19:32

refactor network configs into core to prevent circular deps

92f95a7

justinkaseman reviewed Aug 30, 2024

View reviewed changes

more updates

a1c61f1

cedric-cordenier reviewed Sep 3, 2024

View reviewed changes

cedric-cordenier reviewed Sep 4, 2024

View reviewed changes

DavidOrchard closed this Sep 5, 2024

DavidOrchard deleted the feature/CAPPL-19-connector-service branch September 5, 2024 22:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add gateway connector capability config #14238

add gateway connector capability config #14238

DavidOrchard commented Aug 26, 2024 •

edited

Loading

cl-sonarqube-production bot commented Aug 26, 2024

bolekk Aug 27, 2024

cedric-cordenier Aug 29, 2024

DavidOrchard Aug 29, 2024

DavidOrchard Aug 29, 2024

cedric-cordenier Aug 29, 2024 •

edited

Loading

cedric-cordenier Aug 29, 2024

bolekk Aug 29, 2024

justinkaseman Aug 29, 2024 •

edited

Loading

cedric-cordenier Aug 30, 2024

cedric-cordenier Aug 30, 2024

DavidOrchard Aug 30, 2024

DavidOrchard Aug 30, 2024

bolekk Sep 3, 2024

cedric-cordenier Aug 30, 2024

DavidOrchard Aug 30, 2024

justinkaseman Aug 30, 2024

justinkaseman Aug 30, 2024

justinkaseman Aug 30, 2024 •

edited

Loading

justinkaseman Aug 30, 2024

justinkaseman Aug 30, 2024 •

edited

Loading

justinkaseman Aug 30, 2024 •

edited

Loading

cedric-cordenier Sep 3, 2024

cedric-cordenier Sep 3, 2024

cedric-cordenier Sep 4, 2024

DavidOrchard commented Sep 5, 2024

	func parseTOMLConfig(t testing.T, tomlConfig string) config.ConnectorConfig {
	func parseTOMLConfig(t testing.T, tomlConfig string) config.GatewayConnectorConfig {

	var cfg config.ConnectorConfig
	var cfg config.GatewayConnectorConfig

add gateway connector capability config #14238

add gateway connector capability config #14238

Conversation

DavidOrchard commented Aug 26, 2024 • edited Loading

cl-sonarqube-production bot commented Aug 26, 2024

Quality Gate passed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cedric-cordenier Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinkaseman Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinkaseman Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinkaseman Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

justinkaseman Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidOrchard commented Sep 5, 2024

DavidOrchard commented Aug 26, 2024 •

edited

Loading

cedric-cordenier Aug 29, 2024 •

edited

Loading

justinkaseman Aug 29, 2024 •

edited

Loading

justinkaseman Aug 30, 2024 •

edited

Loading

justinkaseman Aug 30, 2024 •

edited

Loading

justinkaseman Aug 30, 2024 •

edited

Loading