From 5f27f7b444c6e328c918fe2614aa5b5ff0ceb669 Mon Sep 17 00:00:00 2001
From: Nina Barbakadze <barbakadzeninaa@gmail.com>
Date: Wed, 31 Jul 2024 15:35:34 +0200
Subject: [PATCH 1/5] fix: DNS TTL not respected
---
p2p/peer.go | 13 +++++++++----
p2p/peer_set_test.go | 1 +
p2p/switch.go | 38 +++++++++++++++++++++++++++-----------
3 files changed, 37 insertions(+), 15 deletions(-)
diff --git a/p2p/peer.go b/p2p/peer.go
index f43cff9d51..b77be635d8 100644
--- a/p2p/peer.go
+++ b/p2p/peer.go
@@ -32,6 +32,8 @@ type Peer interface {
IsOutbound() bool // did we dial the peer
IsPersistent() bool // do we redial this peer when we disconnect
+ IPHasChanged() bool // has the peer's IP changed
+
CloseConn() error // close original connection
NodeInfo() NodeInfo // peer's info
@@ -138,10 +140,6 @@ func (pc peerConn) ID() ID {
// Return the IP from the connection RemoteAddr
func (pc peerConn) RemoteIP() net.IP {
- if pc.ip != nil {
- return pc.ip
- }
-
host, _, err := net.SplitHostPort(pc.conn.RemoteAddr().String())
if err != nil {
panic(err)
@@ -300,6 +298,13 @@ func (p *peer) IsPersistent() bool {
return p.peerConn.persistent
}
+// IPHasChanged returns true and the new IP if the peer's IP has changed.
+func (p *peer) IPHasChanged() bool {
+ oldIP := p.ip
+ newIP := p.RemoteIP()
+ return !oldIP.Equal(newIP)
+}
+
// NodeInfo returns a copy of the peer's NodeInfo.
func (p *peer) NodeInfo() NodeInfo {
return p.nodeInfo
diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go
index 6501dd77a5..d1f9b5b139 100644
--- a/p2p/peer_set_test.go
+++ b/p2p/peer_set_test.go
@@ -27,6 +27,7 @@ func (mp *mockPeer) NodeInfo() NodeInfo { return DefaultNodeInfo{}
func (mp *mockPeer) Status() ConnectionStatus { return ConnectionStatus{} }
func (mp *mockPeer) ID() ID { return mp.id }
func (mp *mockPeer) IsOutbound() bool { return false }
+func (mp *mockPeer) IPHasChanged() bool { return true }
func (mp *mockPeer) IsPersistent() bool { return true }
func (mp *mockPeer) Get(s string) interface{} { return s }
func (mp *mockPeer) Set(string, interface{}) {}
diff --git a/p2p/switch.go b/p2p/switch.go
index af0607e037..140056f52c 100644
--- a/p2p/switch.go
+++ b/p2p/switch.go
@@ -381,22 +381,38 @@ func (sw *Switch) StopPeerForError(peer Peer, reason interface{}) {
sw.stopAndRemovePeer(peer, reason)
if peer.IsPersistent() {
- var addr *NetAddress
- if peer.IsOutbound() { // socket address for outbound peers
- addr = peer.SocketAddr()
- } else { // self-reported address for inbound peers
- var err error
- addr, err = peer.NodeInfo().NetAddress()
- if err != nil {
- sw.Logger.Error("Wanted to reconnect to inbound peer, but self-reported address is wrong",
- "peer", peer, "err", err)
- return
- }
+ addr, err := sw.getPeerAddress(peer)
+ if err != nil {
+ return
+ }
+ go sw.reconnectToPeer(addr)
+ }
+
+ if peer.IPHasChanged() {
+ addr, err := sw.getPeerAddress(peer)
+ if err != nil {
+ return
}
go sw.reconnectToPeer(addr)
}
}
+// getPeerAddress returns the appropriate NetAddress for a given peer,
+// handling both outbound and inbound peers.
+func (sw *Switch) getPeerAddress(peer Peer) (*NetAddress, error) {
+ if peer.IsOutbound() {
+ return peer.SocketAddr(), nil
+ }
+ // For inbound peers, get the self-reported address
+ addr, err := peer.NodeInfo().NetAddress()
+ if err != nil {
+ sw.Logger.Error("Failed to get address for inbound peer",
+ "peer", peer, "err", err)
+ return nil, err
+ }
+ return addr, nil
+}
+
// StopPeerGracefully disconnects from a peer gracefully.
// TODO: handle graceful disconnects.
func (sw *Switch) StopPeerGracefully(peer Peer) {
From e3dc4cfac30ad1fa9ebed937262fce1dc63a1323 Mon Sep 17 00:00:00 2001
From: Nina Barbakadze <barbakadzeninaa@gmail.com>
Date: Wed, 31 Jul 2024 16:18:27 +0200
Subject: [PATCH 2/5] fix: compile errors
---
blockchain/v2/reactor_test.go | 1 +
p2p/mock/peer.go | 1 +
p2p/mocks/peer.go | 14 ++++++++++++++
p2p/mocks/peer_envelope_sender.go | 14 ++++++++++++++
p2p/peer.go | 2 +-
p2p/peer_set_test.go | 2 +-
test/fuzz/p2p/pex/reactor_receive.go | 6 ++++--
7 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/blockchain/v2/reactor_test.go b/blockchain/v2/reactor_test.go
index d3231a4ae3..bb3afe4953 100644
--- a/blockchain/v2/reactor_test.go
+++ b/blockchain/v2/reactor_test.go
@@ -43,6 +43,7 @@ func (mp mockPeer) RemoteAddr() net.Addr { return &net.TCPAddr{IP: mp.RemoteIP()
func (mp mockPeer) IsOutbound() bool { return true }
func (mp mockPeer) IsPersistent() bool { return true }
+func (mp mockPeer) IPHasChanged() bool { return false }
func (mp mockPeer) CloseConn() error { return nil }
func (mp mockPeer) NodeInfo() p2p.NodeInfo {
diff --git a/p2p/mock/peer.go b/p2p/mock/peer.go
index 31ce856237..ca56085c04 100644
--- a/p2p/mock/peer.go
+++ b/p2p/mock/peer.go
@@ -57,6 +57,7 @@ func (mp *Peer) Status() conn.ConnectionStatus { return conn.ConnectionStatus{}
func (mp *Peer) ID() p2p.ID { return mp.id }
func (mp *Peer) IsOutbound() bool { return mp.Outbound }
func (mp *Peer) IsPersistent() bool { return mp.Persistent }
+func (mp *Peer) IPHasChanged() bool { return false }
func (mp *Peer) Get(key string) interface{} {
if value, ok := mp.kv[key]; ok {
return value
diff --git a/p2p/mocks/peer.go b/p2p/mocks/peer.go
index 92f0106e16..aa37f65d3d 100644
--- a/p2p/mocks/peer.go
+++ b/p2p/mocks/peer.go
@@ -81,6 +81,20 @@ func (_m *Peer) ID() p2p.ID {
return r0
}
+// IPHasChanged provides a mock function with given fields:
+func (_m *Peer) IPHasChanged() bool {
+ ret := _m.Called()
+
+ var r0 bool
+ if rf, ok := ret.Get(0).(func() bool); ok {
+ r0 = rf()
+ } else {
+ r0 = ret.Get(0).(bool)
+ }
+
+ return r0
+}
+
// IsOutbound provides a mock function with given fields:
func (_m *Peer) IsOutbound() bool {
ret := _m.Called()
diff --git a/p2p/mocks/peer_envelope_sender.go b/p2p/mocks/peer_envelope_sender.go
index 89f231104d..b896c3adac 100644
--- a/p2p/mocks/peer_envelope_sender.go
+++ b/p2p/mocks/peer_envelope_sender.go
@@ -109,6 +109,20 @@ func (_m *PeerEnvelopeSender) IsPersistent() bool {
return r0
}
+// IPHasChanged provides a mock function for given fields:
+func (_m *PeerEnvelopeSender) IPHasChanged() bool {
+ ret := _m.Called()
+
+ var r0 bool
+ if rf, ok := ret.Get(0).(func() bool); ok {
+ r0 = rf()
+ } else {
+ r0 = ret.Get(0).(bool)
+ }
+
+ return r0
+}
+
// IsRunning provides a mock function with given fields:
func (_m *PeerEnvelopeSender) IsRunning() bool {
ret := _m.Called()
diff --git a/p2p/peer.go b/p2p/peer.go
index b77be635d8..8912c81cb9 100644
--- a/p2p/peer.go
+++ b/p2p/peer.go
@@ -302,7 +302,7 @@ func (p *peer) IsPersistent() bool {
func (p *peer) IPHasChanged() bool {
oldIP := p.ip
newIP := p.RemoteIP()
- return !oldIP.Equal(newIP)
+ return !oldIP.Equal(newIP)
}
// NodeInfo returns a copy of the peer's NodeInfo.
diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go
index d1f9b5b139..5ab936a57c 100644
--- a/p2p/peer_set_test.go
+++ b/p2p/peer_set_test.go
@@ -27,7 +27,7 @@ func (mp *mockPeer) NodeInfo() NodeInfo { return DefaultNodeInfo{}
func (mp *mockPeer) Status() ConnectionStatus { return ConnectionStatus{} }
func (mp *mockPeer) ID() ID { return mp.id }
func (mp *mockPeer) IsOutbound() bool { return false }
-func (mp *mockPeer) IPHasChanged() bool { return true }
+func (mp *mockPeer) IPHasChanged() bool { return false }
func (mp *mockPeer) IsPersistent() bool { return true }
func (mp *mockPeer) Get(s string) interface{} { return s }
func (mp *mockPeer) Set(string, interface{}) {}
diff --git a/test/fuzz/p2p/pex/reactor_receive.go b/test/fuzz/p2p/pex/reactor_receive.go
index be9c6bba0f..861e51bd28 100644
--- a/test/fuzz/p2p/pex/reactor_receive.go
+++ b/test/fuzz/p2p/pex/reactor_receive.go
@@ -74,8 +74,10 @@ func (fp *fuzzPeer) RemoteIP() net.IP { return net.IPv4(0, 0, 0, 0) }
func (fp *fuzzPeer) RemoteAddr() net.Addr {
return &net.TCPAddr{IP: fp.RemoteIP(), Port: 98991, Zone: ""}
}
-func (fp *fuzzPeer) IsOutbound() bool { return false }
-func (fp *fuzzPeer) IsPersistent() bool { return false }
+func (fp *fuzzPeer) IsOutbound() bool { return false }
+func (fp *fuzzPeer) IsPersistent() bool { return false }
+func (fp *fuzzPeer) IPHasChanged() bool { return false }
+
func (fp *fuzzPeer) CloseConn() error { return nil }
func (fp *fuzzPeer) NodeInfo() p2p.NodeInfo { return defaultNodeInfo }
func (fp *fuzzPeer) Status() p2p.ConnectionStatus { var cs p2p.ConnectionStatus; return cs }
From c80f553347aaf0be1469bd9b3ac15bdd2c60bc6d Mon Sep 17 00:00:00 2001
From: Nina Barbakadze <barbakadzeninaa@gmail.com>
Date: Wed, 31 Jul 2024 16:33:41 +0200
Subject: [PATCH 3/5] fix: do not refetch the ip if current ip is zero
---
p2p/peer.go | 3 +++
1 file changed, 3 insertions(+)
diff --git a/p2p/peer.go b/p2p/peer.go
index 8912c81cb9..1076fe7f45 100644
--- a/p2p/peer.go
+++ b/p2p/peer.go
@@ -301,6 +301,9 @@ func (p *peer) IsPersistent() bool {
// IPHasChanged returns true and the new IP if the peer's IP has changed.
func (p *peer) IPHasChanged() bool {
oldIP := p.ip
+ if oldIP == nil {
+ return false
+ }
newIP := p.RemoteIP()
return !oldIP.Equal(newIP)
}
From 02032accf9161daf63d04b76df998f8ae7dc0fa3 Mon Sep 17 00:00:00 2001
From: Nina Barbakadze <barbakadzeninaa@gmail.com>
Date: Wed, 31 Jul 2024 16:50:11 +0200
Subject: [PATCH 4/5] refactor: make errors more specific
---
p2p/switch.go | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/p2p/switch.go b/p2p/switch.go
index 140056f52c..0795049ff9 100644
--- a/p2p/switch.go
+++ b/p2p/switch.go
@@ -383,6 +383,7 @@ func (sw *Switch) StopPeerForError(peer Peer, reason interface{}) {
if peer.IsPersistent() {
addr, err := sw.getPeerAddress(peer)
if err != nil {
+ sw.Logger.Error("Failed to get address for persistent peer", "peer", peer, "err", err)
return
}
go sw.reconnectToPeer(addr)
@@ -391,7 +392,7 @@ func (sw *Switch) StopPeerForError(peer Peer, reason interface{}) {
if peer.IPHasChanged() {
addr, err := sw.getPeerAddress(peer)
if err != nil {
- return
+ sw.Logger.Error("Failed to get address for peer with changed IP", "peer", peer, "err", err)
}
go sw.reconnectToPeer(addr)
}
From 9c47113e275bef1af2cf36263061ed35288b890e Mon Sep 17 00:00:00 2001
From: Nina Barbakadze <barbakadzeninaa@gmail.com>
Date: Wed, 31 Jul 2024 17:26:44 +0200
Subject: [PATCH 5/5] refactor: address review comments
---
blockchain/v2/reactor_test.go | 2 +-
p2p/mock/peer.go | 2 +-
p2p/mocks/peer.go | 4 ++--
p2p/mocks/peer_envelope_sender.go | 4 ++--
p2p/peer.go | 12 +++++++++---
p2p/peer_set_test.go | 2 +-
p2p/switch.go | 2 +-
test/fuzz/p2p/pex/reactor_receive.go | 2 +-
8 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/blockchain/v2/reactor_test.go b/blockchain/v2/reactor_test.go
index bb3afe4953..02c991cc6d 100644
--- a/blockchain/v2/reactor_test.go
+++ b/blockchain/v2/reactor_test.go
@@ -43,7 +43,7 @@ func (mp mockPeer) RemoteAddr() net.Addr { return &net.TCPAddr{IP: mp.RemoteIP()
func (mp mockPeer) IsOutbound() bool { return true }
func (mp mockPeer) IsPersistent() bool { return true }
-func (mp mockPeer) IPHasChanged() bool { return false }
+func (mp mockPeer) HasIPChanged() bool { return false }
func (mp mockPeer) CloseConn() error { return nil }
func (mp mockPeer) NodeInfo() p2p.NodeInfo {
diff --git a/p2p/mock/peer.go b/p2p/mock/peer.go
index ca56085c04..0ceea3c538 100644
--- a/p2p/mock/peer.go
+++ b/p2p/mock/peer.go
@@ -57,7 +57,7 @@ func (mp *Peer) Status() conn.ConnectionStatus { return conn.ConnectionStatus{}
func (mp *Peer) ID() p2p.ID { return mp.id }
func (mp *Peer) IsOutbound() bool { return mp.Outbound }
func (mp *Peer) IsPersistent() bool { return mp.Persistent }
-func (mp *Peer) IPHasChanged() bool { return false }
+func (mp *Peer) HasIPChanged() bool { return false }
func (mp *Peer) Get(key string) interface{} {
if value, ok := mp.kv[key]; ok {
return value
diff --git a/p2p/mocks/peer.go b/p2p/mocks/peer.go
index aa37f65d3d..8086127154 100644
--- a/p2p/mocks/peer.go
+++ b/p2p/mocks/peer.go
@@ -81,8 +81,8 @@ func (_m *Peer) ID() p2p.ID {
return r0
}
-// IPHasChanged provides a mock function with given fields:
-func (_m *Peer) IPHasChanged() bool {
+// HasIPChanged provides a mock function with given fields:
+func (_m *Peer) HasIPChanged() bool {
ret := _m.Called()
var r0 bool
diff --git a/p2p/mocks/peer_envelope_sender.go b/p2p/mocks/peer_envelope_sender.go
index b896c3adac..56f2d4a063 100644
--- a/p2p/mocks/peer_envelope_sender.go
+++ b/p2p/mocks/peer_envelope_sender.go
@@ -109,8 +109,8 @@ func (_m *PeerEnvelopeSender) IsPersistent() bool {
return r0
}
-// IPHasChanged provides a mock function for given fields:
-func (_m *PeerEnvelopeSender) IPHasChanged() bool {
+// HasIPChanged provides a mock function for given fields:
+func (_m *PeerEnvelopeSender) HasIPChanged() bool {
ret := _m.Called()
var r0 bool
diff --git a/p2p/peer.go b/p2p/peer.go
index 1076fe7f45..bb04cd36c7 100644
--- a/p2p/peer.go
+++ b/p2p/peer.go
@@ -32,7 +32,7 @@ type Peer interface {
IsOutbound() bool // did we dial the peer
IsPersistent() bool // do we redial this peer when we disconnect
- IPHasChanged() bool // has the peer's IP changed
+ HasIPChanged() bool // has the peer's IP changed
CloseConn() error // close original connection
@@ -140,6 +140,10 @@ func (pc peerConn) ID() ID {
// Return the IP from the connection RemoteAddr
func (pc peerConn) RemoteIP() net.IP {
+ if pc.ip != nil {
+ return pc.ip
+ }
+
host, _, err := net.SplitHostPort(pc.conn.RemoteAddr().String())
if err != nil {
panic(err)
@@ -298,12 +302,14 @@ func (p *peer) IsPersistent() bool {
return p.peerConn.persistent
}
-// IPHasChanged returns true and the new IP if the peer's IP has changed.
-func (p *peer) IPHasChanged() bool {
+// HasIPChanged returns true and the new IP if the peer's IP has changed.
+func (p *peer) HasIPChanged() bool {
oldIP := p.ip
if oldIP == nil {
return false
}
+ // Reset the IP so we can get the new one
+ p.ip = nil
newIP := p.RemoteIP()
return !oldIP.Equal(newIP)
}
diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go
index 5ab936a57c..ca92c65cba 100644
--- a/p2p/peer_set_test.go
+++ b/p2p/peer_set_test.go
@@ -27,7 +27,7 @@ func (mp *mockPeer) NodeInfo() NodeInfo { return DefaultNodeInfo{}
func (mp *mockPeer) Status() ConnectionStatus { return ConnectionStatus{} }
func (mp *mockPeer) ID() ID { return mp.id }
func (mp *mockPeer) IsOutbound() bool { return false }
-func (mp *mockPeer) IPHasChanged() bool { return false }
+func (mp *mockPeer) HasIPChanged() bool { return false }
func (mp *mockPeer) IsPersistent() bool { return true }
func (mp *mockPeer) Get(s string) interface{} { return s }
func (mp *mockPeer) Set(string, interface{}) {}
diff --git a/p2p/switch.go b/p2p/switch.go
index 0795049ff9..8bdaa96175 100644
--- a/p2p/switch.go
+++ b/p2p/switch.go
@@ -389,7 +389,7 @@ func (sw *Switch) StopPeerForError(peer Peer, reason interface{}) {
go sw.reconnectToPeer(addr)
}
- if peer.IPHasChanged() {
+ if peer.HasIPChanged() {
addr, err := sw.getPeerAddress(peer)
if err != nil {
sw.Logger.Error("Failed to get address for peer with changed IP", "peer", peer, "err", err)
diff --git a/test/fuzz/p2p/pex/reactor_receive.go b/test/fuzz/p2p/pex/reactor_receive.go
index 861e51bd28..7cf94c71fa 100644
--- a/test/fuzz/p2p/pex/reactor_receive.go
+++ b/test/fuzz/p2p/pex/reactor_receive.go
@@ -76,7 +76,7 @@ func (fp *fuzzPeer) RemoteAddr() net.Addr {
}
func (fp *fuzzPeer) IsOutbound() bool { return false }
func (fp *fuzzPeer) IsPersistent() bool { return false }
-func (fp *fuzzPeer) IPHasChanged() bool { return false }
+func (fp *fuzzPeer) HasIPChanged() bool { return false }
func (fp *fuzzPeer) CloseConn() error { return nil }
func (fp *fuzzPeer) NodeInfo() p2p.NodeInfo { return defaultNodeInfo }