fixup! Apply suggestions from code review

anoma · Aug 27, 2024 · c71daae · c71daae
1 parent 130f149
commit c71daae
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 21 deletions.
diff --git a/crates/core/src/chain.rs b/crates/core/src/chain.rs
@@ -13,7 +13,6 @@ use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
 use thiserror::Error;
 
-use crate::borsh::BorshSerializeExt;
 use crate::bytes::ByteBuf;
 use crate::hash::Hash;
 use crate::time::DateTimeUtc;
@@ -286,14 +285,6 @@ pub enum ParseBlockHashError {
     ParseBlockHash(String),
 }
 
-impl TryFrom<Vec<u8>> for BlockHash {
-    type Error = ParseBlockHashError;
-
-    fn try_from(value: Vec<u8>) -> Result<Self, ParseBlockHashError> {
-        value.as_slice().try_into()
-    }
-}
-
 impl core::fmt::Debug for BlockHash {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let hash = format!("{}", ByteBuf(&self.0));
@@ -535,8 +526,9 @@ pub struct BlockHeader {
 
 impl BlockHeader {
     /// The number of bytes when this header is encoded
-    pub fn encoded_len(&self) -> usize {
-        self.serialize_to_vec().len()
+    pub const fn encoded_len() -> usize {
+        // checked in `test_block_header_encoded_len`
+        103
     }
 }
 
@@ -749,6 +741,7 @@ mod tests {
     use proptest::prelude::*;
 
     use super::*;
+    use crate::borsh::BorshSerializeExt;
 
     proptest! {
         /// Test any chain ID that is generated via `from_genesis` function is valid.
@@ -933,4 +926,16 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    fn test_block_header_encoded_len() {
+        #[allow(clippy::disallowed_methods)]
+        let header = BlockHeader {
+            hash: Hash::zero(),
+            time: DateTimeUtc::now(),
+            next_validators_hash: Hash::zero(),
+        };
+        let len = header.serialize_to_vec().len();
+        assert_eq!(len, BlockHeader::encoded_len())
+    }
 }
diff --git a/crates/proof_of_stake/src/parameters.rs b/crates/proof_of_stake/src/parameters.rs
@@ -232,7 +232,7 @@ impl OwnedPosParams {
         infraction_epoch: Epoch,
     ) -> (Epoch, Epoch) {
         let start = infraction_epoch
-            .sub_or_default(Epoch(self.cubic_slashing_window_length));
+            .saturating_sub(Epoch(self.cubic_slashing_window_length));
         let end =
             infraction_epoch.unchecked_add(self.cubic_slashing_window_length);
         (start, end)

diff --git a/crates/proof_of_stake/src/tests/test_validator.rs b/crates/proof_of_stake/src/tests/test_validator.rs
@@ -967,8 +967,8 @@ fn test_validator_sets() {
     for e in Epoch::iter_bounds_inclusive(
         start_epoch,
         last_epoch
-            .sub_or_default(Epoch(DEFAULT_NUM_PAST_EPOCHS))
-            .sub_or_default(Epoch(1)),
+            .saturating_sub(Epoch(DEFAULT_NUM_PAST_EPOCHS))
+            .saturating_sub(Epoch(1)),
     ) {
         assert!(
             !consensus_validator_set_handle()

diff --git a/crates/state/src/lib.rs b/crates/state/src/lib.rs
@@ -166,18 +166,17 @@ pub trait StateRead: StorageRead + Debug {
         match height {
             Some(h) if h == self.in_mem().get_block_height().0 => {
                 let header = self.in_mem().header.clone();
-                let gas = match header {
-                    Some(ref header) => {
-                        let len = header.encoded_len() as u64;
-                        checked!(len * MEMORY_ACCESS_GAS_PER_BYTE)?
-                    }
-                    None => MEMORY_ACCESS_GAS_PER_BYTE,
+                let gas = if header.is_some() {
+                    let len = BlockHeader::encoded_len() as u64;
+                    checked!(len * MEMORY_ACCESS_GAS_PER_BYTE)?
+                } else {
+                    MEMORY_ACCESS_GAS_PER_BYTE
                 };
                 Ok((header, gas))
             }
             Some(h) => match self.db().read_block_header(h)? {
                 Some(header) => {
-                    let len = header.encoded_len() as u64;
+                    let len = BlockHeader::encoded_len() as u64;
                     let gas = checked!(len * STORAGE_ACCESS_GAS_PER_BYTE)?;
                     Ok((Some(header), gas))
                 }