add gateway connector capability config #14325

DavidOrchard · 2024-09-03T23:08:19Z

toml/types.go
add
type GatewayConnectorConfig struct {

and add
2. GatewayConnectorConfig GatewayConnectorConfig toml:", omitempty"
to Capabilities.

core/config/capabilities_config.go
add
type ConnectorConfig interface {
type ConnectorGatewayConfig interface {
services/chainlink/config_capabilities.go
add
type GatewayConnectorConfig struct {

bolekk · 2024-09-04T19:42:12Z

core/config/capabilities_config.go

+	URL() string
+}
+
+type WorkflowConnectorConfig interface {


I think we should merge these two structs for simplicity. WorkflowConnectorConfig is not a great name (I know I proposed it myself). How about we flatten everything into GatewayConnectorConfig? Simply move ChainIDForNodeKey() in there.

bolekk · 2024-09-05T01:37:40Z

core/config/docs/core.toml

@@ -509,6 +509,28 @@ DeltaReconcile = '1m' # Default
 # but the host and port must be fully specified and cannot be empty. You can specify `0.0.0.0` (IPv4) or `::` (IPv6) to listen on all interfaces, but that is not recommended.
 ListenAddresses = ['1.2.3.4:9999', '[a52d:0:a88:1274::abcd]:1337'] # Example

+[Capabilities.WorkflowConnectorConfig]
+# ChainIDForNodeKey is the ChainID of the network
+ChainIDForNodeKey = '1' # Example


As discussed before, you'll also need to provide the desired address of a key you want to use. You can handle it in a separate PR but it will save you some trouble if you don't have to re-generate all test outputs again.

I've generated 0xfd29Dd9C980D715a64dace97F7A2AB98bcaE0fed so I'll use that as the default.

jmank88 · 2024-09-05T14:07:29Z

core/config/capabilities_config.go

+type GatewayConnectorConfig interface {
+	ChainIDForNodeKey() string
+	NodeAddress() string
+	DonID() string
+	Gateways() []ConnectorGatewayConfig
+	WsHandshakeTimeoutMillis() uint32
+	AuthMinChallengeLen() int
+	AuthTimestampToleranceSec() uint32
+}
+
+type ConnectorGatewayConfig interface {
+	ID() string
+	URL() string
+}
+
 type Capabilities interface {
 	Peering() P2P
 	Dispatcher() Dispatcher
 	ExternalRegistry() CapabilitiesExternalRegistry
+	GatewayConnectorConfig() GatewayConnectorConfig


In most places, the Config suffix is redundant:

Suggested change

type GatewayConnectorConfig interface {

ChainIDForNodeKey() string

NodeAddress() string

DonID() string

Gateways() []ConnectorGatewayConfig

WsHandshakeTimeoutMillis() uint32

AuthMinChallengeLen() int

AuthTimestampToleranceSec() uint32

}

type ConnectorGatewayConfig interface {

ID() string

URL() string

}

type Capabilities interface {

Peering() P2P

Dispatcher() Dispatcher

ExternalRegistry() CapabilitiesExternalRegistry

GatewayConnectorConfig() GatewayConnectorConfig

type GatewayConnector interface {

ChainIDForNodeKey() string

NodeAddress() string

DonID() string

Gateways() []ConnectorGateway

WsHandshakeTimeoutMillis() uint32

AuthMinChallengeLen() int

AuthTimestampToleranceSec() uint32

}

type ConnectorGateway interface {

ID() string

URL() string

}

type Capabilities interface {

Peering() P2P

Dispatcher() Dispatcher

ExternalRegistry() CapabilitiesExternalRegistry

GatewayConnector() GatewayConnector

jmank88 · 2024-09-05T14:07:54Z

core/config/docs/core.toml

+[Capabilities.GatewayConnectorConfig]
+# ChainIDForNodeKey is the ChainID of the network
+ChainIDForNodeKey = '0xfd29Dd9C980D715a64dace97F7A2AB98bcaE0fed' # Default
+# NodeAddress is Workflow Node address
+NodeAddress = '0x68902d681c28119f9b2531473a417088bf008e59' # Example
+# DonID is the Id of the Don
+DonID = 'example_don' # Example
+# WsHandshakeTimeoutMillis is Websocket handshake timeout
+WsHandshakeTimeoutMillis = 1000 # Example
+# AuthMinChallengeLen is the minimum number of bytes in Authentication
+AuthMinChallengeLen = 10 # Example
+# AuthTimestampToleranceSec is Authentication timestamp tolerance
+AuthTimestampToleranceSec = 10 # Example
+
+[[Capabilities.GatewayConnectorConfig.Gateways]]


Suggested change

[Capabilities.GatewayConnectorConfig]

# ChainIDForNodeKey is the ChainID of the network

ChainIDForNodeKey = '0xfd29Dd9C980D715a64dace97F7A2AB98bcaE0fed' # Default

# NodeAddress is Workflow Node address

NodeAddress = '0x68902d681c28119f9b2531473a417088bf008e59' # Example

# DonID is the Id of the Don

DonID = 'example_don' # Example

# WsHandshakeTimeoutMillis is Websocket handshake timeout

WsHandshakeTimeoutMillis = 1000 # Example

# AuthMinChallengeLen is the minimum number of bytes in Authentication

AuthMinChallengeLen = 10 # Example

# AuthTimestampToleranceSec is Authentication timestamp tolerance

AuthTimestampToleranceSec = 10 # Example

[[Capabilities.GatewayConnectorConfig.Gateways]]

[Capabilities.GatewayConnector]

# ChainIDForNodeKey is the ChainID of the network

ChainIDForNodeKey = '0xfd29Dd9C980D715a64dace97F7A2AB98bcaE0fed' # Default

# NodeAddress is Workflow Node address

NodeAddress = '0x68902d681c28119f9b2531473a417088bf008e59' # Example

# DonID is the Id of the Don

DonID = 'example_don' # Example

# WsHandshakeTimeoutMillis is Websocket handshake timeout

WsHandshakeTimeoutMillis = 1000 # Example

# AuthMinChallengeLen is the minimum number of bytes in Authentication

AuthMinChallengeLen = 10 # Example

# AuthTimestampToleranceSec is Authentication timestamp tolerance

AuthTimestampToleranceSec = 10 # Example

[[Capabilities.GatewayConnector.Gateways]]

Actually, does Connector add value? Or would this make sense and be clear to users as just Gateway?

Yeah Connector is important as it distinguishes when a node connects to gateways or is a gateway itself. Similarly we talk about Gateway handlers and Connector handlers. Let's keep it.

jmank88 · 2024-09-05T14:09:27Z

main_test.go

@@ -60,7 +60,7 @@ func TestScripts(t *testing.T) {
 				Dir:             path,
 				Setup:           commonEnv,
 				ContinueOnError: true,
-				// UpdateScripts:   true, // uncomment to update golden files
+				UpdateScripts:   true, // uncomment to update golden files


jmank88 · 2024-09-05T14:10:32Z

testdata/scripts/node/db/help.txtar

This looks like you ran update with a production (non dev/test mode) chainlink binary

AFAIK, my build has always been a dev binary via make chainlink-dev. This file keeps alternating back and forth fwiw.

testscripts-update appears to be running chainlink-test which is not dev mode - just a plain build.

bolekk

Left some minor suggestions but looks good overall!

bolekk · 2024-09-05T14:11:28Z

core/config/docs/core.toml

@@ -509,6 +509,26 @@ DeltaReconcile = '1m' # Default
 # but the host and port must be fully specified and cannot be empty. You can specify `0.0.0.0` (IPv4) or `::` (IPv6) to listen on all interfaces, but that is not recommended.
 ListenAddresses = ['1.2.3.4:9999', '[a52d:0:a88:1274::abcd]:1337'] # Example

+[Capabilities.GatewayConnectorConfig]
+# ChainIDForNodeKey is the ChainID of the network
+ChainIDForNodeKey = '0xfd29Dd9C980D715a64dace97F7A2AB98bcaE0fed' # Default


The default here is an integer such as 11155111 for Ethereum Sepolia, not a hex address.

But still a string to be agnostic? And this should be an example, no?

Suggested change

ChainIDForNodeKey = '0xfd29Dd9C980D715a64dace97F7A2AB98bcaE0fed' # Default

ChainIDForNodeKey = '11155111' # Example

Yes, a string, thanks

bolekk · 2024-09-05T14:12:50Z

core/config/docs/core.toml

@@ -509,6 +509,26 @@ DeltaReconcile = '1m' # Default
 # but the host and port must be fully specified and cannot be empty. You can specify `0.0.0.0` (IPv4) or `::` (IPv6) to listen on all interfaces, but that is not recommended.
 ListenAddresses = ['1.2.3.4:9999', '[a52d:0:a88:1274::abcd]:1337'] # Example

+[Capabilities.GatewayConnectorConfig]
+# ChainIDForNodeKey is the ChainID of the network


"ChainIDForNodeKey is the ChainID of the network associated with a private key to be used for authentication with Gateway nodes"

bolekk · 2024-09-05T14:13:19Z

core/config/docs/core.toml

+[Capabilities.GatewayConnectorConfig]
+# ChainIDForNodeKey is the ChainID of the network
+ChainIDForNodeKey = '0xfd29Dd9C980D715a64dace97F7A2AB98bcaE0fed' # Default
+# NodeAddress is Workflow Node address


"NodeAddress is the address of the desired private key to be used for authentication with Gateway nodes"

bolekk · 2024-09-05T14:13:54Z

core/config/docs/core.toml

+DonID = 'example_don' # Example
+# WsHandshakeTimeoutMillis is Websocket handshake timeout
+WsHandshakeTimeoutMillis = 1000 # Example
+# AuthMinChallengeLen is the minimum number of bytes in Authentication


"... number of bytes in authentication challenge payload"

bolekk · 2024-09-05T14:14:15Z

core/config/docs/core.toml

+# ID of the Gateway
+ID = 'example_gateway' # Example
+# URL of the Gateway
+URL = 'ws://localhost:8081/node' # Example


change "ws" to "wss"

bolekk · 2024-09-05T14:15:01Z

core/config/toml/types.go

+		r.DonID = f.DonID
+	}
+
+	// TODO: verify this copy by reference is ok, or does array need to be copied by value


resolve this TODO please - most likely reference is OK (@cedric-cordenier ?)

I commented this on the previous PR: it's fine to use the reference here, but the interface implementation (the getter) should do a deep copy before returning it. This will prevent bugs with a caller modifying the underlying array.

bolekk · 2024-09-05T14:15:21Z

core/config/toml/types.go

+	Peering                P2P                    `toml:",omitempty"`
+	Dispatcher             Dispatcher             `toml:",omitempty"`
+	ExternalRegistry       ExternalRegistry       `toml:",omitempty"`
+	GatewayConnectorConfig GatewayConnectorConfig `toml:", omitempty"`


nit: unnecessary space before "omitempty"

bolekk · 2024-09-05T14:17:30Z

testdata/scripts/node/db/help.txtar

@@ -9,15 +9,12 @@ USAGE:
   chainlink node db command [command options] [arguments...]

 COMMANDS:
-   reset             Drop, create and migrate database. Useful for setting up the database in order to run tests or resetting the dev database. WARNING: This will ERASE ALL DATA for the specified database, referred to by CL_DATABASE_URL env variable or by the Database.URL field in a secrets TOML config.


not sure why this changed...? try reverting maybe?

This seems to alternate back and forth. I have no idea how or why tbh.

There are more commands available in dev mode. If you just use go test then you won't have this problem.

cedric-cordenier · 2024-09-05T14:47:13Z

core/services/chainlink/config_capabilities.go

+}
+
+func (c *gatewayConnectorConfig) Gateways() []config.ConnectorGatewayConfig {
+	t := []config.ConnectorGatewayConfig{}


@DavidOrchard Does this work? I think it should panic in practice because you're initializing a zero-length array and then adding elements to indexes that don't exist.

You can get around this by constructing an array of the right length:

t := make([]config.ConnectorGatewayConfig, len(c.c.Gateways))

I honestly don't know if it works or not, I do know the tests seem to pass. But I don't know if there's a test that copies the structure.

Please update the config-full.toml test files with example data for all fields so that we have coverage

changes requested were made

jmank88 · 2024-09-05T17:39:46Z

core/config/toml/types.go

+	NodeAddress               *string
+	DonID                     *string
+	Gateways                  []ConnectorGateway
+	WsHandshakeTimeoutMillis  *uint32


I think WS should be upper case according to Go naming rules:

Suggested change

WsHandshakeTimeoutMillis *uint32

WSHandshakeTimeoutMillis *uint32

bolekk · 2024-09-05T19:49:18Z

testdata/scripts/node/db/help.txtar

We probably don't need this extra line?

fwiw, that's what was generated.

cl-sonarqube-production · 2024-09-05T19:57:52Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
51.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

DavidOrchard added wip DO_NOT_MERGE labels Sep 3, 2024

add gateway connector capability config

fc3bc6b

DavidOrchard force-pushed the feature/CAPPL-19-connector-service-2 branch from 50764ea to fc3bc6b Compare September 3, 2024 23:11

use WsHandshakeTimeoutMillis

6bcb60d

DavidOrchard force-pushed the feature/CAPPL-19-connector-service-2 branch 8 times, most recently from cb8b8d4 to d78d6fb Compare September 4, 2024 23:02

fixes, add docs and testdata

80b5f62

DavidOrchard force-pushed the feature/CAPPL-19-connector-service-2 branch from d78d6fb to 80b5f62 Compare September 5, 2024 01:28

bolekk reviewed Sep 5, 2024

View reviewed changes

PR comments: add chainid and collapse workflowconnectorconfig

518b858

DavidOrchard force-pushed the feature/CAPPL-19-connector-service-2 branch from ca54c27 to 518b858 Compare September 5, 2024 03:47

DavidOrchard marked this pull request as ready for review September 5, 2024 05:03

DavidOrchard requested a review from a team as a code owner September 5, 2024 05:03

DavidOrchard requested review from agparadiso, bolekk, cedric-cordenier and ettec and removed request for a team September 5, 2024 05:03

jmank88 previously requested changes Sep 5, 2024

View reviewed changes

bolekk reviewed Sep 5, 2024

View reviewed changes

cedric-cordenier reviewed Sep 5, 2024

View reviewed changes

DavidOrchard force-pushed the feature/CAPPL-19-connector-service-2 branch 2 times, most recently from 6c1f41b to 52bbc8a Compare September 5, 2024 17:02

DavidOrchard force-pushed the feature/CAPPL-19-connector-service-2 branch from 52bbc8a to d1a771a Compare September 5, 2024 17:19

DavidOrchard removed wip DO_NOT_MERGE labels Sep 5, 2024

PR comments, remove Config from names etc

6f0e185

DavidOrchard force-pushed the feature/CAPPL-19-connector-service-2 branch from d1a771a to 6f0e185 Compare September 5, 2024 17:32

jmank88 reviewed Sep 5, 2024

View reviewed changes

DavidOrchard added 2 commits September 5, 2024 12:28

use WSHandshake.

b07269a

Merge branch 'develop' into feature/CAPPL-19-connector-service-2

21b241c

DavidOrchard force-pushed the feature/CAPPL-19-connector-service-2 branch from 37e51f8 to 21b241c Compare September 5, 2024 19:31

use correct array initialization

9e6cc1c

DavidOrchard requested review from bolekk, jmank88 and cedric-cordenier September 5, 2024 19:41

bolekk approved these changes Sep 5, 2024

View reviewed changes

jmank88 approved these changes Sep 5, 2024

View reviewed changes

jmank88 added this pull request to the merge queue Sep 5, 2024

Merged via the queue into develop with commit b1c59dd Sep 5, 2024
134 of 135 checks passed

jmank88 deleted the feature/CAPPL-19-connector-service-2 branch September 5, 2024 23:18

github-actions bot mentioned this pull request Sep 9, 2024

[DO NOT MERGE] Changeset Release Preview - v2.19.0 #13148

Draft

github-actions bot mentioned this pull request Sep 20, 2024

[DO NOT MERGE] Changeset Release Preview - v2.17.0 YoutacRandS-VA/chainlink#23

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add gateway connector capability config #14325

add gateway connector capability config #14325

DavidOrchard commented Sep 3, 2024 •

edited

Loading

bolekk Sep 4, 2024

bolekk Sep 5, 2024

DavidOrchard Sep 5, 2024

jmank88 Sep 5, 2024

jmank88 Sep 5, 2024

jmank88 Sep 5, 2024

bolekk Sep 5, 2024

jmank88 Sep 5, 2024

jmank88 Sep 5, 2024

DavidOrchard Sep 5, 2024

jmank88 Sep 5, 2024

bolekk left a comment

bolekk Sep 5, 2024

jmank88 Sep 5, 2024

bolekk Sep 5, 2024

bolekk Sep 5, 2024

bolekk Sep 5, 2024

bolekk Sep 5, 2024

bolekk Sep 5, 2024

bolekk Sep 5, 2024

cedric-cordenier Sep 5, 2024

bolekk Sep 5, 2024

bolekk Sep 5, 2024

DavidOrchard Sep 5, 2024

jmank88 Sep 5, 2024

cedric-cordenier Sep 5, 2024

DavidOrchard Sep 5, 2024

jmank88 Sep 5, 2024

jmank88 Sep 5, 2024

bolekk Sep 5, 2024

DavidOrchard Sep 5, 2024

cl-sonarqube-production bot commented Sep 5, 2024

	ChainIDForNodeKey = '0xfd29Dd9C980D715a64dace97F7A2AB98bcaE0fed' # Default
	ChainIDForNodeKey = '11155111' # Example

	WsHandshakeTimeoutMillis *uint32
	WSHandshakeTimeoutMillis *uint32

add gateway connector capability config #14325

add gateway connector capability config #14325

Conversation

DavidOrchard commented Sep 3, 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

Choose a reason for hiding this comment

bolekk left a 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

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

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Sep 5, 2024

Quality Gate passed

DavidOrchard commented Sep 3, 2024 •

edited

Loading