-
Notifications
You must be signed in to change notification settings - Fork 26
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
chore(consensus): add documentation to consensus crate #2626
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2626 +/- ##
==========================================
- Coverage 40.10% 9.76% -30.35%
==========================================
Files 26 87 +61
Lines 1895 10491 +8596
Branches 1895 10491 +8596
==========================================
+ Hits 760 1024 +264
- Misses 1100 9430 +8330
- Partials 35 37 +2 ☔ View full report in Codecov by Sentry. |
197a45e
to
67f41c7
Compare
3280825
to
8dd1c7b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 6 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @matan-starkware)
crates/sequencing/papyrus_consensus/src/simulation_network_receiver.rs
line 35 at r2 (raw file):
impl NetworkReceiver { /// Create a new NetworkReceiver.
Suggestion:
/// Creates a new NetworkReceiver.
crates/sequencing/papyrus_consensus/src/simulation_network_receiver.rs
line 39 at r2 (raw file):
/// Inputs: /// - `broadcasted_messages_receiver`: The receiver to listen to. /// - `cache_size`: A small cache risks repeat messages all being handled the same way.
Suggestion:
/// - `cache_size`: Determines the size of the cache. A smaller cache increases the risk of handling
/// repeated messages in the same way.
crates/sequencing/papyrus_consensus/src/manager.rs
line 5 at r2 (raw file):
//! [`run_consensus`] - This is the primary entrypoint for running the consensus component. //! //! [`MultiHeightManager`] - Run consensus repeatedly across different heights
Suggestion:
//! [`MultiHeightManager`] - Runs consensus repeatedly across different heights using
crates/sequencing/papyrus_consensus/src/manager.rs
line 48 at r2 (raw file):
/// - `validator_id`: The ID of this node. /// - `consensus_delay`: The delay before starting consensus. There to allow the network to connect /// to peers.
Suggestion:
/// - `consensus_delay`: The delay before starting consensus, allowing time the network to connect
/// to peers.
crates/sequencing/papyrus_consensus/src/manager.rs
line 51 at r2 (raw file):
/// - `timeouts`: The timeouts for the consensus algorithm. /// - `inbound_vote_receiver`: The channels to receive votes from the network. These are self /// contained messages.
Suggestion:
/// - `vote_receiver`: The channels to receive votes from the network. These are self
/// contained messages.
crates/sequencing/papyrus_consensus/src/manager.rs
line 53 at r2 (raw file):
/// contained messages. /// - `inbound_proposal_receiver`: The channel to receive proposals from the network. Proposals are /// represented as streams (ProposalInit, Content.*, ProposalFin).
Suggestion:
/// - `proposal_receiver`: The channel to receive proposals from the network. Proposals are
/// represented as streams (ProposalInit, Content.*, ProposalFin).
crates/sequencing/papyrus_consensus/src/single_height_consensus.rs
line 154 at r2 (raw file):
/// - Timeouts: the SM returns an event timeout, but SHC then maps that to a task which can be run /// by the Manager. The manager though unaware of the specific task as it has minimal consensus /// logic.
Suggestion:
/// Represents a single height of consensus. It is responsible for mapping between the idealized
/// view of consensus represented in the StateMachine and the real world implementation.
///
/// Example:
/// - Timeouts: the SM returns an event timeout, but SHC then maps that to a task which can be run
/// by the Manager. The manager though unaware of the specific task as it has minimal consensus
/// logic.
crates/sequencing/papyrus_consensus/src/lib.rs
line 18 at r2 (raw file):
//! Consensus can run in two modes: //! 1. Observer - receive consensus messages and update the node when a decision is reached. //! 2. Active - in addition to receiving messages the node can send messages to the network.
Suggestion:
//! Consensus operates in two modes:
//! 1. Observer - Receives consensus messages and updates the node when a decision is reached.
//! 2. Active - In addition to receiving messages, the node can also send messages to the network.
crates/sequencing/papyrus_consensus/src/lib.rs
line 21 at r2 (raw file):
//! //! Observer mode is lower latency than sync, since we process Proposals and votes as they happen, //! not after the decision is made.
Suggestion:
//! Observer mode offers lower latency compared to sync, as Proposals and votes are processed in real-time
//! rather than after a decision has been made.
8dd1c7b
to
7af7abc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 5 of 6 files reviewed, 9 unresolved discussions (waiting on @asmaastarkware)
crates/sequencing/papyrus_consensus/src/simulation_network_receiver.rs
line 35 at r2 (raw file):
impl NetworkReceiver { /// Create a new NetworkReceiver.
Done.
crates/sequencing/papyrus_consensus/src/simulation_network_receiver.rs
line 39 at r2 (raw file):
/// Inputs: /// - `broadcasted_messages_receiver`: The receiver to listen to. /// - `cache_size`: A small cache risks repeat messages all being handled the same way.
Done.
crates/sequencing/papyrus_consensus/src/single_height_consensus.rs
line 154 at r2 (raw file):
/// - Timeouts: the SM returns an event timeout, but SHC then maps that to a task which can be run /// by the Manager. The manager though unaware of the specific task as it has minimal consensus /// logic.
Done.
crates/sequencing/papyrus_consensus/src/manager.rs
line 48 at r2 (raw file):
/// - `validator_id`: The ID of this node. /// - `consensus_delay`: The delay before starting consensus. There to allow the network to connect /// to peers.
Done.
crates/sequencing/papyrus_consensus/src/manager.rs
line 51 at r2 (raw file):
/// - `timeouts`: The timeouts for the consensus algorithm. /// - `inbound_vote_receiver`: The channels to receive votes from the network. These are self /// contained messages.
Done.
crates/sequencing/papyrus_consensus/src/manager.rs
line 53 at r2 (raw file):
/// contained messages. /// - `inbound_proposal_receiver`: The channel to receive proposals from the network. Proposals are /// represented as streams (ProposalInit, Content.*, ProposalFin).
Done.
crates/sequencing/papyrus_consensus/src/lib.rs
line 18 at r2 (raw file):
//! Consensus can run in two modes: //! 1. Observer - receive consensus messages and update the node when a decision is reached. //! 2. Active - in addition to receiving messages the node can send messages to the network.
Done.
crates/sequencing/papyrus_consensus/src/lib.rs
line 21 at r2 (raw file):
//! //! Observer mode is lower latency than sync, since we process Proposals and votes as they happen, //! not after the decision is made.
Done.
7af7abc
to
d53f067
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @matan-starkware)
2230d1a
to
2b9f298
Compare
2b9f298
to
213f6c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: 7 of 8 files reviewed, all discussions resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @matan-starkware)
No description provided.