From 48ca9372939146bbff4de3359bc389538b27bc98 Mon Sep 17 00:00:00 2001 From: bkioshn Date: Fri, 20 Dec 2024 12:29:26 +0700 Subject: [PATCH] fix(cardano-blockchain-types): cleanup Signed-off-by: bkioshn --- rust/cardano-blockchain-types/deps.tmp | 1 - .../src/auxdata/aux_data.rs | 4 +- .../src/multi_era_block_data.rs | 29 ++++--- rust/cardano-blockchain-types/src/point.rs | 87 ++++++++++++------- .../src/txn_witness.rs | 5 +- 5 files changed, 77 insertions(+), 49 deletions(-) diff --git a/rust/cardano-blockchain-types/deps.tmp b/rust/cardano-blockchain-types/deps.tmp index e0b47377c..f65f5bfb5 100644 --- a/rust/cardano-blockchain-types/deps.tmp +++ b/rust/cardano-blockchain-types/deps.tmp @@ -1,4 +1,3 @@ - rbac-registration = { version = "0.0.2", git = "https://github.com/input-output-hk/catalyst-libs.git", tag = "v0.0.8" } thiserror = "1.0.64" diff --git a/rust/cardano-blockchain-types/src/auxdata/aux_data.rs b/rust/cardano-blockchain-types/src/auxdata/aux_data.rs index a4c02ba68..d51bb32f3 100644 --- a/rust/cardano-blockchain-types/src/auxdata/aux_data.rs +++ b/rust/cardano-blockchain-types/src/auxdata/aux_data.rs @@ -158,9 +158,9 @@ impl TransactionAuxData { let script_array = ScriptArray::decode(d, &mut ctx)?; if scripts.insert(script_type, script_array).is_some() { - return Err(minicbor::decode::Error::message( + return Err(minicbor::decode::Error::message(format!( "Multiple Alonzo+ Script entries of type {script_type} found. Invalid.", - )); + ))); } } diff --git a/rust/cardano-blockchain-types/src/multi_era_block_data.rs b/rust/cardano-blockchain-types/src/multi_era_block_data.rs index 5e0e3b065..bc9de6d51 100644 --- a/rust/cardano-blockchain-types/src/multi_era_block_data.rs +++ b/rust/cardano-blockchain-types/src/multi_era_block_data.rs @@ -45,9 +45,8 @@ struct SelfReferencedMultiEraBlock { /// Multi-era block - inner. #[derive(Debug)] struct MultiEraBlockInner { - /// What blockchain was the block produced on. - //#[allow(dead_code)] - pub chain: Network, + /// What blockchain network was the block produced on. + network: Network, /// The Point on the blockchain this block can be found. point: Point, /// The previous point on the blockchain before this block. @@ -93,7 +92,7 @@ impl MultiEraBlock { /// /// If the given bytes cannot be decoded as a multi-era block, an error is returned. fn new_block( - chain: Network, raw_data: Vec, previous: &Point, fork: Fork, + network: Network, raw_data: Vec, previous: &Point, fork: Fork, ) -> anyhow::Result { let builder = SelfReferencedMultiEraBlockTryBuilder { raw_data, @@ -140,7 +139,7 @@ impl MultiEraBlock { Ok(Self { fork, inner: Arc::new(MultiEraBlockInner { - chain, + network, point, previous: previous.clone(), data: self_ref_block, @@ -156,9 +155,9 @@ impl MultiEraBlock { /// /// If the given bytes cannot be decoded as a multi-era block, an error is returned. pub fn new( - chain: Network, raw_data: Vec, previous: &Point, fork: Fork, + network: Network, raw_data: Vec, previous: &Point, fork: Fork, ) -> anyhow::Result { - MultiEraBlock::new_block(chain, raw_data, previous, fork) + MultiEraBlock::new_block(network, raw_data, previous, fork) } /// Remake the block on a new fork. @@ -212,7 +211,7 @@ impl MultiEraBlock { /// # Returns /// `true` if the block is immutable, `false` otherwise. #[must_use] - pub fn immutable(&self) -> bool { + pub fn is_immutable(&self) -> bool { self.fork == 0.into() } @@ -231,13 +230,13 @@ impl MultiEraBlock { self.fork } - /// What chain was the block from + /// What blockchain network was the block from /// /// # Returns - /// - The chain that this block originated on. + /// - The network that this block originated on. #[must_use] - pub fn chain(&self) -> Network { - self.inner.chain + pub fn network(&self) -> Network { + self.inner.network } /// Get The Metadata fora a transaction and known label from the block @@ -286,7 +285,7 @@ impl Display for MultiEraBlock { let txns = block.tx_count(); let aux_data = block.has_aux_data(); - let fork = if self.immutable() { + let fork = if self.is_immutable() { "Immutable".to_string() } else { format!("Fork: {fork:?}") @@ -461,6 +460,10 @@ pub(crate) mod tests { ); assert!(block.is_err()); + assert!(block + .unwrap_err() + .to_string() + .contains("Previous slot is not less than current slot")); } Ok(()) diff --git a/rust/cardano-blockchain-types/src/point.rs b/rust/cardano-blockchain-types/src/point.rs index ebeeac25e..96e8f41c7 100644 --- a/rust/cardano-blockchain-types/src/point.rs +++ b/rust/cardano-blockchain-types/src/point.rs @@ -10,7 +10,10 @@ use std::{ use pallas::crypto::hash::Hash; -use crate::{hashes::Blake2bHash, Slot}; +use crate::{ + hashes::{Blake2b256Hash, Blake2bHash}, + Slot, +}; /// A specific point in the blockchain. It can be used to /// identify a particular location within the blockchain, such as the tip (the @@ -81,7 +84,7 @@ impl Point { /// # Parameters /// /// * `slot` - A `Slot` representing the slot number in the blockchain. - /// * `hash` - A `Blake2bHash` size 32, block hash at the specified slot. + /// * `hash` - A `Blake2b256Hash` , block hash at the specified slot. /// /// # Returns /// @@ -97,7 +100,7 @@ impl Point { /// let point = Point::new(slot.into(), hash.into()); /// ``` #[must_use] - pub fn new(slot: Slot, hash: Blake2bHash<32>) -> Self { + pub fn new(slot: Slot, hash: Blake2b256Hash) -> Self { Self(pallas::network::miniprotocols::Point::Specific( slot.into(), hash.into(), @@ -269,29 +272,29 @@ impl Point { } /// Retrieves the hash from the `Point`. If the `Point` is - /// the origin, it returns a default hash value, which is an empty `Vec`. + /// the origin, it returns `None`. /// /// # Returns /// - /// A `Vec` representing the hash. If the `Point` is the `Origin`, it - /// returns an empty vector. + /// A `Blake2b256Hash` representing the hash. If the `Point` is the `Origin`, it + /// returns `None`. /// /// # Examples /// /// ``` - /// use cardano_blockchain_types::Point; + /// use cardano_blockchain_types::{Point, hashes::Blake2bHash}; /// /// let specific_point = Point::new(42.into(), [0; 32].into()); - /// assert_eq!(specific_point.hash_or_default(), Some([0; 32].into())); + /// assert_eq!(specific_point.hash_or_default(), Some(Blake2bHash::new(&[0; 32]))); /// /// let origin_point = Point::ORIGIN; /// assert_eq!(origin_point.hash_or_default(), None); /// ``` #[must_use] - pub fn hash_or_default(&self) -> Option> { + pub fn hash_or_default(&self) -> Option { match &self.0 { pallas::network::miniprotocols::Point::Specific(_, hash) => { - Some(Hash::from(hash.as_slice())) + Some(Blake2bHash::new(hash)) }, // Origin has empty hash, so set it to None pallas::network::miniprotocols::Point::Origin => None, @@ -328,8 +331,34 @@ impl Point { } } +impl PartialEq> for Point { + /// Compares the hash stored in the `Point` with a `Blake2b256Hash`. + /// It returns `true` if the hashes match and `false` otherwise. If the + /// provided hash is `None`, the function checks if the `Point` has an + /// empty hash. + fn eq(&self, other: &Option) -> bool { + match other { + Some(cmp_hash) => { + match self.0 { + pallas::network::miniprotocols::Point::Specific(_, ref hash) => { + // Compare vec to vec + *hash == >>::into(*cmp_hash) + }, + pallas::network::miniprotocols::Point::Origin => false, + } + }, + None => { + match self.0 { + pallas::network::miniprotocols::Point::Specific(_, ref hash) => hash.is_empty(), + pallas::network::miniprotocols::Point::Origin => true, + } + }, + } + } +} + impl PartialEq>> for Point { - /// Compares the hash stored in the `Point` with a known hash. + /// Compares the hash stored in the `Point` with a Pallas `Hash`. /// It returns `true` if the hashes match and `false` otherwise. If the /// provided hash is `None`, the function checks if the `Point` has an /// empty hash. @@ -366,7 +395,13 @@ impl Display for Point { let slot = self.slot_or_default(); let hash = self.hash_or_default(); match hash { - Some(hash) => write!(f, "Point @ {slot:?}:{}", hex::encode(hash)), + Some(hash) => { + write!( + f, + "Point @ {slot:?}:{}", + hex::encode(>>::into(hash)) + ) + }, None => write!(f, "Point @ {slot:?}"), } } @@ -407,20 +442,16 @@ impl PartialEq for Point { } impl PartialOrd for Point { - /// Allows to compare a `Point` against a `u64` (Just the Immutable File Number). - /// - /// Equality ONLY checks the Immutable File Number, not the path. - /// This is because the Filename is already the Immutable File Number. + /// Allows to compare a `Point` against a `u64` fn partial_cmp(&self, other: &u64) -> Option { self.0.slot_or_default().partial_cmp(other) } } impl PartialEq> for Point { - /// Allows to compare a `SnapshotID` against `u64` (Just the Immutable File Number). - /// - /// Equality ONLY checks the Immutable File Number, not the path. - /// This is because the Filename is already the Immutable File Number. + /// Allows for direct comparison between a `Point` and an `Option`, + /// returning `true` only if the `Option` contains a `Point` that is equal to the + /// `self` instance. fn eq(&self, other: &Option) -> bool { if let Some(other) = other { *self == *other @@ -431,11 +462,8 @@ impl PartialEq> for Point { } impl PartialOrd> for Point { - /// Allows to compare a `Point` against a `u64` (Just the Immutable File Number). - /// - /// Equality ONLY checks the Immutable File Number, not the path. - /// This is because the Filename is already the Immutable File Number. - /// Any point is greater than None. + /// Allows comparing a `Point` with an `Option`, where a `Point` is always + /// considered greater than `None`. fn partial_cmp(&self, other: &Option) -> Option { if let Some(other) = other { self.partial_cmp(other) @@ -476,8 +504,6 @@ fn cmp_point( #[cfg(test)] mod tests { - use pallas::crypto::hash::Hash; - use crate::point::*; #[test] @@ -485,10 +511,9 @@ mod tests { let origin1 = Point::ORIGIN; let point1 = Point::new(100u64.into(), [8; 32].into()); - assert!(origin1 != Some(Hash::new([0; 32]))); - assert!(origin1 == None::>); - - assert!(point1 == Some(Hash::new([8; 32]))); + assert!(origin1 != Some(Blake2bHash::<32>::new(&[0; 32]))); + assert!(origin1 == None::); + assert!(point1 == Some(Hash::<32>::new([8; 32]))); assert!(point1 != None::>); } diff --git a/rust/cardano-blockchain-types/src/txn_witness.rs b/rust/cardano-blockchain-types/src/txn_witness.rs index 532f0c6ec..ade0cda61 100644 --- a/rust/cardano-blockchain-types/src/txn_witness.rs +++ b/rust/cardano-blockchain-types/src/txn_witness.rs @@ -24,7 +24,8 @@ impl TxnWitness { /// /// # Errors /// - /// Only if the witness map does not contain a valid ED25519 public key + /// If the witness map does not contain a valid ED25519 public key, + /// or unsupported transaction era. pub fn new(txs: &[MultiEraTx]) -> anyhow::Result { /// Update the temporary map with the witnesses. fn update_map( @@ -67,7 +68,7 @@ impl TxnWitness { } }, _ => { - return Err(anyhow::anyhow!("Unsupported transaction type")); + return Err(anyhow::anyhow!("Unsupported transaction Era")); }, }; }