From d94a675ad6dc2130f16af9915db4b54e8dab239d Mon Sep 17 00:00:00 2001 From: cam-schultz Date: Wed, 23 Oct 2024 17:24:50 -0500 Subject: [PATCH 1/4] allow non primary network validators to request all peers --- network/network.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/network/network.go b/network/network.go index eab4ecca085e..bde8ab9b2706 100644 --- a/network/network.go +++ b/network/network.go @@ -555,8 +555,7 @@ func (n *network) Peers( } } - _, areTheyAPrimaryNetworkValidator := n.config.Validators.GetValidator(constants.PrimaryNetworkID, peerID) - if areWeAPrimaryNetworkValidator && requestAllPeers && areTheyAPrimaryNetworkValidator { + if areWeAPrimaryNetworkValidator && requestAllPeers { // Return IPs for all subnets. return getGossipableIPs( n.ipTracker, From 767144fbd159cdaa044eefd799a5603c1fcc1b56 Mon Sep 17 00:00:00 2001 From: cam-schultz Date: Thu, 24 Oct 2024 10:40:12 -0500 Subject: [PATCH 2/4] unit test wip --- network/network_test.go | 93 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/network/network_test.go b/network/network_test.go index a79d5d0cf717..d055868305a8 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -28,6 +28,7 @@ import ( "github.com/ava-labs/avalanchego/subnets" "github.com/ava-labs/avalanchego/upgrade" "github.com/ava-labs/avalanchego/utils" + "github.com/ava-labs/avalanchego/utils/bloom" "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/ips" @@ -352,6 +353,98 @@ func TestSend(t *testing.T) { wg.Wait() } +func TestGetPeerList(t *testing.T) { + require := require.New(t) + + // Create a non-validator peer + dialer, listeners, _, configs := newTestNetwork(t, 1) + + configs[0].Beacons = validators.NewManager() + configs[0].Validators = validators.NewManager() + nonValidatorNetwork, err := NewNetwork( + configs[0], + upgrade.InitiallyActiveTime, + newMessageCreator(t), + prometheus.NewRegistry(), + logging.NoLog{}, + listeners[0], + dialer, + &testHandler{ + InboundHandler: nil, + ConnectedF: nil, + DisconnectedF: nil, + }, + ) + require.NoError(err) + + receivedPeers := make(chan []*ips.ClaimedIPPort) + // Create a network of validators + // One validator returns the peer list consisting of all the validators + nodeIDs, networks, wg := newFullyConnectedTestNetwork( + t, + []router.InboundHandler{ + nil, // overwritten below + router.InboundHandlerFunc(func(context.Context, message.InboundMessage) { + require.FailNow("unexpected message received") + }), + router.InboundHandlerFunc(func(context.Context, message.InboundMessage) { + require.FailNow("unexpected message received") + }), + }, + ) + + // Overwrite the validator inbound message handler + handler := &testHandler{ + InboundHandler: router.InboundHandlerFunc(func(_ context.Context, msg message.InboundMessage) { + p := networks[0].Peers(msg.NodeID(), nil, true, &bloom.ReadFilter{}, []byte{}) + receivedPeers <- p + }), + ConnectedF: nil, + DisconnectedF: nil, + } + networks[0].peerConfig.Router = handler + networks[0].router = handler + + // Connect the non-validator peer to the validator network + wg.Add(1) + nonValidatorNetwork.ManuallyTrack(networks[0].config.MyNodeID, networks[0].config.MyIPPort.Get()) + go func() { + defer wg.Done() + + require.NoError(nonValidatorNetwork.Dispatch()) + }() + + mc := newMessageCreator(t) + outboundPeersMsg, err := mc.GetPeerList(nil, nil, true) + require.NoError(err) + + toSend := set.Of(nodeIDs[0]) + sentTo := nonValidatorNetwork.Send( + outboundPeersMsg, + common.SendConfig{ + NodeIDs: toSend, + }, + constants.PrimaryNetworkID, + subnets.NoOpAllower, + ) + require.Equal(toSend, sentTo) + + peersList := <-receivedPeers + require.Len(peersList, len(nodeIDs)-1) + peerNodes := set.NewSet[ids.NodeID](len(peersList)) + for _, peer := range peersList { + peerNodes.Add(peer.NodeID) + } + for _, nodeID := range nodeIDs[1:] { + require.True(peerNodes.Contains(nodeID)) + } + + for _, net := range networks { + net.StartClose() + } + wg.Wait() +} + func TestSendWithFilter(t *testing.T) { require := require.New(t) From 583e16c4418c4f58e911bbe4e6a8ef8fda880252 Mon Sep 17 00:00:00 2001 From: cam-schultz Date: Thu, 24 Oct 2024 12:46:58 -0500 Subject: [PATCH 3/4] TestGetAllPeers --- network/network_test.go | 166 ++++++++++++++++++---------------------- 1 file changed, 74 insertions(+), 92 deletions(-) diff --git a/network/network_test.go b/network/network_test.go index d055868305a8..e21617551560 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -353,98 +353,6 @@ func TestSend(t *testing.T) { wg.Wait() } -func TestGetPeerList(t *testing.T) { - require := require.New(t) - - // Create a non-validator peer - dialer, listeners, _, configs := newTestNetwork(t, 1) - - configs[0].Beacons = validators.NewManager() - configs[0].Validators = validators.NewManager() - nonValidatorNetwork, err := NewNetwork( - configs[0], - upgrade.InitiallyActiveTime, - newMessageCreator(t), - prometheus.NewRegistry(), - logging.NoLog{}, - listeners[0], - dialer, - &testHandler{ - InboundHandler: nil, - ConnectedF: nil, - DisconnectedF: nil, - }, - ) - require.NoError(err) - - receivedPeers := make(chan []*ips.ClaimedIPPort) - // Create a network of validators - // One validator returns the peer list consisting of all the validators - nodeIDs, networks, wg := newFullyConnectedTestNetwork( - t, - []router.InboundHandler{ - nil, // overwritten below - router.InboundHandlerFunc(func(context.Context, message.InboundMessage) { - require.FailNow("unexpected message received") - }), - router.InboundHandlerFunc(func(context.Context, message.InboundMessage) { - require.FailNow("unexpected message received") - }), - }, - ) - - // Overwrite the validator inbound message handler - handler := &testHandler{ - InboundHandler: router.InboundHandlerFunc(func(_ context.Context, msg message.InboundMessage) { - p := networks[0].Peers(msg.NodeID(), nil, true, &bloom.ReadFilter{}, []byte{}) - receivedPeers <- p - }), - ConnectedF: nil, - DisconnectedF: nil, - } - networks[0].peerConfig.Router = handler - networks[0].router = handler - - // Connect the non-validator peer to the validator network - wg.Add(1) - nonValidatorNetwork.ManuallyTrack(networks[0].config.MyNodeID, networks[0].config.MyIPPort.Get()) - go func() { - defer wg.Done() - - require.NoError(nonValidatorNetwork.Dispatch()) - }() - - mc := newMessageCreator(t) - outboundPeersMsg, err := mc.GetPeerList(nil, nil, true) - require.NoError(err) - - toSend := set.Of(nodeIDs[0]) - sentTo := nonValidatorNetwork.Send( - outboundPeersMsg, - common.SendConfig{ - NodeIDs: toSend, - }, - constants.PrimaryNetworkID, - subnets.NoOpAllower, - ) - require.Equal(toSend, sentTo) - - peersList := <-receivedPeers - require.Len(peersList, len(nodeIDs)-1) - peerNodes := set.NewSet[ids.NodeID](len(peersList)) - for _, peer := range peersList { - peerNodes.Add(peer.NodeID) - } - for _, nodeID := range nodeIDs[1:] { - require.True(peerNodes.Contains(nodeID)) - } - - for _, net := range networks { - net.StartClose() - } - wg.Wait() -} - func TestSendWithFilter(t *testing.T) { require := require.New(t) @@ -846,3 +754,77 @@ func TestAllowConnectionAsAValidator(t *testing.T) { } wg.Wait() } + +func TestGetAllPeers(t *testing.T) { + require := require.New(t) + + // Create a non-validator peer + dialer, listeners, nonVdrNodeIDs, configs := newTestNetwork(t, 1) + + configs[0].Beacons = validators.NewManager() + configs[0].Validators = validators.NewManager() + nonValidatorNetwork, err := NewNetwork( + configs[0], + upgrade.InitiallyActiveTime, + newMessageCreator(t), + prometheus.NewRegistry(), + logging.NoLog{}, + listeners[0], + dialer, + &testHandler{ + InboundHandler: nil, + ConnectedF: nil, + DisconnectedF: nil, + }, + ) + require.NoError(err) + + // Create a network of validators + nodeIDs, networks, wg := newFullyConnectedTestNetwork( + t, + []router.InboundHandler{ + nil, nil, nil, + }, + ) + + // Connect the non-validator peer to the validator network + wg.Add(1) + nonValidatorNetwork.ManuallyTrack(networks[0].config.MyNodeID, networks[0].config.MyIPPort.Get()) + go func() { + defer wg.Done() + + require.NoError(nonValidatorNetwork.Dispatch()) + }() + + { + // The non-validator peer should be able to get all the peers in the network + peersListFromNonVdr := networks[0].Peers(nonVdrNodeIDs[0], nil, true, bloom.EmptyFilter, []byte{}) + require.Len(peersListFromNonVdr, len(nodeIDs)-1) + peerNodes := set.NewSet[ids.NodeID](len(peersListFromNonVdr)) + for _, peer := range peersListFromNonVdr { + peerNodes.Add(peer.NodeID) + } + for _, nodeID := range nodeIDs[1:] { + require.True(peerNodes.Contains(nodeID)) + } + } + + { + // A validator peer should be able to get all the peers in the network + peersListFromVdr := networks[0].Peers(nodeIDs[1], nil, true, bloom.EmptyFilter, []byte{}) + require.Len(peersListFromVdr, len(nodeIDs)-2) // GetPeerList doesn't return the peer that requested it + peerNodes := set.NewSet[ids.NodeID](len(peersListFromVdr)) + for _, peer := range peersListFromVdr { + peerNodes.Add(peer.NodeID) + } + for _, nodeID := range nodeIDs[2:] { + require.True(peerNodes.Contains(nodeID)) + } + } + + nonValidatorNetwork.StartClose() + for _, net := range networks { + net.StartClose() + } + wg.Wait() +} From 8c3d15c1b89601ec6c9bca65e6970200e32990ee Mon Sep 17 00:00:00 2001 From: cam-schultz Date: Thu, 24 Oct 2024 16:40:30 -0500 Subject: [PATCH 4/4] update Peers docstring --- network/network.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/network/network.go b/network/network.go index bde8ab9b2706..af85233beed2 100644 --- a/network/network.go +++ b/network/network.go @@ -525,12 +525,10 @@ func (n *network) KnownPeers() ([]byte, []byte) { // // - Respond with all subnet IPs // - The peer requests all peers -// - We believe the peer to be a primary network validator // - We believe ourself to be a primary network validator // // - Respond with subnet IPs tracked by the peer -// - Either the peer does not request all peers or we don't consider them to -// be a primary network validator +// - The peer does not request all peers // - We believe ourself to be a primary network validator // // The reason we allow the peer to request all peers is so that we can avoid