fix: utilize p2p.ExternalAddress properly for dialing

tm2/pkg/bft/node/node.go

@@ -12,8 +12,6 @@
 	"sync"
 	"time"
 
-	goErrors "errors"
-
 	"github.com/gnolang/gno/tm2/pkg/bft/appconn"
 	"github.com/gnolang/gno/tm2/pkg/bft/state/eventstore/file"
 	"github.com/gnolang/gno/tm2/pkg/p2p/conn"
@@ -604,12 +602,10 @@
 	}
 
 	// Start the transport.
-	lAddr := n.config.P2P.ExternalAddress
-	if lAddr == "" {
-		lAddr = n.config.P2P.ListenAddress
-	}
+	// The listen address for the transport needs to be an address within reach of the machine NIC
+	listenAddress := p2pTypes.NetAddressString(n.nodeKey.ID(), n.config.P2P.ListenAddress)
 
-	addr, err := p2pTypes.NewNetAddressFromString(p2pTypes.NetAddressString(n.nodeKey.ID(), lAddr))
+	addr, err := p2pTypes.NewNetAddressFromString(listenAddress)
 	if err != nil {
 		return fmt.Errorf("unable to parse network address, %w", err)
 	}
@@ -893,7 +889,7 @@
 
 	nodeInfo := p2pTypes.NodeInfo{
 		VersionSet: vset,
-		PeerID:     nodeKey.ID(),
+		NetAddress: nil, // The shared address depends on the configuration
 		Network:    genDoc.ChainID,
 		Version:    version.Version,
 		Channels: []byte{
@@ -908,13 +904,44 @@
 		},
 	}
 
+	// Make sure the discovery channel is shared with peers
+	// in case peer discovery is enabled
 	if config.P2P.PeerExchange {
 		nodeInfo.Channels = append(nodeInfo.Channels, discovery.Channel)
 	}
 
+	// Grab the supplied listen address.
+	// This address needs to be valid, but it can be unspecified.
+	// If the listen address is unspecified (port / IP unbound),
+	// then this address cannot be used by peers for dialing
+	addr, err := p2pTypes.NewNetAddressFromString(
+		p2pTypes.NetAddressString(nodeKey.ID(), config.P2P.ListenAddress),
+	)
+	if err != nil {
+		return p2pTypes.NodeInfo{}, fmt.Errorf("unable to parse network address, %w", err)
+	}
+
+	// Use the transport listen address as the advertised address
+	nodeInfo.NetAddress = addr
+
+	// Prepare the advertised dial address (if any)
+	// for the node, which other peers can use to dial
+	if config.P2P.ExternalAddress != "" {
+		addr, err = p2pTypes.NewNetAddressFromString(
+			p2pTypes.NetAddressString(
+				nodeKey.ID(),
+				config.P2P.ExternalAddress,
+			),
+		)
+		if err != nil {
+			return p2pTypes.NodeInfo{}, fmt.Errorf("invalid p2p external address: %w", err)
+		}
+
+		nodeInfo.NetAddress = addr
+	}
+
 	// Validate the node info
-	err := nodeInfo.Validate()
-	if err != nil && !goErrors.Is(err, p2pTypes.ErrUnspecifiedIP) {
+	if err := nodeInfo.Validate(); err != nil {
 		return p2pTypes.NodeInfo{}, fmt.Errorf("unable to validate node info, %w", err)
 	}
 

tm2/pkg/internal/p2p/p2p.go

@@ -70,12 +70,12 @@
 			VersionSet: versionset.VersionSet{
 				versionset.VersionInfo{Name: "p2p", Version: "v0.0.0"},
 			},
-			PeerID:   key.ID(),
-			Network:  "testing",
-			Software: "p2ptest",
-			Version:  "v1.2.3-rc.0-deadbeef",
-			Channels: cfg.Channels,
-			Moniker:  fmt.Sprintf("node-%d", index),
+			NetAddress: addr,
+			Network:    "testing",
+			Software:   "p2ptest",
+			Version:    "v1.2.3-rc.0-deadbeef",
+			Channels:   cfg.Channels,
+			Moniker:    fmt.Sprintf("node-%d", index),
 			Other: p2pTypes.NodeInfoOther{
 				TxIndex:    "off",
 				RPCAddress: fmt.Sprintf("127.0.0.1:%d", 0),
@@ -231,7 +231,7 @@
 func (mp *Peer) Send(_ byte, _ []byte) bool    { return true }
 func (mp *Peer) NodeInfo() p2pTypes.NodeInfo {
 	return p2pTypes.NodeInfo{
-		PeerID: mp.id,
+		NetAddress: mp.addr,
 	}
 }
 func (mp *Peer) Status() conn.ConnectionStatus { return conn.ConnectionStatus{} }

tm2/pkg/p2p/discovery/discovery.go

@@ -207,7 +207,8 @@ func (r *Reactor) handleDiscoveryRequest(peer p2p.PeerConn) error {
 	}
 
 	for _, p := range localPeers {
-		peers = append(peers, p.SocketAddr())
+		// Make sure only routable peers are shared
+		peers = append(peers, p.NodeInfo().DialAddress())
 	}
 
 	// Create the response, and marshal

tm2/pkg/p2p/discovery/discovery_test.go

@@ -166,7 +166,7 @@ func TestReactor_DiscoveryResponse(t *testing.T) {
 
 		slices.ContainsFunc(resp.Peers, func(addr *types.NetAddress) bool {
 			for _, localP := range peers {
-				if localP.SocketAddr().Equals(*addr) {
+				if localP.NodeInfo().DialAddress().Equals(*addr) {
 					return true
 				}
 			}
@@ -317,7 +317,7 @@ func TestReactor_DiscoveryResponse(t *testing.T) {
 
 		slices.ContainsFunc(resp.Peers, func(addr *types.NetAddress) bool {
 			for _, localP := range peers {
-				if localP.SocketAddr().Equals(*addr) {
+				if localP.NodeInfo().DialAddress().Equals(*addr) {
 					return true
 				}
 			}
@@ -373,7 +373,7 @@ func TestReactor_DiscoveryResponse(t *testing.T) {
 		peerAddrs := make([]*types.NetAddress, 0, len(peers))
 
 		for _, p := range peers {
-			peerAddrs = append(peerAddrs, p.SocketAddr())
+			peerAddrs = append(peerAddrs, p.NodeInfo().DialAddress())
 		}
 
 		// Prepare the message

tm2/pkg/p2p/mock/peer.go

@@ -57,7 +57,7 @@ func GeneratePeers(t *testing.T, count int) []*Peer {
 			},
 			NodeInfoFn: func() types.NodeInfo {
 				return types.NodeInfo{
-					PeerID: key.ID(),
+					NetAddress: addr,
 				}
 			},
 			SocketAddrFn: func() *types.NetAddress {

tm2/pkg/p2p/peer.go

@@ -160,7 +160,7 @@ func (p *peer) OnStop() {
 
 // ID returns the peer's ID - the hex encoded hash of its pubkey.
 func (p *peer) ID() types.ID {
-	return p.nodeInfo.PeerID
+	return p.nodeInfo.ID()
 }
 
 // NodeInfo returns a copy of the peer's NodeInfo.

tm2/pkg/p2p/peer_test.go

@@ -243,7 +243,9 @@ func TestPeer_Properties(t *testing.T) {
 								},
 							},
 							nodeInfo: types.NodeInfo{
-								PeerID: id,
+								NetAddress: &types.NetAddress{
+									ID: id,
+								},
 							},
 							connInfo: &ConnInfo{
 								Outbound: testCase.outbound,

tm2/pkg/p2p/switch_test.go

@@ -727,7 +727,7 @@ func TestMultiplexSwitch_DialPeers(t *testing.T) {
 		// as the transport (node)
 		p.NodeInfoFn = func() types.NodeInfo {
 			return types.NodeInfo{
-				PeerID: addr.ID,
+				NetAddress: &addr,
 			}
 		}
 

tm2/pkg/p2p/transport.go

@@ -147,19 +147,24 @@
 }
 
 // Listen starts an active process of listening for incoming connections [NON-BLOCKING]
-func (mt *MultiplexTransport) Listen(addr types.NetAddress) (rerr error) {
-	// Reserve a port, and start listening
-	ln, err := net.Listen("tcp", addr.DialString())
-	if err != nil {
-		return fmt.Errorf("unable to listen on address, %w", err)
-	}
+func (mt *MultiplexTransport) Listen(addr types.NetAddress) error {
+	var (
+		ln  net.Listener
+		err error
+	)
 
 	defer func() {
-		if rerr != nil {
-			ln.Close()
+		if err != nil && ln != nil {
+			_ = ln.Close()
 		}
 	}()
 
+	// Reserve a port, and start listening
+	ln, err = net.Listen("tcp", addr.DialString())
+	if err != nil {
+		return fmt.Errorf("unable to listen on address, %w", err)
+	}
+
 	if addr.Port == 0 {
 		// net.Listen on port 0 means the kernel will auto-allocate a port
 		// - find out which one has been given to us.

tm2/pkg/p2p/transport_test.go

@@ -239,7 +239,7 @@ func TestMultiplexTransport_Accept(t *testing.T) {
 
 			ni := types.NodeInfo{
 				Network:    network, // common network
-				PeerID:     id,
+				NetAddress: na,
 				Version:    "v1.0.0-rc.0",
 				Moniker:    fmt.Sprintf("node-%d", index),
 				VersionSet: make(versionset.VersionSet, 0), // compatible version set
@@ -319,7 +319,7 @@ func TestMultiplexTransport_Accept(t *testing.T) {
 
 			ni := types.NodeInfo{
 				Network:    chainID,
-				PeerID:     id,
+				NetAddress: na,
 				Version:    "v1.0.0-rc.0",
 				Moniker:    fmt.Sprintf("node-%d", index),
 				VersionSet: make(versionset.VersionSet, 0), // compatible version set
@@ -391,7 +391,7 @@ func TestMultiplexTransport_Accept(t *testing.T) {
 
 			ni := types.NodeInfo{
 				Network:    network, // common network
-				PeerID:     key.ID(),
+				NetAddress: na,
 				Version:    "v1.0.0-rc.0",
 				Moniker:    fmt.Sprintf("node-%d", index),
 				VersionSet: make(versionset.VersionSet, 0), // compatible version set
@@ -469,7 +469,7 @@ func TestMultiplexTransport_Accept(t *testing.T) {
 
 			ni := types.NodeInfo{
 				Network:    network, // common network
-				PeerID:     key.ID(),
+				NetAddress: na,
 				Version:    "v1.0.0-rc.0",
 				Moniker:    fmt.Sprintf("node-%d", index),
 				VersionSet: make(versionset.VersionSet, 0), // compatible version set

tm2/pkg/p2p/types/node_info.go

@@ -14,7 +14,6 @@ const (
 )
 
 var (
-	ErrInvalidPeerID        = errors.New("invalid peer ID")
 	ErrInvalidVersion       = errors.New("invalid node version")
 	ErrInvalidMoniker       = errors.New("invalid node moniker")
 	ErrInvalidRPCAddress    = errors.New("invalid node RPC address")
@@ -30,8 +29,8 @@ type NodeInfo struct {
 	// Set of protocol versions
 	VersionSet versionset.VersionSet `json:"version_set"`
 
-	// Unique peer identifier
-	PeerID ID `json:"id"`
+	// The advertised net address of the peer
+	NetAddress *NetAddress `json:"net_address"`
 
 	// Check compatibility.
 	// Channels are HexBytes so easier to read as JSON
@@ -54,12 +53,27 @@ type NodeInfoOther struct {
 // Validate checks the self-reported NodeInfo is safe.
 // It returns an error if there
 // are too many Channels, if there are any duplicate Channels,
-// if the ListenAddr is malformed, or if the ListenAddr is a host name
+// if the NetAddress is malformed, or if the NetAddress is a host name
 // that can not be resolved to some IP
 func (info NodeInfo) Validate() error {
-	// Validate the ID
-	if err := info.PeerID.Validate(); err != nil {
-		return fmt.Errorf("%w, %w", ErrInvalidPeerID, err)
+	// There are a few checks that need to be performed when validating
+	// the node info's net address:
+	// - the ID needs to be valid
+	// - the FORMAT of the net address needs to be valid
+	//
+	// The key nuance here is that the net address is not being validated
+	// for its "dialability", but whether it's of the correct format.
+	//
+	// Unspecified IPs are tolerated (ex. 0.0.0.0 or ::),
+	// because of legacy logic that assumes node info
+	// can have unspecified IPs (ex. no external address is set, use
+	// the listen address which is bound to 0.0.0.0).
+	//
+	// These types of IPs are caught during the
+	// real peer info sharing process, since they are undialable
+	_, err := NewNetAddressFromString(NetAddressString(info.NetAddress.ID, info.NetAddress.DialString()))
+	if err != nil {
+		return fmt.Errorf("invalid net address in node info, %w", err)
 	}
 
 	// Validate Version
@@ -100,7 +114,12 @@ func (info NodeInfo) Validate() error {
 
 // ID returns the local node ID
 func (info NodeInfo) ID() ID {
-	return info.PeerID
+	return info.NetAddress.ID
+}
+
+// DialAddress is the advertised peer dial address (share-able)
+func (info NodeInfo) DialAddress() *NetAddress {
+	return info.NetAddress
 }
 
 // CompatibleWith checks if two NodeInfo are compatible with each other.