node: fix url handling of the value to --suiWS

Fixes: #2827

Previously, it prepended `ws://` to the address unlike any of the other
websocket flags. This allows specifying it the same was as guardiand
v2.16.0 or like the rest. In the future, we can remove the "legacy"
way and make them all consistent.

node/pkg/watchers/sui/watcher.go

@@ -267,6 +267,13 @@ func (e *Watcher) Run(ctx context.Context) error {
 
 	logger := supervisor.Logger(ctx)
 
+	// guardiand v2.16.0 shipped hardcoding "ws://" for the websocket url. This makes
+	// the flag value the same as all of the other uses of rpc websocket values.
+	err := e.fixSuiWsURL(logger)
+	if err != nil {
+		return err
+	}
+
 	logger.Info("Starting watcher",
 		zap.String("watcher_name", "sui"),
 		zap.String("suiRPC", e.suiRPC),
@@ -275,12 +282,7 @@ func (e *Watcher) Run(ctx context.Context) error {
 		zap.Bool("unsafeDevMode", e.unsafeDevMode),
 	)
 
-	u := url.URL{Scheme: "ws", Host: e.suiWS}
-
-	logger.Info("Sui watcher connecting to WS node ", zap.String("url", u.String()))
-	logger.Debug("SUI watcher:", zap.String("suiRPC", e.suiRPC), zap.String("suiWS", e.suiWS), zap.String("suiMoveEventType", e.suiMoveEventType))
-
-	ws, _, err := websocket.Dial(ctx, u.String(), nil)
+	ws, _, err := websocket.Dial(ctx, e.suiWS, nil)
 	if err != nil {
 		p2p.DefaultRegistry.AddErrorCount(vaa.ChainIDSui, 1)
 		suiConnectionErrors.WithLabelValues("websocket_dial_error").Inc()
@@ -498,3 +500,18 @@ func (e *Watcher) Run(ctx context.Context) error {
 		return err
 	}
 }
+
+// fixSuiWsURL ensures the websocket scheme is properly specified
+func (e *Watcher) fixSuiWsURL(logger *zap.Logger) error {
+	u, _ := url.Parse(e.suiWS)
+
+	// When the scheme is empty/nil or when the Host is empty but a scheme eis set
+	if u == nil || u.Scheme == "" || (u.Scheme != "" && u.Host == "") {
+		logger.Warn(fmt.Sprintf("DEPRECATED: Prefix --suiWS address with the url scheme e.g.: ws://%s or wss://%s", e.suiWS, e.suiWS))
+		u = &url.URL{Scheme: "ws", Host: e.suiWS}
+	} else if u.Scheme != "ws" && u.Scheme != "wss" {
+		return fmt.Errorf("invalid url scheme specified for --suiWS, try ws:// or wss://: %s", e.suiWS)
+	}
+	e.suiWS = u.String()
+	return nil
+}

node/pkg/watchers/sui/watcher_test.go

@@ -0,0 +1,84 @@
+package sui
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest/observer"
+)
+
+func Test_fixSuiWsURL(t *testing.T) {
+	tests := []struct {
+		name       string
+		value      string
+		expected   string
+		logMessage string
+		errMessage string
+	}{
+		{
+			name:     "valid",
+			value:    "ws://1.2.3.4:5678",
+			expected: "ws://1.2.3.4:5678",
+		},
+		{
+			name:       "tilt",
+			value:      "sui:9000",
+			expected:   "ws://sui:9000",
+			logMessage: "DEPRECATED: Prefix --suiWS address with the url scheme e.g.: ws://sui:9000 or wss://sui:9000",
+		},
+		{
+			name:     "valid-no-port",
+			value:    "ws://1.2.3.4",
+			expected: "ws://1.2.3.4",
+		},
+		{
+			name:       "no-scheme",
+			value:      "1.2.3.4:5678",
+			expected:   "ws://1.2.3.4:5678",
+			logMessage: "DEPRECATED: Prefix --suiWS address with the url scheme e.g.: ws://1.2.3.4:5678 or wss://1.2.3.4:5678",
+		},
+		{
+			name:       "no-scheme-no-port",
+			value:      "1.2.3.4",
+			expected:   "ws://1.2.3.4",
+			logMessage: "DEPRECATED: Prefix --suiWS address with the url scheme e.g.: ws://1.2.3.4 or wss://1.2.3.4",
+		},
+		{
+			name:       "wrong-scheme",
+			value:      "http://1.2.3.4",
+			errMessage: "invalid url scheme specified for --suiWS, try ws:// or wss://: http://1.2.3.4",
+		},
+	}
+	for _, testCase := range tests {
+		t.Run(testCase.name, func(t *testing.T) {
+			testCore, logs := observer.New(zap.InfoLevel)
+			testLogger := zap.New(testCore)
+
+			suiWatcher := Watcher{
+				suiWS: testCase.value,
+			}
+			err := suiWatcher.fixSuiWsURL(testLogger)
+			if testCase.errMessage != "" {
+				require.EqualError(t, err, testCase.errMessage)
+			} else {
+				require.NoError(t, err)
+				// Only verify the value if no error was returned
+				assert.Equal(t, testCase.expected, suiWatcher.suiWS)
+			}
+
+			if len(testCase.logMessage) != 0 {
+				// If the testcase expects a log, then there should only be 1 log
+				require.Equal(t, 1, logs.Len())
+
+				// Ensure the log message is correct
+				actualLogMessage := logs.All()[0].Message
+				require.Equal(t, testCase.logMessage, actualLogMessage)
+			} else {
+				// If the testcase does not expect a log, none should be emitted
+				require.Equal(t, 0, logs.Len())
+			}
+		})
+	}
+}