This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Refactor out disable/disconnect peer to make API more declarative #394
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tomaka
approved these changes
Jul 21, 2018
| if let (&Some(ref client_version), &Some(ref remote_address)) = (&peer_info.client_version, &peer_info.remote_address) { | ||
| info!(target: "network", "Disconnected peer {} (version: {}, address: {}). {}", peer_id, client_version, remote_address, reason); | ||
| } else { | ||
| info!(target: "network", "Disconnected peer {}. {}", peer_id, reason); |
Contributor
There was a problem hiding this comment.
info! is always printed on stdout even if logging is not enabled, right?
If so then that's probably too high.
Member
Author
There was a problem hiding this comment.
That's the point. Disconnections are hugely problematic right now.
tomaka
added
A5-grumble
and removed
A0-please_review
Member
Author
|
Should be somewhat less verbose now. |
gavofyork
added
A0-please_review
tomaka
approved these changes
Jul 21, 2018
| } | ||
|
|
||
| /// Get the info on a peer, if there's an active connection. | ||
| pub fn peer_info(&self, who: PeerId) -> Option<PeerInfo> { |
Contributor
There was a problem hiding this comment.
I feel like this should be merged with session_info, but I haven't looked at how the higher-level layers use session_info().
tomaka
added
A6-mustntgrumble
and removed
A0-please_review
liuchengxu
added a commit
to chainx-org/substrate
that referenced
this pull request
Aug 23, 2021
`id` of ChainX mainnet is _chainx_.
helin6
pushed a commit
to boolnetwork/substrate
that referenced
this pull request
Jul 25, 2023
paritytech#394) * WIP DispatchError generic param * main crate now compiling with new E generic param for DispatchError * Remove E param from RpcClient since it doesn't really need it * Point to generated DispatchError in codegen so no need for additional param there * More error hunting; cargo check --all-targets passes now * Use our own RuntimeVersion struct (for now) to avoid error decoding into sp_version::RuntimeVersion * cargo fmt * fix docs and expose private documented thing that I think should be pub * move error info to compile time so that we can make DispatchError a little nicer to work with * cargo fmt * clippy * Rework error handling to remove <E> param in most cases * fix Error doc ambiguity (hopefully) * doc tweaks * docs: remove dismbiguation thing that isn't needed now * One more Error<E> that can be a BasicError * rewrite pallet errors thing into normal loops to tidy * tidy errors codegen a little * tidy examples/custom_type_derives.rs a little * cargo fmt * silcnce clippy in example
helin6
pushed a commit
to boolnetwork/substrate
that referenced
this pull request
Jul 25, 2023
* Add error information back into metadata to roll back removal in paritytech#394 * Go back to obtaining runtime error info * re-do codegen too to check that it's all gravy * Convert DispatchError module errors into a module variant to make them easier to work with * Fix broken doc link
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the peer/
SyncIo/NetworkContextAPI to make it more declarative and less instructive. There are no changes to logic (expect, I think in one instance), but semantics are tweaked to make it clear that it's not up the high-level (Substrate protocol) to manage the peer set first-hand. Rather the high-level can inform the libp2p about certain (non-)events that could only be knowable by the high-level API, such as invalid or non-existent responses, badly formatted queries or inconsistent data. The lower, libp2p, layer can then decide what course of action to take.Rather than two separate functions (which is something of a pain), there is a single API for reporting high-level misbehaviour information on a peer:
report_peer. There is always areasongiven when calling reporting, and that reason implies aSeverity, which semantically informs substrate-libp2p on what course of action to take in response (e.g. lowering a reputation score, simple disconnect, temporary ban or permanent ban).I envisage that further information could be added to
Severitydown the line in order to express richer concepts of misbehaviour, but for now it's just three options (Bad, which bans the peer;UselessandTimeout, which currently both just disconnect the peer but don't attempt to prevent further connections).One immediately useful point of this PR is to ensure that all explicit peer disconnections come with an information notice including a reason for why they have been disconnected.