From 115f8750c24115dde48e6aecb37418765971225c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Mon, 20 Jan 2025 18:35:44 +0100 Subject: [PATCH 1/8] Use listen address for transport binding --- tm2/pkg/bft/node/node.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tm2/pkg/bft/node/node.go b/tm2/pkg/bft/node/node.go index c1afb2996fa..8eff3ea2d90 100644 --- a/tm2/pkg/bft/node/node.go +++ b/tm2/pkg/bft/node/node.go @@ -604,12 +604,10 @@ func (n *Node) OnStart() error { } // 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) } From 036451952415bc500aba1b4f563f4434d378b76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Wed, 22 Jan 2025 15:14:06 +0100 Subject: [PATCH 2/8] Add hacky limits for hacky code --- tm2/pkg/bft/node/node.go | 39 +++++++++++++++++--- tm2/pkg/internal/p2p/p2p.go | 14 ++++---- tm2/pkg/overflow/overflow_impl.go | 12 +++---- tm2/pkg/p2p/discovery/discovery.go | 3 +- tm2/pkg/p2p/discovery/discovery_test.go | 6 ++-- tm2/pkg/p2p/mock/peer.go | 2 +- tm2/pkg/p2p/peer.go | 2 +- tm2/pkg/p2p/peer_test.go | 4 ++- tm2/pkg/p2p/switch_test.go | 2 +- tm2/pkg/p2p/transport.go | 21 ++++++----- tm2/pkg/p2p/transport_test.go | 8 ++--- tm2/pkg/p2p/types/node_info.go | 34 ++++++++++++++---- tm2/pkg/p2p/types/node_info_test.go | 48 +++++++++++++++++-------- 13 files changed, 134 insertions(+), 61 deletions(-) diff --git a/tm2/pkg/bft/node/node.go b/tm2/pkg/bft/node/node.go index 8eff3ea2d90..c10a6b21a1d 100644 --- a/tm2/pkg/bft/node/node.go +++ b/tm2/pkg/bft/node/node.go @@ -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" @@ -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{ @@ -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) } diff --git a/tm2/pkg/internal/p2p/p2p.go b/tm2/pkg/internal/p2p/p2p.go index 1e650e0cd25..0c8f1529b85 100644 --- a/tm2/pkg/internal/p2p/p2p.go +++ b/tm2/pkg/internal/p2p/p2p.go @@ -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), @@ -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{} } diff --git a/tm2/pkg/overflow/overflow_impl.go b/tm2/pkg/overflow/overflow_impl.go index 0f057f65387..ab9f13c163d 100644 --- a/tm2/pkg/overflow/overflow_impl.go +++ b/tm2/pkg/overflow/overflow_impl.go @@ -84,10 +84,9 @@ func Quotient8(a, b int8) (int8, int8, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a%b, status + return c, a % b, status } - // Add16 performs + operation on two int16 operands, returning a result and status. func Add16(a, b int16) (int16, bool) { c := a + b @@ -170,10 +169,9 @@ func Quotient16(a, b int16) (int16, int16, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a%b, status + return c, a % b, status } - // Add32 performs + operation on two int32 operands, returning a result and status. func Add32(a, b int32) (int32, bool) { c := a + b @@ -256,10 +254,9 @@ func Quotient32(a, b int32) (int32, int32, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a%b, status + return c, a % b, status } - // Add64 performs + operation on two int64 operands, returning a result and status. func Add64(a, b int64) (int64, bool) { c := a + b @@ -342,6 +339,5 @@ func Quotient64(a, b int64) (int64, int64, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a%b, status + return c, a % b, status } - diff --git a/tm2/pkg/p2p/discovery/discovery.go b/tm2/pkg/p2p/discovery/discovery.go index d884b118c75..db1d6119de1 100644 --- a/tm2/pkg/p2p/discovery/discovery.go +++ b/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 diff --git a/tm2/pkg/p2p/discovery/discovery_test.go b/tm2/pkg/p2p/discovery/discovery_test.go index 17404e6039a..91741c648db 100644 --- a/tm2/pkg/p2p/discovery/discovery_test.go +++ b/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 diff --git a/tm2/pkg/p2p/mock/peer.go b/tm2/pkg/p2p/mock/peer.go index e5a01952831..5be34121924 100644 --- a/tm2/pkg/p2p/mock/peer.go +++ b/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 { diff --git a/tm2/pkg/p2p/peer.go b/tm2/pkg/p2p/peer.go index 135bf4b250c..dcca81ca097 100644 --- a/tm2/pkg/p2p/peer.go +++ b/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. diff --git a/tm2/pkg/p2p/peer_test.go b/tm2/pkg/p2p/peer_test.go index a74ea9e96a4..75f5172ee66 100644 --- a/tm2/pkg/p2p/peer_test.go +++ b/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, diff --git a/tm2/pkg/p2p/switch_test.go b/tm2/pkg/p2p/switch_test.go index 19a5db2efa5..917cd2f1b72 100644 --- a/tm2/pkg/p2p/switch_test.go +++ b/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, } } diff --git a/tm2/pkg/p2p/transport.go b/tm2/pkg/p2p/transport.go index 150072ad5eb..ad7adb1e342 100644 --- a/tm2/pkg/p2p/transport.go +++ b/tm2/pkg/p2p/transport.go @@ -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. diff --git a/tm2/pkg/p2p/transport_test.go b/tm2/pkg/p2p/transport_test.go index 3eb3264ec2b..960a4134590 100644 --- a/tm2/pkg/p2p/transport_test.go +++ b/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 diff --git a/tm2/pkg/p2p/types/node_info.go b/tm2/pkg/p2p/types/node_info.go index 8452cb43cb8..3a9f10c8d1e 100644 --- a/tm2/pkg/p2p/types/node_info.go +++ b/tm2/pkg/p2p/types/node_info.go @@ -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 @@ -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 @@ -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. diff --git a/tm2/pkg/p2p/types/node_info_test.go b/tm2/pkg/p2p/types/node_info_test.go index d03d77e608f..575d8ae5fbd 100644 --- a/tm2/pkg/p2p/types/node_info_test.go +++ b/tm2/pkg/p2p/types/node_info_test.go @@ -2,23 +2,43 @@ package types import ( "fmt" + "net" "testing" + "github.com/gnolang/gno/tm2/pkg/crypto" "github.com/gnolang/gno/tm2/pkg/versionset" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNodeInfo_Validate(t *testing.T) { t.Parallel() + generateNetAddress := func() *NetAddress { + var ( + key = GenerateNodeKey() + address = "127.0.0.1:8080" + ) + + tcpAddr, err := net.ResolveTCPAddr("tcp", address) + require.NoError(t, err) + + addr, err := NewNetAddress(key.ID(), tcpAddr) + require.NoError(t, err) + + return addr + } + t.Run("invalid peer ID", func(t *testing.T) { t.Parallel() info := &NodeInfo{ - PeerID: "", // zero + NetAddress: &NetAddress{ + ID: "", // zero + }, } - assert.ErrorIs(t, info.Validate(), ErrInvalidPeerID) + assert.ErrorIs(t, info.Validate(), crypto.ErrZeroID) }) t.Run("invalid version", func(t *testing.T) { @@ -47,8 +67,8 @@ func TestNodeInfo_Validate(t *testing.T) { t.Parallel() info := &NodeInfo{ - PeerID: GenerateNodeKey().ID(), - Version: testCase.version, + NetAddress: generateNetAddress(), + Version: testCase.version, } assert.ErrorIs(t, info.Validate(), ErrInvalidVersion) @@ -86,8 +106,8 @@ func TestNodeInfo_Validate(t *testing.T) { t.Parallel() info := &NodeInfo{ - PeerID: GenerateNodeKey().ID(), - Moniker: testCase.moniker, + NetAddress: generateNetAddress(), + Moniker: testCase.moniker, } assert.ErrorIs(t, info.Validate(), ErrInvalidMoniker) @@ -121,8 +141,8 @@ func TestNodeInfo_Validate(t *testing.T) { t.Parallel() info := &NodeInfo{ - PeerID: GenerateNodeKey().ID(), - Moniker: "valid moniker", + NetAddress: generateNetAddress(), + Moniker: "valid moniker", Other: NodeInfoOther{ RPCAddress: testCase.rpcAddress, }, @@ -162,9 +182,9 @@ func TestNodeInfo_Validate(t *testing.T) { t.Parallel() info := &NodeInfo{ - PeerID: GenerateNodeKey().ID(), - Moniker: "valid moniker", - Channels: testCase.channels, + NetAddress: generateNetAddress(), + Moniker: "valid moniker", + Channels: testCase.channels, } assert.ErrorIs(t, info.Validate(), testCase.expectedErr) @@ -176,9 +196,9 @@ func TestNodeInfo_Validate(t *testing.T) { t.Parallel() info := &NodeInfo{ - PeerID: GenerateNodeKey().ID(), - Moniker: "valid moniker", - Channels: []byte{10, 20, 30}, + NetAddress: generateNetAddress(), + Moniker: "valid moniker", + Channels: []byte{10, 20, 30}, Other: NodeInfoOther{ RPCAddress: "0.0.0.0:26657", }, From 936495136a684088644f7d9c8f06e846b8efb48f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Wed, 22 Jan 2025 15:17:54 +0100 Subject: [PATCH 3/8] Remove unused code --- tm2/pkg/p2p/types/node_info.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tm2/pkg/p2p/types/node_info.go b/tm2/pkg/p2p/types/node_info.go index 3a9f10c8d1e..4080ff2d8aa 100644 --- a/tm2/pkg/p2p/types/node_info.go +++ b/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") From 29d5e0cd7928da0b9ad4c64ec34dabaeed4db125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Wed, 22 Jan 2025 15:19:51 +0100 Subject: [PATCH 4/8] Run go generate --- .../internal/softfloat/runtime_softfloat64.go | 2 +- .../internal/softfloat/runtime_softfloat64_test.go | 7 +++---- tm2/pkg/overflow/overflow_impl.go | 12 ++++++++---- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64.go b/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64.go index cf2ad5afd8a..6c919c097f2 100644 --- a/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64.go +++ b/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64.go @@ -10,7 +10,7 @@ // Only referred to (and thus linked in) by softfloat targets // and by tests in this directory. -package softfloat +package runtime const ( mantbits64 uint = 52 diff --git a/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64_test.go b/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64_test.go index c57fe08b0ef..9d628c8fe03 100644 --- a/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64_test.go +++ b/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64_test.go @@ -7,14 +7,13 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package softfloat_test +package runtime_test import ( "math" "math/rand" - . "github.com/gnolang/gno/gnovm/pkg/gnolang/internal/softfloat" + . "runtime" "testing" - "runtime" ) // turn uint64 op into float64 op @@ -84,7 +83,7 @@ func TestFloat64(t *testing.T) { test(t, "+", add, fop(Fadd64), all) test(t, "-", sub, fop(Fsub64), all) - if runtime.GOARCH != "386" { // 386 is not precise! + if GOARCH != "386" { // 386 is not precise! test(t, "*", mul, fop(Fmul64), all) test(t, "/", div, fop(Fdiv64), all) } diff --git a/tm2/pkg/overflow/overflow_impl.go b/tm2/pkg/overflow/overflow_impl.go index ab9f13c163d..0f057f65387 100644 --- a/tm2/pkg/overflow/overflow_impl.go +++ b/tm2/pkg/overflow/overflow_impl.go @@ -84,9 +84,10 @@ func Quotient8(a, b int8) (int8, int8, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a % b, status + return c, a%b, status } + // Add16 performs + operation on two int16 operands, returning a result and status. func Add16(a, b int16) (int16, bool) { c := a + b @@ -169,9 +170,10 @@ func Quotient16(a, b int16) (int16, int16, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a % b, status + return c, a%b, status } + // Add32 performs + operation on two int32 operands, returning a result and status. func Add32(a, b int32) (int32, bool) { c := a + b @@ -254,9 +256,10 @@ func Quotient32(a, b int32) (int32, int32, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a % b, status + return c, a%b, status } + // Add64 performs + operation on two int64 operands, returning a result and status. func Add64(a, b int64) (int64, bool) { c := a + b @@ -339,5 +342,6 @@ func Quotient64(a, b int64) (int64, int64, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a % b, status + return c, a%b, status } + From 2701003aa8fd92465c8b12fa3de3668bc56801ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Wed, 22 Jan 2025 15:21:02 +0100 Subject: [PATCH 5/8] Revert "Run go generate" This reverts commit 29d5e0cd7928da0b9ad4c64ec34dabaeed4db125. --- .../internal/softfloat/runtime_softfloat64.go | 2 +- .../internal/softfloat/runtime_softfloat64_test.go | 7 ++++--- tm2/pkg/overflow/overflow_impl.go | 12 ++++-------- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64.go b/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64.go index 6c919c097f2..cf2ad5afd8a 100644 --- a/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64.go +++ b/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64.go @@ -10,7 +10,7 @@ // Only referred to (and thus linked in) by softfloat targets // and by tests in this directory. -package runtime +package softfloat const ( mantbits64 uint = 52 diff --git a/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64_test.go b/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64_test.go index 9d628c8fe03..c57fe08b0ef 100644 --- a/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64_test.go +++ b/gnovm/pkg/gnolang/internal/softfloat/runtime_softfloat64_test.go @@ -7,13 +7,14 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package runtime_test +package softfloat_test import ( "math" "math/rand" - . "runtime" + . "github.com/gnolang/gno/gnovm/pkg/gnolang/internal/softfloat" "testing" + "runtime" ) // turn uint64 op into float64 op @@ -83,7 +84,7 @@ func TestFloat64(t *testing.T) { test(t, "+", add, fop(Fadd64), all) test(t, "-", sub, fop(Fsub64), all) - if GOARCH != "386" { // 386 is not precise! + if runtime.GOARCH != "386" { // 386 is not precise! test(t, "*", mul, fop(Fmul64), all) test(t, "/", div, fop(Fdiv64), all) } diff --git a/tm2/pkg/overflow/overflow_impl.go b/tm2/pkg/overflow/overflow_impl.go index 0f057f65387..ab9f13c163d 100644 --- a/tm2/pkg/overflow/overflow_impl.go +++ b/tm2/pkg/overflow/overflow_impl.go @@ -84,10 +84,9 @@ func Quotient8(a, b int8) (int8, int8, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a%b, status + return c, a % b, status } - // Add16 performs + operation on two int16 operands, returning a result and status. func Add16(a, b int16) (int16, bool) { c := a + b @@ -170,10 +169,9 @@ func Quotient16(a, b int16) (int16, int16, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a%b, status + return c, a % b, status } - // Add32 performs + operation on two int32 operands, returning a result and status. func Add32(a, b int32) (int32, bool) { c := a + b @@ -256,10 +254,9 @@ func Quotient32(a, b int32) (int32, int32, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a%b, status + return c, a % b, status } - // Add64 performs + operation on two int64 operands, returning a result and status. func Add64(a, b int64) (int64, bool) { c := a + b @@ -342,6 +339,5 @@ func Quotient64(a, b int64) (int64, int64, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a%b, status + return c, a % b, status } - From 6f41b3946425a949a6595bfabf2f7409df776f1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Wed, 22 Jan 2025 15:34:11 +0100 Subject: [PATCH 6/8] Revert overflow_impl.go changes --- tm2/pkg/overflow/overflow_impl.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tm2/pkg/overflow/overflow_impl.go b/tm2/pkg/overflow/overflow_impl.go index ab9f13c163d..0f057f65387 100644 --- a/tm2/pkg/overflow/overflow_impl.go +++ b/tm2/pkg/overflow/overflow_impl.go @@ -84,9 +84,10 @@ func Quotient8(a, b int8) (int8, int8, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a % b, status + return c, a%b, status } + // Add16 performs + operation on two int16 operands, returning a result and status. func Add16(a, b int16) (int16, bool) { c := a + b @@ -169,9 +170,10 @@ func Quotient16(a, b int16) (int16, int16, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a % b, status + return c, a%b, status } + // Add32 performs + operation on two int32 operands, returning a result and status. func Add32(a, b int32) (int32, bool) { c := a + b @@ -254,9 +256,10 @@ func Quotient32(a, b int32) (int32, int32, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a % b, status + return c, a%b, status } + // Add64 performs + operation on two int64 operands, returning a result and status. func Add64(a, b int64) (int64, bool) { c := a + b @@ -339,5 +342,6 @@ func Quotient64(a, b int64) (int64, int64, bool) { } c := a / b status := (c < 0) == ((a < 0) != (b < 0)) || (c == 0) // no sign check for 0 quotient - return c, a % b, status + return c, a%b, status } + From e6b0f01491399dbc212788c39e821a8a5fd13804 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Tue, 4 Feb 2025 15:25:03 +0100 Subject: [PATCH 7/8] Update condition on peer filtering --- tm2/pkg/p2p/discovery/discovery.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tm2/pkg/p2p/discovery/discovery.go b/tm2/pkg/p2p/discovery/discovery.go index db1d6119de1..774a7ebf1fb 100644 --- a/tm2/pkg/p2p/discovery/discovery.go +++ b/tm2/pkg/p2p/discovery/discovery.go @@ -186,9 +186,21 @@ func (r *Reactor) handleDiscoveryRequest(peer p2p.PeerConn) error { peers = make([]*types.NetAddress, 0, len(localPeers)) ) - // Exclude the private peers from being shared + // Exclude the private peers from being shared, + // as well as peers who are not dialable localPeers = slices.DeleteFunc(localPeers, func(p p2p.PeerConn) bool { - return p.IsPrivate() + var ( + // Private peers are peers whose information is kept private to the node + privatePeer = p.IsPrivate() + // The reason we don't validate the net address with .Routable() + // is because of legacy logic that supports local loopbacks as advertised + // peer addresses. Introducing a .Routable() constraint will filter all + // local loopback addresses shared by peers, and will cause local deployments + // (and unit test deployments) to break and require additional setup + invalidDialAddress = p.NodeInfo().DialAddress().Validate() != nil + ) + + return privatePeer || invalidDialAddress }) // Check if there is anything to share, From e0596dd05693c285b0ec76033276c76cbaf004b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Tue, 4 Feb 2025 16:22:22 +0100 Subject: [PATCH 8/8] Fixup redial --- tm2/pkg/p2p/discovery/discovery.go | 4 +- tm2/pkg/p2p/switch.go | 61 ++++++++++++++++++++---------- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/tm2/pkg/p2p/discovery/discovery.go b/tm2/pkg/p2p/discovery/discovery.go index 774a7ebf1fb..7a9da3726c0 100644 --- a/tm2/pkg/p2p/discovery/discovery.go +++ b/tm2/pkg/p2p/discovery/discovery.go @@ -160,7 +160,7 @@ func (r *Reactor) Receive(chID byte, peer p2p.PeerConn, msgBytes []byte) { // Validate the message if err := msg.ValidateBasic(); err != nil { - r.Logger.Error("unable to validate discovery message", "err", err) + r.Logger.Warn("unable to validate discovery message", "err", err) return } @@ -168,7 +168,7 @@ func (r *Reactor) Receive(chID byte, peer p2p.PeerConn, msgBytes []byte) { switch msg := msg.(type) { case *Request: if err := r.handleDiscoveryRequest(peer); err != nil { - r.Logger.Error("unable to handle discovery request", "err", err) + r.Logger.Warn("unable to handle discovery request", "err", err) } case *Response: // Make the peers available for dialing on the switch diff --git a/tm2/pkg/p2p/switch.go b/tm2/pkg/p2p/switch.go index 7d9e768dd4b..09902322697 100644 --- a/tm2/pkg/p2p/switch.go +++ b/tm2/pkg/p2p/switch.go @@ -406,57 +406,76 @@ func (sw *MultiplexSwitch) runRedialLoop(ctx context.Context) { peersToDial = make([]*types.NetAddress, 0) ) + // Gather addresses of persistent peers that are missing or + // not already in the dial queue sw.persistentPeers.Range(func(key, value any) bool { var ( id = key.(types.ID) addr = value.(*types.NetAddress) ) - // Check if the peer is part of the peer set - // or is scheduled for dialing - if peers.Has(id) || sw.dialQueue.Has(addr) { - return true + if !peers.Has(id) && !sw.dialQueue.Has(addr) { + peersToDial = append(peersToDial, addr) } - peersToDial = append(peersToDial, addr) - return true }) if len(peersToDial) == 0 { - // No persistent peers are missing + // No persistent peers need dialing return } - // Calculate the dial items + // Prepare dial items with the appropriate backoff dialItems := make([]dial.Item, 0, len(peersToDial)) - for _, p := range peersToDial { - item := getBackoffItem(p.ID) + for _, addr := range peersToDial { + item := getBackoffItem(addr.ID) + if item == nil { - dialItem := dial.Item{ - Time: time.Now(), - Address: p, - } + // First attempt + now := time.Now() + + dialItems = append(dialItems, + dial.Item{ + Time: now, + Address: addr, + }, + ) - dialItems = append(dialItems, dialItem) - setBackoffItem(p.ID, &backoffItem{dialItem.Time, 0}) + setBackoffItem(addr.ID, &backoffItem{ + lastDialTime: now, + attempts: 0, + }) continue } - setBackoffItem(p.ID, &backoffItem{ - lastDialTime: time.Now().Add( + // Subsequent attempt: apply backoff + var ( + attempts = item.attempts + 1 + dialTime = time.Now().Add( calculateBackoff( item.attempts, time.Second, 10*time.Minute, ), - ), - attempts: item.attempts + 1, + ) + ) + + dialItems = append(dialItems, + dial.Item{ + Time: dialTime, + Address: addr, + }, + ) + + setBackoffItem(addr.ID, &backoffItem{ + lastDialTime: dialTime, + attempts: attempts, }) } - // Add the peers to the dial queue + // Add these items to the dial queue sw.dialItems(dialItems...) }