8000 fix(p2p/pex): respect MaxNumOutboundPeers limit while dialing peers provided by a seed node (backport #3360) by mergify[bot] · Pull Request #3396 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(p2p/pex): respect MaxNumOutboundPeers limit while dialing peers provided by a seed node (backport #3360) #3396

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

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

mergify[bot]
Copy link
Contributor
@mergify mergify bot commented Jul 2, 2024

Closes #486.

Commits:

  1. Make TestConnectionSpeedForPeerReceivedFromSeed to fail, as it will make the node to connect to more than the configured MaxNumOutboundPeers
  2. Solve the issue by introducing a channel to wake up ensurePeersRoutine(). In this way, the node will not to wait until the next defaultEnsurePeersPeriod (30s) before attempting again to connect to new peers (ensurePeers() method) whose addresses were provide by the seed node.

PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

This is an automatic backport of pull request #3360 done by [Mergify](https://mergify.com).

…rovided by a seed node (#3360)

Closes #486.

Commits:

1. Make `TestConnectionSpeedForPeerReceivedFromSeed` to fail, as it will
make the node to connect to more than the configured
`MaxNumOutboundPeers`
2. Solve the issue by introducing a channel to wake up
`ensurePeersRoutine()`. In this way, the node will not to wait until the
next `defaultEnsurePeersPeriod` (30s) before attempting again to connect
to new peers (`ensurePeers()` method) whose addresses were provide by
the seed node.

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 4241776)

# Conflicts:
#	.changelog/v0.38.3/bug-fixes/486-p2p-max-outbound.md
#	p2p/pex/pex_reactor.go
#	p2p/pex/pex_reactor_test.go
@mergify mergify bot requested review from a team as code owners July 2, 2024 09:50
@mergify mergify bot added the conflicts label Jul 2, 2024

This comment was marked as resolved.

Copy link
Contributor
@cason cason left a comment

Choose a reason for hiding this comment

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

Diffs match. The only exception is that require.Nil was replaced by require.NoError in this new version. This only affects the test unit changed by this PR, so it should be good.

@cason cason merged commit ecd56ec into v0.38.x Jul 2, 2024
21 checks passed
@cason cason deleted the mergify/bp/v0.38.x/pr-3360 branch July 2, 2024 10:20
@sergio-mena
Copy link
Contributor

@cason the changelog ended up in the wrong directory here. It should be in unreleased. Can you fix it?

@cason
Copy link
Contributor
cason commented Jul 8, 2024

Here: #3457

melekes pushed a commit that referenced this pull request Jul 8, 2024
Fixes issue identified by @sergio-mena:
#3396 (comment)

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
@sergio-mena @cason
0