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

chore: Improve error definitions #15

Merged
merged 2 commits into from
Oct 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions node/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 17 additions & 3 deletions node/actors/consensus/src/leader/replica_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,41 @@ use network::io::{ConsensusInputMessage, Target};
use roles::validator;
use tracing::instrument;

#[derive(thiserror::Error, Debug)]
#[allow(clippy::missing_docs_in_private_items)]
/// Errors that can occur when processing a "replica commit" message.
#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
/// Unexpected proposal.
#[error("unexpected proposal")]
UnexpectedProposal,
/// Past view or phase.
#[error("past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
Old {
/// Current view.
current_view: validator::ViewNumber,
/// Current phase.
current_phase: validator::Phase,
},
/// The processing node is not a lead for this message's view.
#[error("we are not a leader for this message's view")]
NotLeaderInView,
/// Duplicate message from a replica.
#[error("duplicate message from a replica (existing message: {existing_message:?}")]
DuplicateMessage {
/// Existing message from the same replica.
existing_message: validator::ReplicaCommit,
},
#[error("number of received messages is below threshold. waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
/// Number of received messages is below threshold.
#[error(
"number of received messages is below threshold. waiting for more (received: {num_messages:?}, \
threshold: {threshold:?}"
)]
NumReceivedBelowThreshold {
/// Number of received messages.
num_messages: usize,
/// Threshold for message count.
threshold: usize,
},
/// Invalid message signature.
#[error("invalid signature: {0:#}")]
InvalidSignature(#[source] crypto::bls12_381::Error),
}
Expand Down
23 changes: 20 additions & 3 deletions node/actors/consensus/src/leader/replica_prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,51 @@ use roles::validator;
use std::collections::HashMap;
use tracing::instrument;

#[derive(thiserror::Error, Debug)]
#[allow(clippy::missing_docs_in_private_items)]
/// Errors that can occur when processing a "replica prepare" message.
#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
/// Past view or phase.
#[error("past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
Old {
/// Current view.
current_view: validator::ViewNumber,
/// Current phase.
current_phase: validator::Phase,
},
/// The node is not a leader for this message's view.
#[error("we are not a leader for this message's view")]
NotLeaderInView,
/// Duplicate message from a replica.
#[error("duplicate message from a replica (existing message: {existing_message:?}")]
Exists {
/// Existing message from the same replica.
existing_message: validator::ReplicaPrepare,
},
#[error("number of received messages below threshold. waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
/// Number of received messages is below a threshold.
#[error(
"number of received messages below threshold. waiting for more (received: {num_messages:?}, \
threshold: {threshold:?}"
)]
NumReceivedBelowThreshold {
/// Number of received messages.
num_messages: usize,
/// Threshold for message count.
threshold: usize,
},
/// High QC of a future view.
#[error(
"high QC of a future view (high QC view: {high_qc_view:?}, current view: {current_view:?}"
)]
HighQCOfFutureView {
/// Received high QC view.
high_qc_view: validator::ViewNumber,
/// Current view.
current_view: validator::ViewNumber,
},
/// Invalid message signature.
#[error("invalid signature: {0:#}")]
InvalidSignature(#[source] crypto::bls12_381::Error),
/// Invalid `HighQC` message.
#[error("invalid high QC: {0:#}")]
InvalidHighQC(#[source] anyhow::Error),
}
Expand Down
17 changes: 14 additions & 3 deletions node/actors/consensus/src/replica/leader_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,34 @@ use concurrency::ctx;
use roles::validator;
use tracing::instrument;

#[derive(thiserror::Error, Debug)]
#[allow(clippy::missing_docs_in_private_items)]
/// Errors that can occur when processing a "leader commit" message.
#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
#[error("invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})]")]
/// Invalid leader.
#[error(
"invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})"
)]
InvalidLeader {
/// Correct leader.
correct_leader: validator::PublicKey,
/// Received leader.
received_leader: validator::PublicKey,
},
/// Past view of phase.
#[error("past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
Old {
/// Current view.
current_view: validator::ViewNumber,
/// Current phase.
current_phase: validator::Phase,
},
/// Invalid message signature.
#[error("invalid signature: {0:#}")]
InvalidSignature(#[source] crypto::bls12_381::Error),
/// Invalid justification for the message.
#[error("invalid justification: {0:#}")]
InvalidJustification(#[source] anyhow::Error),
/// Internal error. Unlike other error types, this one isn't supposed to be easily recoverable.
#[error("internal error: {0:#}")]
Internal(#[from] anyhow::Error),
}
Expand Down
51 changes: 45 additions & 6 deletions node/actors/consensus/src/replica/leader_prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,61 +7,100 @@ use roles::validator;
use std::collections::HashMap;
use tracing::instrument;

#[derive(thiserror::Error, Debug)]
#[allow(clippy::missing_docs_in_private_items)]
/// Errors that can occur when processing a "leader prepare" message.
#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
#[error("invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?}])")]
/// Invalid leader.
#[error(
"invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})"
)]
InvalidLeader {
/// Correct leader.
correct_leader: validator::PublicKey,
/// Received leader.
received_leader: validator::PublicKey,
},
#[error("message for a past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
/// Message for a past view or phase.
#[error(
"message for a past view / phase (current view: {current_view:?}, current phase: {current_phase:?})"
)]
Old {
/// Current view.
current_view: validator::ViewNumber,
/// Current phase.
current_phase: validator::Phase,
},
/// Invalid message signature.
#[error("invalid signature: {0:#}")]
InvalidSignature(#[source] crypto::bls12_381::Error),
/// Invalid `PrepareQC` message.
#[error("invalid PrepareQC: {0:#}")]
InvalidPrepareQC(#[source] anyhow::Error),
/// Invalid `HighQC` message.
#[error("invalid high QC: {0:#}")]
InvalidHighQC(#[source] anyhow::Error),
/// High QC of a future view.
#[error(
"high QC of a future view (high QC view: {high_qc_view:?}, current view: {current_view:?}"
)]
HighQCOfFutureView {
/// Received high QC view.
high_qc_view: validator::ViewNumber,
/// Current view.
current_view: validator::ViewNumber,
},
/// Previous proposal was not finalized.
#[error("new block proposal when the previous proposal was not finalized")]
ProposalWhenPreviousNotFinalized,
#[error("block proposal with invalid parent hash (correct parent hash: {correct_parent_hash:#?}, received parent hash: {received_parent_hash:#?}, block: {header:?})")]
/// Invalid parent hash.
#[error(
"block proposal with invalid parent hash (correct parent hash: {correct_parent_hash:#?}, \
received parent hash: {received_parent_hash:#?}, block: {header:?})"
)]
ProposalInvalidParentHash {
/// Correct parent hash.
correct_parent_hash: validator::BlockHeaderHash,
/// Received parent hash.
received_parent_hash: validator::BlockHeaderHash,
/// Header including the incorrect parent hash.
header: validator::BlockHeader,
},
#[error("block proposal with non-sequential number (correct proposal number: {correct_number}, received proposal number: {received_number}, block: {header:?})")]
/// Non-sequential proposal number.
#[error(
"block proposal with non-sequential number (correct proposal number: {correct_number}, \
received proposal number: {received_number}, block: {header:?})"
)]
ProposalNonSequentialNumber {
/// Correct proposal number.
correct_number: validator::BlockNumber,
/// Received proposal number.
received_number: validator::BlockNumber,
/// Header including the incorrect proposal number.
header: validator::BlockHeader,
},
/// Mismatched payload.
#[error("block proposal with mismatched payload")]
ProposalMismatchedPayload,
/// Oversized payload.
#[error(
"block proposal with an oversized payload (payload size: {payload_size}, block: {header:?}"
)]
ProposalOversizedPayload {
/// Size of the payload.
payload_size: usize,
/// Proposal header corresponding to the payload.
header: validator::BlockHeader,
},
/// Re-proposal without quorum.
#[error("block re-proposal without quorum for the re-proposal")]
ReproposalWithoutQuorum,
/// Re-proposal when the previous proposal was finalized.
#[error("block re-proposal when the previous proposal was finalized")]
ReproposalWhenFinalized,
/// Re-proposal of invalid block.
#[error("block re-proposal of invalid block")]
ReproposalInvalidBlock,
/// Internal error. Unlike other error types, this one isn't supposed to be easily recoverable.
#[error("internal error: {0:#}")]
Internal(#[from] anyhow::Error),
}
Expand Down
4 changes: 2 additions & 2 deletions node/actors/network/src/consensus/handshake/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ impl ProtoFmt for Handshake {
}
}

/// Error returned by AuthStream::handshake.
#[derive(thiserror::Error, Debug)]
/// Error returned by handshake logic.
#[derive(Debug, thiserror::Error)]
pub(super) enum Error {
#[error("session id mismatch")]
SessionIdMismatch,
Expand Down
6 changes: 3 additions & 3 deletions node/actors/network/src/gossip/handshake/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ impl ProtoFmt for Handshake {
}
}

/// Error returned by AuthStream::handshake.
#[derive(thiserror::Error, Debug)]
/// Error returned by gossip handshake logic.
#[derive(Debug, thiserror::Error)]
pub(super) enum Error {
#[error("session id mismatch")]
SessionIdMismatch,
#[error("unexpected peer")]
PeerMismatch,
#[error("validator signature")]
Signature(#[from] node::ErrInvalidSignature),
Signature(#[from] node::InvalidSignatureError),
#[error("stream")]
Stream(#[source] anyhow::Error),
}
Expand Down
3 changes: 1 addition & 2 deletions node/actors/network/src/io.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#![allow(missing_docs)]
use concurrency::oneshot;
use roles::{node, validator};
use thiserror::Error;

/// All the messages that other actors can send to the Network actor.
#[derive(Debug)]
Expand Down Expand Up @@ -84,7 +83,7 @@ impl SyncState {
/// Errors returned from a [`GetBlockResponse`].
///
/// Note that these errors don't include network-level errors, only app-level ones.
#[derive(Debug, Error)]
#[derive(Debug, thiserror::Error)]
pub enum GetBlockError {
/// Transient error: the node doesn't have the requested L2 block and plans to get it in the future
/// by syncing.
Expand Down
2 changes: 1 addition & 1 deletion node/actors/network/src/mux/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl Mux {
}

/// Error returned by `Mux::run()`.
#[derive(thiserror::Error, Debug)]
#[derive(Debug, thiserror::Error)]
#[allow(clippy::missing_docs_in_private_items)]
pub(crate) enum RunError {
#[error("config: {0:#}")]
Expand Down
2 changes: 1 addition & 1 deletion node/actors/network/src/mux/transient_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub(crate) struct ReadStream(pub(super) sync::ExclusiveLock<ReadReusableStream>)
pub(crate) struct WriteStream(pub(super) sync::ExclusiveLock<WriteReusableStream>);

/// Error returned by `ReadStream::read` in case the stream has been closed by peer.
#[derive(thiserror::Error, Debug)]
#[derive(Debug, thiserror::Error)]
#[error("end of stream")]
pub(crate) struct EndOfStream;

Expand Down
1 change: 1 addition & 0 deletions node/actors/sync_blocks/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ license.workspace = true

[dependencies]
anyhow.workspace = true
thiserror.workspace = true
tracing.workspace = true

concurrency = { path = "../../libs/concurrency" }
Expand Down
Loading
Loading