Refactor: Replace PoolId::new and KeyHash::new with .into() for hash conversions across modules

lowhung · lowhung · commit 87b2ea674293 · 2025-10-29T16:59:44.000-07:00
diff --git a/codec/src/map_parameters.rs b/codec/src/map_parameters.rs
@@ -58,7 +58,7 @@ pub fn to_hash_opt<const N: usize>(
 
 /// Convert a Pallas Hash<28> reference to an Acropolis PoolId
 pub fn to_pool_id(pallas_hash: &pallas_primitives::Hash<28>) -> PoolId {
-    PoolId::from(to_hash(pallas_hash))
+    to_hash(pallas_hash).into()
 }
 
 /// Convert a Pallas Hash<32> reference to an Acropolis VRFKey
@@ -72,7 +72,7 @@ pub fn bytes_to_hash<const N: usize>(bytes: &pallas_primitives::Bytes) -> Hash<N
     let slice: &[u8] = bytes.as_ref();
     let mut array = [0u8; N];
     array.copy_from_slice(&slice[..N]);
-    Hash::from(array)
+    array.into()
 }
 
 /// Derive our Address from a Pallas address
diff --git a/common/src/address.rs b/common/src/address.rs
@@ -105,7 +105,7 @@ pub enum ShelleyAddressPaymentPart {
 
 impl Default for ShelleyAddressPaymentPart {
     fn default() -> Self {
-        Self::PaymentKeyHash(KeyHash::new([0u8; 28]))
+        Self::PaymentKeyHash([0u8; 28].into())
     }
 }
 
@@ -211,19 +211,31 @@ impl ShelleyAddress {
 
             let header = *header;
 
-            let to_28_bytes = |slice: &[u8]| -> Result<KeyHash> {
-                slice.try_into().map_err(|_| anyhow!("Invalid hash"))
-            };
-
             let payment_part = match (header >> 4) & 0x01 {
-                0 => ShelleyAddressPaymentPart::PaymentKeyHash(to_28_bytes(&data[1..29])?),
-                1 => ShelleyAddressPaymentPart::ScriptHash(to_28_bytes(&data[1..29])?),
+                0 => ShelleyAddressPaymentPart::PaymentKeyHash(
+                    data[1..29]
+                        .try_into()
+                        .map_err(|e| anyhow!("Failed to parse payment key hash: {}", e))?,
+                ),
+                1 => ShelleyAddressPaymentPart::ScriptHash(
+                    data[1..29]
+                        .try_into()
+                        .map_err(|e| anyhow!("Failed to parse payment script hash: {}", e))?,
+                ),
                 _ => panic!(),
             };
 
             let delegation_part = match (header >> 5) & 0x03 {
-                0 => ShelleyAddressDelegationPart::StakeKeyHash(to_28_bytes(&data[29..57])?),
-                1 => ShelleyAddressDelegationPart::ScriptHash(to_28_bytes(&data[29..57])?),
+                0 => ShelleyAddressDelegationPart::StakeKeyHash(
+                    data[29..57]
+                        .try_into()
+                        .map_err(|e| anyhow!("Failed to parse stake key hash: {}", e))?,
+                ),
+                1 => ShelleyAddressDelegationPart::ScriptHash(
+                    data[29..57]
+                        .try_into()
+                        .map_err(|e| anyhow!("Failed to parse stake script hash: {}", e))?,
+                ),
                 2 => {
                     let mut decoder = VarIntDecoder::new(&data[29..]);
                     let slot = decoder.read()?;
@@ -301,24 +313,24 @@ impl ShelleyAddress {
             ShelleyAddressDelegationPart::None => {
                 let header = build_header(3);
                 data.push(header);
-                data.extend(payment_hash.as_ref());
+                data.extend_from_slice(payment_hash.as_ref());
             }
             ShelleyAddressDelegationPart::StakeKeyHash(hash) => {
                 let header = build_header(0);
                 data.push(header);
-                data.extend(payment_hash.as_ref());
-                data.extend(hash.as_ref());
+                data.extend_from_slice(payment_hash.as_ref());
+                data.extend_from_slice(hash.as_ref());
             }
             ShelleyAddressDelegationPart::ScriptHash(hash) => {
                 let header = build_header(1);
                 data.push(header);
-                data.extend(payment_hash.as_ref());
-                data.extend(hash.as_ref());
+                data.extend_from_slice(payment_hash.as_ref());
+                data.extend_from_slice(hash.as_ref());
             }
             ShelleyAddressDelegationPart::Pointer(pointer) => {
                 let header = build_header(2);
                 data.push(header);
-                data.extend(payment_hash.as_ref());
+                data.extend_from_slice(payment_hash.as_ref());
 
                 let mut encoder = VarIntEncoder::new();
                 encoder.push(pointer.slot);
@@ -413,10 +425,12 @@ impl StakeAddress {
 
             let credential = match (header >> 4) & 0x0Fu8 {
                 0b1110 => StakeCredential::AddrKeyHash(
-                    data[1..].try_into().map_err(|_| anyhow!("Invalid key hash size"))?,
+                    data[1..].try_into().map_err(|e| anyhow!("Failed to parse key hash: {}", e))?,
                 ),
                 0b1111 => StakeCredential::ScriptHash(
-                    data[1..].try_into().map_err(|_| anyhow!("Invalid script hash size"))?,
+                    data[1..]
+                        .try_into()
+                        .map_err(|e| anyhow!("Failed to parse script hash: {}", e))?,
                 ),
                 _ => return Err(anyhow!("Unknown header {header} in stake address")),
             };
@@ -437,9 +451,9 @@ impl StakeAddress {
             NetworkId::Testnet => 0b0u8,
         };
 
-        let (stake_bits, stake_hash): (u8, &Vec<u8>) = match &self.credential {
-            StakeCredential::AddrKeyHash(data) => (0b1110, &data.to_vec()),
-            StakeCredential::ScriptHash(data) => (0b1111, &data.to_vec()),
+        let (stake_bits, stake_hash) = match &self.credential {
+            StakeCredential::AddrKeyHash(data) => (0b1110, data.as_ref()),
+            StakeCredential::ScriptHash(data) => (0b1111, data.as_ref()),
         };
 
         let mut data = vec![network_bits | (stake_bits << 4)];
@@ -526,7 +540,7 @@ impl Default for StakeAddress {
     fn default() -> Self {
         StakeAddress {
             network: NetworkId::Mainnet,
-            credential: StakeCredential::AddrKeyHash(KeyHash::new([0u8; 28])),
+            credential: StakeCredential::AddrKeyHash([0u8; 28].into()),
         }
     }
 }
diff --git a/common/src/types.rs b/common/src/types.rs
@@ -2090,7 +2090,7 @@ mod tests {
         let mut test_hash_bytes = [0u8; 28];
         test_hash_bytes[0..4].copy_from_slice(&[1, 2, 3, 4]);
         voting.votes.insert(
-            Voter::StakePoolKey(PoolId::new(Hash::new(test_hash_bytes))),
+            Voter::StakePoolKey(test_hash_bytes.into()),
             SingleVoterVotes::default(),
         );
 
diff --git a/modules/accounts_state/src/snapshot.rs b/modules/accounts_state/src/snapshot.rs
@@ -195,23 +195,21 @@ mod tests {
     use super::*;
     use acropolis_common::stake_addresses::StakeAddressState;
     use acropolis_common::NetworkId::Mainnet;
-    use acropolis_common::{KeyHash, PoolId, StakeAddress, StakeCredential};
+    use acropolis_common::{PoolId, StakeAddress, StakeCredential};
 
-    // Helper function to create stake addresses for testing
     fn create_test_stake_address(id: u8) -> StakeAddress {
         let mut hash = [0u8; 28];
         hash[0] = id;
         StakeAddress {
             network: Mainnet,
-            credential: StakeCredential::AddrKeyHash(KeyHash::new(hash)),
+            credential: StakeCredential::AddrKeyHash(hash.into()),
         }
     }
 
-    // Helper function to create SPO key hashes for testing
     fn create_test_spo_hash(id: u8) -> PoolId {
-        let mut hash = vec![0u8; 28];
+        let mut hash = [0u8; 28];
         hash[0] = id;
-        PoolId::try_from(hash).unwrap()
+        hash.into()
     }
 
     #[test]
diff --git a/modules/accounts_state/src/spo_distribution_store.rs b/modules/accounts_state/src/spo_distribution_store.rs
@@ -32,8 +32,8 @@ fn encode_epoch_pool_prefix(epoch: u64, pool_id: &PoolId) -> Vec<u8> {
 
 fn decode_key(key: &[u8]) -> Result<(u64, PoolId, AddrKeyhash)> {
     let epoch = u64::from_be_bytes(key[..EPOCH_LEN].try_into()?);
-    let pool_id: PoolId = key[EPOCH_LEN..EPOCH_LEN + POOL_ID_LENGTH].try_into()?;
-    let stake_key: AddrKeyhash = key[EPOCH_LEN + POOL_ID_LENGTH..].try_into()?;
+    let pool_id = key[EPOCH_LEN..EPOCH_LEN + POOL_ID_LENGTH].try_into()?;
+    let stake_key = key[EPOCH_LEN + POOL_ID_LENGTH..].try_into()?;
     Ok((epoch, pool_id, stake_key))
 }
 
@@ -230,7 +230,7 @@ mod tests {
     const DB_PATH: &str = "spdd_db";
 
     fn test_pool_hash(byte: u8) -> PoolId {
-        PoolId::new(keyhash_224(&vec![byte]))
+        keyhash_224(&vec![byte]).into()
     }
 
     fn test_addr_hash(byte: u8) -> AddrKeyhash {
diff --git a/modules/accounts_state/src/state.rs b/modules/accounts_state/src/state.rs
@@ -1022,7 +1022,7 @@ mod tests {
     }
 
     fn test_vrf_keyhash(byte: u8) -> VrfKeyHash {
-        VrfKeyHash::new(keyhash_256(&vec![byte]))
+        keyhash_256(&vec![byte]).into()
     }
 
     const STAKE_KEY_HASH: [u8; 3] = [0x99, 0x0f, 0x00];
@@ -1078,8 +1078,8 @@ mod tests {
     fn spdd_from_delegation_with_utxo_values_and_pledge() {
         let mut state = State::default();
 
-        let spo1: PoolId = PoolId::new(test_keyhash(0x01));
-        let spo2: PoolId = PoolId::new(test_keyhash(0x02));
+        let spo1 = test_keyhash(0x01).into();
+        let spo2 = test_keyhash(0x02).into();
 
         let vrf_key_hash_1 = test_vrf_keyhash(0x03);
         let vrf_key_hash_2 = test_vrf_keyhash(0x04);
@@ -1090,7 +1090,7 @@ mod tests {
                 epoch: 1,
                 spos: vec![
                     PoolRegistration {
-                        operator: spo1.clone(),
+                        operator: spo1,
                         vrf_key_hash: vrf_key_hash_1,
                         pledge: 26,
                         cost: 0,
@@ -1104,7 +1104,7 @@ mod tests {
                         pool_metadata: None,
                     },
                     PoolRegistration {
-                        operator: spo2.clone(),
+                        operator: spo2,
                         vrf_key_hash: vrf_key_hash_2,
                         pledge: 47,
                         cost: 10,
@@ -1393,7 +1393,7 @@ mod tests {
         // Get the KeyHash once
         let spo1_hash = spo1.get_hash();
         let drep_key_hash = test_keyhash_from_bytes(&DREP_HASH);
-        let pool_id_1 = PoolId::new(*spo1_hash);
+        let pool_id_1 = (*spo1_hash).into();
 
         let certificates = vec![
             // register the first two SPOs separately from their delegation
diff --git a/modules/epochs_state/src/state.rs b/modules/epochs_state/src/state.rs
@@ -188,7 +188,7 @@ impl State {
         self.last_block_time = block_info.timestamp;
         self.last_block_height = block_info.number;
         self.epoch_blocks += 1;
-        let spo_id = PoolId::new(keyhash_224(issuer_vkey));
+        let spo_id = PoolId::from(keyhash_224(issuer_vkey));
 
         // Count one on this hash
         *(self.blocks_minted.entry(spo_id.clone()).or_insert(0)) += 1;
diff --git a/modules/spo_state/src/epochs_history.rs b/modules/spo_state/src/epochs_history.rs
@@ -214,7 +214,7 @@ mod tests {
 
     #[test]
     fn get_pool_history_returns_none_when_spo_is_not_found() {
-        let key_hash = KeyHash::new([1; 28]);
+        let key_hash = [1; 28].into();
         let epochs_history = EpochsHistoryState::new(save_history_store_config());
         let pool_history = epochs_history.get_pool_history(&key_hash);
         assert!(pool_history.is_none());
@@ -223,8 +223,8 @@ mod tests {
     #[test]
     fn get_pool_history_returns_data() {
         let epochs_history = EpochsHistoryState::new(save_history_store_config());
-        let pool_id = PoolId::new(KeyHash::new([1; 28]));
-        let spo_block_key_hash = PoolId::new(KeyHash::new([2; 28]));
+        let pool_id = [1; 28].into();
+        let spo_block_key_hash = [2; 28].into();
 
         let block = new_block(2);
         let mut spdd_msg = new_spdd_message(1);
diff --git a/modules/spo_state/src/retired_pools_history.rs b/modules/spo_state/src/retired_pools_history.rs
@@ -66,7 +66,6 @@ impl RetiredPoolsHistoryState {
 mod tests {
     use super::*;
     use crate::test_utils::*;
-    use acropolis_common::KeyHash;
 
     #[test]
     fn retired_pools_history_is_none_when_store_retired_pools_is_false() {
@@ -91,8 +90,8 @@ mod tests {
         let state = RetiredPoolsHistoryState::new(save_retired_pools_store_config());
 
         let block = new_block(2);
-        let spo_1 = PoolId::new(KeyHash::new([1; 28]));
-        let spo_2 = PoolId::new(KeyHash::new([2; 28]));
+        let spo_1 = [1; 28].into();
+        let spo_2 = [2; 28].into();
         let retired_spos = vec![spo_1, spo_2];
         state.handle_deregistrations(&block, &retired_spos);
 
diff --git a/modules/spo_state/src/state.rs b/modules/spo_state/src/state.rs
@@ -668,7 +668,6 @@ impl State {
 mod tests {
     use super::*;
     use crate::test_utils::*;
-    use acropolis_common::hash::Hash;
     use acropolis_common::{
         state_history::{StateHistory, StateHistoryStore},
         PoolRetirement, Ratio, StakeAddress, TxCertificate, TxCertificateWithPos, TxIdentifier,
@@ -677,11 +676,11 @@ mod tests {
     use tokio::sync::Mutex;
 
     fn test_pool_id(byte: u8) -> PoolId {
-        PoolId::new(Hash::new([byte; 28]))
+        [byte; 28].into()
     }
 
     fn test_pool_id_from_bytes(bytes: &[u8]) -> PoolId {
-        PoolId::new(keyhash_224(bytes))
+        keyhash_224(bytes).into()
     }
 
     fn default_pool_registration(

Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ pub fn to_hash_opt<const N: usize>(`
`58`	`58`
`59`	`59`	`/// Convert a Pallas Hash<28> reference to an Acropolis PoolId`
`60`	`60`	`pub fn to_pool_id(pallas_hash: &pallas_primitives::Hash<28>) -> PoolId {`
`61`		`- PoolId::from(to_hash(pallas_hash))`
	`61`	`+ to_hash(pallas_hash).into()`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`/// Convert a Pallas Hash<32> reference to an Acropolis VRFKey`
`@@ -72,7 +72,7 @@ pub fn bytes_to_hash<const N: usize>(bytes: &pallas_primitives::Bytes) -> Hash<N`
`72`	`72`	`let slice: &[u8] = bytes.as_ref();`
`73`	`73`	`let mut array = [0u8; N];`
`74`	`74`	`array.copy_from_slice(&slice[..N]);`
`75`		`- Hash::from(array)`
	`75`	`+ array.into()`
`76`	`76`	`}`
`77`	`77`
`78`	`78`	`/// Derive our Address from a Pallas address`