Light Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

refactor(*): Transport redesign#4568

Merged
mergify[bot] merged 240 commits intolibp2p:masterfrom
umgefahren:transport-redesign
Aug 3, 2024
Merged

refactor(*): Transport redesign#4568
mergify[bot] merged 240 commits intolibp2p:masterfrom
umgefahren:transport-redesign

Conversation

Copy link
Contributor

umgefahren commented Sep 27, 2023 *
edited
Loading

Description

Resolves: #4226.
Resolves: #3953.
Resolves: #3889.

Notes & open questions

What do you think of the implementation?

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

umgefahren changed the title Transport redesign refactor(core): Transport redesign Sep 27, 2023
umgefahren mentioned this pull request Sep 27, 2023
dariusc93 reviewed Sep 27, 2023
Copy link
Contributor

thomaseizinger left a comment

Choose a reason for hiding this comment

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

Great start! I left some high-level comments :)

Copy link
Contributor Author

umgefahren commented Sep 28, 2023 *
edited
Loading

I have implemented some of the suggestions. However I didn't touch the question on whether DialOpts should be struct or the field should just be passed as arguments.

I also moved some methods into a macro.

Copy link
Contributor Author

umgefahren commented Sep 28, 2023

Since we are touching PortUse anyway and we don't want to introduce breaking changes in the future: Is there a use for choosing not only if we should use an existing or new one, but also a specific one?

Copy link
Contributor Author

umgefahren commented Oct 4, 2023

At the moment I'm pretty pleased with the implementation. But I don't really like having the list of things that we wan't to strip. It feels like something that might cause a bug later on, but I honestly don't have a better idea.

umgefahren changed the title refactor(core): Transport redesign refactor(core, swarm): Transport redesign Oct 4, 2023
Copy link
Contributor

thomaseizinger commented Oct 5, 2023

But I don't really like having the list of things that we wan't to strip. It feels like something that might cause a bug later on, but I honestly don't have a better idea.

As long as we cover it with unit tests, that is fine. We can always ship improvements as patch releases later if we find that we are missing something :)

Copy link
Contributor Author

umgefahren commented Aug 3, 2024

I think this is a fine solution. It is known that one of the disadvantages of enabling port reuse is that you can only have a single connection to another peer. Thus, our best way of fulfilling the request to establish another connection is to ignore port reuse and simply allocate a new port. I think that is better than the connection failing.

I have two suggestions for slightly revising the log and bumping its severity.

I'm glad you are fine with the solution and I implemented your suggestions. I'm ready to merge.

thomaseizinger approved these changes Aug 3, 2024
Copy link
Contributor

thomaseizinger left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

thomaseizinger commented Aug 3, 2024

Great to see this being so close to merging. Thanks for pushing this over the finish line @umgefahren.

happybeing reacted with thumbs up emoji

Copy link
Contributor Author

umgefahren commented Aug 3, 2024

Is there something left for me to do for merging?

Copy link
Contributor

thomaseizinger commented Aug 3, 2024

Is there something left for me to do for merging?

Not at this stage. I'll leave it to @jxs to coordinate when it is good to go in!

jxs reacted with thumbs up emoji

jxs approved these changes Aug 3, 2024
Copy link
Member

jxs left a comment

Choose a reason for hiding this comment

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

LGTM Thanks Hannes for your hard work, and Thomas for the review!
I will try to release 0.54 on Monday

jxs added the send-it label Aug 3, 2024
mergify bot merged commit 079b2d6 into libp2p:master Aug 3, 2024
Copy link
Contributor Author

umgefahren commented Aug 3, 2024

Thanks to everyone involved, I'm sorry it took so much longer than anticipated.

umgefahren deleted the transport-redesign branch August 3, 2024 15:02
Copy link
Contributor

thomaseizinger commented Aug 3, 2024 *
edited
Loading

I will try to release 0.54 on Monday

Do you not want to release AutoNatV2 with this together? There is another PR coming for that I'd assume.

umgefahren mentioned this pull request Aug 4, 2024
5 tasks
Copy link
Contributor Author

umgefahren commented Aug 4, 2024

I will try to release 0.54 on Monday

Do you not want to release AutoNatV2 with this together? There is another PR coming for that I'd assume.

In case you do: I have opened #5526.

b-zee mentioned this pull request Aug 5, 2024
mergify bot pushed a commit that referenced this pull request Aug 8, 2024
Closes: #4524

This is the implementation of the evolved AutoNAT protocol, named AutonatV2 as defined in the [spec](https://github.com/libp2p/specs/blob/03718ef0f2dea4a756a85ba716ee33f97e4a6d6c/autonat/autonat-v2.md).
The stabilization PR for the spec can be found under libp2p/specs#538.

The work on the Rust implementation can be found in the PR to my fork: umgefahren#1.

The implementation has been smoke-tested with the Go implementation (PR: libp2p/go-libp2p#2469).

The new protocol addresses shortcomings of the original AutoNAT protocol:

- Since the server now always dials back over a newly allocated port, this made #4568 necessary; the client can be sure of the reachability state for other peers, even if the connection to the server was made through a hole punch.
- The server can now test addresses different from the observed address (i.e., the connection to the server was made through a `p2p-circuit`). To mitigate against DDoS attacks, the client has to send more data to the server than the dial-back costs.

Pull-Request: #5526.
TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this pull request Sep 14, 2024
TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this pull request Sep 14, 2024
Closes: libp2p#4524

This is the implementation of the evolved AutoNAT protocol, named AutonatV2 as defined in the [spec](https://github.com/libp2p/specs/blob/03718ef0f2dea4a756a85ba716ee33f97e4a6d6c/autonat/autonat-v2.md).
The stabilization PR for the spec can be found under libp2p/specs#538.

The work on the Rust implementation can be found in the PR to my fork: umgefahren#1.

The implementation has been smoke-tested with the Go implementation (PR: libp2p/go-libp2p#2469).

The new protocol addresses shortcomings of the original AutoNAT protocol:

- Since the server now always dials back over a newly allocated port, this made libp2p#4568 necessary; the client can be sure of the reachability state for other peers, even if the connection to the server was made through a hole punch.
- The server can now test addresses different from the observed address (i.e., the connection to the server was made through a `p2p-circuit`). To mitigate against DDoS attacks, the client has to send more data to the server than the dial-back costs.

Pull-Request: libp2p#5526.
nazar-pc mentioned this pull request Oct 27, 2024
2 tasks
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this pull request Dec 12, 2024
# Description

Fixes #5996

https://github.com/libp2p/rust-libp2p/releases/tag/libp2p-v0.53.0
https://github.com/libp2p/rust-libp2p/blob/master/CHANGELOG.md

## Integration

Nothing special is needed, just note that `yamux_window_size` is no
longer applicable to libp2p (litep2p seems to still have it though).

## Review Notes

There are a few simplifications and improvements done in libp2p 0.53
regarding swarm interface, I'll list a few key/applicable here.

libp2p/rust-libp2p#4788 removed
`write_length_prefixed` function, so I inlined its code instead.

libp2p/rust-libp2p#4120 introduced new
`libp2p::SwarmBuilder` instead of now deprecated
`libp2p::swarm::SwarmBuilder`, the transition is straightforward and
quite ergonomic (can be seen in tests).

libp2p/rust-libp2p#4581 is the most annoying
change I have seen that basically makes many enums `#[non_exhaustive]`.
I mapped some, but those that couldn't be mapped I dealt with by
printing log messages once they are hit (the best solution I could come
up with, at least with stable Rust).

libp2p/rust-libp2p#4306 makes connection close
as soon as there are no handler using it, so I had to replace
`KeepAlive::Until` with an explicit future that flips internal boolean
after timeout, achieving the old behavior, though it should ideally be
removed completely at some point.

`yamux_window_size` is no longer used by libp2p thanks to
libp2p/rust-libp2p#4970 and generally Yamux
should have a higher performance now.

I have resolved and cleaned up all deprecations related to libp2p except
`BandwidthSinks`. Libp2p deprecated it (though it is still present in
0.54.1, which is why I didn't handle it just yet). Ideally Substrate
would finally [switch to the official Prometheus
client](paritytech/substrate#12699), in which
case we'd get metrics for free. Otherwise a bit of code will need to be
copy-pasted to maintain current behavior with `BandwidthSinks` gone,
which I left a TODO about.

The biggest change in 0.54.0 is
libp2p/rust-libp2p#4568 that changed transport
APIs and enabled unconditional potential port reuse, which can lead to
very confusing errors if running two Substrate nodes on the same machine
without changing listening port explicitly.

Overall nothing scary here, but testing is always appreciated.

# Checklist

* [x] My PR includes a detailed description as outlined in the
"Description" and its two subsections above.
* [x] My PR follows the [labeling requirements](

https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process
) of this project (at minimum one label for `T` required)
* External contributors: ask maintainers to put the right label on your
PR.

---

Polkadot Address: 1vSxzbyz2cJREAuVWjhXUT1ds8vBzoxn2w4asNpusQKwjJd

---------

Co-authored-by: Dmitry Markin
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this pull request Dec 12, 2024
# Description

Fixes #5996

https://github.com/libp2p/rust-libp2p/releases/tag/libp2p-v0.53.0
https://github.com/libp2p/rust-libp2p/blob/master/CHANGELOG.md

## Integration

Nothing special is needed, just note that `yamux_window_size` is no
longer applicable to libp2p (litep2p seems to still have it though).

## Review Notes

There are a few simplifications and improvements done in libp2p 0.53
regarding swarm interface, I'll list a few key/applicable here.

libp2p/rust-libp2p#4788 removed
`write_length_prefixed` function, so I inlined its code instead.

libp2p/rust-libp2p#4120 introduced new
`libp2p::SwarmBuilder` instead of now deprecated
`libp2p::swarm::SwarmBuilder`, the transition is straightforward and
quite ergonomic (can be seen in tests).

libp2p/rust-libp2p#4581 is the most annoying
change I have seen that basically makes many enums `#[non_exhaustive]`.
I mapped some, but those that couldn't be mapped I dealt with by
printing log messages once they are hit (the best solution I could come
up with, at least with stable Rust).

libp2p/rust-libp2p#4306 makes connection close
as soon as there are no handler using it, so I had to replace
`KeepAlive::Until` with an explicit future that flips internal boolean
after timeout, achieving the old behavior, though it should ideally be
removed completely at some point.

`yamux_window_size` is no longer used by libp2p thanks to
libp2p/rust-libp2p#4970 and generally Yamux
should have a higher performance now.

I have resolved and cleaned up all deprecations related to libp2p except
`BandwidthSinks`. Libp2p deprecated it (though it is still present in
0.54.1, which is why I didn't handle it just yet). Ideally Substrate
would finally [switch to the official Prometheus
client](paritytech/substrate#12699), in which
case we'd get metrics for free. Otherwise a bit of code will need to be
copy-pasted to maintain current behavior with `BandwidthSinks` gone,
which I left a TODO about.

The biggest change in 0.54.0 is
libp2p/rust-libp2p#4568 that changed transport
APIs and enabled unconditional potential port reuse, which can lead to
very confusing errors if running two Substrate nodes on the same machine
without changing listening port explicitly.

Overall nothing scary here, but testing is always appreciated.

# Checklist

* [x] My PR includes a detailed description as outlined in the
"Description" and its two subsections above.
* [x] My PR follows the [labeling requirements](

https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process
) of this project (at minimum one label for `T` required)
* External contributors: ask maintainers to put the right label on your
PR.

---

Polkadot Address: 1vSxzbyz2cJREAuVWjhXUT1ds8vBzoxn2w4asNpusQKwjJd

---------

Co-authored-by: Dmitry Markin
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this pull request Dec 12, 2024
# Description

Fixes #5996

https://github.com/libp2p/rust-libp2p/releases/tag/libp2p-v0.53.0
https://github.com/libp2p/rust-libp2p/blob/master/CHANGELOG.md

## Integration

Nothing special is needed, just note that `yamux_window_size` is no
longer applicable to libp2p (litep2p seems to still have it though).

## Review Notes

There are a few simplifications and improvements done in libp2p 0.53
regarding swarm interface, I'll list a few key/applicable here.

libp2p/rust-libp2p#4788 removed
`write_length_prefixed` function, so I inlined its code instead.

libp2p/rust-libp2p#4120 introduced new
`libp2p::SwarmBuilder` instead of now deprecated
`libp2p::swarm::SwarmBuilder`, the transition is straightforward and
quite ergonomic (can be seen in tests).

libp2p/rust-libp2p#4581 is the most annoying
change I have seen that basically makes many enums `#[non_exhaustive]`.
I mapped some, but those that couldn't be mapped I dealt with by
printing log messages once they are hit (the best solution I could come
up with, at least with stable Rust).

libp2p/rust-libp2p#4306 makes connection close
as soon as there are no handler using it, so I had to replace
`KeepAlive::Until` with an explicit future that flips internal boolean
after timeout, achieving the old behavior, though it should ideally be
removed completely at some point.

`yamux_window_size` is no longer used by libp2p thanks to
libp2p/rust-libp2p#4970 and generally Yamux
should have a higher performance now.

I have resolved and cleaned up all deprecations related to libp2p except
`BandwidthSinks`. Libp2p deprecated it (though it is still present in
0.54.1, which is why I didn't handle it just yet). Ideally Substrate
would finally [switch to the official Prometheus
client](paritytech/substrate#12699), in which
case we'd get metrics for free. Otherwise a bit of code will need to be
copy-pasted to maintain current behavior with `BandwidthSinks` gone,
which I left a TODO about.

The biggest change in 0.54.0 is
libp2p/rust-libp2p#4568 that changed transport
APIs and enabled unconditional potential port reuse, which can lead to
very confusing errors if running two Substrate nodes on the same machine
without changing listening port explicitly.

Overall nothing scary here, but testing is always appreciated.

# Checklist

* [x] My PR includes a detailed description as outlined in the
"Description" and its two subsections above.
* [x] My PR follows the [labeling requirements](

https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process
) of this project (at minimum one label for `T` required)
* External contributors: ask maintainers to put the right label on your
PR.

---

Polkadot Address: 1vSxzbyz2cJREAuVWjhXUT1ds8vBzoxn2w4asNpusQKwjJd

---------

Co-authored-by: Dmitry Markin
github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this pull request Dec 16, 2024
# Description

Fixes #5996

https://github.com/libp2p/rust-libp2p/releases/tag/libp2p-v0.53.0
https://github.com/libp2p/rust-libp2p/blob/master/CHANGELOG.md

## Integration

Nothing special is needed, just note that `yamux_window_size` is no
longer applicable to libp2p (litep2p seems to still have it though).

## Review Notes

There are a few simplifications and improvements done in libp2p 0.53
regarding swarm interface, I'll list a few key/applicable here.

libp2p/rust-libp2p#4788 removed
`write_length_prefixed` function, so I inlined its code instead.

libp2p/rust-libp2p#4120 introduced new
`libp2p::SwarmBuilder` instead of now deprecated
`libp2p::swarm::SwarmBuilder`, the transition is straightforward and
quite ergonomic (can be seen in tests).

libp2p/rust-libp2p#4581 is the most annoying
change I have seen that basically makes many enums `#[non_exhaustive]`.
I mapped some, but those that couldn't be mapped I dealt with by
printing log messages once they are hit (the best solution I could come
up with, at least with stable Rust).

libp2p/rust-libp2p#4306 makes connection close
as soon as there are no handler using it, so I had to replace
`KeepAlive::Until` with an explicit future that flips internal boolean
after timeout, achieving the old behavior, though it should ideally be
removed completely at some point.

`yamux_window_size` is no longer used by libp2p thanks to
libp2p/rust-libp2p#4970 and generally Yamux
should have a higher performance now.

I have resolved and cleaned up all deprecations related to libp2p except
`BandwidthSinks`. Libp2p deprecated it (though it is still present in
0.54.1, which is why I didn't handle it just yet). Ideally Substrate
would finally [switch to the official Prometheus
client](paritytech/substrate#12699), in which
case we'd get metrics for free. Otherwise a bit of code will need to be
copy-pasted to maintain current behavior with `BandwidthSinks` gone,
which I left a TODO about.

The biggest change in 0.54.0 is
libp2p/rust-libp2p#4568 that changed transport
APIs and enabled unconditional potential port reuse, which can lead to
very confusing errors if running two Substrate nodes on the same machine
without changing listening port explicitly.

Overall nothing scary here, but testing is always appreciated.

# Checklist

* [x] My PR includes a detailed description as outlined in the
"Description" and its two subsections above.
* [x] My PR follows the [labeling requirements](

https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process
) of this project (at minimum one label for `T` required)
* External contributors: ask maintainers to put the right label on your
PR.

---

Polkadot Address: 1vSxzbyz2cJREAuVWjhXUT1ds8vBzoxn2w4asNpusQKwjJd

---------

Co-authored-by: Dmitry Markin
nazar-pc added a commit to autonomys/polkadot-sdk that referenced this pull request Dec 20, 2024
# Description

Fixes paritytech#5996

https://github.com/libp2p/rust-libp2p/releases/tag/libp2p-v0.53.0
https://github.com/libp2p/rust-libp2p/blob/master/CHANGELOG.md

## Integration

Nothing special is needed, just note that `yamux_window_size` is no
longer applicable to libp2p (litep2p seems to still have it though).

## Review Notes

There are a few simplifications and improvements done in libp2p 0.53
regarding swarm interface, I'll list a few key/applicable here.

libp2p/rust-libp2p#4788 removed
`write_length_prefixed` function, so I inlined its code instead.

libp2p/rust-libp2p#4120 introduced new
`libp2p::SwarmBuilder` instead of now deprecated
`libp2p::swarm::SwarmBuilder`, the transition is straightforward and
quite ergonomic (can be seen in tests).

libp2p/rust-libp2p#4581 is the most annoying
change I have seen that basically makes many enums `#[non_exhaustive]`.
I mapped some, but those that couldn't be mapped I dealt with by
printing log messages once they are hit (the best solution I could come
up with, at least with stable Rust).

libp2p/rust-libp2p#4306 makes connection close
as soon as there are no handler using it, so I had to replace
`KeepAlive::Until` with an explicit future that flips internal boolean
after timeout, achieving the old behavior, though it should ideally be
removed completely at some point.

`yamux_window_size` is no longer used by libp2p thanks to
libp2p/rust-libp2p#4970 and generally Yamux
should have a higher performance now.

I have resolved and cleaned up all deprecations related to libp2p except
`BandwidthSinks`. Libp2p deprecated it (though it is still present in
0.54.1, which is why I didn't handle it just yet). Ideally Substrate
would finally [switch to the official Prometheus
client](paritytech/substrate#12699), in which
case we'd get metrics for free. Otherwise a bit of code will need to be
copy-pasted to maintain current behavior with `BandwidthSinks` gone,
which I left a TODO about.

The biggest change in 0.54.0 is
libp2p/rust-libp2p#4568 that changed transport
APIs and enabled unconditional potential port reuse, which can lead to
very confusing errors if running two Substrate nodes on the same machine
without changing listening port explicitly.

Overall nothing scary here, but testing is always appreciated.

# Checklist

* [x] My PR includes a detailed description as outlined in the
"Description" and its two subsections above.
* [x] My PR follows the [labeling requirements](

https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process
) of this project (at minimum one label for `T` required)
* External contributors: ask maintainers to put the right label on your
PR.

---

Polkadot Address: 1vSxzbyz2cJREAuVWjhXUT1ds8vBzoxn2w4asNpusQKwjJd

---------

Co-authored-by: Dmitry Markin

(cherry picked from commit c881288)
dudo50 pushed a commit to paraspell-research/polkadot-sdk that referenced this pull request Jan 4, 2025
# Description

Fixes paritytech#5996

https://github.com/libp2p/rust-libp2p/releases/tag/libp2p-v0.53.0
https://github.com/libp2p/rust-libp2p/blob/master/CHANGELOG.md

## Integration

Nothing special is needed, just note that `yamux_window_size` is no
longer applicable to libp2p (litep2p seems to still have it though).

## Review Notes

There are a few simplifications and improvements done in libp2p 0.53
regarding swarm interface, I'll list a few key/applicable here.

libp2p/rust-libp2p#4788 removed
`write_length_prefixed` function, so I inlined its code instead.

libp2p/rust-libp2p#4120 introduced new
`libp2p::SwarmBuilder` instead of now deprecated
`libp2p::swarm::SwarmBuilder`, the transition is straightforward and
quite ergonomic (can be seen in tests).

libp2p/rust-libp2p#4581 is the most annoying
change I have seen that basically makes many enums `#[non_exhaustive]`.
I mapped some, but those that couldn't be mapped I dealt with by
printing log messages once they are hit (the best solution I could come
up with, at least with stable Rust).

libp2p/rust-libp2p#4306 makes connection close
as soon as there are no handler using it, so I had to replace
`KeepAlive::Until` with an explicit future that flips internal boolean
after timeout, achieving the old behavior, though it should ideally be
removed completely at some point.

`yamux_window_size` is no longer used by libp2p thanks to
libp2p/rust-libp2p#4970 and generally Yamux
should have a higher performance now.

I have resolved and cleaned up all deprecations related to libp2p except
`BandwidthSinks`. Libp2p deprecated it (though it is still present in
0.54.1, which is why I didn't handle it just yet). Ideally Substrate
would finally [switch to the official Prometheus
client](paritytech/substrate#12699), in which
case we'd get metrics for free. Otherwise a bit of code will need to be
copy-pasted to maintain current behavior with `BandwidthSinks` gone,
which I left a TODO about.

The biggest change in 0.54.0 is
libp2p/rust-libp2p#4568 that changed transport
APIs and enabled unconditional potential port reuse, which can lead to
very confusing errors if running two Substrate nodes on the same machine
without changing listening port explicitly.

Overall nothing scary here, but testing is always appreciated.

# Checklist

* [x] My PR includes a detailed description as outlined in the
"Description" and its two subsections above.
* [x] My PR follows the [labeling requirements](

https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process
) of this project (at minimum one label for `T` required)
* External contributors: ask maintainers to put the right label on your
PR.

---

Polkadot Address: 1vSxzbyz2cJREAuVWjhXUT1ds8vBzoxn2w4asNpusQKwjJd

---------

Co-authored-by: Dmitry Markin
acul71 mentioned this pull request Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

dariusc93 dariusc93 left review comments

mxinden mxinden left review comments

jxs jxs approved these changes

thomaseizinger thomaseizinger approved these changes

+1 more reviewer

stormshield-ebzh stormshield-ebzh left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Projects

None yet

Milestone

v0.54.0

7 participants