Skip to content

Commit

Permalink
chore(workspace): Use thiserror for Error Types (#269)
Browse files Browse the repository at this point in the history
### Description

Now that `thiserror` supports `no_std`, we can use `thiserror` for error
types over `derive_more::Display`.

This PR updates the workspace to use `thiserror`.
  • Loading branch information
refcell authored Nov 16, 2024
1 parent 067001f commit 16b2164
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 144 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ async-trait = "0.1.83"
unsigned-varint = "0.8.0"
spin = { version = "0.9.8", features = ["mutex"] }
derive_more = { version = "1.0", default-features = false }
thiserror = { version = "2.0", default-features = false }
similar-asserts = "1.6"

# tracing
Expand All @@ -112,7 +113,6 @@ tracing = { version = "0.1.40", default-features = false }
arbitrary = { version = "1.3", features = ["derive"] }
arbtest = "0.3"
rand = "0.8"
thiserror = "1.0"
proptest = "1.5"
proptest-derive = "0.5"
tokio = "1"
Expand Down
2 changes: 1 addition & 1 deletion crates/protocol/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ alloy-consensus.workspace = true

# Misc
tracing.workspace = true
derive_more.workspace = true
thiserror.workspace = true
async-trait.workspace = true
unsigned-varint.workspace = true

Expand Down
94 changes: 28 additions & 66 deletions crates/protocol/src/batch/errors.rs
Original file line number Diff line number Diff line change
@@ -1,123 +1,85 @@
//! Span Batch Errors

/// Span Batch Errors
#[derive(Debug, derive_more::Display, Clone, PartialEq, Eq)]
#[derive(Debug, thiserror::Error, Clone, PartialEq, Eq)]
pub enum SpanBatchError {
/// The span batch is too big
#[display("The span batch is too big.")]
#[error("The span batch is too big.")]
TooBigSpanBatchSize,
/// The bit field is too long
#[display("The bit field is too long")]
#[error("The bit field is too long")]
BitfieldTooLong,
/// Empty Span Batch
#[display("Empty span batch")]
#[error("Empty span batch")]
EmptySpanBatch,
/// Missing L1 origin
#[display("Missing L1 origin")]
#[error("Missing L1 origin")]
MissingL1Origin,
/// Decoding errors
#[display("Span batch decoding error: {_0}")]
Decoding(SpanDecodingError),
}

impl From<SpanDecodingError> for SpanBatchError {
fn from(err: SpanDecodingError) -> Self {
Self::Decoding(err)
}
}

impl core::error::Error for SpanBatchError {
fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
match self {
Self::Decoding(err) => Some(err),
_ => None,
}
}
#[error("Span batch decoding error: {0}")]
Decoding(#[from] SpanDecodingError),
}

/// An error encoding a batch.
#[derive(Debug, derive_more::Display, Clone, PartialEq, Eq)]
#[derive(Debug, thiserror::Error, Clone, PartialEq, Eq)]
pub enum BatchEncodingError {
/// Error encoding an Alloy RLP
#[display("Error encoding an Alloy RLP: {_0}")]
#[error("Error encoding an Alloy RLP: {0}")]
AlloyRlpError(alloy_rlp::Error),
/// Error encoding a span batch
#[display("Error encoding a span batch: {_0}")]
SpanBatchError(SpanBatchError),
#[error("Error encoding a span batch: {0}")]
SpanBatchError(#[from] SpanBatchError),
}

/// An error decoding a batch.
#[derive(Debug, derive_more::Display, Clone, PartialEq, Eq)]
#[derive(Debug, thiserror::Error, Clone, PartialEq, Eq)]
pub enum BatchDecodingError {
/// Empty buffer
#[display("Empty buffer")]
#[error("Empty buffer")]
EmptyBuffer,
/// Error decoding an Alloy RLP
#[display("Error decoding an Alloy RLP: {_0}")]
#[error("Error decoding an Alloy RLP: {0}")]
AlloyRlpError(alloy_rlp::Error),
/// Error decoding a span batch
#[display("Error decoding a span batch: {_0}")]
SpanBatchError(SpanBatchError),
}

impl From<alloy_rlp::Error> for BatchDecodingError {
fn from(err: alloy_rlp::Error) -> Self {
Self::AlloyRlpError(err)
}
}

impl From<SpanBatchError> for BatchDecodingError {
fn from(err: SpanBatchError) -> Self {
Self::SpanBatchError(err)
}
}

impl core::error::Error for BatchDecodingError {
fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
match self {
Self::SpanBatchError(err) => Some(err),
_ => None,
}
}
#[error("Error decoding a span batch: {0}")]
SpanBatchError(#[from] SpanBatchError),
}

/// Decoding Error
#[derive(Debug, derive_more::Display, Clone, PartialEq, Eq)]
#[derive(Debug, thiserror::Error, Clone, PartialEq, Eq)]
pub enum SpanDecodingError {
/// Failed to decode relative timestamp
#[display("Failed to decode relative timestamp")]
#[error("Failed to decode relative timestamp")]
RelativeTimestamp,
/// Failed to decode L1 origin number
#[display("Failed to decode L1 origin number")]
#[error("Failed to decode L1 origin number")]
L1OriginNumber,
/// Failed to decode parent check
#[display("Failed to decode parent check")]
#[error("Failed to decode parent check")]
ParentCheck,
/// Failed to decode L1 origin check
#[display("Failed to decode L1 origin check")]
#[error("Failed to decode L1 origin check")]
L1OriginCheck,
/// Failed to decode block count
#[display("Failed to decode block count")]
#[error("Failed to decode block count")]
BlockCount,
/// Failed to decode block tx counts
#[display("Failed to decode block tx counts")]
#[error("Failed to decode block tx counts")]
BlockTxCounts,
/// Failed to decode transaction nonces
#[display("Failed to decode transaction nonces")]
#[error("Failed to decode transaction nonces")]
TxNonces,
/// Mismatch in length between the transaction type and signature arrays in a span batch
/// transaction payload.
#[display("Mismatch in length between the transaction type and signature arrays")]
#[error("Mismatch in length between the transaction type and signature arrays")]
TypeSignatureLenMismatch,
/// Invalid transaction type
#[display("Invalid transaction type")]
#[error("Invalid transaction type")]
InvalidTransactionType,
/// Invalid transaction data
#[display("Invalid transaction data")]
#[error("Invalid transaction data")]
InvalidTransactionData,
/// Invalid transaction signature
#[display("Invalid transaction signature")]
#[error("Invalid transaction signature")]
InvalidTransactionSignature,
}

impl core::error::Error for SpanDecodingError {}
41 changes: 11 additions & 30 deletions crates/protocol/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,50 +89,31 @@ impl arbitrary::Arbitrary<'_> for L2BlockInfo {
}

/// An error that can occur when converting an [OpBlock] to an [L2BlockInfo].
#[derive(Debug, derive_more::Display)]
#[derive(Debug, thiserror::Error)]
pub enum FromBlockError {
/// The genesis block hash does not match the expected value.
#[display("Invalid genesis hash")]
#[error("Invalid genesis hash")]
InvalidGenesisHash,
/// The L2 block is missing the L1 info deposit transaction.
#[display("L2 block is missing L1 info deposit transaction ({_0})")]
#[error("L2 block is missing L1 info deposit transaction ({0})")]
MissingL1InfoDeposit(B256),
/// The first payload transaction has an unexpected type.
#[display("First payload transaction has unexpected type: {_0}")]
#[error("First payload transaction has unexpected type: {0}")]
UnexpectedTxType(u8),
/// Failed to decode the first transaction into an [OpTxEnvelope].
#[display("Failed to decode the first transaction into an OpTxEnvelope: {_0}")]
#[error("Failed to decode the first transaction into an OpTxEnvelope: {0}")]
TxEnvelopeDecodeError(Eip2718Error),
/// The first payload transaction is not a deposit transaction.
#[display("First payload transaction is not a deposit transaction, type: {_0}")]
#[error("First payload transaction is not a deposit transaction, type: {0}")]
FirstTxNonDeposit(u8),
/// Failed to decode the [L1BlockInfoTx] from the deposit transaction.
#[display("Failed to decode the L1BlockInfoTx from the deposit transaction: {_0}")]
BlockInfoDecodeError(DecodeError),
#[error("Failed to decode the L1BlockInfoTx from the deposit transaction: {0}")]
BlockInfoDecodeError(#[from] DecodeError),
}

// Since `Eip2718Error` uses an msrv prior to rust `1.81`, the `core::error::Error` type
// is not stabalized and `Eip2718Error` only implements `std::error::Error` and not
// `core::error::Error`. So we need to implement `std::error::Error` to provide the `Eip2718Error`
// as a source when the `std` feature is enabled.
#[cfg(feature = "std")]
impl std::error::Error for FromBlockError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::TxEnvelopeDecodeError(err) => Some(err),
Self::BlockInfoDecodeError(err) => Some(err),
_ => None,
}
}
}

#[cfg(not(feature = "std"))]
impl core::error::Error for FromBlockError {
fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
match self {
Self::BlockInfoDecodeError(err) => Some(err),
_ => None,
}
impl From<Eip2718Error> for FromBlockError {
fn from(value: Eip2718Error) -> Self {
Self::TxEnvelopeDecodeError(value)
}
}

Expand Down
6 changes: 2 additions & 4 deletions crates/protocol/src/brotli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ use core::ops;
use crate::MAX_SPAN_BATCH_ELEMENTS;

/// A frame decompression error.
#[derive(derive_more::Display, Debug, PartialEq, Eq)]
#[derive(thiserror::Error, Debug, PartialEq, Eq)]
pub enum BatchDecompressionError {
/// The buffer exceeds the [MAX_SPAN_BATCH_ELEMENTS] protocol parameter.
#[display("The batch exceeds the maximum number of elements: {max_size}", max_size = MAX_SPAN_BATCH_ELEMENTS)]
#[error("The batch exceeds the maximum number of elements: {max_size}", max_size = MAX_SPAN_BATCH_ELEMENTS)]
BatchTooLarge,
}

impl core::error::Error for BatchDecompressionError {}

/// Decompresses the given bytes data using the Brotli decompressor implemented
/// in the [`brotli`](https://crates.io/crates/brotli) crate.
pub fn decompress_brotli(
Expand Down
9 changes: 6 additions & 3 deletions crates/protocol/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,25 @@ pub const CHANNEL_ID_LENGTH: usize = 16;
pub type ChannelId = [u8; CHANNEL_ID_LENGTH];

/// An error returned by the [ChannelOut] when adding single batches.
#[derive(Debug, Clone, derive_more::Display)]
#[derive(Debug, Clone, thiserror::Error)]
pub enum ChannelOutError {
/// The channel is closed.
#[error("The channel is already closed")]
ChannelClosed,
/// The max frame size is too small.
#[error("The max frame size is too small")]
MaxFrameSizeTooSmall,
/// Missing compressed batch data.
#[error("Missing compressed batch data")]
MissingData,
/// An error from brotli compression.
#[error("Error from Brotli compression")]
BrotliCompression,
/// An error encoding the `Batch`.
#[error("Error encoding the batch")]
BatchEncoding,
}

impl core::error::Error for ChannelOutError {}

/// [ChannelOut] constructs a channel from compressed, encoded batch data.
#[derive(Debug, Clone)]
pub struct ChannelOut<'a> {
Expand Down
31 changes: 8 additions & 23 deletions crates/protocol/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,24 @@ use alloy_primitives::B256;
/// An error encountered during [OpBlock] conversion.
///
/// [OpBlock]: op_alloy_consensus::OpBlock
#[derive(Debug, derive_more::Display)]
#[derive(Debug, thiserror::Error)]
pub enum OpBlockConversionError {
/// Invalid genesis hash.
#[display("Invalid genesis hash. Expected {_0}, got {_1}")]
#[error("Invalid genesis hash. Expected {0}, got {1}")]
InvalidGenesisHash(B256, B256),
/// Invalid transaction type.
#[display("First payload transaction has unexpected type: {_0}")]
#[error("First payload transaction has unexpected type: {0}")]
InvalidTxType(u8),
/// L1 Info error
#[display("Failed to decode L1 info: {_0}")]
L1InfoError(DecodeError),
#[error("Failed to decode L1 info: {0}")]
L1InfoError(#[from] DecodeError),
/// Missing system config in genesis block.
#[display("Missing system config in genesis block")]
#[error("Missing system config in genesis block")]
MissingSystemConfigGenesis,
/// Empty transactions.
#[display("Empty transactions in payload. Block hash: {_0}")]
#[error("Empty transactions in payload. Block hash: {0}")]
EmptyTransactions(B256),
/// EIP-1559 parameter decoding error.
#[display("Failed to decode EIP-1559 parameters from header's `nonce` field.")]
#[error("Failed to decode EIP-1559 parameters from header's `nonce` field.")]
Eip1559DecodeError,
}

impl core::error::Error for OpBlockConversionError {
fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
match self {
Self::L1InfoError(err) => Some(err),
_ => None,
}
}
}

impl From<DecodeError> for OpBlockConversionError {
fn from(e: DecodeError) -> Self {
Self::L1InfoError(e)
}
}
8 changes: 3 additions & 5 deletions crates/protocol/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,16 @@ use tracing_subscriber::{layer::Context, Layer};
use crate::{BatchValidationProvider, L2BlockInfo};

/// An error for implementations of the [BatchValidationProvider] trait.
#[derive(Debug, derive_more::Display)]
#[derive(Debug, thiserror::Error)]
pub enum TestBatchValidatorError {
/// The block was not found.
#[display("Block not found")]
#[error("Block not found")]
BlockNotFound,
/// The L2 block was not found.
#[display("L2 Block not found")]
#[error("L2 Block not found")]
L2BlockNotFound,
}

impl core::error::Error for TestBatchValidatorError {}

/// An [TestBatchValidator] implementation for testing.
#[derive(Debug, Default, Clone)]
pub struct TestBatchValidator {
Expand Down
4 changes: 2 additions & 2 deletions crates/protocol/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Utility methods used by protocol types.

use alloc::{vec, vec::Vec};
use alloc::vec::Vec;
use alloy_consensus::TxType;
use alloy_primitives::B256;
use alloy_rlp::{Buf, Header};
Expand All @@ -17,7 +17,7 @@ use crate::{
#[cfg(feature = "std")]
pub fn compress_brotli(mut input: &[u8]) -> Vec<u8> {
use brotli::enc::{BrotliCompress, BrotliEncoderParams};
let mut output = vec![];
let mut output = alloc::vec![];
BrotliCompress(&mut input, &mut output, &BrotliEncoderParams::default()).expect("succeeds");
output
}
Expand Down
1 change: 1 addition & 0 deletions crates/rpc-types-engine/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ serde = { workspace = true, optional = true }
alloy-serde = { workspace = true, optional = true }

# misc
thiserror.workspace = true
derive_more = { workspace = true, features = ["display"] }
arbitrary = { workspace = true, features = ["derive"], optional = true }

Expand Down
Loading

0 comments on commit 16b2164

Please sign in to comment.