Simplify hash and error conversions (#395)

Since `BitcoinNodeHash` implements `From<bitcoin_hashes::sha256::Hash>`, we can directly apply `Into::into`. Then for str conversions we can use `parse` instead, and map the arrays directly. Finally in `chain_state.rs` and `partial_chain.rs` I simplify `BlockchainError::BlockValidation` conversions with `?`.
vinteumorg · Mar 5, 2025 · 3b0a243 · 3b0a243
1 parent d0e0708
commit 3b0a243
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 66 deletions.
diff --git a/crates/floresta-chain/src/pruned_utreexo/chain_state.rs b/crates/floresta-chain/src/pruned_utreexo/chain_state.rs
@@ -15,7 +15,6 @@ use bitcoin::consensus::deserialize_partial;
 use bitcoin::consensus::Decodable;
 use bitcoin::consensus::Encodable;
 use bitcoin::hashes::sha256;
-use bitcoin::hashes::Hash;
 use bitcoin::script;
 use bitcoin::Block;
 use bitcoin::BlockHash;
@@ -170,7 +169,7 @@ impl<PersistedState: ChainStore> ChainState<PersistedState> {
         let prev_block = self.get_disk_block_header(&block_header.prev_blockhash)?;
         let prev_block_height = prev_block.height();
         if prev_block_height.is_none() {
-            return Err(BlockValidationErrors::BlockExtendsAnOrphanChain.into());
+            return Err(BlockValidationErrors::BlockExtendsAnOrphanChain)?;
         }
         let height = prev_block_height.unwrap() + 1;
 
@@ -179,12 +178,12 @@ impl<PersistedState: ChainStore> ChainState<PersistedState> {
 
         let actual_target = block_header.target();
         if actual_target > expected_target {
-            return Err(BlockValidationErrors::NotEnoughPow.into());
+            return Err(BlockValidationErrors::NotEnoughPow)?;
         }
 
         let block_hash = block_header
             .validate_pow(actual_target)
-            .map_err(|_| BlockchainError::BlockValidation(BlockValidationErrors::NotEnoughPow))?;
+            .map_err(|_| BlockValidationErrors::NotEnoughPow)?;
         Ok(block_hash)
     }
 
@@ -753,21 +752,21 @@ impl<PersistedState: ChainStore> ChainState<PersistedState> {
         inputs: HashMap<OutPoint, TxOut>,
     ) -> Result<(), BlockchainError> {
         if !block.check_merkle_root() {
-            return Err(BlockValidationErrors::BadMerkleRoot.into());
+            return Err(BlockValidationErrors::BadMerkleRoot)?;
         }
 
         let bip34_height = self.chain_params().params.bip34_height;
         // If bip34 is active, check that the encoded block height is correct
         if height >= bip34_height && self.get_bip34_height(block) != Some(height) {
-            return Err(BlockValidationErrors::BadBip34.into());
+            return Err(BlockValidationErrors::BadBip34)?;
         }
 
         if !block.check_witness_commitment() {
-            return Err(BlockValidationErrors::BadWitnessCommitment.into());
+            return Err(BlockValidationErrors::BadWitnessCommitment)?;
         }
 
         if block.weight().to_wu() > 4_000_000 {
-            return Err(BlockValidationErrors::BlockTooBig.into());
+            return Err(BlockValidationErrors::BlockTooBig)?;
         }
 
         // Validate block transactions
@@ -830,14 +829,11 @@ impl<PersistedState: ChainStore> BlockchainInterface for ChainState<PersistedSta
         del_hashes: Vec<sha256::Hash>,
         acc: Stump,
     ) -> Result<(), Self::Error> {
-        // verify the proof
-        let del_hashes = del_hashes
-            .iter()
-            .map(|hash| BitcoinNodeHash::from(hash.as_byte_array()))
-            .collect::<Vec<_>>();
+        // Convert to BitcoinNodeHashes, from rustreexo
+        let del_hashes: Vec<_> = del_hashes.into_iter().map(Into::into).collect();
 
         if !acc.verify(&proof, &del_hashes)? {
-            return Err(BlockValidationErrors::InvalidProof.into());
+            return Err(BlockValidationErrors::InvalidProof)?;
         }
 
         let height = self

diff --git a/crates/floresta-chain/src/pruned_utreexo/chainparams.rs b/crates/floresta-chain/src/pruned_utreexo/chainparams.rs
@@ -84,10 +84,9 @@ impl ChainParams {
         let genesis = genesis_block(Params::new(network.into()));
         match network {
             Network::Bitcoin => AssumeUtreexoValue {
-                block_hash: BlockHash::from_str(
-                    "00000000000000000000569f4d863c27e667cbee8acc8da195e7e5551658e6e9",
-                )
-                .unwrap(),
+                block_hash: "00000000000000000000569f4d863c27e667cbee8acc8da195e7e5551658e6e9"
+                    .parse()
+                    .unwrap(),
                 height: 855571,
                 roots: [
                     "4dcc014cc23611dda2dcf0f34a3e62e7d302146df4b0b01ac701d440358c19d6",
@@ -108,9 +107,8 @@ impl ChainParams {
                     "67ba89afe6bce9bafbf0b88013e4446c861e6c746e291c3921e0b65c93671ba3",
                     "972ea2c7472c22e4eab49e9c2db5757a048b271b6251883ce89ccfeaa38b47ab",
                 ]
-                .into_iter()
-                .map(|x| BitcoinNodeHash::from_str(x).unwrap())
-                .collect(),
+                .map(|s| s.parse().unwrap())
+                .to_vec(),
                 leaves: 2587882501,
             },
             Network::Testnet => AssumeUtreexoValue {

diff --git a/crates/floresta-chain/src/pruned_utreexo/consensus.rs b/crates/floresta-chain/src/pruned_utreexo/consensus.rs
@@ -19,7 +19,6 @@ use bitcoin::TxIn;
 use bitcoin::TxOut;
 use bitcoin::Txid;
 use floresta_common::prelude::*;
-use rustreexo::accumulator::node_hash::BitcoinNodeHash;
 use rustreexo::accumulator::proof::Proof;
 use rustreexo::accumulator::stump::Stump;
 
@@ -258,10 +257,8 @@ impl Consensus {
         del_hashes: Vec<sha256::Hash>,
     ) -> Result<Stump, BlockchainError> {
         let block_hash = block.block_hash();
-        let del_hashes = del_hashes
-            .iter()
-            .map(|hash| BitcoinNodeHash::from(hash.as_byte_array()))
-            .collect::<Vec<_>>();
+        // Convert to BitcoinNodeHashes, from rustreexo
+        let del_hashes: Vec<_> = del_hashes.into_iter().map(Into::into).collect();
 
         let adds = udata::proof_util::get_block_adds(block, height, block_hash);
 

diff --git a/crates/floresta-chain/src/pruned_utreexo/partial_chain.rs b/crates/floresta-chain/src/pruned_utreexo/partial_chain.rs
@@ -212,30 +212,23 @@ impl PartialChainStateInner {
         inputs: HashMap<bitcoin::OutPoint, bitcoin::TxOut>,
     ) -> Result<(), BlockchainError> {
         if !block.check_merkle_root() {
-            return Err(BlockchainError::BlockValidation(
-                BlockValidationErrors::BadMerkleRoot,
-            ));
+            return Err(BlockValidationErrors::BadMerkleRoot)?;
         }
         if height >= self.chain_params().params.bip34_height
             && block.bip34_block_height() != Ok(height as u64)
         {
-            return Err(BlockchainError::BlockValidation(
-                BlockValidationErrors::BadBip34,
-            ));
+            return Err(BlockValidationErrors::BadBip34)?;
         }
 
         if !block.check_witness_commitment() {
-            return Err(BlockchainError::BlockValidation(
-                BlockValidationErrors::BadWitnessCommitment,
-            ));
+            return Err(BlockValidationErrors::BadWitnessCommitment)?;
         }
 
         let prev_block = self.get_ancestor(height)?;
         if block.header.prev_blockhash != prev_block.block_hash() {
-            return Err(BlockchainError::BlockValidation(
-                BlockValidationErrors::BlockExtendsAnOrphanChain,
-            ));
+            return Err(BlockValidationErrors::BlockExtendsAnOrphanChain)?;
         }
+
         // Validate block transactions
         let subsidy = self.consensus.get_subsidy(height);
         let verify_script = self.assume_valid;
@@ -501,13 +494,11 @@ impl From<PartialChainStateInner> for PartialChainState {
 
 #[cfg(test)]
 mod tests {
-    use core::str::FromStr;
     use std::collections::HashMap;
 
     use bitcoin::block::Header;
     use bitcoin::consensus::deserialize;
     use bitcoin::Block;
-    use rustreexo::accumulator::node_hash::BitcoinNodeHash;
     use rustreexo::accumulator::proof::Proof;
     use rustreexo::accumulator::stump::Stump;
 
@@ -534,7 +525,7 @@ mod tests {
                 _ => panic!("unexpected {res:?}"),
             };
         }
-        run("0000002000226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f39adbcd7823048d34357bdca86cd47172afe2a4af8366b5b34db36df89386d49b23ec964ffff7f20000000000101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff165108feddb99c6b8435060b2f503253482f627463642fffffffff0100f2052a01000000160014806cef41295922d32ddfca09c26cc4acd36c3ed000000000",super::BlockValidationErrors::BlockExtendsAnOrphanChain);
+        run("0000002000226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f39adbcd7823048d34357bdca86cd47172afe2a4af8366b5b34db36df89386d49b23ec964ffff7f20000000000101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff165108feddb99c6b8435060b2f503253482f627463642fffffffff0100f2052a01000000160014806cef41295922d32ddfca09c26cc4acd36c3ed000000000", BlockValidationErrors::BlockExtendsAnOrphanChain);
         run("0000002000226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f40adbcd7823048d34357bdca86cd47172afe2a4af8366b5b34db36df89386d49b23ec964ffff7f20000000000101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff165108feddb99c6b8435060b2f503253482f627463642fffffffff0100f2052a01000000160014806cef41295922d32ddfca09c26cc4acd36c3ed000000000", BlockValidationErrors::BadMerkleRoot);
     }
     fn parse_block(hex: &str) -> Block {
@@ -641,9 +632,8 @@ mod tests {
             "b21aae30bc74e9aef600a5d507ef27d799b9b6ba08e514656d34d717bdb569d2",
             "bedb648c9a3c5741660f926c1552d83ebb4cb1842cca6855b6d1089bb4951ce1",
         ]
-        .iter()
-        .map(|hash| BitcoinNodeHash::from_str(hash).unwrap())
-        .collect();
+        .map(|s| s.parse().unwrap())
+        .to_vec();
 
         let acc2 = Stump { roots, leaves: 100 };
 
@@ -682,9 +672,8 @@ mod tests {
             "e329a7ddcc888130bb6e4f82ce9f5cf5a712a7b0ae05a1aaf21b363866a9b05e",
             "1864a4982532447dcb3d9a5d2fea9f8ed4e3b1e759d55b8a427fb599fed0c302",
         ]
-        .iter()
-        .map(|x| BitcoinNodeHash::from(hex::decode(x).unwrap().as_slice()))
-        .collect::<Vec<_>>();
+        .map(|s| s.parse().unwrap())
+        .to_vec();
 
         let expected_acc: Stump = Stump { leaves: 150, roots };
 

diff --git a/crates/floresta-chain/src/pruned_utreexo/udata.rs b/crates/floresta-chain/src/pruned_utreexo/udata.rs
@@ -421,16 +421,13 @@ pub mod proof_util {
                 if !is_unspendable(&output.script_pubkey)
                     && !block_inputs.contains(&(transaction.compute_txid(), i as u32))
                 {
-                    leaf_hashes.push(get_leaf_hashes(transaction, i as u32, height, block_hash))
+                    leaf_hashes
+                        .push(get_leaf_hashes(transaction, i as u32, height, block_hash).into())
                 }
             }
         }
 
-        // Convert the leaf hashes to NodeHashes used in Rustreexo
         leaf_hashes
-            .iter()
-            .map(|&hash| BitcoinNodeHash::from(hash.as_byte_array()))
-            .collect()
     }
 
     #[allow(clippy::type_complexity)]

diff --git a/crates/floresta-wire/src/p2p_wire/mempool.rs b/crates/floresta-wire/src/p2p_wire/mempool.rs
@@ -906,9 +906,8 @@ mod tests {
             "6096c8421c1f86a9caa26e972dccdb964e280164fb060a576d51f5844e259569",
             "fd46029ebb0c19e2d468a9b24d20519c64ccc342e6a32b95c86a57489b6d2504",
         ]
-        .into_iter()
-        .map(|x| BitcoinNodeHash::from_str(x).unwrap())
-        .collect::<Vec<BitcoinNodeHash>>();
+        .map(|s| s.parse().unwrap())
+        .to_vec();
 
         let acc = Pollard::from_roots(roots, 169);
         let proof_hashes = [
@@ -920,15 +919,12 @@ mod tests {
             "15aba691713052033954935777d8089f4ca6b0573c7ad89fe1d0d85bbbe21846",
             "8f22055465f568fd2bf9d19b285fcf2539ffea59a3cb096a3a0645366adea1b0",
         ]
-        .into_iter()
-        .map(|x| BitcoinNodeHash::from_str(x).unwrap())
-        .collect::<Vec<BitcoinNodeHash>>();
+        .map(|s| s.parse().unwrap())
+        .to_vec();
 
         let proof = Proof::new(vec![8], proof_hashes);
         let del_hashes = ["427aceafd82c11cb53a2b78f408ece6fcacf2a5b9feb5fc45cdcf36627d68d76"]
-            .into_iter()
-            .map(|x| BitcoinNodeHash::from_str(x).unwrap())
-            .collect::<Vec<BitcoinNodeHash>>();
+            .map(|s| s.parse().unwrap());
 
         let prevout: LeafData = deserialize_hex("0508085c47cc849eb80ea905cc7800a3be674ffc57263cf210c59d8d00000000c997a5e56e104102fa209c6a852dd90660a20b2d9c352423edce25857fcd3704000000001300000000f2052a0100000043410411db93e1dcdb8a016b49840f8c53bc1eb68a382e97b1482ecad7b148a6909a5cb2e0eaddfb84ccf9744464f82e160bfa9b8b64f9d4c03f999b8643f656b412a3ac").unwrap();
 

diff --git a/crates/floresta-wire/src/p2p_wire/running_node.rs b/crates/floresta-wire/src/p2p_wire/running_node.rs
@@ -9,7 +9,6 @@ use std::time::Duration;
 use std::time::Instant;
 
 use bitcoin::bip158::BlockFilter;
-use bitcoin::hashes::Hash;
 use bitcoin::p2p::address::AddrV2;
 use bitcoin::p2p::address::AddrV2Message;
 use bitcoin::p2p::message_blockdata::Inventory;
@@ -718,10 +717,8 @@ where
             }
 
             if !self.chain.is_in_idb() {
-                let del_hashes: Vec<BitcoinNodeHash> = del_hashes
-                    .iter()
-                    .map(|hash| BitcoinNodeHash::from(hash.as_byte_array()))
-                    .collect();
+                // Convert to BitcoinNodeHashes, from rustreexo
+                let del_hashes: Vec<_> = del_hashes.into_iter().map(Into::into).collect();
 
                 let block_height = self
                     .chain