Skip to content

fix(p2p/pex): do not send PEX request in fast dial mode#4644

Merged
jmalicevic merged 5 commits intomainfrom
cason/p2p-pex-requests
Dec 12, 2024
Merged

fix(p2p/pex): do not send PEX request in fast dial mode#4644
jmalicevic merged 5 commits intomainfrom
cason/p2p-pex-requests

Conversation

@cason
Copy link

@cason cason commented Dec 11, 2024

Solves #4620.

Fixes an issue introduced by #3360.

In short, when receiving addresses from a configured seed node, the peer immediately dials the received addresses, without waiting for the defaultEnsurePeersPeriod (30s). This is a desired behavior, the "fast dial mode" in the title.

However, for preventing abuse, a node only accepts PEX requests from a peer every minReceiveRequestInterval() time, set to defaultEnsurePeersPeriod/3 (10s). When running this "fast dial mode", however, a PEX request can be send, in some unlucky setup, to the same peer without waiting for the full defaultEnsurePeersPeriod` (30s).

The problem is that at the receive side, a node keeps track of the latest PEX request received from each peer. If two requests are received with an interval lower than minReceiveRequestInterval(), the peer is considered abusive: it is disconnected with an ErrReceivedPEXRequestTooSoon error and banned from the address book.

This PR proposes a workaround to prevent the above mentioned scenario.


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

@cason cason requested a review from a team as a code owner December 11, 2024 11:00
@cason cason requested a review from a team December 11, 2024 11:00
@cason cason added bug Something isn't working p2p backport-to-v1.x backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x labels Dec 11, 2024
@cason cason linked an issue Dec 11, 2024 that may be closed by this pull request
@cason cason assigned cason and jmalicevic and unassigned cason Dec 11, 2024
Copy link
Collaborator

@melekes melekes left a comment

Choose a reason for hiding this comment

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

lgtm

@jmalicevic jmalicevic added this pull request to the merge queue Dec 12, 2024
Merged via the queue into main with commit 491379f Dec 12, 2024
@jmalicevic jmalicevic deleted the cason/p2p-pex-requests branch December 12, 2024 10:34
mergify bot pushed a commit that referenced this pull request Dec 12, 2024
Solves #4620.

Fixes an issue introduced by
#3360.

In short, when receiving addresses from a configured seed node, the peer
immediately dials the received addresses, without waiting for the
`defaultEnsurePeersPeriod` (30s). This is a desired behavior, the "fast
dial mode" in the title.

However, for preventing abuse, a node only accepts PEX requests from a
peer every `minReceiveRequestInterval()` time, set to
`defaultEnsurePeersPeriod/3` (10s). When running this "fast dial mode",
however, a PEX request can be send, in some unlucky setup, to the same
peer without waiting for the full defaultEnsurePeersPeriod` (30s).

The problem is that at the receive side, a node keeps track of the
latest PEX request received from each peer. If two requests are received
with an interval lower than `minReceiveRequestInterval()`, the peer is
considered abusive: it is disconnected with an
`ErrReceivedPEXRequestTooSoon` error and banned from the address book.

This PR proposes a workaround to prevent the above mentioned scenario.

---

#### 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

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 491379f)

# Conflicts:
#	p2p/pex/pex_reactor.go
mergify bot added a commit that referenced this pull request Dec 12, 2024
Solves #4620.

Fixes an issue introduced by
#3360.

In short, when receiving addresses from a configured seed node, the peer
immediately dials the received addresses, without waiting for the
`defaultEnsurePeersPeriod` (30s). This is a desired behavior, the "fast
dial mode" in the title.

However, for preventing abuse, a node only accepts PEX requests from a
peer every `minReceiveRequestInterval()` time, set to
`defaultEnsurePeersPeriod/3` (10s). When running this "fast dial mode",
however, a PEX request can be send, in some unlucky setup, to the same
peer without waiting for the full defaultEnsurePeersPeriod` (30s).

The problem is that at the receive side, a node keeps track of the
latest PEX request received from each peer. If two requests are received
with an interval lower than `minReceiveRequestInterval()`, the peer is
considered abusive: it is disconnected with an
`ErrReceivedPEXRequestTooSoon` error and banned from the address book.

This PR proposes a workaround to prevent the above mentioned scenario.

---

#### 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

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 491379f)
jmalicevic added a commit that referenced this pull request Dec 12, 2024
…) (#4649)

Solves #4620.

Fixes an issue introduced by
#3360.

---------

Co-authored-by: Daniel <daniel.cason@informal.systems>
Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>
jmalicevic pushed a commit that referenced this pull request Dec 12, 2024
…) (#4648)

Solves #4620.

Fixes an issue introduced by
#3360.


Co-authored-by: Daniel <daniel.cason@informal.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x bug Something isn't working p2p

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

p2p: sent next PEX request too soon

3 participants