From 594a7f0f759bb670c2cd4a25e4c4862784d50400 Mon Sep 17 00:00:00 2001 From: amit-momin <108959691+amit-momin@users.noreply.github.com> Date: Wed, 13 Mar 2024 16:00:11 -0500 Subject: [PATCH] Consolidate configs for the EVM chain client for easier external use (#12294) * Consolidated configs for easier external use and moved the evm client out of legacyevm * Reused existing config types and created centralized models for client types * Removed deprecated client constructor * Renamed the client config builder file * Added evm client and config builder tests * Fixed linting * Reverted client type aliases --- core/chains/evm/client/chain_client.go | 15 +- core/chains/evm/client/config_builder.go | 100 ++++++++++ core/chains/evm/client/config_builder_test.go | 181 ++++++++++++++++++ core/chains/evm/client/evm_client.go | 37 ++++ core/chains/evm/client/evm_client_test.go | 37 ++++ core/chains/evm/client/helpers_test.go | 7 + core/chains/evm/config/chain_scoped.go | 2 +- .../evm/config/chain_scoped_node_pool.go | 28 +-- core/chains/legacyevm/chain.go | 32 +--- 9 files changed, 380 insertions(+), 59 deletions(-) create mode 100644 core/chains/evm/client/config_builder.go create mode 100644 core/chains/evm/client/config_builder_test.go create mode 100644 core/chains/evm/client/evm_client.go create mode 100644 core/chains/evm/client/evm_client_test.go diff --git a/core/chains/evm/client/chain_client.go b/core/chains/evm/client/chain_client.go index 64e854f1de4..1eb2347c474 100644 --- a/core/chains/evm/client/chain_client.go +++ b/core/chains/evm/client/chain_client.go @@ -51,20 +51,7 @@ func NewChainClient( chainID *big.Int, chainType config.ChainType, ) Client { - multiNode := commonclient.NewMultiNode[ - *big.Int, - evmtypes.Nonce, - common.Address, - common.Hash, - *types.Transaction, - common.Hash, - types.Log, - ethereum.FilterQuery, - *evmtypes.Receipt, - *assets.Wei, - *evmtypes.Head, - RPCClient, - ]( + multiNode := commonclient.NewMultiNode( lggr, selectionMode, leaseDuration, diff --git a/core/chains/evm/client/config_builder.go b/core/chains/evm/client/config_builder.go new file mode 100644 index 00000000000..c004bc4e9c6 --- /dev/null +++ b/core/chains/evm/client/config_builder.go @@ -0,0 +1,100 @@ +package client + +import ( + "fmt" + "net/url" + "time" + + "go.uber.org/multierr" + + commonconfig "github.com/smartcontractkit/chainlink-common/pkg/config" + "github.com/smartcontractkit/chainlink/v2/common/config" + evmconfig "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml" +) + +type nodeConfig struct { + Name *string + WSURL *string + HTTPURL *string + SendOnly *bool + Order *int32 +} + +// Build the configs needed to initialize the chain client +// Parameters should only be basic go types to make it accessible for external users +// Configs can be stored in a variety of ways +func NewClientConfigs( + selectionMode *string, + leaseDuration time.Duration, + chainType string, + nodeCfgs []nodeConfig, + pollFailureThreshold *uint32, + pollInterval time.Duration, + syncThreshold *uint32, + nodeIsSyncingEnabled *bool, +) (evmconfig.NodePool, []*toml.Node, config.ChainType, error) { + nodes, err := parseNodeConfigs(nodeCfgs) + if err != nil { + return nil, nil, "", err + } + nodePool := toml.NodePool{ + SelectionMode: selectionMode, + LeaseDuration: commonconfig.MustNewDuration(leaseDuration), + PollFailureThreshold: pollFailureThreshold, + PollInterval: commonconfig.MustNewDuration(pollInterval), + SyncThreshold: syncThreshold, + NodeIsSyncingEnabled: nodeIsSyncingEnabled, + } + nodePoolCfg := &evmconfig.NodePoolConfig{C: nodePool} + return nodePoolCfg, nodes, config.ChainType(chainType), nil +} + +func parseNodeConfigs(nodeCfgs []nodeConfig) ([]*toml.Node, error) { + nodes := make([]*toml.Node, len(nodeCfgs)) + for i, nodeCfg := range nodeCfgs { + if nodeCfg.WSURL == nil || nodeCfg.HTTPURL == nil { + return nil, fmt.Errorf("node config [%d]: missing WS or HTTP URL", i) + } + wsUrl := commonconfig.MustParseURL(*nodeCfg.WSURL) + httpUrl := commonconfig.MustParseURL(*nodeCfg.HTTPURL) + node := &toml.Node{ + Name: nodeCfg.Name, + WSURL: wsUrl, + HTTPURL: httpUrl, + SendOnly: nodeCfg.SendOnly, + Order: nodeCfg.Order, + } + nodes[i] = node + } + + if err := validateNodeConfigs(nodes); err != nil { + return nil, err + } + + return nodes, nil +} + +func validateNodeConfigs(nodes []*toml.Node) (err error) { + names := commonconfig.UniqueStrings{} + wsURLs := commonconfig.UniqueStrings{} + httpURLs := commonconfig.UniqueStrings{} + for i, node := range nodes { + if nodeErr := node.ValidateConfig(); nodeErr != nil { + err = multierr.Append(err, nodeErr) + } + if names.IsDupe(node.Name) { + err = multierr.Append(err, commonconfig.NewErrDuplicate(fmt.Sprintf("Nodes.%d.Name", i), *node.Name)) + } + u := (*url.URL)(node.WSURL) + if wsURLs.IsDupeFmt(u) { + err = multierr.Append(err, commonconfig.NewErrDuplicate(fmt.Sprintf("Nodes.%d.WSURL", i), u.String())) + } + u = (*url.URL)(node.HTTPURL) + if httpURLs.IsDupeFmt(u) { + err = multierr.Append(err, commonconfig.NewErrDuplicate(fmt.Sprintf("Nodes.%d.HTTPURL", i), u.String())) + } + } + + return err +} diff --git a/core/chains/evm/client/config_builder_test.go b/core/chains/evm/client/config_builder_test.go new file mode 100644 index 00000000000..cc00029d270 --- /dev/null +++ b/core/chains/evm/client/config_builder_test.go @@ -0,0 +1,181 @@ +package client_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" +) + +func TestClientConfigBuilder(t *testing.T) { + t.Parallel() + + selectionMode := ptr("HighestHead") + leaseDuration := 0 * time.Second + pollFailureThreshold := ptr(uint32(5)) + pollInterval := 10 * time.Second + syncThreshold := ptr(uint32(5)) + nodeIsSyncingEnabled := ptr(false) + chainTypeStr := "" + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo"), + WSURL: ptr("ws://foo.test"), + HTTPURL: ptr("http://foo.test"), + }, + } + nodePool, nodes, chainType, err := client.NewClientConfigs(selectionMode, leaseDuration, chainTypeStr, nodeConfigs, pollFailureThreshold, pollInterval, syncThreshold, nodeIsSyncingEnabled) + require.NoError(t, err) + + // Validate node pool configs + require.Equal(t, *selectionMode, nodePool.SelectionMode()) + require.Equal(t, leaseDuration, nodePool.LeaseDuration()) + require.Equal(t, *pollFailureThreshold, nodePool.PollFailureThreshold()) + require.Equal(t, pollInterval, nodePool.PollInterval()) + require.Equal(t, *syncThreshold, nodePool.SyncThreshold()) + require.Equal(t, *nodeIsSyncingEnabled, nodePool.NodeIsSyncingEnabled()) + + // Validate node configs + require.Equal(t, *nodeConfigs[0].Name, *nodes[0].Name) + require.Equal(t, *nodeConfigs[0].WSURL, (*nodes[0].WSURL).String()) + require.Equal(t, *nodeConfigs[0].HTTPURL, (*nodes[0].HTTPURL).String()) + + // Validate chain type + require.Equal(t, chainTypeStr, string(chainType)) +} + +func TestNodeConfigs(t *testing.T) { + t.Parallel() + + t.Run("parsing unique node configs succeeds", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + WSURL: ptr("ws://foo1.test"), + HTTPURL: ptr("http://foo1.test"), + }, + { + Name: ptr("foo2"), + WSURL: ptr("ws://foo2.test"), + HTTPURL: ptr("http://foo2.test"), + }, + } + tomlNodes, err := client.ParseTestNodeConfigs(nodeConfigs) + require.NoError(t, err) + require.Len(t, tomlNodes, len(nodeConfigs)) + }) + + t.Run("parsing missing ws url fails", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + HTTPURL: ptr("http://foo1.test"), + }, + } + _, err := client.ParseTestNodeConfigs(nodeConfigs) + require.Error(t, err) + }) + + t.Run("parsing missing http url fails", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + WSURL: ptr("ws://foo1.test"), + }, + } + _, err := client.ParseTestNodeConfigs(nodeConfigs) + require.Error(t, err) + }) + + t.Run("parsing invalid ws url fails", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + WSURL: ptr("http://foo1.test"), + HTTPURL: ptr("http://foo1.test"), + }, + } + _, err := client.ParseTestNodeConfigs(nodeConfigs) + require.Error(t, err) + }) + + t.Run("parsing duplicate http url fails", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + WSURL: ptr("ws://foo1.test"), + HTTPURL: ptr("ws://foo1.test"), + }, + } + _, err := client.ParseTestNodeConfigs(nodeConfigs) + require.Error(t, err) + }) + + t.Run("parsing duplicate node names fails", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + WSURL: ptr("ws://foo1.test"), + HTTPURL: ptr("http://foo1.test"), + }, + { + Name: ptr("foo1"), + WSURL: ptr("ws://foo2.test"), + HTTPURL: ptr("http://foo2.test"), + }, + } + _, err := client.ParseTestNodeConfigs(nodeConfigs) + require.Error(t, err) + }) + + t.Run("parsing duplicate node ws urls fails", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + WSURL: ptr("ws://foo1.test"), + HTTPURL: ptr("http://foo1.test"), + }, + { + Name: ptr("foo2"), + WSURL: ptr("ws://foo2.test"), + HTTPURL: ptr("http://foo1.test"), + }, + } + _, err := client.ParseTestNodeConfigs(nodeConfigs) + require.Error(t, err) + }) + + t.Run("parsing duplicate node http urls fails", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + WSURL: ptr("ws://foo1.test"), + HTTPURL: ptr("http://foo1.test"), + }, + { + Name: ptr("foo2"), + WSURL: ptr("ws://foo1.test"), + HTTPURL: ptr("http://foo2.test"), + }, + } + _, err := client.ParseTestNodeConfigs(nodeConfigs) + require.Error(t, err) + }) + + t.Run("parsing order too large fails", func(t *testing.T) { + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo1"), + WSURL: ptr("ws://foo1.test"), + HTTPURL: ptr("http://foo1.test"), + Order: ptr(int32(101)), + }, + } + _, err := client.ParseTestNodeConfigs(nodeConfigs) + require.Error(t, err) + }) +} + +func ptr[T any](t T) *T { return &t } diff --git a/core/chains/evm/client/evm_client.go b/core/chains/evm/client/evm_client.go new file mode 100644 index 00000000000..cd7d4a74b80 --- /dev/null +++ b/core/chains/evm/client/evm_client.go @@ -0,0 +1,37 @@ +package client + +import ( + "math/big" + "net/url" + "time" + + "github.com/smartcontractkit/chainlink-common/pkg/logger" + commonclient "github.com/smartcontractkit/chainlink/v2/common/client" + "github.com/smartcontractkit/chainlink/v2/common/config" + evmconfig "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml" + evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" +) + +func NewEvmClient(cfg evmconfig.NodePool, noNewHeadsThreshold time.Duration, lggr logger.Logger, chainID *big.Int, chainType config.ChainType, nodes []*toml.Node) Client { + var empty url.URL + var primaries []commonclient.Node[*big.Int, *evmtypes.Head, RPCClient] + var sendonlys []commonclient.SendOnlyNode[*big.Int, RPCClient] + for i, node := range nodes { + if node.SendOnly != nil && *node.SendOnly { + rpc := NewRPCClient(lggr, empty, (*url.URL)(node.HTTPURL), *node.Name, int32(i), chainID, + commonclient.Secondary) + sendonly := commonclient.NewSendOnlyNode(lggr, (url.URL)(*node.HTTPURL), + *node.Name, chainID, rpc) + sendonlys = append(sendonlys, sendonly) + } else { + rpc := NewRPCClient(lggr, (url.URL)(*node.WSURL), (*url.URL)(node.HTTPURL), *node.Name, int32(i), + chainID, commonclient.Primary) + primaryNode := commonclient.NewNode(cfg, noNewHeadsThreshold, + lggr, (url.URL)(*node.WSURL), (*url.URL)(node.HTTPURL), *node.Name, int32(i), chainID, *node.Order, + rpc, "EVM") + primaries = append(primaries, primaryNode) + } + } + return NewChainClient(lggr, cfg.SelectionMode(), cfg.LeaseDuration(), noNewHeadsThreshold, primaries, sendonlys, chainID, chainType) +} diff --git a/core/chains/evm/client/evm_client_test.go b/core/chains/evm/client/evm_client_test.go new file mode 100644 index 00000000000..2764a2b3611 --- /dev/null +++ b/core/chains/evm/client/evm_client_test.go @@ -0,0 +1,37 @@ +package client_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/logger" +) + +func TestNewEvmClient(t *testing.T) { + t.Parallel() + + noNewHeadsThreshold := 3 * time.Minute + selectionMode := ptr("HighestHead") + leaseDuration := 0 * time.Second + pollFailureThreshold := ptr(uint32(5)) + pollInterval := 10 * time.Second + syncThreshold := ptr(uint32(5)) + nodeIsSyncingEnabled := ptr(false) + chainTypeStr := "" + nodeConfigs := []client.TestNodeConfig{ + { + Name: ptr("foo"), + WSURL: ptr("ws://foo.test"), + HTTPURL: ptr("http://foo.test"), + }, + } + nodePool, nodes, chainType, err := client.NewClientConfigs(selectionMode, leaseDuration, chainTypeStr, nodeConfigs, pollFailureThreshold, pollInterval, syncThreshold, nodeIsSyncingEnabled) + require.NoError(t, err) + + client := client.NewEvmClient(nodePool, noNewHeadsThreshold, logger.TestLogger(t), testutils.FixtureChainID, chainType, nodes) + require.NotNil(t, client) +} diff --git a/core/chains/evm/client/helpers_test.go b/core/chains/evm/client/helpers_test.go index e400d95bef4..1decf3ed89d 100644 --- a/core/chains/evm/client/helpers_test.go +++ b/core/chains/evm/client/helpers_test.go @@ -14,6 +14,7 @@ import ( commonclient "github.com/smartcontractkit/chainlink/v2/common/client" commonconfig "github.com/smartcontractkit/chainlink/v2/common/config" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" ) @@ -185,3 +186,9 @@ func (mes *mockSubscription) Unsubscribe() { mes.unsubscribed = true close(mes.Errors) } + +type TestNodeConfig = nodeConfig + +func ParseTestNodeConfigs(nodes []TestNodeConfig) ([]*toml.Node, error) { + return parseNodeConfigs(nodes) +} diff --git a/core/chains/evm/config/chain_scoped.go b/core/chains/evm/config/chain_scoped.go index 9247e77ba9d..aa13b9a2282 100644 --- a/core/chains/evm/config/chain_scoped.go +++ b/core/chains/evm/config/chain_scoped.go @@ -166,7 +166,7 @@ func (e *evmConfig) MinIncomingConfirmations() uint32 { } func (e *evmConfig) NodePool() NodePool { - return &nodePoolConfig{c: e.c.NodePool} + return &NodePoolConfig{C: e.c.NodePool} } func (e *evmConfig) NodeNoNewHeadsThreshold() time.Duration { diff --git a/core/chains/evm/config/chain_scoped_node_pool.go b/core/chains/evm/config/chain_scoped_node_pool.go index fc52caa0aa3..0796d004cae 100644 --- a/core/chains/evm/config/chain_scoped_node_pool.go +++ b/core/chains/evm/config/chain_scoped_node_pool.go @@ -6,30 +6,30 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml" ) -type nodePoolConfig struct { - c toml.NodePool +type NodePoolConfig struct { + C toml.NodePool } -func (n *nodePoolConfig) PollFailureThreshold() uint32 { - return *n.c.PollFailureThreshold +func (n *NodePoolConfig) PollFailureThreshold() uint32 { + return *n.C.PollFailureThreshold } -func (n *nodePoolConfig) PollInterval() time.Duration { - return n.c.PollInterval.Duration() +func (n *NodePoolConfig) PollInterval() time.Duration { + return n.C.PollInterval.Duration() } -func (n *nodePoolConfig) SelectionMode() string { - return *n.c.SelectionMode +func (n *NodePoolConfig) SelectionMode() string { + return *n.C.SelectionMode } -func (n *nodePoolConfig) SyncThreshold() uint32 { - return *n.c.SyncThreshold +func (n *NodePoolConfig) SyncThreshold() uint32 { + return *n.C.SyncThreshold } -func (n *nodePoolConfig) LeaseDuration() time.Duration { - return n.c.LeaseDuration.Duration() +func (n *NodePoolConfig) LeaseDuration() time.Duration { + return n.C.LeaseDuration.Duration() } -func (n *nodePoolConfig) NodeIsSyncingEnabled() bool { - return *n.c.NodeIsSyncingEnabled +func (n *NodePoolConfig) NodeIsSyncingEnabled() bool { + return *n.C.NodeIsSyncingEnabled } diff --git a/core/chains/legacyevm/chain.go b/core/chains/legacyevm/chain.go index 50e6d3914c6..77ae62acd21 100644 --- a/core/chains/legacyevm/chain.go +++ b/core/chains/legacyevm/chain.go @@ -5,8 +5,6 @@ import ( "errors" "fmt" "math/big" - "net/url" - "time" gotoml "github.com/pelletier/go-toml/v2" "go.uber.org/multierr" @@ -18,10 +16,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/types" "github.com/smartcontractkit/chainlink-common/pkg/utils/mailbox" - commonclient "github.com/smartcontractkit/chainlink/v2/common/client" - commonconfig "github.com/smartcontractkit/chainlink/v2/common/config" "github.com/smartcontractkit/chainlink/v2/core/chains" - "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" evmconfig "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml" @@ -170,7 +165,7 @@ type ChainOpts struct { // TODO BCF-2513 remove test code from the API // Gen-functions are useful for dependency injection by tests - GenEthClient func(*big.Int) client.Client + GenEthClient func(*big.Int) evmclient.Client GenLogBroadcaster func(*big.Int) log.Broadcaster GenLogPoller func(*big.Int) logpoller.LogPoller GenHeadTracker func(*big.Int, httypes.HeadBroadcaster) httypes.HeadTracker @@ -218,7 +213,7 @@ func newChain(ctx context.Context, cfg *evmconfig.ChainScoped, nodes []*toml.Nod if !cfg.EVMRPCEnabled() { client = evmclient.NewNullClient(chainID, l) } else if opts.GenEthClient == nil { - client = newEthClientFromCfg(cfg.EVM().NodePool(), cfg.EVM().NodeNoNewHeadsThreshold(), l, chainID, chainType, nodes) + client = evmclient.NewEvmClient(cfg.EVM().NodePool(), cfg.EVM().NodeNoNewHeadsThreshold(), l, chainID, chainType, nodes) } else { client = opts.GenEthClient(chainID) } @@ -468,26 +463,3 @@ func (c *chain) HeadTracker() httypes.HeadTracker { return c.headTracker func (c *chain) Logger() logger.Logger { return c.logger } func (c *chain) BalanceMonitor() monitor.BalanceMonitor { return c.balanceMonitor } func (c *chain) GasEstimator() gas.EvmFeeEstimator { return c.gasEstimator } - -func newEthClientFromCfg(cfg evmconfig.NodePool, noNewHeadsThreshold time.Duration, lggr logger.Logger, chainID *big.Int, chainType commonconfig.ChainType, nodes []*toml.Node) evmclient.Client { - var empty url.URL - var primaries []commonclient.Node[*big.Int, *evmtypes.Head, evmclient.RPCClient] - var sendonlys []commonclient.SendOnlyNode[*big.Int, evmclient.RPCClient] - for i, node := range nodes { - if node.SendOnly != nil && *node.SendOnly { - rpc := evmclient.NewRPCClient(lggr, empty, (*url.URL)(node.HTTPURL), *node.Name, int32(i), chainID, - commonclient.Secondary) - sendonly := commonclient.NewSendOnlyNode[*big.Int, evmclient.RPCClient](lggr, (url.URL)(*node.HTTPURL), - *node.Name, chainID, rpc) - sendonlys = append(sendonlys, sendonly) - } else { - rpc := evmclient.NewRPCClient(lggr, (url.URL)(*node.WSURL), (*url.URL)(node.HTTPURL), *node.Name, int32(i), - chainID, commonclient.Primary) - primaryNode := commonclient.NewNode[*big.Int, *evmtypes.Head, evmclient.RPCClient](cfg, noNewHeadsThreshold, - lggr, (url.URL)(*node.WSURL), (*url.URL)(node.HTTPURL), *node.Name, int32(i), chainID, *node.Order, - rpc, "EVM") - primaries = append(primaries, primaryNode) - } - } - return evmclient.NewChainClient(lggr, cfg.SelectionMode(), cfg.LeaseDuration(), noNewHeadsThreshold, primaries, sendonlys, chainID, chainType) -}