[Network] upgrade libp2p and libp2p-pubsub libraries #6963

peterargue · 2025-01-30T21:26:02Z

Replaces: #6821

Upgrade libp2p and libp2p-pubsub to the latest versions.

github.com/libp2p/go-libp2p v0.32.2 => v0.38.2
github.com/libp2p/go-libp2p-pubsub v0.10.0 => v0.13.0

codecov-commenter · 2025-01-30T21:31:12Z

Codecov Report

Attention: Patch coverage is 20.00000% with 56 lines in your changes missing coverage. Please review.

Project coverage is 41.15%. Comparing base (e833530) to head (3144c07).

Files with missing lines	Patch %	Lines
integration/testnet/client.go	0.00%	19 Missing ⚠️
network/p2p/mock/node_builder.go	0.00%	13 Missing ⚠️
network/p2p/test/fixtures.go	12.50%	6 Missing and 1 partial ⚠️
network/p2p/builder/libp2pNodeBuilder.go	0.00%	6 Missing ⚠️
network/p2p/builder/gossipsub/gossipSubBuilder.go	16.66%	4 Missing and 1 partial ⚠️
network/p2p/mock/gossip_sub_builder.go	0.00%	2 Missing ⚠️
network/p2p/mock/pub_sub_adapter_config.go	0.00%	2 Missing ⚠️
network/p2p/node/gossipSubAdapterConfig.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6963      +/-   ##
==========================================
- Coverage   41.15%   41.15%   -0.01%     
==========================================
  Files        2131     2131              
  Lines      186855   186911      +56     
==========================================
+ Hits        76899    76919      +20     
- Misses     103516   103556      +40     
+ Partials     6440     6436       -4

Flag	Coverage Δ
unittests	`41.15% <20.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

peterargue · 2025-01-30T22:33:20Z

insecure/corruptlibp2p/gossipsub_spammer.go

@@ -68,7 +67,7 @@ func NewGossipSubRouterSpammerWithRpcInspector(t *testing.T,
 // ctlMessages is the list of spam messages to send to the victim node.
 func (s *GossipSubRouterSpammer) SpamControlMessage(t *testing.T, victim p2p.LibP2PNode, ctlMessages []pb.ControlMessage, msgs ...*pb.Message) {
 	for _, ctlMessage := range ctlMessages {
-		require.True(t, s.router.Get().SendControl(victim.ID(), &ctlMessage, msgs...))
+		s.router.Get().SendControl(victim.ID(), &ctlMessage, msgs...)


the only difference between the mainline and forked version of pubsub was that in the fork SendControl returned a boolean. Since that was only used in this test, I'm removing the check to simplify our dependency management.

peterargue · 2025-01-30T22:34:52Z

network/p2p/builder/libp2pNodeBuilder.go

@@ -211,7 +212,7 @@ func (builder *LibP2PNodeBuilder) Build() (p2p.LibP2PNode, error) {
 			return nil, fmt.Errorf("could not create resolver: %w", err)
 		}

-		opts = append(opts, libp2p.MultiaddrResolver(resolver))
+		opts = append(opts, libp2p.MultiaddrResolver(swarm.ResolverFromMaDNS{Resolver: resolver}))


this was required for the libp2p/go-libp2p upgrade

peterargue · 2025-02-01T00:31:38Z

network/test/cohort1/network_test.go

@@ -199,7 +199,7 @@ func (suite *NetworkTestSuite) TestUpdateNodeAddresses() {

 	// create a new staked identity
 	ids, libP2PNodes := testutils.LibP2PNodeForNetworkFixture(suite.T(), suite.sporkId, 1)
-	idProvider := unittest.NewUpdatableIDProvider(ids)
+	idProvider := unittest.NewUpdatableIDProvider(append(suite.ids, ids...))


I have no idea how this test originally worked since it was completely broken. maybe the networking logic was so slow that the message was never actually delivered. 🤷

peterargue · 2025-02-01T00:32:59Z

network/p2p/node/libp2pNode_test.go

@@ -319,7 +314,31 @@ func createConcurrentStreams(t *testing.T, ctx context.Context, nodes []p2p.LibP
 		// in 2 connections 1 created by each node, this happens because we are calling CreateStream concurrently.
 		time.Sleep(500 * time.Millisecond)
 	}
-	unittest.RequireReturnsBefore(t, wg.Wait, 3*time.Second, "could not create streams on time")


I think this test started running faster, so this check started to fail because it got here sooner and had to wait for the context timeout.

peterargue · 2025-02-01T01:28:10Z

network/p2p/builder/libp2pNodeBuilder.go

@@ -487,7 +488,7 @@ func (b *LibP2PNodeBuilder) configureRoutingSystem(
 	} else {
 		// bitswap requires a content routing system. this returns a stub instead of a full DHT
 		b.SetRoutingSystem(func(ctx context.Context, host host.Host) (routing.Routing, error) {
-			return none.ConstructNilRouting(ctx, host, nil, nil)
+			return routinghelpers.Null{}, nil


none.ConstructNilRouting was removed. Using recommended alternative. The implementations differ in that the none version returned nil from PutValue, but the new version returns an error. Will need to see how this behaves with testing.

janezpodhostnik · 2025-02-04T13:46:25Z

Are we ok with updating go-libp2p-pubsub to an unreleased version? Should we wait for a release?

… messages

peterargue · 2025-02-04T21:32:00Z

Are we ok with updating go-libp2p-pubsub to an unreleased version? Should we wait for a release?

We needed a change that was merged after the tagged release to remove our forked version. I can poke the team about pushing a new tag.

peterargue · 2025-02-04T23:02:23Z

insecure/integration/functional/test/gossipsub/scoring/scoring_test.go

@@ -115,7 +116,9 @@ func testGossipSubInvalidMessageDeliveryScoring(t *testing.T, spamMsgFactory fun
 		t.Name(),
 		idProvider,
 		p2ptest.WithRole(role),
-		p2ptest.OverrideFlowConfig(cfg))
+		p2ptest.OverrideFlowConfig(cfg),
+		p2ptest.WithValidateQueueSize(3000), // prevent node from dropping messages on slow CI machines


the invalid signature test seems to run slower on the new version. Added this to prevent the victim node from dropping too many messages during the test.

network/test/cohort1/network_test.go

Co-authored-by: Leo Zhang <[email protected]>

durkmurder

Looks good, thank you for fixing broken tests. 🥇

durkmurder · 2025-02-07T20:32:23Z

insecure/integration/functional/test/gossipsub/scoring/scoring_test.go

+
+	spammerScore, ok := victimNode.PeerScoreExposer().GetScore(spammer.SpammerNode.ID())
+	require.True(t, ok)
+	t.Logf("spammer score: %f", spammerScore)


do we still need this or this was debugging code?

not strictly. they're only logged in verbose mode and I think they are helpful to debug the test.

durkmurder · 2025-02-07T20:34:01Z

integration/testnet/client.go

+	// when this is run against an Access node with indexing enabled, occasionally the indexed height
+	// lags behind the sealed height, especially after first starting up (like in a test).
+	// Retry using the same block until we get the account.
+	for {


Usually operations like this have an upper time limit so we don't run it indefinitely

yea, I left it to the context to manage that. the caller uses a 5 second timeout

durkmurder · 2025-02-07T20:35:34Z

network/p2p/node/libp2pNode_test.go

+	// cancel the context to trigger streams to shutdown
+	cancel()
+
+	unittest.RequireReturnsBefore(t, wg.Wait, 1*time.Second, "could not shutdown streams on time")


nice sequence

durkmurder · 2025-02-07T20:39:46Z

network/test/cohort1/network_test.go

 	_, err = newNet.Register(channels.TestNetworkChannel, newEngine)
 	require.NoError(suite.T(), err)
-	newEngine.On("Process", channels.TestNetworkChannel, suite.ids[0].NodeID, mockery.Anything).Return(nil)
+
+	callCount := 0


is it safe to use a non-atomic here? Do we have a guarantee that everything runs in the same goroutine?

you're right, this could cause flakiness. ee0881e

[Network] upgrade libp2p, libp2p-pubsub, and go-multiaddr-dns libraries

0d89220

peterargue requested a review from a team as a code owner January 30, 2025 21:26

peterargue added 2 commits January 30, 2025 14:30

update to the latest libp2p-pubsub

84b6d79

Remove special pubsub fork used for insecure testing

3c7f326

peterargue commented Jan 30, 2025

View reviewed changes

peterargue added 3 commits January 30, 2025 14:41

fix api change to tcp.NewTCPTransport

94e4897

fix flakiness in TestCreateStream_SinglePairwiseConnection

638abea

fix flaky TestUpdateNodeAddresses

9fcd7ea

peterargue commented Feb 1, 2025

View reviewed changes

peterargue added 2 commits January 31, 2025 17:12

fix flaky test TestUnicastRateLimit_Bandwidth

6118977

none client was removed. switched to recommended alternative

fbe57d5

peterargue commented Feb 1, 2025

View reviewed changes

peterargue added 7 commits January 31, 2025 17:31

update boxo

60a22fc

update boxo fork

194713b

Merge branch 'master' into petera/upgrade-libp2p-take2

ad57faf

undo boxo upgrade

4da50ab

update gamazero/workerpool

d282ba6

add boxo override to insecure module

18ec7d7

add boxo override to integration module

a893bba

peterargue mentioned this pull request Feb 3, 2025

[Network] upgrade libp2p, libp2p-pubsub, and go-multiaddr-dns libraries #6821

Closed

peterargue added 3 commits February 3, 2025 13:45

readd mods_override scripts

74c7341

fix flakiness in TestGossipSubInvalidMessageDelivery_Integration

4877fae

relax testing update

bbd5a43

peterargue added 3 commits February 4, 2025 13:05

add support for setting validate queue size to prevent tests dropping…

2dbb527

… messages

Merge branch 'master' into petera/upgrade-libp2p-take2

416d620

undo downgrade from merge conflict

52632b9

peterargue added 3 commits February 4, 2025 13:37

update mocks

6d32200

increase validate queue on victim, revert assertion changes

92946d7

fix flaky access test

e06926f

peterargue commented Feb 4, 2025

View reviewed changes

add missing error check

9715998

peterargue mentioned this pull request Feb 4, 2025

[Network] upgrade libp2p and libp2p-pubsub libraries - v0.37 #6978

Closed

peterargue added 2 commits February 7, 2025 09:13

switch to tagged libp2p/go-libp2p-pubsub

788ff7c

Merge branch 'master' into petera/upgrade-libp2p-take2

d89b9cd

zhangchiqing approved these changes Feb 7, 2025

View reviewed changes

network/test/cohort1/network_test.go Outdated Show resolved Hide resolved

Update network/test/cohort1/network_test.go

f13fbd7

Co-authored-by: Leo Zhang <[email protected]>

durkmurder approved these changes Feb 7, 2025

View reviewed changes

peterargue added 4 commits February 7, 2025 12:54

switch to atomic for callCount in network test

ee0881e

Merge branch 'master' into petera/upgrade-libp2p-take2

f8ed3e8

fixt lint

857d346

fix type in test assert

3144c07

peterargue added this pull request to the merge queue Feb 7, 2025

Merged via the queue into master with commit 8fbbd56 Feb 7, 2025
56 checks passed

peterargue deleted the petera/upgrade-libp2p-take2 branch February 7, 2025 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Network] upgrade libp2p and libp2p-pubsub libraries #6963

[Network] upgrade libp2p and libp2p-pubsub libraries #6963

peterargue commented Jan 30, 2025 •

edited

Loading

codecov-commenter commented Jan 30, 2025 •

edited

Loading

peterargue Jan 30, 2025 •

edited

Loading

peterargue Jan 30, 2025

peterargue Feb 1, 2025

peterargue Feb 1, 2025 •

edited

Loading

peterargue Feb 1, 2025 •

edited

Loading

janezpodhostnik commented Feb 4, 2025

peterargue commented Feb 4, 2025

peterargue Feb 4, 2025

durkmurder left a comment

durkmurder Feb 7, 2025

peterargue Feb 7, 2025

durkmurder Feb 7, 2025

peterargue Feb 7, 2025

durkmurder Feb 7, 2025

durkmurder Feb 7, 2025

peterargue Feb 7, 2025

[Network] upgrade libp2p and libp2p-pubsub libraries #6963

[Network] upgrade libp2p and libp2p-pubsub libraries #6963

Conversation

peterargue commented Jan 30, 2025 • edited Loading

codecov-commenter commented Jan 30, 2025 • edited Loading

Codecov Report

peterargue Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterargue Feb 1, 2025 • edited Loading

Choose a reason for hiding this comment

peterargue Feb 1, 2025 • edited Loading

Choose a reason for hiding this comment

janezpodhostnik commented Feb 4, 2025

peterargue commented Feb 4, 2025

Choose a reason for hiding this comment

durkmurder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterargue commented Jan 30, 2025 •

edited

Loading

codecov-commenter commented Jan 30, 2025 •

edited

Loading

peterargue Jan 30, 2025 •

edited

Loading

peterargue Feb 1, 2025 •

edited

Loading

peterargue Feb 1, 2025 •

edited

Loading