Skip to content

Commit

Permalink
fix: explicity define v1 nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
darrenvechain committed Jun 5, 2024
1 parent 25c7a8b commit 4ce5e48
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 54 deletions.
2 changes: 1 addition & 1 deletion cmd/thor/p2p/p2p.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
26 changes: 9 additions & 17 deletions p2psrv/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -70,7 +69,6 @@ func New(opts *Options, version string) *Server {
knownNodes: knownNodes,
discoveredNodes: discoveredNodes,
dialingNodes: newNodeMap(),
version: version,
}
}

Expand All @@ -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())
Expand Down Expand Up @@ -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"
}
49 changes: 13 additions & 36 deletions p2psrv/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
})
}
Expand Down

0 comments on commit 4ce5e48

Please sign in to comment.