Skip to content

p2p: minor cleanup + update router options#6353

Merged
alexanderbez merged 7 commits intomasterfrom
bez/p2p-cleanup
Apr 14, 2021
Merged

p2p: minor cleanup + update router options#6353
alexanderbez merged 7 commits intomasterfrom
bez/p2p-cleanup

Conversation

@alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Apr 13, 2021

I wanted to address the point Merge MaxNumInboundPeers and MaxNumOutboundPeers into MaxConnections. from the p2p tracking issue, but realized we cannot remove those fields until we remove all of the legacy stack.

So this PR just tweaks the current router options to use both max in and out plus some minor cleanup:

  • Rename legacy flag
  • Add new MaxIncomingConnectionAttempts config
  • Add new MaxConnections config (will replace MaxNumInboundPeers and MaxNumOutboundPeers)

/cc @tessr

@alexanderbez alexanderbez added the C:p2p Component: P2P pkg label Apr 13, 2021
@alexanderbez alexanderbez changed the title p2p: minor cleanup p2p: minor cleanup + update router options Apr 13, 2021
@codecov
Copy link

codecov bot commented Apr 13, 2021

Codecov Report

Merging #6353 (860a012) into master (ca7dbea) will increase coverage by 0.00%.
The diff coverage is 42.85%.

@@           Coverage Diff           @@
##           master    #6353   +/-   ##
=======================================
  Coverage   60.80%   60.81%           
=======================================
  Files         282      282           
  Lines       26885    26901   +16     
=======================================
+ Hits        16347    16359   +12     
- Misses       8835     8836    +1     
- Partials     1703     1706    +3     
Impacted Files Coverage Δ
config/toml.go 59.09% <ø> (ø)
test/e2e/generator/generate.go 0.00% <0.00%> (ø)
node/node.go 57.30% <25.00%> (-0.51%) ⬇️
p2p/router.go 78.26% <66.66%> (+0.61%) ⬆️
config/config.go 74.92% <100.00%> (+0.15%) ⬆️
privval/signer_endpoint.go 75.75% <0.00%> (-6.07%) ⬇️
types/tx.go 82.97% <0.00%> (-4.26%) ⬇️
libs/clist/clist.go 63.09% <0.00%> (-3.58%) ⬇️
privval/signer_listener_endpoint.go 87.05% <0.00%> (-2.36%) ⬇️
blockchain/v0/pool.go 78.32% <0.00%> (-2.29%) ⬇️
... and 11 more

# functionality with a single network.

initial_height = 1000
disable_legacy_p2p = false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can ignore most of these changes. My vsc TOML extension automatically sorts :)

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should all be running that extension, tbh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yah I think we should ^__^

Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

LGTM

Question: For v0.35 is the default the new p2p stack or the legacy one (I see currently that it's the legacy one but just wanting to make sure we are explicit about this)?

@alexanderbez
Copy link
Contributor Author

Good question @cmwaters -- I'm not sure tbh.

@alexanderbez alexanderbez merged commit 47b28fd into master Apr 14, 2021
@alexanderbez alexanderbez deleted the bez/p2p-cleanup branch April 14, 2021 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C:p2p Component: P2P pkg

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants