Skip to content

Commit

Permalink
Merge #1139: Do error cleanups
Browse files Browse the repository at this point in the history
abc014a Move rustdoc above attribute (Tobin C. Harding)
e923001 Implement std::error::Error::source for MerkleBlockError (Tobin C. Harding)
613f1cf Implement std::error::Error::source for bip152 (Tobin C. Harding)
a37ab82 Add non_exhaustive to _all_ errors (Tobin C. Harding)

Pull request description:

  Do error cleanups, and add a script to help find missing `non_exhaustive` on error types

  - Patch 1: Audit the codebase and put `non_exhaustive` on all  error types.
  - Patch 2: Implement `std::error::Error::source` for `bip152::Error`
  - Patch 3: Implement `Display` and `std::error::Error::source` for `MerkleBlockError`
  - Patch 4: Move rustdocs to above attributes on one error type
  - ~Patch 5: Add a python script to `contrib` that checks the codebase for missing non exhaustive on the line above the results of regex `pub enum .*Error`~

  I removed the Python script patch, I can't be bothered working on this for now but the clean ups and `non_exhaustive` additions are useful IMO.

  ### labels
  - I added 'API break', I think the `non_exhaustive` addition requires major bump but not sure?
  - release notes mention is just for the `non_exhaustive` patch.

ACKs for top commit:
  Kixunil:
    ACK abc014a
  apoelstra:
    ACK abc014a

Tree-SHA512: 16ea15014eae97de7ac9cca1e9b76304aa3702a98cde577c2d71343022f840d3b33a39d2ab6d3fba0f0f1ebaa1631a0595eea1d794ba9727fe6ccfcf08e2feae
  • Loading branch information
apoelstra committed Aug 15, 2022
2 parents 67e88e7 + abc014a commit 9f37fdd
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/blockdata/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,9 @@ impl Default for TxIn {
#[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
pub struct Sequence(pub u32);

/// An error in creating relative lock-times.
#[derive(Clone, PartialEq, Eq, Debug)]
#[non_exhaustive]
/// An error in creating relative lock-times.
pub enum RelativeLockTimeError {
/// The input was too large
IntegerOverflow(u32)
Expand Down
12 changes: 11 additions & 1 deletion src/util/bip152.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::{Block, BlockHash, BlockHeader, Transaction};

/// A BIP-152 error
#[derive(Clone, PartialEq, Eq, Debug, Copy, PartialOrd, Ord, Hash)]
#[non_exhaustive]
pub enum Error {
/// An unknown version number was used.
UnknownVersion,
Expand All @@ -37,7 +38,16 @@ impl fmt::Display for Error {
}

#[cfg(feature = "std")]
impl error::Error for Error {}
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
impl std::error::Error for Error {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use self::Error::*;

match *self {
UnknownVersion | InvalidPrefill => None,
}
}
}

/// A [PrefilledTransaction] structure is used in [HeaderAndShortIds] to
/// provide a list of a few transactions explicitly.
Expand Down
1 change: 1 addition & 0 deletions src/util/bip158.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const M: u64 = 784931;

/// Errors for blockfilter.
#[derive(Debug)]
#[non_exhaustive]
pub enum Error {
/// Missing UTXO, cannot calculate script filter.
UtxoMissing(OutPoint),
Expand Down
38 changes: 33 additions & 5 deletions src/util/merkleblock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
//! assert_eq!(1, index[0]);
//! ```

use core::fmt;

use crate::prelude::*;

use crate::io;
Expand All @@ -53,19 +55,45 @@ use crate::consensus::encode::{self, Decodable, Encodable};
use crate::util::merkleblock::MerkleBlockError::*;
use crate::{Block, BlockHeader};

/// An error when verifying the merkle block
/// An error when verifying the merkle block.
#[derive(Clone, PartialEq, Eq, Debug)]
#[non_exhaustive]
pub enum MerkleBlockError {
/// When header merkle root don't match to the root calculated from the partial merkle tree
/// Merkle root in the header doesn't match to the root calculated from partial merkle tree.
MerkleRootMismatch,
/// When partial merkle tree contains no transactions
/// Partial merkle tree contains no transactions.
NoTransactions,
/// When there are too many transactions
/// There are too many transactions.
TooManyTransactions,
/// General format error
/// General format error.
BadFormat(String),
}

impl fmt::Display for MerkleBlockError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use self::MerkleBlockError::*;

match *self {
MerkleRootMismatch => write!(f, "merkle header root doesn't match to the root calculated from the partial merkle tree"),
NoTransactions => write!(f, "partial merkle tree contains no transactions"),
TooManyTransactions => write!(f, "too many transactions"),
BadFormat(ref s) => write!(f, "general format error: {}", s),
}
}
}

#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
impl std::error::Error for MerkleBlockError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use self::MerkleBlockError::*;

match *self {
MerkleRootMismatch | NoTransactions | TooManyTransactions | BadFormat(_) => None,
}
}
}

/// Data structure that represents a partial merkle tree.
///
/// It represents a subset of the txid's of a known block, in a way that
Expand Down

0 comments on commit 9f37fdd

Please sign in to comment.