fix: remove hash caches to avoid JSON deserialization bug

chain/src/chain.rs

@@ -155,7 +155,7 @@ impl<CI: ChainIndex + 'static> ChainService<CI> {
             };
 
             self.shared.store().insert_block(batch, block);
-            self.shared.store().insert_output_root(batch, block.header().hash(), &root);
+            self.shared.store().insert_output_root(batch, &block.header().hash(), &root);
             self.shared.store().insert_block_ext(batch, &block.header().hash(), &ext);
 
             let current_total_difficulty = tip_header.total_difficulty();
@@ -572,7 +572,7 @@ pub mod test {
                 .expect("process block ok");
         }
         assert_eq!(
-            shared.block_hash(8).as_ref(),
+            shared.block_hash(8),
             chain2.get(7).map(|b| b.header().hash())
         );
     }
@@ -632,12 +632,9 @@ pub mod test {
         } else {
             chain2
         };
+        assert_eq!(shared.block_hash(8), best.get(7).map(|b| b.header().hash()));
         assert_eq!(
-            shared.block_hash(8).as_ref(),
-            best.get(7).map(|b| b.header().hash())
-        );
-        assert_eq!(
-            shared.block_hash(19).as_ref(),
+            shared.block_hash(19),
             best.get(18).map(|b| b.header().hash())
         );
     }

core/src/block.rs

@@ -139,7 +139,6 @@ impl BlockBuilder {
                 .commit_transactions
                 .iter()
                 .map(|t| t.hash())
-                .cloned()
                 .collect::<Vec<_>>(),
         );
 

core/src/header.rs

@@ -48,11 +48,7 @@ impl RawHeader {
 
     pub fn with_seal(self, seal: Seal) -> Header {
         let builder = HeaderBuilder {
-            inner: Header {
-                raw: self,
-                seal,
-                hash: H256::zero(),
-            },
+            inner: Header { raw: self, seal },
         };
         builder.build()
     }
@@ -79,8 +75,6 @@ pub struct Header {
     raw: RawHeader,
     /// proof seal
     seal: Seal,
-    #[serde(skip)]
-    hash: H256,
 }
 
 impl Header {
@@ -108,8 +102,8 @@ impl Header {
         self.seal.nonce
     }
 
-    pub fn hash(&self) -> &H256 {
-        &self.hash
+    pub fn hash(&self) -> H256 {
+        sha3_256(serialize(&self).unwrap()).into()
     }
 
     pub fn pow_hash(&self) -> H256 {
@@ -233,13 +227,6 @@ impl HeaderBuilder {
     }
 
     pub fn build(self) -> Header {
-        let hash: H256 = sha3_256(serialize(&self.inner).unwrap()).into();
-        self.with_hash(hash)
-    }
-
-    pub fn with_hash(self, hash: H256) -> Header {
-        let mut header = self.inner;
-        header.hash = hash;
-        header
+        self.inner
     }
 }

core/src/transaction.rs

@@ -103,8 +103,6 @@ pub struct Transaction {
     deps: Vec<OutPoint>,
     inputs: Vec<CellInput>,
     outputs: Vec<CellOutput>,
-    #[serde(skip)]
-    hash: H256,
 }
 
 impl CellOutput {
@@ -181,8 +179,8 @@ impl Transaction {
         self.inputs.len() == 1 && self.inputs[0].previous_output.is_null()
     }
 
-    pub fn hash(&self) -> &H256 {
-        &self.hash
+    pub fn hash(&self) -> H256 {
+        sha3_256(serialize(&self).unwrap()).into()
     }
 
     pub fn check_lock(&self, unlock: &[u8], lock: &[u8]) -> bool {
@@ -287,13 +285,6 @@ impl TransactionBuilder {
     }
 
     pub fn build(self) -> Transaction {
-        let hash: H256 = sha3_256(serialize(&self.inner).unwrap()).into();
-        self.with_hash(hash)
-    }
-
-    pub fn with_hash(self, hash: H256) -> Transaction {
-        let mut transaction = self.inner;
-        transaction.hash = hash;
-        transaction
+        self.inner
     }
 }

shared/src/index.rs

@@ -96,7 +96,7 @@ impl<T: 'static + KeyValueDB> ChainIndex for ChainKVStore<T> {
                 )
             })
             .map(|ref serialized_transaction| {
-                TransactionBuilder::new(serialized_transaction).with_hash(h.clone())
+                TransactionBuilder::new(serialized_transaction).build()
             })
     }
 
@@ -175,7 +175,7 @@ mod tests {
         let block = consensus.genesis_block();
         let hash = block.header().hash();
         store.init(&block);
-        assert_eq!(hash, &store.get_block_hash(0).unwrap());
+        assert_eq!(&hash, &store.get_block_hash(0).unwrap());
 
         assert_eq!(
             block.header().difficulty(),

shared/src/shared.rs

@@ -41,7 +41,7 @@ impl TipHeader {
         self.inner.number()
     }
 
-    pub fn hash(&self) -> &H256 {
+    pub fn hash(&self) -> H256 {
         self.inner.hash()
     }
 
@@ -192,7 +192,7 @@ pub trait ChainProvider: Sync + Send {
 
     fn block(&self, hash: &H256) -> Option<Block>;
 
-    fn genesis_hash(&self) -> &H256;
+    fn genesis_hash(&self) -> H256;
 
     fn get_transaction(&self, hash: &H256) -> Option<Transaction>;
 
@@ -250,7 +250,7 @@ impl<CI: ChainIndex> ChainProvider for Shared<CI> {
         self.store.get_block_number(hash)
     }
 
-    fn genesis_hash(&self) -> &H256 {
+    fn genesis_hash(&self) -> H256 {
         self.consensus.genesis_block().header().hash()
     }
 

shared/src/store.rs

@@ -147,7 +147,7 @@ impl<T: 'static + KeyValueDB> ChainStore for ChainKVStore<T> {
 
     fn get_header(&self, h: &H256) -> Option<Header> {
         self.get(COLUMN_BLOCK_HEADER, h.as_bytes())
-            .map(|ref raw| HeaderBuilder::new(raw).with_hash(h.clone()))
+            .map(|ref raw| HeaderBuilder::new(raw).build())
     }
 
     fn get_block_uncles(&self, h: &H256) -> Option<Vec<UncleBlock>> {
@@ -166,7 +166,7 @@ impl<T: 'static + KeyValueDB> ChainStore for ChainKVStore<T> {
             .and_then(|serialized_addresses| {
                 let addresses: Vec<Address> = deserialize(&serialized_addresses).unwrap();
                 self.get(COLUMN_BLOCK_BODY, h.as_bytes())
-                    .and_then(|serialized_body| {
+                    .map(|serialized_body| {
                         let txs: Vec<TransactionBuilder> = addresses
                             .iter()
                             .filter_map(|address| {
@@ -176,17 +176,10 @@ impl<T: 'static + KeyValueDB> ChainStore for ChainKVStore<T> {
                             })
                             .collect();
 
-                        self.get(COLUMN_BLOCK_TRANSACTION_IDS, h.as_bytes())
-                            .map(|serialized_ids| (txs, serialized_ids))
+                        txs
                     })
             })
-            .map(|(txs, serialized_ids)| {
-                let txs_ids: Vec<H256> = deserialize(&serialized_ids[..]).unwrap();
-                txs.into_iter()
-                    .zip(txs_ids.into_iter())
-                    .map(|(tx, id)| tx.with_hash(id))
-                    .collect()
-            })
+            .map(|txs| txs.into_iter().map(|tx| tx.build()).collect())
     }
 
     fn get_block_ext(&self, block_hash: &H256) -> Option<BlockExt> {
@@ -281,7 +274,6 @@ impl<T: 'static + KeyValueDB> ChainStore for ChainKVStore<T> {
             .commit_transactions()
             .iter()
             .map(|tx| tx.hash())
-            .cloned()
             .collect::<Vec<H256>>();
         batch.insert(
             COLUMN_BLOCK_HEADER,

sync/src/relayer/compact_block_process.rs

@@ -38,8 +38,8 @@ where
         let compact_block: CompactBlock = (*self.message).into();
         let block_hash = compact_block.header.hash();
         let pending_compact_blocks = self.relayer.state.pending_compact_blocks.upgradable_read();
-        if pending_compact_blocks.get(block_hash).is_none()
-            && self.relayer.get_block(block_hash).is_none()
+        if pending_compact_blocks.get(&block_hash).is_none()
+            && self.relayer.get_block(&block_hash).is_none()
         {
             let resolver =
                 HeaderResolverWrapper::new(&compact_block.header, self.relayer.shared.clone());

sync/src/synchronizer/block_fetcher.rs

@@ -184,7 +184,7 @@ where
                     .get_ancestor(&best_known_header.hash(), n_height));
                 let to_fetch_hash = to_fetch.hash();
 
-                let block_status = self.synchronizer.get_block_status(to_fetch_hash);
+                let block_status = self.synchronizer.get_block_status(&to_fetch_hash);
                 if block_status == BlockStatus::VALID_MASK
                     && inflight.insert(to_fetch_hash.clone().clone())
                 {

sync/src/synchronizer/header_view.rs

@@ -22,7 +22,7 @@ impl HeaderView {
         self.inner.number()
     }
 
-    pub fn hash(&self) -> &H256 {
+    pub fn hash(&self) -> H256 {
         self.inner.hash()
     }
 

sync/src/synchronizer/headers_process.rs

@@ -140,7 +140,7 @@ where
         );
         for window in headers.windows(2) {
             if let [parent, header] = &window {
-                if header.parent_hash() != parent.hash() {
+                if header.parent_hash() != &parent.hash() {
                     debug!(
                         target: "sync",
                         "header.parent_hash {:?} parent.hash {:?}",

sync/src/synchronizer/mod.rs

@@ -242,7 +242,7 @@ impl<CI: ChainIndex> Synchronizer<CI> {
             return None;
         }
 
-        if locator.last().expect("empty checked") != self.shared.genesis_hash() {
+        if locator.last().expect("empty checked") != &self.shared.genesis_hash() {
             return None;
         }
 
@@ -1017,7 +1017,7 @@ mod tests {
 
         for window in headers.windows(2) {
             if let [parent, header] = &window {
-                assert_eq!(header.parent_hash(), parent.hash());
+                assert_eq!(header.parent_hash(), &parent.hash());
             }
         }
     }
@@ -1138,11 +1138,11 @@ mod tests {
 
         assert_eq!(
             headers.first().unwrap().hash(),
-            &shared2.block_hash(193).unwrap()
+            shared2.block_hash(193).unwrap()
         );
         assert_eq!(
             headers.last().unwrap().hash(),
-            &shared2.block_hash(200).unwrap()
+            shared2.block_hash(200).unwrap()
         );
 
         println!(
@@ -1198,7 +1198,7 @@ mod tests {
         }
 
         assert_eq!(
-            synchronizer1
+            &synchronizer1
                 .peers
                 .last_common_headers
                 .read()

verification/src/block_verifier.rs

@@ -194,7 +194,6 @@ impl MerkleRootVerifier {
             .commit_transactions()
             .iter()
             .map(|tx| tx.hash())
-            .cloned()
             .collect::<Vec<_>>();
 
         if block.header().txs_commit() != &merkle_root(&commits[..]) {
@@ -362,7 +361,7 @@ impl<CP: ChainProvider + Clone> UnclesVerifier<CP> {
                 return Err(Error::Uncles(UnclesError::InvalidDifficultyEpoch));
             }
 
-            if uncle.header().cellbase_id() != uncle.cellbase().hash() {
+            if uncle.header().cellbase_id() != &uncle.cellbase().hash() {
                 return Err(Error::Uncles(UnclesError::InvalidCellbase));
             }
 
@@ -427,7 +426,7 @@ impl<P: ChainProvider + CellProvider + Clone> ::std::clone::Clone for Transactio
 struct TransactionsVerifierWrapper<'a, P: CellProvider + 'a> {
     verifier: &'a TransactionsVerifier<P>,
     block: &'a Block,
-    output_indexs: FnvHashMap<&'a H256, usize>,
+    output_indexs: FnvHashMap<H256, usize>,
 }
 
 impl<'a, P: CellProvider> CellProvider for TransactionsVerifierWrapper<'a, P> {

verification/src/tests/dummy.rs

@@ -38,7 +38,7 @@ impl ChainProvider for DummyChainProvider {
         panic!("Not implemented!");
     }
 
-    fn genesis_hash(&self) -> &H256 {
+    fn genesis_hash(&self) -> H256 {
         panic!("Not implemented!");
     }