From e951bdba202b2371168d89f9841607b93c38e920 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Thu, 5 Sep 2024 16:24:11 -0700 Subject: [PATCH 01/25] service wrapper --- .../gateway_connector/connector_handler.go | 65 +++++++++ .../gateway_connector/service_wrapper.go | 128 ++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 core/capabilities/gateway_connector/connector_handler.go create mode 100644 core/capabilities/gateway_connector/service_wrapper.go diff --git a/core/capabilities/gateway_connector/connector_handler.go b/core/capabilities/gateway_connector/connector_handler.go new file mode 100644 index 00000000000..516174bacf9 --- /dev/null +++ b/core/capabilities/gateway_connector/connector_handler.go @@ -0,0 +1,65 @@ +package gateway_connector + +import ( + "context" + "crypto/ecdsa" + + "github.com/smartcontractkit/chainlink-common/pkg/services" + + "github.com/smartcontractkit/chainlink/v2/core/config" + "github.com/smartcontractkit/chainlink/v2/core/logger" + "github.com/smartcontractkit/chainlink/v2/core/services/gateway/api" + "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common" + "github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector" +) + +type workflowConnectorHandler struct { + services.StateMachine + + connector connector.GatewayConnector + signerKey *ecdsa.PrivateKey + nodeAddress string + lggr logger.Logger +} + +type Response struct { + Success bool `json:"success"` + ErrorMessage string `json:"error_message,omitempty"` +} + +var ( + _ connector.Signer = &workflowConnectorHandler{} + // _ connector.GatewayConnectorHandler = &workflowConnectorHandler{} + // _ services.Service = &workflowConnectorHandler{} +) + +// NOTE: the name is a little misleading - it's a generic handler to support all communication between Gateways and Workflow DONs +// We might want to come up with a cleaner split between capabilities. +func NewWorkflowConnectorHandler(config *config.GatewayConnector, signerKey *ecdsa.PrivateKey, lggr logger.Logger) (*workflowConnectorHandler, error) { + return &workflowConnectorHandler{ + nodeAddress: (*config).NodeAddress(), + signerKey: signerKey, + lggr: lggr.Named("WorkflowConnectorHandler"), + }, nil +} + +func (h *workflowConnectorHandler) Sign(data ...[]byte) ([]byte, error) { + return common.SignData(h.signerKey, data...) +} + +func (h *workflowConnectorHandler) HandleGatewayMessage(ctx context.Context, gatewayId string, msg *api.Message) { +} +func (h *workflowConnectorHandler) Start(ctx context.Context) error { + return h.StartOnce("WorkflowConnectorHandler", func() error { + return nil + }) +} +func (h *workflowConnectorHandler) Close() error { + return h.StopOnce("WorkflowConnectorHandler", func() (err error) { + return nil + }) +} + +func (h *workflowConnectorHandler) SetConnector(connector connector.GatewayConnector) { + h.connector = connector +} diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go new file mode 100644 index 00000000000..93a11691d80 --- /dev/null +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -0,0 +1,128 @@ +package gateway_connector + +import ( + "context" + "errors" + "math/big" + "reflect" + "slices" + + "github.com/ethereum/go-ethereum/common" + "github.com/jonboulle/clockwork" + + "github.com/smartcontractkit/chainlink-common/pkg/services" + "github.com/smartcontractkit/chainlink/v2/core/config" + gwConfig "github.com/smartcontractkit/chainlink/v2/core/config" + "github.com/smartcontractkit/chainlink/v2/core/logger" + "github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector" + "github.com/smartcontractkit/chainlink/v2/core/services/gateway/network" + "github.com/smartcontractkit/chainlink/v2/core/services/keystore" + "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey" +) + +type serviceWrapper struct { + services.StateMachine + + config *config.GatewayConnector + keystore keystore.Eth + connector connector.GatewayConnector + lggr logger.Logger +} + +func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { + r := connector.ConnectorConfig{} + if f.NodeAddress != nil { + r.NodeAddress = f.NodeAddress() + } + + if f.DonID != nil { + r.DonId = f.DonID() + } + + if f.Gateways != nil { + r.Gateways = make([]connector.ConnectorGatewayConfig, len(r.Gateways)) + for index, element := range f.Gateways() { + r.Gateways[index] = connector.ConnectorGatewayConfig{Id: element.ID(), URL: element.URL()} + } + } + + if !reflect.ValueOf(f.WSHandshakeTimeoutMillis).IsZero() { + r.WsClientConfig = network.WebSocketClientConfig{HandshakeTimeoutMillis: f.WSHandshakeTimeoutMillis()} + } + + if f.AuthMinChallengeLen != nil { + r.AuthMinChallengeLen = f.AuthMinChallengeLen() + } + + if f.AuthTimestampToleranceSec != nil { + r.AuthTimestampToleranceSec = f.AuthTimestampToleranceSec() + } + return r +} + +// NOTE: this wrapper is needed to make sure that our services are started after Keystore. +func NewGatewayConnectorServiceWrapper(config *gwConfig.GatewayConnector, keystore keystore.Eth, lggr logger.Logger) *serviceWrapper { + return &serviceWrapper{ + config: config, + keystore: keystore, + lggr: lggr, + } +} + +func (e *serviceWrapper) Start(ctx context.Context) error { + return e.StartOnce("GatewayConnectorServiceWrapper", func() error { + conf := *e.config + e.lggr.Infow("Starting GatewayConnectorServiceWrapper", "chainID", conf.ChainIDForNodeKey()) + chainId, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) + enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainId) + if err != nil { + return err + } + if len(enabledKeys) == 0 { + return errors.New("no available keys found") + } + configuredNodeAddress := common.HexToAddress(conf.NodeAddress()) + idx := slices.IndexFunc(enabledKeys, func(key ethkey.KeyV2) bool { return key.Address == configuredNodeAddress }) + if idx == -1 { + return errors.New("key for configured node address not found") + } + signerKey := enabledKeys[idx].ToEcdsaPrivKey() + if enabledKeys[idx].ID() != conf.NodeAddress() { + return errors.New("node address mismatch") + } + // cannot assign to conf.NodeAddress (neither addressable nor a map index expression + // I think I'm still confused about mixing interfaces and structures, and how to use a setter on an interface. + conf.NodeAddress = enabledKeys[idx].ID() + + handler, err := NewWorkflowConnectorHandler(e.config, signerKey, e.lggr) + if err != nil { + return err + } + translated := translateConfigs(conf) + e.connector, err = connector.NewGatewayConnector(&translated, handler, handler, clockwork.NewRealClock(), e.lggr) + if err != nil { + return err + } + handler.SetConnector(e.connector) + + return e.connector.Start(ctx) + }) +} + +func (e *serviceWrapper) Close() error { + return e.StopOnce("WorkflowConnectorHandler", func() (err error) { + return e.connector.Close() + }) +} + +func (e *serviceWrapper) Ready() error { + return nil +} + +func (e *serviceWrapper) HealthReport() map[string]error { + return nil +} + +func (e *serviceWrapper) Name() string { + return "GatewayConnectorServiceWrapper" +} From d4ce0dbc389a1d4cfda895b044d83eb88b6b13af Mon Sep 17 00:00:00 2001 From: David Orchard Date: Fri, 6 Sep 2024 11:33:58 -0700 Subject: [PATCH 02/25] PR updates: simplify connector into signer and consolidate files --- .../gateway_connector/connector_handler.go | 65 ------------------- .../gateway_connector/service_wrapper.go | 39 ++++++++--- 2 files changed, 31 insertions(+), 73 deletions(-) delete mode 100644 core/capabilities/gateway_connector/connector_handler.go diff --git a/core/capabilities/gateway_connector/connector_handler.go b/core/capabilities/gateway_connector/connector_handler.go deleted file mode 100644 index 516174bacf9..00000000000 --- a/core/capabilities/gateway_connector/connector_handler.go +++ /dev/null @@ -1,65 +0,0 @@ -package gateway_connector - -import ( - "context" - "crypto/ecdsa" - - "github.com/smartcontractkit/chainlink-common/pkg/services" - - "github.com/smartcontractkit/chainlink/v2/core/config" - "github.com/smartcontractkit/chainlink/v2/core/logger" - "github.com/smartcontractkit/chainlink/v2/core/services/gateway/api" - "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common" - "github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector" -) - -type workflowConnectorHandler struct { - services.StateMachine - - connector connector.GatewayConnector - signerKey *ecdsa.PrivateKey - nodeAddress string - lggr logger.Logger -} - -type Response struct { - Success bool `json:"success"` - ErrorMessage string `json:"error_message,omitempty"` -} - -var ( - _ connector.Signer = &workflowConnectorHandler{} - // _ connector.GatewayConnectorHandler = &workflowConnectorHandler{} - // _ services.Service = &workflowConnectorHandler{} -) - -// NOTE: the name is a little misleading - it's a generic handler to support all communication between Gateways and Workflow DONs -// We might want to come up with a cleaner split between capabilities. -func NewWorkflowConnectorHandler(config *config.GatewayConnector, signerKey *ecdsa.PrivateKey, lggr logger.Logger) (*workflowConnectorHandler, error) { - return &workflowConnectorHandler{ - nodeAddress: (*config).NodeAddress(), - signerKey: signerKey, - lggr: lggr.Named("WorkflowConnectorHandler"), - }, nil -} - -func (h *workflowConnectorHandler) Sign(data ...[]byte) ([]byte, error) { - return common.SignData(h.signerKey, data...) -} - -func (h *workflowConnectorHandler) HandleGatewayMessage(ctx context.Context, gatewayId string, msg *api.Message) { -} -func (h *workflowConnectorHandler) Start(ctx context.Context) error { - return h.StartOnce("WorkflowConnectorHandler", func() error { - return nil - }) -} -func (h *workflowConnectorHandler) Close() error { - return h.StopOnce("WorkflowConnectorHandler", func() (err error) { - return nil - }) -} - -func (h *workflowConnectorHandler) SetConnector(connector connector.GatewayConnector) { - h.connector = connector -} diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 93a11691d80..cd11d9e509e 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -2,6 +2,7 @@ package gateway_connector import ( "context" + "crypto/ecdsa" "errors" "math/big" "reflect" @@ -14,6 +15,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/config" gwConfig "github.com/smartcontractkit/chainlink/v2/core/config" "github.com/smartcontractkit/chainlink/v2/core/logger" + gwCommon "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/network" "github.com/smartcontractkit/chainlink/v2/core/services/keystore" @@ -29,6 +31,31 @@ type serviceWrapper struct { lggr logger.Logger } +type workflowConnectorSigner struct { + services.StateMachine + + connector connector.GatewayConnector + signerKey *ecdsa.PrivateKey + nodeAddress string + lggr logger.Logger +} + +var ( + _ connector.Signer = &workflowConnectorSigner{} +) + +func NewWorkflowConnectorSigner(config *config.GatewayConnector, signerKey *ecdsa.PrivateKey, lggr logger.Logger) (*workflowConnectorSigner, error) { + return &workflowConnectorSigner{ + nodeAddress: (*config).NodeAddress(), + signerKey: signerKey, + lggr: lggr.Named("WorkflowConnectorSigner"), + }, nil +} + +func (h *workflowConnectorSigner) Sign(data ...[]byte) ([]byte, error) { + return gwCommon.SignData(h.signerKey, data...) +} + func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { r := connector.ConnectorConfig{} if f.NodeAddress != nil { @@ -90,27 +117,23 @@ func (e *serviceWrapper) Start(ctx context.Context) error { if enabledKeys[idx].ID() != conf.NodeAddress() { return errors.New("node address mismatch") } - // cannot assign to conf.NodeAddress (neither addressable nor a map index expression - // I think I'm still confused about mixing interfaces and structures, and how to use a setter on an interface. - conf.NodeAddress = enabledKeys[idx].ID() - handler, err := NewWorkflowConnectorHandler(e.config, signerKey, e.lggr) + signer, err := NewWorkflowConnectorSigner(e.config, signerKey, e.lggr) if err != nil { return err } translated := translateConfigs(conf) - e.connector, err = connector.NewGatewayConnector(&translated, handler, handler, clockwork.NewRealClock(), e.lggr) + // cannot use signer (variable of type *workflowConnectorSigner) as connector.GatewayConnectorHandler value in argument to connector.NewGatewayConnector: *workflowConnectorSigner does not implement connector.GatewayConnectorHandler (missing method Close)compilerInvalidIfaceAssign + e.connector, err = connector.NewGatewayConnector(&translated, signer, signer, clockwork.NewRealClock(), e.lggr) if err != nil { return err } - handler.SetConnector(e.connector) - return e.connector.Start(ctx) }) } func (e *serviceWrapper) Close() error { - return e.StopOnce("WorkflowConnectorHandler", func() (err error) { + return e.StopOnce("GatewayConnectorServiceWrapper", func() (err error) { return e.connector.Close() }) } From 7784141d8ef52a6055f8f0453794110e95a3984d Mon Sep 17 00:00:00 2001 From: David Orchard Date: Fri, 6 Sep 2024 12:59:13 -0700 Subject: [PATCH 03/25] keep signer as handler with empty methods for now --- .../gateway_connector/service_wrapper.go | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index cd11d9e509e..b67014e1836 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -15,6 +15,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/config" gwConfig "github.com/smartcontractkit/chainlink/v2/core/config" "github.com/smartcontractkit/chainlink/v2/core/logger" + "github.com/smartcontractkit/chainlink/v2/core/services/gateway/api" gwCommon "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/network" @@ -56,13 +57,30 @@ func (h *workflowConnectorSigner) Sign(data ...[]byte) ([]byte, error) { return gwCommon.SignData(h.signerKey, data...) } +func (h *workflowConnectorSigner) HandleGatewayMessage(ctx context.Context, gatewayId string, msg *api.Message) { +} +func (h *workflowConnectorSigner) Start(ctx context.Context) error { + return h.StartOnce("WorkflowConnectorHandler", func() error { + return nil + }) +} +func (h *workflowConnectorSigner) Close() error { + return h.StopOnce("WorkflowConnectorHandler", func() (err error) { + return nil + }) +} + +func (h *workflowConnectorSigner) SetConnector(connector connector.GatewayConnector) { + h.connector = connector +} + func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { r := connector.ConnectorConfig{} - if f.NodeAddress != nil { + if f.NodeAddress() != "" { r.NodeAddress = f.NodeAddress() } - if f.DonID != nil { + if f.DonID() != "" { r.DonId = f.DonID() } @@ -77,13 +95,9 @@ func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { r.WsClientConfig = network.WebSocketClientConfig{HandshakeTimeoutMillis: f.WSHandshakeTimeoutMillis()} } - if f.AuthMinChallengeLen != nil { - r.AuthMinChallengeLen = f.AuthMinChallengeLen() - } - - if f.AuthTimestampToleranceSec != nil { - r.AuthTimestampToleranceSec = f.AuthTimestampToleranceSec() - } + // 0 are valid values + r.AuthMinChallengeLen = f.AuthMinChallengeLen() + r.AuthTimestampToleranceSec = f.AuthTimestampToleranceSec() return r } @@ -123,7 +137,6 @@ func (e *serviceWrapper) Start(ctx context.Context) error { return err } translated := translateConfigs(conf) - // cannot use signer (variable of type *workflowConnectorSigner) as connector.GatewayConnectorHandler value in argument to connector.NewGatewayConnector: *workflowConnectorSigner does not implement connector.GatewayConnectorHandler (missing method Close)compilerInvalidIfaceAssign e.connector, err = connector.NewGatewayConnector(&translated, signer, signer, clockwork.NewRealClock(), e.lggr) if err != nil { return err From ac908dde8e1df8b8d51baa0944ab7ed0b1f27105 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Fri, 6 Sep 2024 13:30:14 -0700 Subject: [PATCH 04/25] lint --- .../gateway_connector/service_wrapper.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index b67014e1836..0a9685a5f4d 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -1,4 +1,4 @@ -package gateway_connector +package gatewayConnector import ( "context" @@ -57,7 +57,7 @@ func (h *workflowConnectorSigner) Sign(data ...[]byte) ([]byte, error) { return gwCommon.SignData(h.signerKey, data...) } -func (h *workflowConnectorSigner) HandleGatewayMessage(ctx context.Context, gatewayId string, msg *api.Message) { +func (h *workflowConnectorSigner) HandleGatewayMessage(ctx context.Context, gatewayID string, msg *api.Message) { } func (h *workflowConnectorSigner) Start(ctx context.Context) error { return h.StartOnce("WorkflowConnectorHandler", func() error { @@ -84,8 +84,8 @@ func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { r.DonId = f.DonID() } - if f.Gateways != nil { - r.Gateways = make([]connector.ConnectorGatewayConfig, len(r.Gateways)) + if len(f.Gateways()) != 0 { + r.Gateways = make([]connector.ConnectorGatewayConfig, len(f.Gateways())) for index, element := range f.Gateways() { r.Gateways[index] = connector.ConnectorGatewayConfig{Id: element.ID(), URL: element.URL()} } @@ -114,8 +114,8 @@ func (e *serviceWrapper) Start(ctx context.Context) error { return e.StartOnce("GatewayConnectorServiceWrapper", func() error { conf := *e.config e.lggr.Infow("Starting GatewayConnectorServiceWrapper", "chainID", conf.ChainIDForNodeKey()) - chainId, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) - enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainId) + chainID, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) + enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainID) if err != nil { return err } From cb3088df4b4c04ada8016178301caacac0db0dcd Mon Sep 17 00:00:00 2001 From: David Orchard Date: Fri, 6 Sep 2024 13:57:58 -0700 Subject: [PATCH 05/25] PR comments except for unit test --- .../gateway_connector/service_wrapper.go | 44 +++++++------------ 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 0a9685a5f4d..faebcc42b07 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -5,7 +5,6 @@ import ( "crypto/ecdsa" "errors" "math/big" - "reflect" "slices" "github.com/ethereum/go-ethereum/common" @@ -32,7 +31,7 @@ type serviceWrapper struct { lggr logger.Logger } -type workflowConnectorSigner struct { +type connectorSigner struct { services.StateMachine connector connector.GatewayConnector @@ -41,48 +40,41 @@ type workflowConnectorSigner struct { lggr logger.Logger } -var ( - _ connector.Signer = &workflowConnectorSigner{} -) +var _ connector.Signer = &connectorSigner{} -func NewWorkflowConnectorSigner(config *config.GatewayConnector, signerKey *ecdsa.PrivateKey, lggr logger.Logger) (*workflowConnectorSigner, error) { - return &workflowConnectorSigner{ +func NewConnectorSigner(config *config.GatewayConnector, signerKey *ecdsa.PrivateKey, lggr logger.Logger) (*connectorSigner, error) { + return &connectorSigner{ nodeAddress: (*config).NodeAddress(), signerKey: signerKey, - lggr: lggr.Named("WorkflowConnectorSigner"), + lggr: lggr.Named("ConnectorSigner"), }, nil } -func (h *workflowConnectorSigner) Sign(data ...[]byte) ([]byte, error) { +func (h *connectorSigner) Sign(data ...[]byte) ([]byte, error) { return gwCommon.SignData(h.signerKey, data...) } -func (h *workflowConnectorSigner) HandleGatewayMessage(ctx context.Context, gatewayID string, msg *api.Message) { +func (h *connectorSigner) HandleGatewayMessage(ctx context.Context, gatewayID string, msg *api.Message) { } -func (h *workflowConnectorSigner) Start(ctx context.Context) error { - return h.StartOnce("WorkflowConnectorHandler", func() error { +func (h *connectorSigner) Start(ctx context.Context) error { + return h.StartOnce("ConnectorSigner", func() error { return nil }) } -func (h *workflowConnectorSigner) Close() error { - return h.StopOnce("WorkflowConnectorHandler", func() (err error) { +func (h *connectorSigner) Close() error { + return h.StopOnce("ConnectorSigner", func() (err error) { return nil }) } -func (h *workflowConnectorSigner) SetConnector(connector connector.GatewayConnector) { +func (h *connectorSigner) SetConnector(connector connector.GatewayConnector) { h.connector = connector } func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { r := connector.ConnectorConfig{} - if f.NodeAddress() != "" { - r.NodeAddress = f.NodeAddress() - } - - if f.DonID() != "" { - r.DonId = f.DonID() - } + r.NodeAddress = f.NodeAddress() + r.DonId = f.DonID() if len(f.Gateways()) != 0 { r.Gateways = make([]connector.ConnectorGatewayConfig, len(f.Gateways())) @@ -91,11 +83,7 @@ func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { } } - if !reflect.ValueOf(f.WSHandshakeTimeoutMillis).IsZero() { - r.WsClientConfig = network.WebSocketClientConfig{HandshakeTimeoutMillis: f.WSHandshakeTimeoutMillis()} - } - - // 0 are valid values + r.WsClientConfig = network.WebSocketClientConfig{HandshakeTimeoutMillis: f.WSHandshakeTimeoutMillis()} r.AuthMinChallengeLen = f.AuthMinChallengeLen() r.AuthTimestampToleranceSec = f.AuthTimestampToleranceSec() return r @@ -132,7 +120,7 @@ func (e *serviceWrapper) Start(ctx context.Context) error { return errors.New("node address mismatch") } - signer, err := NewWorkflowConnectorSigner(e.config, signerKey, e.lggr) + signer, err := NewConnectorSigner(e.config, signerKey, e.lggr) if err != nil { return err } From b5bafecfbec7c887aa2435e1d035c3011bee76d8 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Fri, 6 Sep 2024 14:15:27 -0700 Subject: [PATCH 06/25] package lint --- core/capabilities/gateway_connector/service_wrapper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index faebcc42b07..6c436dafcda 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -1,4 +1,4 @@ -package gatewayConnector +package gatewayconnector import ( "context" From 79d6e4cd6bc6cc78be1cf3e206f60e424196d450 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Mon, 9 Sep 2024 15:19:12 -0700 Subject: [PATCH 07/25] unit test v1 --- .../gateway_connector/service_wrapper.go | 3 +- .../gateway_connector/service_wrapper_test.go | 66 +++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 core/capabilities/gateway_connector/service_wrapper_test.go diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 6c436dafcda..0014a1e265b 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -12,7 +12,6 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/services" "github.com/smartcontractkit/chainlink/v2/core/config" - gwConfig "github.com/smartcontractkit/chainlink/v2/core/config" "github.com/smartcontractkit/chainlink/v2/core/logger" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/api" gwCommon "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common" @@ -90,7 +89,7 @@ func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { } // NOTE: this wrapper is needed to make sure that our services are started after Keystore. -func NewGatewayConnectorServiceWrapper(config *gwConfig.GatewayConnector, keystore keystore.Eth, lggr logger.Logger) *serviceWrapper { +func NewGatewayConnectorServiceWrapper(config *config.GatewayConnector, keystore keystore.Eth, lggr logger.Logger) *serviceWrapper { return &serviceWrapper{ config: config, keystore: keystore, diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go new file mode 100644 index 00000000000..2e7b2a38ca2 --- /dev/null +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -0,0 +1,66 @@ +package gatewayconnector + +import ( + "testing" + + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/logger" + chainlink "github.com/smartcontractkit/chainlink/v2/core/services/chainlink/config" + "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey" + ksmocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/test-go/testify/mock" +) + +// Unit test that creates the ServiceWrapper object and then calls Start() can Close() on it. +// Take inspiration from functions/plugin_test.go and functions/connector_handler_test.go on how to mock the dependencies. +// +// Test valid NodeAddress and an invalid one (i.e. key doesn't exit). + +func TestGatewayConnectorServiceWrapper(t *testing.T) { + t.Parallel() + + logger := logger.TestLogger(t) + _, addr := testutils.NewPrivateKeyAndAddress(t) + + config, err := chainlink.GeneralConfigOpts{}.New() + // I don't think this is the right way to get the GatewayConnector in. + config.Capabilities().GatewayConnector() = chainlink.GatewayConnector{ + ChainIDForNodeKey: "1", + NodeAddress: addr.Hex(), + DonId: "5", + WSHandshakeTimeoutMillis: 500, + AuthMinChallengeLen: 0, + AuthTimestampToleranceSec: 10, + Gateways: []chainlink.ConnectorGateway{{ID: ptr("example_gateway"), URL: ptr("wss://localhost:8081/node")}}, + } + ethKeystore := ksmocks.NewEth(t) + ethKeystore.On("EnabledKeysForChain", mock.Anything).Return([]ethkey.KeyV2{{Address: addr}}) + + handler := NewGatewayConnectorServiceWrapper(config, ethKeystore, logger) + require.NoError(t, err) + + t.Cleanup(func() { + assert.NoError(t, handler.Close()) + }) + + t.Run("Start & Stop Success", func(t *testing.T) { + ctx := testutils.Context(t) + + err := handler.Start(ctx) + require.NoError(t, err) + err = handler.Close() + require.NoError(t, err) + }) + + t.Run("Start Error", func(t *testing.T) { + // Question, what's the best practices when testing a different configuration, is it 2 configs and then + // 2 handlers, or does the test do the initialization as in copy from line 27 to 41 into here? + ctx := testutils.Context(t) + err := handler.Start(ctx) + require.NoError(t, err) + }) +} + +func ptr[T any](t T) *T { return &t } From a2d5ab6ba7b6055641c3b556e17de545609c513e Mon Sep 17 00:00:00 2001 From: David Orchard Date: Mon, 9 Sep 2024 15:55:18 -0700 Subject: [PATCH 08/25] fix unit tests --- .../gateway_connector/service_wrapper_test.go | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index 2e7b2a38ca2..f4358edc4d7 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -5,9 +5,10 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" "github.com/smartcontractkit/chainlink/v2/core/logger" - chainlink "github.com/smartcontractkit/chainlink/v2/core/services/chainlink/config" + chainlink "github.com/smartcontractkit/chainlink/v2/core/services/chainlink" "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey" ksmocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" + "github.com/smartcontractkit/chainlink/v2/core/toml" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/test-go/testify/mock" @@ -24,17 +25,17 @@ func TestGatewayConnectorServiceWrapper(t *testing.T) { logger := logger.TestLogger(t) _, addr := testutils.NewPrivateKeyAndAddress(t) - config, err := chainlink.GeneralConfigOpts{}.New() - // I don't think this is the right way to get the GatewayConnector in. - config.Capabilities().GatewayConnector() = chainlink.GatewayConnector{ - ChainIDForNodeKey: "1", - NodeAddress: addr.Hex(), - DonId: "5", - WSHandshakeTimeoutMillis: 500, - AuthMinChallengeLen: 0, - AuthTimestampToleranceSec: 10, - Gateways: []chainlink.ConnectorGateway{{ID: ptr("example_gateway"), URL: ptr("wss://localhost:8081/node")}}, - } + config, err := chainlink.GeneralConfigOpts{ + Config: chainlink.Config{ + Core: toml.Core{ + Capabilities: toml.Capabilities{ + GatewayConnector: toml.GatewayConnector{ + // all the fields here + }, + }, + }, + }, + }.New() ethKeystore := ksmocks.NewEth(t) ethKeystore.On("EnabledKeysForChain", mock.Anything).Return([]ethkey.KeyV2{{Address: addr}}) From ef63be6fa3380a7f19766c4bb6c081a269b32187 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Mon, 9 Sep 2024 16:36:20 -0700 Subject: [PATCH 09/25] PR Review: fix config, separate tests --- .../gateway_connector/service_wrapper.go | 2 +- .../gateway_connector/service_wrapper_test.go | 63 ++++++++++++++----- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 0014a1e265b..c7cd6e0718b 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -124,7 +124,7 @@ func (e *serviceWrapper) Start(ctx context.Context) error { return err } translated := translateConfigs(conf) - e.connector, err = connector.NewGatewayConnector(&translated, signer, signer, clockwork.NewRealClock(), e.lggr) + e.connector, err = connector.NewGatewayConnector(&translated, signer, clockwork.NewRealClock(), e.lggr) if err != nil { return err } diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index f4358edc4d7..3e62ee3f7f8 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -3,17 +3,38 @@ package gatewayconnector import ( "testing" + "github.com/ethereum/go-ethereum/common" + "github.com/smartcontractkit/chainlink/v2/core/config/toml" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" "github.com/smartcontractkit/chainlink/v2/core/logger" chainlink "github.com/smartcontractkit/chainlink/v2/core/services/chainlink" "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey" ksmocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" - "github.com/smartcontractkit/chainlink/v2/core/toml" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/test-go/testify/mock" ) +func generateConfig(addr common.Address) (chainlink.GeneralConfig, error) { + return chainlink.GeneralConfigOpts{ + Config: chainlink.Config{ + Core: toml.Core{ + Capabilities: toml.Capabilities{ + GatewayConnector: toml.GatewayConnector{ + ChainIDForNodeKey: ptr("1"), + NodeAddress: ptr(addr.Hex()), + DonID: ptr("5"), + WSHandshakeTimeoutMillis: ptr[uint32](100), + AuthMinChallengeLen: ptr[int](0), + AuthTimestampToleranceSec: ptr[uint32](10), + Gateways: []toml.ConnectorGateway{{ID: ptr("example_gateway"), URL: ptr("wss://localhost:8081/node")}}, + }, + }, + }, + }, + }.New() +} + // Unit test that creates the ServiceWrapper object and then calls Start() can Close() on it. // Take inspiration from functions/plugin_test.go and functions/connector_handler_test.go on how to mock the dependencies. // @@ -25,21 +46,12 @@ func TestGatewayConnectorServiceWrapper(t *testing.T) { logger := logger.TestLogger(t) _, addr := testutils.NewPrivateKeyAndAddress(t) - config, err := chainlink.GeneralConfigOpts{ - Config: chainlink.Config{ - Core: toml.Core{ - Capabilities: toml.Capabilities{ - GatewayConnector: toml.GatewayConnector{ - // all the fields here - }, - }, - }, - }, - }.New() + config, err := generateConfig(addr) ethKeystore := ksmocks.NewEth(t) ethKeystore.On("EnabledKeysForChain", mock.Anything).Return([]ethkey.KeyV2{{Address: addr}}) - handler := NewGatewayConnectorServiceWrapper(config, ethKeystore, logger) + gc := config.Capabilities().GatewayConnector() + handler := NewGatewayConnectorServiceWrapper(&gc, ethKeystore, logger) require.NoError(t, err) t.Cleanup(func() { @@ -54,13 +66,32 @@ func TestGatewayConnectorServiceWrapper(t *testing.T) { err = handler.Close() require.NoError(t, err) }) +} + +func TestGatewayConnectorServiceWrapperConfigError(t *testing.T) { + t.Parallel() + + logger := logger.TestLogger(t) + _, addr := testutils.NewPrivateKeyAndAddress(t) + + config, err := generateConfig(addr) + ethKeystore := ksmocks.NewEth(t) + _, addr2 := testutils.NewPrivateKeyAndAddress(t) + + ethKeystore.On("EnabledKeysForChain", mock.Anything).Return([]ethkey.KeyV2{{Address: addr2}}) + + gc := config.Capabilities().GatewayConnector() + handler := NewGatewayConnectorServiceWrapper(&gc, ethKeystore, logger) + require.NoError(t, err) + + t.Cleanup(func() { + assert.NoError(t, handler.Close()) + }) t.Run("Start Error", func(t *testing.T) { - // Question, what's the best practices when testing a different configuration, is it 2 configs and then - // 2 handlers, or does the test do the initialization as in copy from line 27 to 41 into here? ctx := testutils.Context(t) err := handler.Start(ctx) - require.NoError(t, err) + require.Error(t, err) }) } From 2b66be0421b8196569638c3d974173ba3e57f8d7 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 07:54:27 -0700 Subject: [PATCH 10/25] PR review: clean up pointers, service wrapper generation. --- .../gateway_connector/service_wrapper.go | 32 +++++---- .../gateway_connector/service_wrapper_test.go | 72 ++++++++----------- 2 files changed, 47 insertions(+), 57 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index c7cd6e0718b..c90221f05a5 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -21,10 +21,10 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey" ) -type serviceWrapper struct { +type ServiceWrapper struct { services.StateMachine - config *config.GatewayConnector + config config.GatewayConnector keystore keystore.Eth connector connector.GatewayConnector lggr logger.Logger @@ -41,9 +41,9 @@ type connectorSigner struct { var _ connector.Signer = &connectorSigner{} -func NewConnectorSigner(config *config.GatewayConnector, signerKey *ecdsa.PrivateKey, lggr logger.Logger) (*connectorSigner, error) { +func NewConnectorSigner(config config.GatewayConnector, signerKey *ecdsa.PrivateKey, lggr logger.Logger) (*connectorSigner, error) { return &connectorSigner{ - nodeAddress: (*config).NodeAddress(), + nodeAddress: config.NodeAddress(), signerKey: signerKey, lggr: lggr.Named("ConnectorSigner"), }, nil @@ -89,18 +89,24 @@ func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { } // NOTE: this wrapper is needed to make sure that our services are started after Keystore. -func NewGatewayConnectorServiceWrapper(config *config.GatewayConnector, keystore keystore.Eth, lggr logger.Logger) *serviceWrapper { - return &serviceWrapper{ +func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore keystore.Eth, lggr logger.Logger) *ServiceWrapper { + return &ServiceWrapper{ config: config, keystore: keystore, lggr: lggr, } } -func (e *serviceWrapper) Start(ctx context.Context) error { +func (e *ServiceWrapper) Start(ctx context.Context) error { return e.StartOnce("GatewayConnectorServiceWrapper", func() error { - conf := *e.config - e.lggr.Infow("Starting GatewayConnectorServiceWrapper", "chainID", conf.ChainIDForNodeKey()) + conf := e.config + e.lggr.Infow("Starting GatewayConnectorServiceWrapper", "chainID") + + // logger.go:146: 2024-09-10T07:52:35.248-0700 ERROR zap@v1.27.0/sugar.go:257 Ignored key without a value. {"version": "unset@unset", "ignored": "chainID"} + // go.uber.org/zap.(*SugaredLogger).Infow + ///Users/davidorchard/go/pkg/mod/go.uber.org/zap@v1.27.0/sugar.go:257 + + e.lggr.Infow("Starting GatewayConnectorServiceWrapper2", "chainID", conf.ChainIDForNodeKey()) chainID, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainID) if err != nil { @@ -132,20 +138,20 @@ func (e *serviceWrapper) Start(ctx context.Context) error { }) } -func (e *serviceWrapper) Close() error { +func (e *ServiceWrapper) Close() error { return e.StopOnce("GatewayConnectorServiceWrapper", func() (err error) { return e.connector.Close() }) } -func (e *serviceWrapper) Ready() error { +func (e *ServiceWrapper) Ready() error { return nil } -func (e *serviceWrapper) HealthReport() map[string]error { +func (e *ServiceWrapper) HealthReport() map[string]error { return nil } -func (e *serviceWrapper) Name() string { +func (e *ServiceWrapper) Name() string { return "GatewayConnectorServiceWrapper" } diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index 3e62ee3f7f8..5c61d2ded90 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -15,8 +15,10 @@ import ( "github.com/test-go/testify/mock" ) -func generateConfig(addr common.Address) (chainlink.GeneralConfig, error) { - return chainlink.GeneralConfigOpts{ +func generateWrapper(t *testing.T, addr common.Address, keystoreAddr common.Address) (*ServiceWrapper, error) { + logger := logger.TestLogger(t) + + config, err := chainlink.GeneralConfigOpts{ Config: chainlink.Config{ Core: toml.Core{ Capabilities: toml.Capabilities{ @@ -33,66 +35,48 @@ func generateConfig(addr common.Address) (chainlink.GeneralConfig, error) { }, }, }.New() -} - -// Unit test that creates the ServiceWrapper object and then calls Start() can Close() on it. -// Take inspiration from functions/plugin_test.go and functions/connector_handler_test.go on how to mock the dependencies. -// -// Test valid NodeAddress and an invalid one (i.e. key doesn't exit). - -func TestGatewayConnectorServiceWrapper(t *testing.T) { - t.Parallel() - - logger := logger.TestLogger(t) - _, addr := testutils.NewPrivateKeyAndAddress(t) - - config, err := generateConfig(addr) ethKeystore := ksmocks.NewEth(t) - ethKeystore.On("EnabledKeysForChain", mock.Anything).Return([]ethkey.KeyV2{{Address: addr}}) + // github.com/stretchr/testify/mock.Arguments.Get(...) + // /Users/davidorchard/go/pkg/mod/github.com/stretchr/testify@v1.9.0/mock/mock.go:900 + // github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks.(*Eth).EnabledKeysForChain(0xc0017a0f00, {0x1071e02d0, 0xc000776770}, 0xc0009148c0) + ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{{Address: keystoreAddr}}) gc := config.Capabilities().GatewayConnector() - handler := NewGatewayConnectorServiceWrapper(&gc, ethKeystore, logger) + wrapper := NewGatewayConnectorServiceWrapper(gc, ethKeystore, logger) require.NoError(t, err) + return wrapper, err - t.Cleanup(func() { - assert.NoError(t, handler.Close()) - }) - - t.Run("Start & Stop Success", func(t *testing.T) { - ctx := testutils.Context(t) - - err := handler.Start(ctx) - require.NoError(t, err) - err = handler.Close() - require.NoError(t, err) - }) } -func TestGatewayConnectorServiceWrapperConfigError(t *testing.T) { +func TestGatewayConnectorServiceWrapper_CleanStartClose(t *testing.T) { t.Parallel() - logger := logger.TestLogger(t) _, addr := testutils.NewPrivateKeyAndAddress(t) + wrapper, err := generateWrapper(t, addr, addr) - config, err := generateConfig(addr) - ethKeystore := ksmocks.NewEth(t) - _, addr2 := testutils.NewPrivateKeyAndAddress(t) + require.NoError(t, err) - ethKeystore.On("EnabledKeysForChain", mock.Anything).Return([]ethkey.KeyV2{{Address: addr2}}) + ctx := testutils.Context(t) - gc := config.Capabilities().GatewayConnector() - handler := NewGatewayConnectorServiceWrapper(&gc, ethKeystore, logger) + err = wrapper.Start(ctx) require.NoError(t, err) t.Cleanup(func() { - assert.NoError(t, handler.Close()) + assert.NoError(t, wrapper.Close()) }) +} - t.Run("Start Error", func(t *testing.T) { - ctx := testutils.Context(t) - err := handler.Start(ctx) - require.Error(t, err) - }) +func TestGatewayConnectorServiceWrapper_NonexistantKey(t *testing.T) { + t.Parallel() + + _, addr := testutils.NewPrivateKeyAndAddress(t) + _, keystoreAddr := testutils.NewPrivateKeyAndAddress(t) + wrapper, err := generateWrapper(t, addr, keystoreAddr) + + require.NoError(t, err) + ctx := testutils.Context(t) + err = wrapper.Start(ctx) + require.Error(t, err) } func ptr[T any](t T) *T { return &t } From 65191071ecff9ac3882f0429899af55390ef6e8b Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 10:29:27 -0700 Subject: [PATCH 11/25] fix mocks return args issue --- .../gateway_connector/service_wrapper.go | 6 ----- .../gateway_connector/service_wrapper_test.go | 27 +++++++++++++++---- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index c90221f05a5..de8696f182b 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -100,12 +100,6 @@ func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore func (e *ServiceWrapper) Start(ctx context.Context) error { return e.StartOnce("GatewayConnectorServiceWrapper", func() error { conf := e.config - e.lggr.Infow("Starting GatewayConnectorServiceWrapper", "chainID") - - // logger.go:146: 2024-09-10T07:52:35.248-0700 ERROR zap@v1.27.0/sugar.go:257 Ignored key without a value. {"version": "unset@unset", "ignored": "chainID"} - // go.uber.org/zap.(*SugaredLogger).Infow - ///Users/davidorchard/go/pkg/mod/go.uber.org/zap@v1.27.0/sugar.go:257 - e.lggr.Infow("Starting GatewayConnectorServiceWrapper2", "chainID", conf.ChainIDForNodeKey()) chainID, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainID) diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index 5c61d2ded90..4da82fcfb30 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -36,10 +36,7 @@ func generateWrapper(t *testing.T, addr common.Address, keystoreAddr common.Addr }, }.New() ethKeystore := ksmocks.NewEth(t) - // github.com/stretchr/testify/mock.Arguments.Get(...) - // /Users/davidorchard/go/pkg/mod/github.com/stretchr/testify@v1.9.0/mock/mock.go:900 - // github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks.(*Eth).EnabledKeysForChain(0xc0017a0f00, {0x1071e02d0, 0xc000776770}, 0xc0009148c0) - ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{{Address: keystoreAddr}}) + ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{{Address: keystoreAddr}}, nil) gc := config.Capabilities().GatewayConnector() wrapper := NewGatewayConnectorServiceWrapper(gc, ethKeystore, logger) @@ -57,7 +54,27 @@ func TestGatewayConnectorServiceWrapper_CleanStartClose(t *testing.T) { require.NoError(t, err) ctx := testutils.Context(t) - + // panic: runtime error: invalid memory address or nil pointer dereference + // [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1008fbb6b] + + // goroutine 280 [running]: + // github.com/ethereum/go-ethereum/crypto.Sign({0xc001a1b880, 0x20, 0x20}, 0x0) + // /Users/davidorchard/go/pkg/mod/github.com/ethereum/go-ethereum@v1.13.8/crypto/signature_cgo.go:60 +0x10b + // github.com/smartcontractkit/chainlink/v2/core/utils.GenerateEthSignature(0x0, {0xc000d128f0, 0xc4, 0xd0}) + // /Users/davidorchard/code/chainlink/core/utils/eth_signatures.go:46 +0x6e + // github.com/smartcontractkit/chainlink/v2/core/services/gateway/common.SignData(0x0, {0xc000bc1ef0?, 0x102412600?, 0xc000d12801?}) + // /Users/davidorchard/code/chainlink/core/services/gateway/common/utils.go:45 +0xdb + // github.com/smartcontractkit/chainlink/v2/core/capabilities/gateway_connector.(*connectorSigner).Sign(0xc001a6c280?, {0xc000bc1ef0?, 0xc001a3d140?, 0x101bdfaae?}) + // /Users/davidorchard/code/chainlink/core/capabilities/gateway_connector/service_wrapper.go:53 +0x1c + // github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector.(*gatewayConnector).NewAuthHeader(0xc00194bc20, 0x0?) + // /Users/davidorchard/code/chainlink/core/services/gateway/connector/connector.go:256 +0x28f + // github.com/smartcontractkit/chainlink/v2/core/services/gateway/network.(*webSocketClient).Connect(0xc00194d530, {0x1027f9260, 0xc001a6e140}, 0xc001950d80) + // /Users/davidorchard/code/chainlink/core/services/gateway/network/wsclient.go:42 +0x50 + // github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector.(*gatewayConnector).reconnectLoop(0xc00194bc20, 0xc00195e780) + // /Users/davidorchard/code/chainlink/core/services/gateway/connector/connector.go:195 +0x14a + // created by github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector.(*gatewayConnector).Start.func1 in goroutine 277 + // /Users/davidorchard/code/chainlink/core/services/gateway/connector/connector.go:226 +0xd4 + // FAIL github.com/smartcontractkit/chainlink/v2/core/capabilities/gateway_connector 3.241s err = wrapper.Start(ctx) require.NoError(t, err) From 493f94c688a4cc850f7401155cf1eb867cc130cf Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 11:19:58 -0700 Subject: [PATCH 12/25] add logs to figure out Sign issue --- core/capabilities/gateway_connector/service_wrapper.go | 9 ++++++++- .../gateway_connector/service_wrapper_test.go | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index de8696f182b..e0d31c78935 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -50,6 +50,7 @@ func NewConnectorSigner(config config.GatewayConnector, signerKey *ecdsa.Private } func (h *connectorSigner) Sign(data ...[]byte) ([]byte, error) { + h.lggr.Debugw("Sign signerKey", h.signerKey) return gwCommon.SignData(h.signerKey, data...) } @@ -100,7 +101,7 @@ func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore func (e *ServiceWrapper) Start(ctx context.Context) error { return e.StartOnce("GatewayConnectorServiceWrapper", func() error { conf := e.config - e.lggr.Infow("Starting GatewayConnectorServiceWrapper2", "chainID", conf.ChainIDForNodeKey()) + e.lggr.Infow("Starting GatewayConnectorServiceWrapper", "chainID", conf.ChainIDForNodeKey()) chainID, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainID) if err != nil { @@ -119,6 +120,8 @@ func (e *ServiceWrapper) Start(ctx context.Context) error { return errors.New("node address mismatch") } + e.lggr.Infow("GatewayConnectorServiceWrapper signerKey", signerKey) + signer, err := NewConnectorSigner(e.config, signerKey, e.lggr) if err != nil { return err @@ -149,3 +152,7 @@ func (e *ServiceWrapper) HealthReport() map[string]error { func (e *ServiceWrapper) Name() string { return "GatewayConnectorServiceWrapper" } + +func (e *ServiceWrapper) GetGatewayConnecgtor() connector.GatewayConnector { + return e.connector +} diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index 4da82fcfb30..6c517cff9ee 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -11,8 +11,8 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey" ksmocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "github.com/test-go/testify/mock" ) func generateWrapper(t *testing.T, addr common.Address, keystoreAddr common.Address) (*ServiceWrapper, error) { @@ -37,16 +37,16 @@ func generateWrapper(t *testing.T, addr common.Address, keystoreAddr common.Addr }.New() ethKeystore := ksmocks.NewEth(t) ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{{Address: keystoreAddr}}, nil) - gc := config.Capabilities().GatewayConnector() wrapper := NewGatewayConnectorServiceWrapper(gc, ethKeystore, logger) require.NoError(t, err) return wrapper, err - } func TestGatewayConnectorServiceWrapper_CleanStartClose(t *testing.T) { t.Parallel() + logger := logger.TestLogger(t) + logger.Infow("TestGatewayConnectorServiceWrapper_CleanStartClose") _, addr := testutils.NewPrivateKeyAndAddress(t) wrapper, err := generateWrapper(t, addr, addr) From cfaf0ca79b08fe0f8223dc75dd2de5df587f8661 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 12:40:10 -0700 Subject: [PATCH 13/25] fix keys issues --- .../gateway_connector/service_wrapper.go | 5 +-- .../gateway_connector/service_wrapper_test.go | 45 +++++-------------- 2 files changed, 13 insertions(+), 37 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index e0d31c78935..15739b6512d 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -50,7 +50,6 @@ func NewConnectorSigner(config config.GatewayConnector, signerKey *ecdsa.Private } func (h *connectorSigner) Sign(data ...[]byte) ([]byte, error) { - h.lggr.Debugw("Sign signerKey", h.signerKey) return gwCommon.SignData(h.signerKey, data...) } @@ -101,7 +100,6 @@ func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore func (e *ServiceWrapper) Start(ctx context.Context) error { return e.StartOnce("GatewayConnectorServiceWrapper", func() error { conf := e.config - e.lggr.Infow("Starting GatewayConnectorServiceWrapper", "chainID", conf.ChainIDForNodeKey()) chainID, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainID) if err != nil { @@ -112,6 +110,7 @@ func (e *ServiceWrapper) Start(ctx context.Context) error { } configuredNodeAddress := common.HexToAddress(conf.NodeAddress()) idx := slices.IndexFunc(enabledKeys, func(key ethkey.KeyV2) bool { return key.Address == configuredNodeAddress }) + if idx == -1 { return errors.New("key for configured node address not found") } @@ -120,8 +119,6 @@ func (e *ServiceWrapper) Start(ctx context.Context) error { return errors.New("node address mismatch") } - e.lggr.Infow("GatewayConnectorServiceWrapper signerKey", signerKey) - signer, err := NewConnectorSigner(e.config, signerKey, e.lggr) if err != nil { return err diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index 6c517cff9ee..09cf2e2a3a7 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -1,9 +1,9 @@ package gatewayconnector import ( + "crypto/ecdsa" "testing" - "github.com/ethereum/go-ethereum/common" "github.com/smartcontractkit/chainlink/v2/core/config/toml" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" "github.com/smartcontractkit/chainlink/v2/core/logger" @@ -15,8 +15,11 @@ import ( "github.com/stretchr/testify/require" ) -func generateWrapper(t *testing.T, addr common.Address, keystoreAddr common.Address) (*ServiceWrapper, error) { +func generateWrapper(t *testing.T, privateKey *ecdsa.PrivateKey, keystoreKey *ecdsa.PrivateKey) (*ServiceWrapper, error) { logger := logger.TestLogger(t) + privateKeyV2 := ethkey.FromPrivateKey(privateKey) + addr := privateKeyV2.Address + keystoreKeyV2 := ethkey.FromPrivateKey(keystoreKey) config, err := chainlink.GeneralConfigOpts{ Config: chainlink.Config{ @@ -36,7 +39,7 @@ func generateWrapper(t *testing.T, addr common.Address, keystoreAddr common.Addr }, }.New() ethKeystore := ksmocks.NewEth(t) - ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{{Address: keystoreAddr}}, nil) + ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{keystoreKeyV2}, nil) gc := config.Capabilities().GatewayConnector() wrapper := NewGatewayConnectorServiceWrapper(gc, ethKeystore, logger) require.NoError(t, err) @@ -45,36 +48,12 @@ func generateWrapper(t *testing.T, addr common.Address, keystoreAddr common.Addr func TestGatewayConnectorServiceWrapper_CleanStartClose(t *testing.T) { t.Parallel() - logger := logger.TestLogger(t) - logger.Infow("TestGatewayConnectorServiceWrapper_CleanStartClose") - - _, addr := testutils.NewPrivateKeyAndAddress(t) - wrapper, err := generateWrapper(t, addr, addr) + key, _ := testutils.NewPrivateKeyAndAddress(t) + wrapper, err := generateWrapper(t, key, key) require.NoError(t, err) ctx := testutils.Context(t) - // panic: runtime error: invalid memory address or nil pointer dereference - // [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1008fbb6b] - - // goroutine 280 [running]: - // github.com/ethereum/go-ethereum/crypto.Sign({0xc001a1b880, 0x20, 0x20}, 0x0) - // /Users/davidorchard/go/pkg/mod/github.com/ethereum/go-ethereum@v1.13.8/crypto/signature_cgo.go:60 +0x10b - // github.com/smartcontractkit/chainlink/v2/core/utils.GenerateEthSignature(0x0, {0xc000d128f0, 0xc4, 0xd0}) - // /Users/davidorchard/code/chainlink/core/utils/eth_signatures.go:46 +0x6e - // github.com/smartcontractkit/chainlink/v2/core/services/gateway/common.SignData(0x0, {0xc000bc1ef0?, 0x102412600?, 0xc000d12801?}) - // /Users/davidorchard/code/chainlink/core/services/gateway/common/utils.go:45 +0xdb - // github.com/smartcontractkit/chainlink/v2/core/capabilities/gateway_connector.(*connectorSigner).Sign(0xc001a6c280?, {0xc000bc1ef0?, 0xc001a3d140?, 0x101bdfaae?}) - // /Users/davidorchard/code/chainlink/core/capabilities/gateway_connector/service_wrapper.go:53 +0x1c - // github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector.(*gatewayConnector).NewAuthHeader(0xc00194bc20, 0x0?) - // /Users/davidorchard/code/chainlink/core/services/gateway/connector/connector.go:256 +0x28f - // github.com/smartcontractkit/chainlink/v2/core/services/gateway/network.(*webSocketClient).Connect(0xc00194d530, {0x1027f9260, 0xc001a6e140}, 0xc001950d80) - // /Users/davidorchard/code/chainlink/core/services/gateway/network/wsclient.go:42 +0x50 - // github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector.(*gatewayConnector).reconnectLoop(0xc00194bc20, 0xc00195e780) - // /Users/davidorchard/code/chainlink/core/services/gateway/connector/connector.go:195 +0x14a - // created by github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector.(*gatewayConnector).Start.func1 in goroutine 277 - // /Users/davidorchard/code/chainlink/core/services/gateway/connector/connector.go:226 +0xd4 - // FAIL github.com/smartcontractkit/chainlink/v2/core/capabilities/gateway_connector 3.241s err = wrapper.Start(ctx) require.NoError(t, err) @@ -86,11 +65,11 @@ func TestGatewayConnectorServiceWrapper_CleanStartClose(t *testing.T) { func TestGatewayConnectorServiceWrapper_NonexistantKey(t *testing.T) { t.Parallel() - _, addr := testutils.NewPrivateKeyAndAddress(t) - _, keystoreAddr := testutils.NewPrivateKeyAndAddress(t) - wrapper, err := generateWrapper(t, addr, keystoreAddr) - + key, _ := testutils.NewPrivateKeyAndAddress(t) + keystoreKey, _ := testutils.NewPrivateKeyAndAddress(t) + wrapper, err := generateWrapper(t, key, keystoreKey) require.NoError(t, err) + ctx := testutils.Context(t) err = wrapper.Start(ctx) require.Error(t, err) From 3b1e3ee228ded59661eff24107d4658e15988243 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 12:53:33 -0700 Subject: [PATCH 14/25] typos --- core/capabilities/gateway_connector/service_wrapper.go | 3 ++- core/capabilities/gateway_connector/service_wrapper_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 15739b6512d..484fffa97be 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -55,6 +55,7 @@ func (h *connectorSigner) Sign(data ...[]byte) ([]byte, error) { func (h *connectorSigner) HandleGatewayMessage(ctx context.Context, gatewayID string, msg *api.Message) { } + func (h *connectorSigner) Start(ctx context.Context) error { return h.StartOnce("ConnectorSigner", func() error { return nil @@ -150,6 +151,6 @@ func (e *ServiceWrapper) Name() string { return "GatewayConnectorServiceWrapper" } -func (e *ServiceWrapper) GetGatewayConnecgtor() connector.GatewayConnector { +func (e *ServiceWrapper) GetGatewayConnector() connector.GatewayConnector { return e.connector } diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index 09cf2e2a3a7..2d32f9a102d 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -62,7 +62,7 @@ func TestGatewayConnectorServiceWrapper_CleanStartClose(t *testing.T) { }) } -func TestGatewayConnectorServiceWrapper_NonexistantKey(t *testing.T) { +func TestGatewayConnectorServiceWrapper_NonexistentKey(t *testing.T) { t.Parallel() key, _ := testutils.NewPrivateKeyAndAddress(t) From 3971608421e48324ec920321ef7fa1e7ef5390f1 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 13:16:02 -0700 Subject: [PATCH 15/25] remove redundant named import --- core/capabilities/gateway_connector/service_wrapper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index 2d32f9a102d..d0394d904b7 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -7,7 +7,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/config/toml" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" "github.com/smartcontractkit/chainlink/v2/core/logger" - chainlink "github.com/smartcontractkit/chainlink/v2/core/services/chainlink" + "github.com/smartcontractkit/chainlink/v2/core/services/chainlink" "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey" ksmocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" "github.com/stretchr/testify/assert" From 1cfa38f3f60c98e311f11e261c051eb96ae43dfd Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 14:33:18 -0700 Subject: [PATCH 16/25] update order of deps to try to fix strange lint error --- .../capabilities/gateway_connector/service_wrapper_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index d0394d904b7..0545ec7e68a 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -4,15 +4,16 @@ import ( "crypto/ecdsa" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/smartcontractkit/chainlink/v2/core/config/toml" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" "github.com/smartcontractkit/chainlink/v2/core/logger" "github.com/smartcontractkit/chainlink/v2/core/services/chainlink" "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey" ksmocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" ) func generateWrapper(t *testing.T, privateKey *ecdsa.PrivateKey, keystoreKey *ecdsa.PrivateKey) (*ServiceWrapper, error) { From 5ac44fa325622a2554928df8adec40abc025a572 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 14:42:36 -0700 Subject: [PATCH 17/25] remove handlegatewaymessage --- core/capabilities/gateway_connector/service_wrapper.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 484fffa97be..832c5f99c0b 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -13,7 +13,6 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/services" "github.com/smartcontractkit/chainlink/v2/core/config" "github.com/smartcontractkit/chainlink/v2/core/logger" - "github.com/smartcontractkit/chainlink/v2/core/services/gateway/api" gwCommon "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/network" @@ -53,9 +52,6 @@ func (h *connectorSigner) Sign(data ...[]byte) ([]byte, error) { return gwCommon.SignData(h.signerKey, data...) } -func (h *connectorSigner) HandleGatewayMessage(ctx context.Context, gatewayID string, msg *api.Message) { -} - func (h *connectorSigner) Start(ctx context.Context) error { return h.StartOnce("ConnectorSigner", func() error { return nil From 8f26d15fcee3b8d3493d7008f4b1feba9e395f19 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 14:52:08 -0700 Subject: [PATCH 18/25] change name to gwcommon --- core/capabilities/gateway_connector/service_wrapper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 832c5f99c0b..c9c0d5cea6d 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -13,7 +13,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/services" "github.com/smartcontractkit/chainlink/v2/core/config" "github.com/smartcontractkit/chainlink/v2/core/logger" - gwCommon "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common" + gwcommon "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/connector" "github.com/smartcontractkit/chainlink/v2/core/services/gateway/network" "github.com/smartcontractkit/chainlink/v2/core/services/keystore" @@ -49,7 +49,7 @@ func NewConnectorSigner(config config.GatewayConnector, signerKey *ecdsa.Private } func (h *connectorSigner) Sign(data ...[]byte) ([]byte, error) { - return gwCommon.SignData(h.signerKey, data...) + return gwcommon.SignData(h.signerKey, data...) } func (h *connectorSigner) Start(ctx context.Context) error { From c3f5616110ed86cc10e5ab3d13f4dcbe657c1224 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 15:04:17 -0700 Subject: [PATCH 19/25] remove unused funcs and clean up var reuse --- .../gateway_connector/service_wrapper.go | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index c9c0d5cea6d..834828616a3 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -52,21 +52,6 @@ func (h *connectorSigner) Sign(data ...[]byte) ([]byte, error) { return gwcommon.SignData(h.signerKey, data...) } -func (h *connectorSigner) Start(ctx context.Context) error { - return h.StartOnce("ConnectorSigner", func() error { - return nil - }) -} -func (h *connectorSigner) Close() error { - return h.StopOnce("ConnectorSigner", func() (err error) { - return nil - }) -} - -func (h *connectorSigner) SetConnector(connector connector.GatewayConnector) { - h.connector = connector -} - func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { r := connector.ConnectorConfig{} r.NodeAddress = f.NodeAddress() @@ -97,7 +82,8 @@ func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore func (e *ServiceWrapper) Start(ctx context.Context) error { return e.StartOnce("GatewayConnectorServiceWrapper", func() error { conf := e.config - chainID, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) + nodeAddress := conf.ChainIDForNodeKey() + chainID, _ := new(big.Int).SetString(nodeAddress, 0) enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainID) if err != nil { return err @@ -105,23 +91,23 @@ func (e *ServiceWrapper) Start(ctx context.Context) error { if len(enabledKeys) == 0 { return errors.New("no available keys found") } - configuredNodeAddress := common.HexToAddress(conf.NodeAddress()) + configuredNodeAddress := common.HexToAddress(nodeAddress) idx := slices.IndexFunc(enabledKeys, func(key ethkey.KeyV2) bool { return key.Address == configuredNodeAddress }) if idx == -1 { return errors.New("key for configured node address not found") } signerKey := enabledKeys[idx].ToEcdsaPrivKey() - if enabledKeys[idx].ID() != conf.NodeAddress() { + if enabledKeys[idx].ID() != nodeAddress { return errors.New("node address mismatch") } - signer, err := NewConnectorSigner(e.config, signerKey, e.lggr) + signer, err := NewConnectorSigner(conf, signerKey, e.lggr) if err != nil { return err } translated := translateConfigs(conf) - e.connector, err = connector.NewGatewayConnector(&translated, signer, clockwork.NewRealClock(), e.lggr) + e.connector, err = connector.NewGatewayConnector(&translated, signer, clockwork.NewFakeClock(), e.lggr) if err != nil { return err } From 558f577b265dd113004ecf8cc73230efcc92caaf Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 15:11:29 -0700 Subject: [PATCH 20/25] add clock to servicewrapper --- core/capabilities/gateway_connector/service_wrapper.go | 6 ++++-- core/capabilities/gateway_connector/service_wrapper_test.go | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 834828616a3..9302a52292d 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -27,6 +27,7 @@ type ServiceWrapper struct { keystore keystore.Eth connector connector.GatewayConnector lggr logger.Logger + clock clockwork.Clock } type connectorSigner struct { @@ -71,10 +72,11 @@ func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { } // NOTE: this wrapper is needed to make sure that our services are started after Keystore. -func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore keystore.Eth, lggr logger.Logger) *ServiceWrapper { +func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore keystore.Eth, clock clockwork.Clock, lggr logger.Logger) *ServiceWrapper { return &ServiceWrapper{ config: config, keystore: keystore, + clock: clock, lggr: lggr, } } @@ -107,7 +109,7 @@ func (e *ServiceWrapper) Start(ctx context.Context) error { return err } translated := translateConfigs(conf) - e.connector, err = connector.NewGatewayConnector(&translated, signer, clockwork.NewFakeClock(), e.lggr) + e.connector, err = connector.NewGatewayConnector(&translated, signer, e.clock, e.lggr) if err != nil { return err } diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index 0545ec7e68a..ecbb896a8ec 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -4,6 +4,7 @@ import ( "crypto/ecdsa" "testing" + "github.com/jonboulle/clockwork" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -42,7 +43,7 @@ func generateWrapper(t *testing.T, privateKey *ecdsa.PrivateKey, keystoreKey *ec ethKeystore := ksmocks.NewEth(t) ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{keystoreKeyV2}, nil) gc := config.Capabilities().GatewayConnector() - wrapper := NewGatewayConnectorServiceWrapper(gc, ethKeystore, logger) + wrapper := NewGatewayConnectorServiceWrapper(gc, ethKeystore, clockwork.NewFakeClock(), logger) require.NoError(t, err) return wrapper, err } From f2f8d9ca862f135848c7c8c2f3a1822bf6651ca4 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 15:20:11 -0700 Subject: [PATCH 21/25] fix bug in configs --- core/capabilities/gateway_connector/service_wrapper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 9302a52292d..6397837ef19 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -84,8 +84,8 @@ func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore func (e *ServiceWrapper) Start(ctx context.Context) error { return e.StartOnce("GatewayConnectorServiceWrapper", func() error { conf := e.config - nodeAddress := conf.ChainIDForNodeKey() - chainID, _ := new(big.Int).SetString(nodeAddress, 0) + nodeAddress := conf.NodeAddress() + chainID, _ := new(big.Int).SetString(conf.ChainIDForNodeKey(), 0) enabledKeys, err := e.keystore.EnabledKeysForChain(ctx, chainID) if err != nil { return err From e119169af86adaaf3c7c12863c9f5d6f085814b4 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 15:22:31 -0700 Subject: [PATCH 22/25] fix Ready and healthreport --- core/capabilities/gateway_connector/service_wrapper.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 6397837ef19..3fb2775a97c 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -123,12 +123,8 @@ func (e *ServiceWrapper) Close() error { }) } -func (e *ServiceWrapper) Ready() error { - return nil -} - func (e *ServiceWrapper) HealthReport() map[string]error { - return nil + return map[string]error{e.Name(): e.Healthy()} } func (e *ServiceWrapper) Name() string { From d572c971b153692709ae8a37580a6e1b625e29f3 Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 18:12:08 -0700 Subject: [PATCH 23/25] remove connectorSigner struct --- .../gateway_connector/service_wrapper.go | 47 +++++-------------- .../gateway_connector/service_wrapper_test.go | 2 +- 2 files changed, 14 insertions(+), 35 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 3fb2775a97c..2fd35e899b9 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -26,33 +26,11 @@ type ServiceWrapper struct { config config.GatewayConnector keystore keystore.Eth connector connector.GatewayConnector + signerKey *ecdsa.PrivateKey lggr logger.Logger clock clockwork.Clock } -type connectorSigner struct { - services.StateMachine - - connector connector.GatewayConnector - signerKey *ecdsa.PrivateKey - nodeAddress string - lggr logger.Logger -} - -var _ connector.Signer = &connectorSigner{} - -func NewConnectorSigner(config config.GatewayConnector, signerKey *ecdsa.PrivateKey, lggr logger.Logger) (*connectorSigner, error) { - return &connectorSigner{ - nodeAddress: config.NodeAddress(), - signerKey: signerKey, - lggr: lggr.Named("ConnectorSigner"), - }, nil -} - -func (h *connectorSigner) Sign(data ...[]byte) ([]byte, error) { - return gwcommon.SignData(h.signerKey, data...) -} - func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { r := connector.ConnectorConfig{} r.NodeAddress = f.NodeAddress() @@ -72,12 +50,13 @@ func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { } // NOTE: this wrapper is needed to make sure that our services are started after Keystore. -func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore keystore.Eth, clock clockwork.Clock, lggr logger.Logger) *ServiceWrapper { +func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, signerKey *ecdsa.PrivateKey, keystore keystore.Eth, clock clockwork.Clock, lggr logger.Logger) *ServiceWrapper { return &ServiceWrapper{ - config: config, - keystore: keystore, - clock: clock, - lggr: lggr, + config: config, + signerKey: signerKey, + keystore: keystore, + clock: clock, + lggr: lggr, } } @@ -99,17 +78,13 @@ func (e *ServiceWrapper) Start(ctx context.Context) error { if idx == -1 { return errors.New("key for configured node address not found") } - signerKey := enabledKeys[idx].ToEcdsaPrivKey() + e.signerKey = enabledKeys[idx].ToEcdsaPrivKey() if enabledKeys[idx].ID() != nodeAddress { return errors.New("node address mismatch") } - signer, err := NewConnectorSigner(conf, signerKey, e.lggr) - if err != nil { - return err - } translated := translateConfigs(conf) - e.connector, err = connector.NewGatewayConnector(&translated, signer, e.clock, e.lggr) + e.connector, err = connector.NewGatewayConnector(&translated, e, e.clock, e.lggr) if err != nil { return err } @@ -117,6 +92,10 @@ func (e *ServiceWrapper) Start(ctx context.Context) error { }) } +func (h *ServiceWrapper) Sign(data ...[]byte) ([]byte, error) { + return gwcommon.SignData(h.signerKey, data...) +} + func (e *ServiceWrapper) Close() error { return e.StopOnce("GatewayConnectorServiceWrapper", func() (err error) { return e.connector.Close() diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index ecbb896a8ec..ba723bc0d49 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -43,7 +43,7 @@ func generateWrapper(t *testing.T, privateKey *ecdsa.PrivateKey, keystoreKey *ec ethKeystore := ksmocks.NewEth(t) ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{keystoreKeyV2}, nil) gc := config.Capabilities().GatewayConnector() - wrapper := NewGatewayConnectorServiceWrapper(gc, ethKeystore, clockwork.NewFakeClock(), logger) + wrapper := NewGatewayConnectorServiceWrapper(gc, privateKey, ethKeystore, clockwork.NewFakeClock(), logger) require.NoError(t, err) return wrapper, err } From d4e0068db52f0f6588f64ac2f76f1c7fd27a085d Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 20:44:24 -0700 Subject: [PATCH 24/25] remove privateKey from constructor, keep initialization in Start --- .../capabilities/gateway_connector/service_wrapper.go | 11 +++++------ .../gateway_connector/service_wrapper_test.go | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index 2fd35e899b9..d0280a72ba0 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -50,13 +50,12 @@ func translateConfigs(f config.GatewayConnector) connector.ConnectorConfig { } // NOTE: this wrapper is needed to make sure that our services are started after Keystore. -func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, signerKey *ecdsa.PrivateKey, keystore keystore.Eth, clock clockwork.Clock, lggr logger.Logger) *ServiceWrapper { +func NewGatewayConnectorServiceWrapper(config config.GatewayConnector, keystore keystore.Eth, clock clockwork.Clock, lggr logger.Logger) *ServiceWrapper { return &ServiceWrapper{ - config: config, - signerKey: signerKey, - keystore: keystore, - clock: clock, - lggr: lggr, + config: config, + keystore: keystore, + clock: clock, + lggr: lggr, } } diff --git a/core/capabilities/gateway_connector/service_wrapper_test.go b/core/capabilities/gateway_connector/service_wrapper_test.go index ba723bc0d49..ecbb896a8ec 100644 --- a/core/capabilities/gateway_connector/service_wrapper_test.go +++ b/core/capabilities/gateway_connector/service_wrapper_test.go @@ -43,7 +43,7 @@ func generateWrapper(t *testing.T, privateKey *ecdsa.PrivateKey, keystoreKey *ec ethKeystore := ksmocks.NewEth(t) ethKeystore.On("EnabledKeysForChain", mock.Anything, mock.Anything).Return([]ethkey.KeyV2{keystoreKeyV2}, nil) gc := config.Capabilities().GatewayConnector() - wrapper := NewGatewayConnectorServiceWrapper(gc, privateKey, ethKeystore, clockwork.NewFakeClock(), logger) + wrapper := NewGatewayConnectorServiceWrapper(gc, ethKeystore, clockwork.NewFakeClock(), logger) require.NoError(t, err) return wrapper, err } From e256b2fa0d9fdf819a7f19e407d0e9b80a19bb6d Mon Sep 17 00:00:00 2001 From: David Orchard Date: Tue, 10 Sep 2024 20:54:48 -0700 Subject: [PATCH 25/25] lint --- core/capabilities/gateway_connector/service_wrapper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/capabilities/gateway_connector/service_wrapper.go b/core/capabilities/gateway_connector/service_wrapper.go index d0280a72ba0..824c92b4f89 100644 --- a/core/capabilities/gateway_connector/service_wrapper.go +++ b/core/capabilities/gateway_connector/service_wrapper.go @@ -91,8 +91,8 @@ func (e *ServiceWrapper) Start(ctx context.Context) error { }) } -func (h *ServiceWrapper) Sign(data ...[]byte) ([]byte, error) { - return gwcommon.SignData(h.signerKey, data...) +func (e *ServiceWrapper) Sign(data ...[]byte) ([]byte, error) { + return gwcommon.SignData(e.signerKey, data...) } func (e *ServiceWrapper) Close() error {