Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

p2p: invert peer discovery integration #989

Merged
merged 2 commits into from
Aug 18, 2022
Merged

Conversation

corverroos
Copy link
Contributor

Inverts the integration between discv5 and libp2p from pull to push.

category: refactor
ticket: #985
feature_flag: invert_discv5

Comment on lines -31 to -35
// UDPNode wraps a discv5 udp node and adds the bootnodes relays.
type UDPNode struct {
*discover.UDPv5
Relays []Peer
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used

@@ -71,7 +71,7 @@ func TestP2PConnGating(t *testing.T) {

err = nodeA.Connect(context.Background(), addr)
require.Error(t, err)
require.Contains(t, err.Error(), fmt.Sprintf("gater rejected connection with peer %s and addr %s", addr.ID, addr.Addrs[0]))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flapping test, somehow used ipv6 on my local machine

@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Merging #989 (29959f0) into main (09bdf33) will decrease coverage by 0.15%.
The diff coverage is 1.92%.

@@            Coverage Diff             @@
##             main     #989      +/-   ##
==========================================
- Coverage   54.44%   54.29%   -0.16%     
==========================================
  Files         117      117              
  Lines       12954    12994      +40     
==========================================
+ Hits         7053     7055       +2     
- Misses       4879     4916      +37     
- Partials     1022     1023       +1     
Impacted Files Coverage Δ
app/featureset/featureset.go 100.00% <ø> (ø)
app/lifecycle/orderstart_string.go 40.00% <ø> (ø)
p2p/discovery.go 30.63% <0.00%> (-17.26%) ⬇️
p2p/p2p.go 21.01% <0.00%> (-0.57%) ⬇️
app/app.go 57.17% <100.00%> (+0.08%) ⬆️
core/qbft/qbft.go 81.11% <0.00%> (ø)
core/leadercast/transport.go 76.33% <0.00%> (+1.18%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -133,3 +137,62 @@ func NewLocalEnode(config Config, key *ecdsa.PrivateKey) (*enode.LocalNode, *eno

return node, db, nil
}

// NewDiscoveryAdapter returns a life cycle hook that links discv5 to libp2p by
// continuously polling discv5 for latest peer ERNs and adding then to libp2p peer store.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// continuously polling discv5 for latest peer ERNs and adding then to libp2p peer store.
// continuously polling discv5 for latest peer ENRs and adding then to libp2p peer store.

ctx = log.WithTopic(ctx, "p2p")
ttl := peerstore.TempAddrTTL
baseDelay := expbackoff.WithBaseDelay(time.Millisecond * 100) // Poll quickly on startup
maxDelay := expbackoff.WithMaxDelay(ttl * 9 / 10) // Slow down to 90% of ttl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
maxDelay := expbackoff.WithMaxDelay(ttl * 9 / 10) // Slow down to 90% of ttl
maxDelay := expbackoff.WithMaxDelay(ttl * 0.9) // Slow down to 90% of ttl

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't compile

Copy link
Contributor

@xenowits xenowits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@corverroos corverroos added the merge when ready Indicates bulldozer bot may merge when all checks pass label Aug 18, 2022
@obol-bulldozer obol-bulldozer bot merged commit a68a2c6 into main Aug 18, 2022
@obol-bulldozer obol-bulldozer bot deleted the corver/pushdisc branch August 18, 2022 05:22
obol-bulldozer bot pushed a commit that referenced this pull request Aug 18, 2022
Inverts relay address routing from pull based to pull based.

category: refactor
ticket: #989 
feature_flag: invert_libp2p_routing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants