fix(txpool): Error in GossipsubMessageAcceptance variant docstrings #2316
Conversation
netrome
requested review from
xgreenx,
Dentosal and
MitchTurner
as code owners
I'm not certain, but the place that caused my eyebrows to shift is correct. I'll go over all references to these variants and see if I find any other error. But the documentation is incorrect, and this PR fixes that at least. |
crates/types/src/services/p2p.rs
Outdated
| @@ -52,10 +52,10 @@ pub enum GossipsubMessageAcceptance { | |||
| /// Report whether the gossiped message is valid and safe to rebroadcast | |||
There was a problem hiding this comment.
"Report whether" is weird wording here. Doesn't it just "Accept" the message?
There was a problem hiding this comment.
Agreed. I wonder if we should even use verbs in these comments like "Report". This is a data structure, and not a behavior we're documenting.
I've reworked the comments a bit now. What do you think of the following?
/// Status of a message received via Gossip
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
pub enum GossipsubMessageAcceptance {
/// The gossiped message is valid and safe to rebroadcast.
Accept,
/// The gossiped message is invalid and should be ignored.
Ignore,
/// The gossiped message is invalid and anyone relaying it should be penalized.
Reject,
}There was a problem hiding this comment.
Sweeped all references now. There seems to only be two places where the individual variants are referenced outside of test code (and this is extensively referenced from test code). The first place is the construction in So yes, I am confident that this is correct now. |
netrome
force-pushed
the
fix/gossipsub_message_info_docs
branch
from
6c589b8 to
7cf9472
Compare
| @@ -44,18 +44,15 @@ pub struct GossipsubMessageInfo { | |||
| pub peer_id: PeerId, | |||
| } | |||
|
|
|||
| // TODO: Maybe we can remove most of types from here directly into P2P | |||
There was a problem hiding this comment.
Removed this comment because it's not clear exactly what it refers to, and we don't have an issue to track it.
## Version v0.39.0 ### Added - [2324](#2324): Added metrics for sync, async processor and for all GraphQL queries. - [2320](#2320): Added new CLI flag `graphql-max-resolver-recursive-depth` to limit recursion within resolver. The default value it "1". ## Fixed - [2320](#2320): Prevent `/health` and `/v1/health` from being throttled by the concurrency limiter. - [2322](#2322): Set the salt of genesis contracts to zero on execution. - [2324](#2324): Ignore peer if we already are syncing transactions from it. #### Breaking - [2320](#2330): Reject queries that are recursive during the resolution of the query. ### Changed #### Breaking - [2311](#2311): Changed the text of the error returned by the executor if gas overflows. ## What's Changed * chore(executor): instrument errors to be more meaningful by @rymnc in #2311 * fix(dummy_da_block_costs): remove dependency on polling_interval and use channel instead by @rymnc in #2314 * fix(txpool): Error in GossipsubMessageAcceptance variant docstrings by @netrome in #2316 * refactor: Eager returns in txpool_v2::service::Task::run by @netrome in #2325 * fix(api_service): exclude health checks from throttling with ConcurrencyLimitLayer by @rymnc in #2320 * Remove the `normalize_rewards_and_costs()` function by @rafal-ch in #2293 * fix(genesis): set salt of contract on execution of genesis state configuration by @rymnc in #2322 * Fixing the issue with duplicate transaction synchronization processes by @xgreenx in #2324 * Reject queries that are recursive during the resolution by @xgreenx in #2330 **Full Changelog**: v0.38.0...v0.39.0
Description
The documentation for the
IgnoreandRejectvariants has been mixed up. This PR corrects it.