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

Why is ExtendedPeerInfo private in release-polkadot-v1.6.0? #3556

Closed
Ali-Usama opened this issue Mar 4, 2024 · 5 comments · Fixed by #3586
Closed

Why is ExtendedPeerInfo private in release-polkadot-v1.6.0? #3556

Ali-Usama opened this issue Mar 4, 2024 · 5 comments · Fixed by #3586
Labels
I10-unconfirmed Issue might be valid, but it's not yet known.

Comments

@Ali-Usama
Copy link
Contributor

Ali-Usama commented Mar 4, 2024

I was working on a custom consensus mechanism for Substrate on branch release-polkadot-v1.2.0, but there was a conflict with some of the dependencies due to which I had to upgrade it to the release-polkadot-v1.6.0, and I noticed that ExtendPeerInfo was moved from sc-network-common to sc-network-sync in this PR.

The issue that I'm facing is the ExtendedPeerInfo struct is now under the types module which is private so I can't access its components.

I wanted to ask if this was intentional and if there's any other way to access that struct?

@github-actions github-actions bot added the I10-unconfirmed Issue might be valid, but it's not yet known. label Mar 4, 2024
@altonen
Copy link
Contributor

altonen commented Mar 4, 2024

The idea is to get rid of sc-network-common because it doesn't serve a purpose anymore and if sc-network-sync is the only crate that use ExtendedPeerInfo, then that's the right place.

This could have been a simple oversight during refactoring but is there a particular reason you need access to ExtendedPeerInfo?

cc @dmitry-markin

@Ali-Usama
Copy link
Contributor Author

This could have been a simple oversight during refactoring but is there a particular reason you need access to ExtendedPeerInfo?

Yes, I was using it to get a list of connected peers in release-polkadot-v1.2.0 but it's not accessible in release-polkadot-v1.6.0. If it's a simple oversight, are there any plans to get it fixed soon enough?

@altonen
Copy link
Contributor

altonen commented Mar 4, 2024

Do you want to open a PR, making types public? Otherwise I'll try and fix it at some point this week

@altonen altonen linked a pull request Mar 4, 2024 that will close this issue
@bkchr
Copy link
Member

bkchr commented Mar 5, 2024

Can you not just copy this type? Is this type somewhere returned by a function?

@Ali-Usama
Copy link
Contributor Author

Can you not just copy this type? Is this type somewhere returned by a function?

I can't copy it because I'm using this PeerInfo command which expects the ExtendedPeerInfo to be coming from sc-network-sync instead of my local copy.

 if response.send(self.handler.peers_info()).is_err() {
    |                             ---- ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `sc_network_sync::types::ExtendedPeerInfo<B>`, found `ExtendedPeerInfo<B>`
    |                             |
    |                             arguments to this method are incorrect

github-merge-queue bot pushed a commit that referenced this issue Mar 6, 2024
This PR makes `sc-network-sync` types public following
[this](#3556) issue.

Fixes #3556
@github-project-automation github-project-automation bot moved this to Blocked ⛔️ in Networking Mar 6, 2024
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
This PR makes `sc-network-sync` types public following
[this](paritytech#3556) issue.

Fixes paritytech#3556
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I10-unconfirmed Issue might be valid, but it's not yet known.
Projects
Status: Blocked ⛔️
3 participants