Skip to content

Commit

Permalink
[Gateway] Fix address lookup in handlers (#10637)
Browse files Browse the repository at this point in the history
Making sure config is always lowercased and adding more nil checks
  • Loading branch information
bolekk authored Sep 14, 2023
1 parent d6363e0 commit d43a809
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 1 deletion.
12 changes: 11 additions & 1 deletion core/services/gateway/connectionmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ func NewConnectionManager(gwConfig *config.GatewayConfig, clock utils.Clock, lgg
return nil, fmt.Errorf("duplicate node address %s in DON %s", nodeAddress, donConfig.DonId)
}
nodes[nodeAddress] = &nodeState{conn: network.NewWSConnectionWrapper()}
if nodes[nodeAddress].conn == nil {
return nil, fmt.Errorf("error creating WSConnectionWrapper for node %s", nodeAddress)
}
}
dons[donConfig.DonId] = &donConnectionManager{
donConfig: &donConfig,
Expand Down Expand Up @@ -232,11 +235,18 @@ func (m *donConnectionManager) SetHandler(handler handlers.Handler) {
}

func (m *donConnectionManager) SendToNode(ctx context.Context, nodeAddress string, msg *api.Message) error {
if msg == nil {
return errors.New("nil message")
}
data, err := m.codec.EncodeRequest(msg)
if err != nil {
return fmt.Errorf("error encoding request for node %s: %v", nodeAddress, err)
}
return m.nodes[nodeAddress].conn.Write(ctx, websocket.BinaryMessage, data)
nodeState := m.nodes[nodeAddress]
if nodeState == nil {
return fmt.Errorf("node %s not found", nodeAddress)
}
return nodeState.conn.Write(ctx, websocket.BinaryMessage, data)
}

func (m *donConnectionManager) readLoop(nodeAddress string, nodeState *nodeState) {
Expand Down
19 changes: 19 additions & 0 deletions core/services/gateway/connectionmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (

"github.com/stretchr/testify/require"

"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
"github.com/smartcontractkit/chainlink/v2/core/logger"
"github.com/smartcontractkit/chainlink/v2/core/services/gateway"
"github.com/smartcontractkit/chainlink/v2/core/services/gateway/api"
gc "github.com/smartcontractkit/chainlink/v2/core/services/gateway/common"
"github.com/smartcontractkit/chainlink/v2/core/services/gateway/config"
"github.com/smartcontractkit/chainlink/v2/core/services/gateway/network"
Expand Down Expand Up @@ -208,3 +210,20 @@ func TestConnectionManager_FinalizeHandshake(t *testing.T) {
err = mgr.FinalizeHandshake(attemptId, response, nil)
require.ErrorIs(t, err, network.ErrChallengeInvalidSignature)
}

func TestConnectionManager_SendToNode_Failures(t *testing.T) {
t.Parallel()

config, nodes := newTestConfig(t, 2)
clock := utils.NewFixedClock(time.Now())
mgr, err := gateway.NewConnectionManager(config, clock, logger.TestLogger(t))
require.NoError(t, err)

donMgr := mgr.DONConnectionManager("my_don_1")
err = donMgr.SendToNode(testutils.Context(t), nodes[0].Address, nil)
require.Error(t, err)

message := &api.Message{}
err = donMgr.SendToNode(testutils.Context(t), "some_other_node", message)
require.Error(t, err)
}
9 changes: 9 additions & 0 deletions core/services/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import (
"context"
"encoding/json"
"fmt"
"strings"

"go.uber.org/multierr"

"github.com/ethereum/go-ethereum/common"

"github.com/smartcontractkit/chainlink/v2/core/logger"
"github.com/smartcontractkit/chainlink/v2/core/services/gateway/api"
"github.com/smartcontractkit/chainlink/v2/core/services/gateway/config"
Expand Down Expand Up @@ -59,6 +62,12 @@ func NewGatewayFromConfig(config *config.GatewayConfig, handlerFactory HandlerFa
if donConnMgr == nil {
return nil, fmt.Errorf("connection manager ID %s not found", donConfig.DonId)
}
for idx, nodeConfig := range donConfig.Members {
donConfig.Members[idx].Address = strings.ToLower(nodeConfig.Address)
if !common.IsHexAddress(nodeConfig.Address) {
return nil, fmt.Errorf("invalid node address %s", nodeConfig.Address)
}
}
handler, err := handlerFactory.NewHandler(donConfig.HandlerName, donConfig.HandlerConfig, &donConfig, donConnMgr)
if err != nil {
return nil, err
Expand Down
22 changes: 22 additions & 0 deletions core/services/gateway/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ HandlerName = "dummy"
[[dons]]
DonId = "my_don_2"
HandlerName = "dummy"
[[dons.Members]]
Name = "node one"
Address = "0x0001020304050607080900010203040506070809"
`)

lggr := logger.TestLogger(t)
Expand Down Expand Up @@ -102,6 +106,24 @@ SomeOtherField = "abcd"
require.Error(t, err)
}

func TestGateway_NewGatewayFromConfig_InvalidNodeAddress(t *testing.T) {
t.Parallel()

tomlConfig := buildConfig(`
[[dons]]
HandlerName = "dummy"
DonId = "my_don"
[[dons.Members]]
Name = "node one"
Address = "0xnot_an_address"
`)

lggr := logger.TestLogger(t)
_, err := gateway.NewGatewayFromConfig(parseTOMLConfig(t, tomlConfig), gateway.NewHandlerFactory(nil, lggr), lggr)
require.Error(t, err)
}

func TestGateway_CleanStartAndClose(t *testing.T) {
t.Parallel()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"crypto/ecdsa"
"fmt"
"net/http"
"strings"
"sync/atomic"
"testing"

Expand Down Expand Up @@ -110,6 +111,8 @@ func TestIntegration_Gateway_NoFullNodes_BasicConnectionAndMessage(t *testing.T)
t.Parallel()

nodeKeys := common.NewTestNodes(t, 1)[0]
// Verify that addresses in config are case-insensitive
nodeKeys.Address = strings.ToUpper(nodeKeys.Address)

// Launch Gateway
lggr := logger.TestLogger(t)
Expand Down

0 comments on commit d43a809

Please sign in to comment.