Skip to content

Commit

Permalink
Add hacky limits for hacky code
Browse files Browse the repository at this point in the history
  • Loading branch information
zivkovicmilos committed Jan 22, 2025
1 parent 115f875 commit 0364519
Show file tree
Hide file tree
Showing 13 changed files with 134 additions and 61 deletions.
39 changes: 34 additions & 5 deletions tm2/pkg/bft/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import (
"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"
Expand Down Expand Up @@ -891,7 +889,7 @@ func makeNodeInfo(

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{
Expand All @@ -906,13 +904,44 @@ func makeNodeInfo(
},
}

// 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)
}

Expand Down
14 changes: 7 additions & 7 deletions tm2/pkg/internal/p2p/p2p.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ func MakeConnectedPeers(
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),
Expand Down Expand Up @@ -231,7 +231,7 @@ func (mp *Peer) TrySend(_ byte, _ []byte) bool { return true }
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{} }
Expand Down
12 changes: 4 additions & 8 deletions tm2/pkg/overflow/overflow_impl.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion tm2/pkg/p2p/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tm2/pkg/p2p/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tm2/pkg/p2p/mock/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tm2/pkg/p2p/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion tm2/pkg/p2p/peer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tm2/pkg/p2p/switch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
21 changes: 13 additions & 8 deletions tm2/pkg/p2p/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,19 +147,24 @@ func (mt *MultiplexTransport) Close() error {
}

// 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.
Expand Down
8 changes: 4 additions & 4 deletions tm2/pkg/p2p/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
34 changes: 27 additions & 7 deletions tm2/pkg/p2p/types/node_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,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
Expand All @@ -54,12 +54,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
Expand Down Expand Up @@ -100,7 +115,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.
Expand Down
Loading

0 comments on commit 0364519

Please sign in to comment.