From c9588637ad4cc63a242329efcf648c9ce26118d1 Mon Sep 17 00:00:00 2001 From: Pavel Zbitskiy Date: Fri, 13 Sep 2024 12:50:56 -0400 Subject: [PATCH] config: ensure both ws and p2p net running the same mode in hybrid --- config/config_test.go | 36 +++++++++++++++++++++++++++++++++++ config/localTemplate.go | 11 +++++++++++ network/hybridNetwork.go | 4 ++++ network/hybridNetwork_test.go | 18 ++++++++++++++++++ 4 files changed, 69 insertions(+) diff --git a/config/config_test.go b/config/config_test.go index 1b1c4c2753..936622b06c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -751,6 +751,42 @@ func TestLocal_RecalculateConnectionLimits(t *testing.T) { } } +func TestLocal_ValidateP2PHybridConfig(t *testing.T) { + partitiontest.PartitionTest(t) + t.Parallel() + + var tests = []struct { + enableP2PHybridMode bool + p2pHybridNetAddress string + netAddress string + err bool + }{ + {false, "", "", false}, + {false, ":0", "", false}, + {false, "", ":0", false}, + {false, ":0", ":0", false}, + {true, "", "", false}, + {true, ":0", "", true}, + {true, "", ":0", true}, + {true, ":0", ":0", false}, + } + + for i, test := range tests { + test := test + t.Run(fmt.Sprintf("test=%d", i), func(t *testing.T) { + t.Parallel() + + c := Local{ + EnableP2PHybridMode: test.enableP2PHybridMode, + P2PHybridNetAddress: test.p2pHybridNetAddress, + NetAddress: test.netAddress, + } + err := c.ValidateP2PHybridConfig() + require.Equal(t, test.err, err != nil, "test=%d", i) + }) + } +} + // Tests that ensureAbsGenesisDir resolves a path to an absolute path, appends the genesis directory, and creates any needed directories func TestEnsureAbsDir(t *testing.T) { partitiontest.PartitionTest(t) diff --git a/config/localTemplate.go b/config/localTemplate.go index c03461c371..ca707be16f 100644 --- a/config/localTemplate.go +++ b/config/localTemplate.go @@ -17,6 +17,7 @@ package config import ( + "errors" "fmt" "os" "path/filepath" @@ -764,6 +765,16 @@ func (cfg Local) IsHybridServer() bool { return cfg.NetAddress != "" && cfg.P2PHybridNetAddress != "" && cfg.EnableP2PHybridMode } +// ValidateP2PHybridConfig checks if both NetAddress and P2PHybridNetAddress are set or unset in hybrid mode. +func (cfg Local) ValidateP2PHybridConfig() error { + if cfg.EnableP2PHybridMode { + if cfg.NetAddress == "" && cfg.P2PHybridNetAddress != "" || cfg.NetAddress != "" && cfg.P2PHybridNetAddress == "" { + return errors.New("both NetAddress and P2PHybridNetAddress must be set or unset") + } + } + return nil +} + // ensureAbsGenesisDir will convert a path to absolute, and will attempt to make a genesis directory there func ensureAbsGenesisDir(path string, genesisID string) (string, error) { pathAbs, err := filepath.Abs(path) diff --git a/network/hybridNetwork.go b/network/hybridNetwork.go index 85621260a9..e64b708a2f 100644 --- a/network/hybridNetwork.go +++ b/network/hybridNetwork.go @@ -38,7 +38,11 @@ type HybridP2PNetwork struct { } // NewHybridP2PNetwork constructs a GossipNode that combines P2PNetwork and WebsocketNetwork +// Hybrid mode requires both P2P and WS to be running in server (NetAddress set) or client (NetAddress empty) mode. func NewHybridP2PNetwork(log logging.Logger, cfg config.Local, datadir string, phonebookAddresses []string, genesisID string, networkID protocol.NetworkID, nodeInfo NodeInfo) (*HybridP2PNetwork, error) { + if err := cfg.ValidateP2PHybridConfig(); err != nil { + return nil, err + } // supply alternate NetAddress for P2P network p2pcfg := cfg p2pcfg.NetAddress = cfg.P2PHybridNetAddress diff --git a/network/hybridNetwork_test.go b/network/hybridNetwork_test.go index 4e1392a2d0..3fac0cefd0 100644 --- a/network/hybridNetwork_test.go +++ b/network/hybridNetwork_test.go @@ -181,3 +181,21 @@ func TestHybridNetwork_DuplicateConn(t *testing.T) { return len(netA.GetPeers(PeersConnectedIn)) == 2 }, 3*time.Second, 50*time.Millisecond) } + +func TestHybridNetwork_ValidateConfig(t *testing.T) { + partitiontest.PartitionTest(t) + t.Parallel() + + cfg := config.GetDefaultLocal() + cfg.EnableP2PHybridMode = true + cfg.NetAddress = ":0" + cfg.P2PHybridNetAddress = "" + + _, err := NewHybridP2PNetwork(logging.TestingLog(t), cfg, "", nil, genesisID, "net", &nopeNodeInfo{}) + require.ErrorContains(t, err, "both NetAddress and P2PHybridNetAddress") + + cfg.NetAddress = "" + cfg.P2PHybridNetAddress = ":0" + _, err = NewHybridP2PNetwork(logging.TestingLog(t), cfg, "", nil, genesisID, "net", &nopeNodeInfo{}) + require.ErrorContains(t, err, "both NetAddress and P2PHybridNetAddress") +}