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

Peer API fix for library users of fuel-core #1542

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

Voxelot
Copy link
Member

@Voxelot Voxelot commented Dec 12, 2023

This PR restructures the changes from #1524 to fix an error that would block users from calling client.chain_info() without having the p2p feature enabled.

While throwing errors on apis that use disabled features is our current convention, it would mean that the sdk testing harness would fail when trying to get the consensus parameters unless it also enabled the p2p feature flag.

What changed in this PR:

  • Information about peers now part of NodeInfo instead of ChainInfo. This seemed like a more suitable location, as chain info relates more towards global network settings and state. NodeInfo on the other hand shows details about the current node instance you've connected to.
  • Peers are not returned in the default query for NodeInfo, and instead use a special query fragment that's used to only return info about peers. This is beneficial since it will allow users without p2p enabled to still fetch NodeInfo, and also reduces api bandwidth by only returning potentially bulky information about peers through a dedicated api request instead of by default.

Note: This is a release blocker for 0.22 to unblock the sdk

@Voxelot Voxelot self-assigned this Dec 12, 2023
…ient.node_info() api

unblock users of client api to fetch chain_info when p2p is disabled.
@Voxelot Voxelot force-pushed the Voxelot/restructure-peer-info-api branch from 931e162 to 69ee11b Compare December 12, 2023 02:34
@Voxelot Voxelot requested a review from a team December 12, 2023 02:37
@Voxelot Voxelot enabled auto-merge (squash) December 12, 2023 02:45
Copy link
Contributor

@bvrooman bvrooman left a comment

Choose a reason for hiding this comment

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

This appears to be a fairly straightforward migration of the peer info query from chain info to node info.

@Voxelot Voxelot merged commit d134579 into master Dec 12, 2023
31 checks passed
@Voxelot Voxelot deleted the Voxelot/restructure-peer-info-api branch December 12, 2023 02:59
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