diff --git a/cmd/thor/p2p/p2p.go b/cmd/thor/p2p/p2p.go index e41802253..86f067b39 100644 --- a/cmd/thor/p2p/p2p.go +++ b/cmd/thor/p2p/p2p.go @@ -78,7 +78,7 @@ func New( return &P2P{ comm: communicator, - p2pSrv: p2psrv.New(opts, version), + p2pSrv: p2psrv.New(opts), peersCachePath: peersCachePath, enode: fmt.Sprintf("enode://%x@[extip]:%v", discover.PubkeyID(&privateKey.PublicKey).Bytes(), listenPort), } diff --git a/p2psrv/server.go b/p2psrv/server.go index ea9b2b13e..040bbb3c3 100644 --- a/p2psrv/server.go +++ b/p2psrv/server.go @@ -38,11 +38,10 @@ type Server struct { knownNodes *cache.PrioCache discoveredNodes *cache.RandCache dialingNodes *nodeMap - version string } // New create a p2p server. -func New(opts *Options, version string) *Server { +func New(opts *Options) *Server { knownNodes := cache.NewPrioCache(5) discoveredNodes := cache.NewRandCache(128) for _, node := range opts.KnownNodes { @@ -70,7 +69,6 @@ func New(opts *Options, version string) *Server { knownNodes: knownNodes, discoveredNodes: discoveredNodes, dialingNodes: newNodeMap(), - version: version, } } @@ -91,8 +89,8 @@ func (s *Server) Start(protocols []*p2p.Protocol, topic discv5.Topic) error { dir = "inbound" } - if !sameMajor(s.version, peer.Name()) { - log.Warn("peer version mismatch", + if isIncompatible(peer.Name()) { + log.Warn("incompatible peer version", "name", peer.Name(), "address", peer.RemoteAddr().String(), "id", peer.ID().String()) @@ -392,24 +390,18 @@ func (s *Server) Options() *Options { return s.opts } -var versionRegex = regexp.MustCompile(`\d+\.\d+\.\d+`) - -// sameMajor returns true if the peer has the same major version -func sameMajor(appVersion, peerName string) bool { - if appVersion == "" || !versionRegex.MatchString(appVersion) { - // Got a bad app version, so accept any peer - return true - } +var versionRegex = regexp.MustCompile(`v\d+\.\d+\.\d+`) +// isIncompatible checks if the peer is running an incompatible version. +func isIncompatible(peerName string) bool { // Extract the semantic version from the name peerVersion := versionRegex.FindString(peerName) if peerVersion == "" { return false } - peerMajorVersion := strings.Split(peerVersion, ".")[0] - givenMajorVersion := strings.Split(appVersion, ".")[0] + major := strings.Split(peerVersion, ".")[0] - // Compare the major versions - return peerMajorVersion == givenMajorVersion + // Only v1 nodes are incompatible for now + return major == "v1" } diff --git a/p2psrv/server_test.go b/p2psrv/server_test.go index 1708376ab..bd87e2798 100644 --- a/p2psrv/server_test.go +++ b/p2psrv/server_test.go @@ -13,8 +13,6 @@ import ( "github.com/stretchr/testify/assert" ) -const version = "2.1.1-88c7c86-release" - func TestNewServer(t *testing.T) { privateKey, err := crypto.GenerateKey() if err != nil { @@ -33,7 +31,7 @@ func TestNewServer(t *testing.T) { KnownNodes: Nodes{node}, } - server := New(opts, version) + server := New(opts) assert.Equal(t, "testNode", server.opts.Name) assert.Equal(t, privateKey, server.opts.PrivateKey) @@ -64,7 +62,7 @@ func TestNewServerConnectOnly(t *testing.T) { KnownNodes: Nodes{knownNode}, } - server := New(opts, version) + server := New(opts) assert.Equal(t, "testNode", server.opts.Name) assert.Equal(t, privateKey, server.opts.PrivateKey) @@ -78,49 +76,28 @@ func TestNewServerConnectOnly(t *testing.T) { assert.True(t, server.knownNodes.Contains(knownNode.ID)) } -func TestSameMajor(t *testing.T) { +func TestIsIncompatible(t *testing.T) { testCases := []struct { - id string - appVersion string - peerName string - expected bool + peerName string + expected bool }{ { - id: "only-major-same", - appVersion: "2.0.0-88c7c86-release", - peerName: "thor/v2.1.1-88c7c86-release/linux/go1.21.9", - expected: true, - }, - { - id: "only-major-different", - appVersion: "2.1.1-88c7c86-release", - peerName: "thor/v1.1.1-88c7c86-release/linux/go1.21.9", - expected: false, - }, - { - id: "app-version-empty", - appVersion: "", - peerName: "thor/v1.1.1-88c7c86-release/linux/go1.21.9", - expected: true, + peerName: "thor/v2.1.1-88c7c86-release/linux/go1.21.9", + expected: false, }, { - id: "exact-match", - appVersion: "1.1.1-88c7c86-release", - peerName: "thor/v1.1.1-88c7c86-release/linux/go1.21.9", - expected: true, + peerName: "thor/v1.1.1-88c7c86-release/linux/go1.21.9", + expected: true, }, { - id: "bad-app-version", - // bad app version, so accept any peer - appVersion: "bad.bad.bad-88c7c86-release", - peerName: "thor/v1.1.1-88c7c86-release/linux/go1.21.9", - expected: true, + peerName: "thor/v3.1.1-88c7c86-release/linux/go1.21.9", + expected: false, }, } for _, tc := range testCases { - t.Run(tc.id, func(t *testing.T) { - result := sameMajor(tc.appVersion, tc.peerName) + t.Run(tc.peerName, func(t *testing.T) { + result := isIncompatible(tc.peerName) assert.Equal(t, tc.expected, result) }) }