Migrate to thiserror (#7177)

* Migrate to thiserror

* Discourage the use of other modules' Result alias

`io::Result` set a bad precedent. Don't import other `Result`
aliases.

ledger/src/bank_forks.rs

@@ -14,6 +14,7 @@ use std::{
     sync::Arc,
     time::Instant,
 };
+use thiserror::Error;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct SnapshotConfig {
@@ -27,24 +28,15 @@ pub struct SnapshotConfig {
     pub snapshot_path: PathBuf,
 }
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum BankForksError {
-    SnapshotError(SnapshotError),
-    SnapshotPackageSendError(SnapshotPackageSendError),
-}
-pub type Result<T> = std::result::Result<T, BankForksError>;
+    #[error("snapshot error")]
+    SnapshotError(#[from] SnapshotError),
 
-impl std::convert::From<SnapshotError> for BankForksError {
-    fn from(e: SnapshotError) -> BankForksError {
-        BankForksError::SnapshotError(e)
-    }
-}
-
-impl std::convert::From<SnapshotPackageSendError> for BankForksError {
-    fn from(e: SnapshotPackageSendError) -> BankForksError {
-        BankForksError::SnapshotPackageSendError(e)
-    }
+    #[error("snapshot package send error")]
+    SnapshotPackageSendError(#[from] SnapshotPackageSendError),
 }
+type Result<T> = std::result::Result<T, BankForksError>;
 
 pub struct BankForks {
     pub banks: HashMap<Slot, Arc<Bank>>,

ledger/src/block_error.rs

@@ -1,18 +1,25 @@
-#[derive(Debug, PartialEq)]
+use thiserror::Error;
+
+#[derive(Error, Debug, PartialEq)]
 pub enum BlockError {
     /// Block entries hashes must all be valid
+    #[error("invalid entry hash")]
     InvalidEntryHash,
 
     /// Blocks must end in a tick that has been marked as the last tick.
+    #[error("invalid last tick")]
     InvalidLastTick,
 
     /// Blocks can not have extra ticks or missing ticks
+    #[error("invalid tick count")]
     InvalidTickCount,
 
     /// All ticks must contain the same number of hashes within a block
+    #[error("invalid tick hash count")]
     InvalidTickHashCount,
 
     /// Blocks must end in a tick entry, trailing transaction entries are not allowed to guarantee
     /// that each block has the same number of hashes
+    #[error("trailing entry")]
     TrailingEntry,
 }

ledger/src/blocktree.rs

@@ -1,9 +1,11 @@
 //! The `blocktree` module provides functions for parallel verification of the
 //! Proof of History ledger as well as iterative read, append write, and random
 //! access read to a persistent file-based ledger.
+pub use crate::{blocktree_db::BlocktreeError, blocktree_meta::SlotMeta};
 use crate::{
     blocktree_db::{
-        columns as cf, Column, Database, IteratorDirection, IteratorMode, LedgerColumn, WriteBatch,
+        columns as cf, Column, Database, IteratorDirection, IteratorMode, LedgerColumn, Result,
+        WriteBatch,
     },
     blocktree_meta::*,
     entry::{create_ticks, Entry},
@@ -11,10 +13,6 @@ use crate::{
     leader_schedule_cache::LeaderScheduleCache,
     shred::{Shred, Shredder},
 };
-pub use crate::{
-    blocktree_db::{BlocktreeError, Result},
-    blocktree_meta::SlotMeta,
-};
 use bincode::deserialize;
 use chrono::{offset::TimeZone, Duration as ChronoDuration, Utc};
 use log::*;

ledger/src/blocktree_db.rs

@@ -12,6 +12,7 @@ use serde::Serialize;
 use solana_client::rpc_request::RpcTransactionStatus;
 use solana_sdk::{clock::Slot, signature::Signature};
 use std::{collections::HashMap, fs, marker::PhantomData, path::Path, sync::Arc};
+use thiserror::Error;
 
 // A good value for this is the number of cores on the machine
 const TOTAL_THREADS: i32 = 8;
@@ -35,18 +36,16 @@ const CODE_SHRED_CF: &str = "code_shred";
 /// Column family for Transaction Status
 const TRANSACTION_STATUS_CF: &str = "transaction_status";
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum BlocktreeError {
     ShredForIndexExists,
     InvalidShredData(Box<bincode::ErrorKind>),
-    RocksDb(rocksdb::Error),
+    RocksDb(#[from] rocksdb::Error),
     SlotNotRooted,
-    IO(std::io::Error),
-    Serialize(std::boxed::Box<bincode::ErrorKind>),
+    IO(#[from] std::io::Error),
+    Serialize(#[from] Box<bincode::ErrorKind>),
 }
-pub type Result<T> = std::result::Result<T, BlocktreeError>;
-
-impl std::error::Error for BlocktreeError {}
+pub(crate) type Result<T> = std::result::Result<T, BlocktreeError>;
 
 impl std::fmt::Display for BlocktreeError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -54,24 +53,6 @@ impl std::fmt::Display for BlocktreeError {
     }
 }
 
-impl std::convert::From<std::io::Error> for BlocktreeError {
-    fn from(e: std::io::Error) -> BlocktreeError {
-        BlocktreeError::IO(e)
-    }
-}
-
-impl std::convert::From<std::boxed::Box<bincode::ErrorKind>> for BlocktreeError {
-    fn from(e: std::boxed::Box<bincode::ErrorKind>) -> BlocktreeError {
-        BlocktreeError::Serialize(e)
-    }
-}
-
-impl std::convert::From<rocksdb::Error> for BlocktreeError {
-    fn from(e: rocksdb::Error) -> BlocktreeError {
-        BlocktreeError::RocksDb(e)
-    }
-}
-
 pub enum IteratorMode<Index> {
     Start,
     End,

ledger/src/blocktree_processor.rs

@@ -31,6 +31,7 @@ use std::{
     sync::Arc,
     time::{Duration, Instant},
 };
+use thiserror::Error;
 
 thread_local!(static PAR_THREAD_POOL: RefCell<ThreadPool> = RefCell::new(rayon::ThreadPoolBuilder::new()
                     .num_threads(get_thread_count())
@@ -224,18 +225,19 @@ pub struct BankForksInfo {
     pub bank_slot: u64,
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Error, Debug, PartialEq)]
 pub enum BlocktreeProcessorError {
+    #[error("failed to load entries")]
     FailedToLoadEntries,
-    FailedToLoadMeta,
-    InvalidBlock(BlockError),
-    InvalidTransaction,
-}
 
-impl From<BlockError> for BlocktreeProcessorError {
-    fn from(block_error: BlockError) -> Self {
-        BlocktreeProcessorError::InvalidBlock(block_error)
-    }
+    #[error("failed to load meta")]
+    FailedToLoadMeta,
+
+    #[error("invalid block")]
+    InvalidBlock(#[from] BlockError),
+
+    #[error("invalid transaction")]
+    InvalidTransaction,
 }
 
 /// Callback for accessing bank state while processing the blocktree

ledger/src/rooted_slot_iterator.rs

@@ -1,3 +1,4 @@
+use crate::blocktree_db::Result;
 use crate::{blocktree::*, blocktree_meta::SlotMeta};
 use log::*;
 use solana_sdk::clock::Slot;

ledger/src/shred.rs

@@ -21,6 +21,7 @@ use solana_sdk::{
 };
 use std::mem::size_of;
 use std::{sync::Arc, time::Instant};
+use thiserror::Error;
 
 /// The following constants are computed by hand, and hardcoded.
 /// `test_shred_constants` ensures that the values are correct.
@@ -52,22 +53,23 @@ pub const SHRED_TICK_REFERENCE_MASK: u8 = 0b0011_1111;
 const LAST_SHRED_IN_SLOT: u8 = 0b1000_0000;
 pub const DATA_COMPLETE_SHRED: u8 = 0b0100_0000;
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum ShredError {
+    #[error("invalid shred type")]
     InvalidShredType,
-    InvalidFecRate(f32), // FEC rate must be more than 0.0 and less than 1.0
-    SlotTooLow { slot: Slot, parent_slot: Slot }, // "Current slot must be > Parent slot, but the difference must not be > u16::MAX
-    Serialize(std::boxed::Box<bincode::ErrorKind>),
+
+    #[error("invalid FEC rate; must be 0.0 < {0} < 1.0")]
+    InvalidFecRate(f32),
+
+    #[error("slot too low; current slot {slot} must be above parent slot {parent_slot}, but the difference must be below u16::MAX")]
+    SlotTooLow { slot: Slot, parent_slot: Slot },
+
+    #[error("serialization error")]
+    Serialize(#[from] Box<bincode::ErrorKind>),
 }
 
 pub type Result<T> = std::result::Result<T, ShredError>;
 
-impl std::convert::From<std::boxed::Box<bincode::ErrorKind>> for ShredError {
-    fn from(e: std::boxed::Box<bincode::ErrorKind>) -> ShredError {
-        ShredError::Serialize(e)
-    }
-}
-
 #[derive(Serialize, Clone, Deserialize, PartialEq, Debug)]
 pub struct ShredType(pub u8);
 impl Default for ShredType {

ledger/src/snapshot_utils.rs

@@ -14,6 +14,7 @@ use std::{
     path::{Path, PathBuf},
 };
 use tar::Archive;
+use thiserror::Error;
 
 pub const SNAPSHOT_STATUS_CACHE_FILE_NAME: &str = "status_cache";
 pub const TAR_SNAPSHOTS_DIR: &str = "snapshots";
@@ -25,32 +26,19 @@ pub struct SlotSnapshotPaths {
     pub snapshot_file_path: PathBuf,
 }
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum SnapshotError {
-    IO(std::io::Error),
-    Serialize(std::boxed::Box<bincode::ErrorKind>),
-    FsExtra(fs_extra::error::Error),
+    #[error("I/O error")]
+    IO(#[from] std::io::Error),
+
+    #[error("serialization error")]
+    Serialize(#[from] Box<bincode::ErrorKind>),
+
+    #[error("file system error")]
+    FsExtra(#[from] fs_extra::error::Error),
 }
 pub type Result<T> = std::result::Result<T, SnapshotError>;
 
-impl std::convert::From<std::io::Error> for SnapshotError {
-    fn from(e: std::io::Error) -> SnapshotError {
-        SnapshotError::IO(e)
-    }
-}
-
-impl std::convert::From<std::boxed::Box<bincode::ErrorKind>> for SnapshotError {
-    fn from(e: std::boxed::Box<bincode::ErrorKind>) -> SnapshotError {
-        SnapshotError::Serialize(e)
-    }
-}
-
-impl std::convert::From<fs_extra::error::Error> for SnapshotError {
-    fn from(e: fs_extra::error::Error) -> SnapshotError {
-        SnapshotError::FsExtra(e)
-    }
-}
-
 impl PartialOrd for SlotSnapshotPaths {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         Some(self.slot.cmp(&other.slot))