Skip to content

improve efficiency of propagating address failures#443

Open
dharjeezy wants to merge 9 commits into
paritytech:masterfrom
dharjeezy:dami/dial-up-addresses
Open

improve efficiency of propagating address failures#443
dharjeezy wants to merge 9 commits into
paritytech:masterfrom
dharjeezy:dami/dial-up-addresses

Conversation

@dharjeezy

Copy link
Copy Markdown
Contributor

closes #409

Comment thread src/transport/manager/mod.rs
Comment thread src/transport/manager/mod.rs Outdated
@dharjeezy
dharjeezy requested a review from lexnv October 14, 2025 09:40
Comment thread src/transport/manager/mod.rs
Comment thread src/transport/manager/mod.rs
Comment thread src/transport/manager/mod.rs Outdated
Comment on lines +1138 to +1140
let adresses = context.dial_failure_addresses(&[address.clone()]);

match context.tx.try_send(make_dial_failure_event(peer, adresses.clone())) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: Is there a way to avoid the cloning here? Maybe changing the signature of the fn to take a & Multiaddr?

I would also inline the make_dial_failure_event:

match context.tx.try_send(InnerTransportEvent::DialFailure { peer, addresses })

Instead of cloning the addreses here (addresses.clone()), we could recreate the addresses on error

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i have changed to this inline way, but can't still do without cloning

@lexnv lexnv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me! Thanks again for contributing 🙏

@lexnv
lexnv requested a review from dmitry-markin October 31, 2025 12:26
peer,
addresses: vec![address.clone()],
})
.send(InnerTransportEvent::DialFailure { peer, addresses })

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@dharjeezy Can you also add to the doc of InnerTransportEvent & TransportEvent that addresses is only forwarded if the protocol was registered with DialFailureAddresses::Required, please?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is done now @dmitry-markin. can we merge it in now?

Comment thread src/transport/mod.rs Outdated
lexnv and others added 3 commits November 20, 2025 17:37
Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
@dharjeezy

dharjeezy commented Dec 14, 2025

Copy link
Copy Markdown
Contributor Author

@lexnv can we please get this merged?

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.

TransportManager: Improve the efficiency of propagating dial failure messages

3 participants