thiserror, docs, remove general Failure case (#9741) (#9810)

automerge

core/src/cluster_info.rs

@@ -161,7 +161,7 @@ pub struct PruneData {
 impl Sanitize for PruneData {
     fn sanitize(&self) -> std::result::Result<(), SanitizeError> {
         if self.wallclock >= MAX_WALLCLOCK {
-            return Err(SanitizeError::ValueOutOfRange);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         Ok(())
     }
@@ -2822,7 +2822,7 @@ mod tests {
         let mut pd = PruneData::default();
         pd.wallclock = MAX_WALLCLOCK;
         let msg = Protocol::PruneMessage(Pubkey::default(), pd);
-        assert_eq!(msg.sanitize(), Err(SanitizeError::ValueOutOfRange));
+        assert_eq!(msg.sanitize(), Err(SanitizeError::ValueOutOfBounds));
     }
 
     // computes the maximum size for pull request blooms

core/src/contact_info.rs

@@ -42,7 +42,7 @@ pub struct ContactInfo {
 impl Sanitize for ContactInfo {
     fn sanitize(&self) -> std::result::Result<(), SanitizeError> {
         if self.wallclock >= MAX_WALLCLOCK {
-            return Err(SanitizeError::Failed);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         Ok(())
     }
@@ -325,4 +325,12 @@ mod tests {
         ci.rpc = socketaddr!("127.0.0.1:234");
         assert!(ci.valid_client_facing_addr().is_some());
     }
+
+    #[test]
+    fn test_sanitize() {
+        let mut ci = ContactInfo::default();
+        assert_eq!(ci.sanitize(), Ok(()));
+        ci.wallclock = MAX_WALLCLOCK;
+        assert_eq!(ci.sanitize(), Err(SanitizeError::ValueOutOfBounds));
+    }
 }

core/src/crds_value.rs

@@ -83,13 +83,13 @@ impl Sanitize for CrdsData {
             CrdsData::ContactInfo(val) => val.sanitize(),
             CrdsData::Vote(ix, val) => {
                 if *ix >= MAX_VOTES {
-                    return Err(SanitizeError::Failed);
+                    return Err(SanitizeError::ValueOutOfBounds);
                 }
                 val.sanitize()
             }
             CrdsData::LowestSlot(ix, val) => {
                 if *ix as usize >= 1 {
-                    return Err(SanitizeError::ValueOutOfRange);
+                    return Err(SanitizeError::ValueOutOfBounds);
                 }
                 val.sanitize()
             }
@@ -97,7 +97,7 @@ impl Sanitize for CrdsData {
             CrdsData::AccountsHashes(val) => val.sanitize(),
             CrdsData::EpochSlots(ix, val) => {
                 if *ix as usize >= MAX_EPOCH_SLOTS as usize {
-                    return Err(SanitizeError::Failed);
+                    return Err(SanitizeError::ValueOutOfBounds);
                 }
                 val.sanitize()
             }
@@ -115,11 +115,11 @@ pub struct SnapshotHash {
 impl Sanitize for SnapshotHash {
     fn sanitize(&self) -> Result<(), SanitizeError> {
         if self.wallclock >= MAX_WALLCLOCK {
-            return Err(SanitizeError::Failed);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         for (slot, _) in &self.hashes {
             if *slot >= MAX_SLOT {
-                return Err(SanitizeError::Failed);
+                return Err(SanitizeError::ValueOutOfBounds);
             }
         }
         self.from.sanitize()
@@ -161,10 +161,10 @@ impl LowestSlot {
 impl Sanitize for LowestSlot {
     fn sanitize(&self) -> Result<(), SanitizeError> {
         if self.wallclock >= MAX_WALLCLOCK {
-            return Err(SanitizeError::ValueOutOfRange);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         if self.lowest >= MAX_SLOT {
-            return Err(SanitizeError::ValueOutOfRange);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         if self.root != 0 {
             return Err(SanitizeError::InvalidValue);
@@ -189,7 +189,7 @@ pub struct Vote {
 impl Sanitize for Vote {
     fn sanitize(&self) -> Result<(), SanitizeError> {
         if self.wallclock >= MAX_WALLCLOCK {
-            return Err(SanitizeError::Failed);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         self.from.sanitize()?;
         self.transaction.sanitize()
@@ -448,7 +448,7 @@ mod test {
 
         let o = ls.clone();
         let v = CrdsValue::new_unsigned(CrdsData::LowestSlot(1, o.clone()));
-        assert_eq!(v.sanitize(), Err(SanitizeError::ValueOutOfRange));
+        assert_eq!(v.sanitize(), Err(SanitizeError::ValueOutOfBounds));
 
         let mut o = ls.clone();
         o.slots.insert(1);
@@ -505,7 +505,7 @@ mod test {
             ),
             &keypair,
         );
-        assert!(item.sanitize().is_err());
+        assert_eq!(item.sanitize(), Err(SanitizeError::ValueOutOfBounds));
     }
     #[test]
     fn test_compute_vote_index_empty() {

core/src/epoch_slots.rs

@@ -19,10 +19,10 @@ pub struct Uncompressed {
 impl Sanitize for Uncompressed {
     fn sanitize(&self) -> std::result::Result<(), SanitizeError> {
         if self.first_slot >= MAX_SLOT {
-            return Err(SanitizeError::ValueOutOfRange);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         if self.num >= MAX_SLOTS_PER_ENTRY {
-            return Err(SanitizeError::ValueOutOfRange);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         Ok(())
     }
@@ -38,10 +38,10 @@ pub struct Flate2 {
 impl Sanitize for Flate2 {
     fn sanitize(&self) -> std::result::Result<(), SanitizeError> {
         if self.first_slot >= MAX_SLOT {
-            return Err(SanitizeError::ValueOutOfRange);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         if self.num >= MAX_SLOTS_PER_ENTRY {
-            return Err(SanitizeError::ValueOutOfRange);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         Ok(())
     }
@@ -221,7 +221,7 @@ pub struct EpochSlots {
 impl Sanitize for EpochSlots {
     fn sanitize(&self) -> std::result::Result<(), SanitizeError> {
         if self.wallclock >= MAX_WALLCLOCK {
-            return Err(SanitizeError::ValueOutOfRange);
+            return Err(SanitizeError::ValueOutOfBounds);
         }
         self.from.sanitize()?;
         self.slots.sanitize()
@@ -387,22 +387,22 @@ mod tests {
 
         let mut o = slots.clone();
         o.first_slot = MAX_SLOT;
-        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange));
+        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds));
 
         let mut o = slots.clone();
         o.num = MAX_SLOTS_PER_ENTRY;
-        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange));
+        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds));
 
         let compressed = Flate2::deflate(slots).unwrap();
         assert!(compressed.sanitize().is_ok());
 
         let mut o = compressed.clone();
         o.first_slot = MAX_SLOT;
-        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange));
+        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds));
 
         let mut o = compressed.clone();
         o.num = MAX_SLOTS_PER_ENTRY;
-        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange));
+        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds));
 
         let mut slots = EpochSlots::default();
         let range: Vec<Slot> = (0..5000).into_iter().collect();
@@ -412,7 +412,7 @@ mod tests {
 
         let mut o = slots.clone();
         o.wallclock = MAX_WALLCLOCK;
-        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange));
+        assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds));
     }
 
     #[test]

sdk/src/hash.rs

@@ -1,5 +1,6 @@
 //! The `hash` module provides functions for creating SHA-256 hashes.
 
+use crate::sanitize::Sanitize;
 use sha2::{Digest, Sha256};
 use std::{convert::TryFrom, fmt, mem, str::FromStr};
 use thiserror::Error;
@@ -30,6 +31,8 @@ impl Hasher {
     }
 }
 
+impl Sanitize for Hash {}
+
 impl AsRef<[u8]> for Hash {
     fn as_ref(&self) -> &[u8] {
         &self.0[..]

sdk/src/instruction.rs

@@ -1,5 +1,6 @@
 //! Defines a composable Instruction type and a memory-efficient CompiledInstruction.
 
+use crate::sanitize::Sanitize;
 use crate::{pubkey::Pubkey, short_vec, system_instruction::SystemError};
 use bincode::serialize;
 use serde::Serialize;
@@ -228,6 +229,8 @@ pub struct CompiledInstruction {
     pub data: Vec<u8>,
 }
 
+impl Sanitize for CompiledInstruction {}
+
 impl CompiledInstruction {
     pub fn new<T: Serialize>(program_ids_index: u8, data: &T, accounts: Vec<u8>) -> Self {
         let data = serialize(data).unwrap();

sdk/src/message.rs

@@ -184,6 +184,9 @@ impl Sanitize for Message {
                 }
             }
         }
+        self.account_keys.sanitize()?;
+        self.recent_blockhash.sanitize()?;
+        self.instructions.sanitize()?;
         Ok(())
     }
 }

sdk/src/sanitize.rs

@@ -1,11 +1,22 @@
-#[derive(PartialEq, Debug)]
+use thiserror::Error;
+
+#[derive(PartialEq, Debug, Error, Eq, Clone)]
 pub enum SanitizeError {
-    Failed,
+    #[error("index out of bounds")]
     IndexOutOfBounds,
-    ValueOutOfRange,
+    #[error("value out of bounds")]
+    ValueOutOfBounds,
+    #[error("invalid value")]
     InvalidValue,
 }
 
+/// Trait for sanitizing values and members of over the wire messages.
+/// Implementation should recursively decent through the data structure
+/// and sanitize all struct members and enum clauses.  Sanitize excludes
+/// signature verification checks, those are handled by another pass.
+/// Sanitize checks should include but are not limited too:
+///   * All index values are in range
+///   * All values are within their static max/min bounds
 pub trait Sanitize {
     fn sanitize(&self) -> Result<(), SanitizeError> {
         Ok(())