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: implement adr-050 #4176

Merged
merged 13 commits into from Dec 4, 2019
Merged

Conversation

@dongsam
Copy link
Contributor

dongsam commented Nov 21, 2019

  • implementation spec of Improved Trusted Peering ADR-050 by B-Harvest
  • Add unconditional_peer_ids and persistent_peers_max_dial_period to config
  • Add unconditionalOutbound, unconditionalInbound arguments to func (sw *Switch) NumPeers() for get unconditional count of outbound/inbound peers
  • Add unconditionalPeerIDs map to Switch struct

default config value of persistent_peers_max_dial_period is 0s(disabled) It could be changed through discussion

Refs

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs tendermint/spec#68
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md
p2p/pex/pex_reactor.go Outdated Show resolved Hide resolved
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 25, 2019

Codecov Report

Merging #4176 into master will increase coverage by <.01%.
The diff coverage is 78.26%.

@@            Coverage Diff             @@
##           master    #4176      +/-   ##
==========================================
+ Coverage   67.33%   67.33%   +<.01%     
==========================================
  Files         223      223              
  Lines       19298    19338      +40     
==========================================
+ Hits        12994    13022      +28     
- Misses       5329     5336       +7     
- Partials      975      980       +5
Impacted Files Coverage Δ
rpc/core/pipe.go 25% <ø> (ø) ⬆️
config/toml.go 65.95% <ø> (ø) ⬆️
cmd/tendermint/commands/run_node.go 0% <0%> (ø) ⬆️
rpc/core/net.go 36.84% <0%> (ø) ⬆️
p2p/pex/pex_reactor.go 83.42% <50%> (-1.01%) ⬇️
node/node.go 62.95% <60%> (-0.09%) ⬇️
p2p/switch.go 69.57% <87.87%> (+0.87%) ⬆️
config/config.go 83.97% <90.9%> (-0.46%) ⬇️
consensus/ticker.go 91.66% <0%> (-4.17%) ⬇️
consensus/replay.go 71.76% <0%> (-0.79%) ⬇️
... and 6 more
@dongsam dongsam force-pushed the dongsam:ADR-050-Improved-Trusted-Peering branch from 73fcd9a to 06f2699 Nov 25, 2019
@dongsam dongsam marked this pull request as ready for review Nov 25, 2019
@dongsam dongsam requested review from ebuchman, melekes and tessr as code owners Nov 25, 2019
@dlguddus

This comment has been minimized.

Copy link
Contributor

dlguddus commented Nov 29, 2019

@tessr @melekes @ebuchman
review requested from B-Harvest.

Copy link
Collaborator

melekes left a comment

Thank you for your contribution 👍 I've left a few comments.

cmd/tendermint/commands/run_node.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
config/toml.go Outdated Show resolved Hide resolved
config/toml.go Outdated Show resolved Hide resolved
p2p/pex/pex_reactor.go Outdated Show resolved Hide resolved
rpc/core/pipe.go Outdated Show resolved Hide resolved
p2p/pex/pex_reactor.go Outdated Show resolved Hide resolved
p2p/pex/pex_reactor.go Outdated Show resolved Hide resolved
p2p/pex/pex_reactor.go Outdated Show resolved Hide resolved
@marbar3778 marbar3778 changed the title [ADR] ADR-050 spec implementation p2p: implement adr-050 Dec 2, 2019
dongsam and others added 3 commits Dec 3, 2019
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
# Conflicts:
#	docs/spec/p2p/config.md
#	docs/spec/reactors/pex/pex.md
@dongsam dongsam force-pushed the dongsam:ADR-050-Improved-Trusted-Peering branch from 74e289d to a607dbc Dec 3, 2019
p2p/pex/pex_reactor.go Outdated Show resolved Hide resolved
CHANGELOG_PENDING.md Outdated Show resolved Hide resolved
p2p/pex/pex_reactor.go Show resolved Hide resolved
p2p/pex/pex_reactor.go Outdated Show resolved Hide resolved
p2p/switch.go Outdated Show resolved Hide resolved
p2p/switch.go Outdated Show resolved Hide resolved
p2p/switch.go Outdated Show resolved Hide resolved
rpc/core/pipe.go Outdated Show resolved Hide resolved
dongsam and others added 2 commits Dec 3, 2019
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
p2p/pex/pex_reactor.go Outdated Show resolved Hide resolved
p2p/pex/pex_reactor.go Show resolved Hide resolved
rpc/core/pipe.go Outdated Show resolved Hide resolved
@dongsam

This comment has been minimized.

Copy link
Contributor Author

dongsam commented Dec 3, 2019

Is there non-decisive test failures in ci/circleci: test_cover?
It didn't fail when running in my local env

// NumPeers returns the count of outbound/inbound and outbound-dialing peers.
func (sw *Switch) NumPeers() (outbound, inbound, dialing int) {
// NumPeers returns the count of outbound/inbound, outbound-dialing and unconditional peers.
func (sw *Switch) NumPeers() (outbound, inbound, dialing, unconditionalOutbound, unconditionalInbound int) {

This comment has been minimized.

Copy link
@melekes

melekes Dec 3, 2019

Collaborator

we can revert this change now

p2p/switch.go Show resolved Hide resolved
p2p/switch_test.go Show resolved Hide resolved
p2p/switch_test.go Show resolved Hide resolved
p2p/switch_test.go Show resolved Hide resolved
p2p/pex/pex_reactor.go Show resolved Hide resolved
@melekes
melekes approved these changes Dec 4, 2019
@melekes melekes merged commit 701e9ca into tendermint:master Dec 4, 2019
18 checks passed
18 checks passed
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
GolangCI No issues found!
Details
LGTM analysis: Go No new or fixed alerts
Details
ci/circleci: build_artifacts Your tests passed on CircleCI!
Details
ci/circleci: contract_tests Your tests passed on CircleCI!
Details
ci/circleci: localnet Your tests passed on CircleCI!
Details
ci/circleci: prepare_build Your tests passed on CircleCI!
Details
ci/circleci: setup_dependencies Your tests passed on CircleCI!
Details
ci/circleci: test_abci_apps Your tests passed on CircleCI!
Details
ci/circleci: test_abci_cli Your tests passed on CircleCI!
Details
ci/circleci: test_apps Your tests passed on CircleCI!
Details
ci/circleci: test_cover Your tests passed on CircleCI!
Details
ci/circleci: test_p2p Your tests passed on CircleCI!
Details
ci/circleci: test_persistence Your tests passed on CircleCI!
Details
ci/circleci: upload_coverage Your tests passed on CircleCI!
Details
codecov/patch 78.26% of diff hit (target 67.33%)
Details
codecov/project 67.33% (+<.01%) compared to 100078c
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.