Repository: dusk-network/rusk (language: rust)

Issue:
node-data: change hash method on transaction to digest
#### Summary

Transaction hash and transaction ID are often used as semantically equivalent terms in the crypto/blockchain world. To avoid any ambiguity, we should refrain from employing the terms "hash" and "id" in a manner that result in their denotations having different meanings (different hashes/values).

#### Relevant Context
- The value of `hash(&self)` is only being used in the Merkle tree.
- The value of `id(&self)` is the actual transaction identifier being used in the explorer, wallet, to identify transactions etc. - This is the equivalent of the txID/txHash terminology on other chains.

https://github.com/dusk-network/rusk/blob/08f95c328db1ea022f246e963ae46ae02b1237a0/node-data/src/ledger/transaction.rs#L57-L81

#### Possible Solution (optional)

As `hash(&self)` is rarely used and is not really needed by end-users or integrators, we should introduce another name for it (potentially `digest`) so that the value of `id(&self)` can be safely referred to as TxID or TxHash.



Discussion / hints:

Fix patch:
diff --git a/consensus/src/proposal/block_generator.rs b/consensus/src/proposal/block_generator.rs
index 5ee57eddcd..18f258328b 100644
--- a/consensus/src/proposal/block_generator.rs
+++ b/consensus/src/proposal/block_generator.rs
@@ -113,15 +113,15 @@ impl<T: Operations> Generator<T> {
 
         // We always write the faults len in a u32
         let mut faults_size = u32::SIZE;
-        let faults_hashes: Vec<_> = faults
+        let fault_digests: Vec<_> = faults
             .iter()
             .map(|f| {
                 faults_size += f.size();
-                f.hash()
+                f.digest()
             })
             .collect();
 
-        blk_header.faultroot = merkle_root(&faults_hashes);
+        blk_header.faultroot = merkle_root(&fault_digests);
 
         // We know for sure that this operation cannot underflow
         let max_txs_bytes = MAX_BLOCK_SIZE - header_size - faults_size;
@@ -140,10 +140,10 @@ impl<T: Operations> Generator<T> {
         blk_header.state_hash = result.verification_output.state_root;
         blk_header.event_bloom = result.verification_output.event_bloom;
 
-        let tx_hashes: Vec<_> =
-            result.txs.iter().map(|t| t.inner.hash()).collect();
+        let tx_digests: Vec<_> =
+            result.txs.iter().map(|t| t.inner.digest()).collect();
         let txs: Vec<_> = result.txs.into_iter().map(|t| t.inner).collect();
-        blk_header.txroot = merkle_root(&tx_hashes[..]);
+        blk_header.txroot = merkle_root(&tx_digests[..]);
 
         blk_header.timestamp = max(
             ru.timestamp() + *MINIMUM_BLOCK_TIME,
diff --git a/consensus/src/proposal/handler.rs b/consensus/src/proposal/handler.rs
index 248aeb2fc0..d98d1d70ee 100644
--- a/consensus/src/proposal/handler.rs
+++ b/consensus/src/proposal/handler.rs
@@ -173,9 +173,9 @@ fn verify_candidate_msg(
     }
 
     // Verify tx_root
-    let tx_hashes: Vec<_> =
-        p.candidate.txs().iter().map(|t| t.hash()).collect();
-    let tx_root = merkle_root(&tx_hashes[..]);
+    let tx_digests: Vec<_> =
+        p.candidate.txs().iter().map(|t| t.digest()).collect();
+    let tx_root = merkle_root(&tx_digests[..]);
     if tx_root != p.candidate.header().txroot {
         return Err(ConsensusError::InvalidBlock);
     }
@@ -186,9 +186,9 @@ fn verify_candidate_msg(
     }
 
     // Verify fault_root
-    let fault_hashes: Vec<_> =
-        p.candidate.faults().iter().map(|t| t.hash()).collect();
-    let fault_root = merkle_root(&fault_hashes[..]);
+    let fault_digests: Vec<_> =
+        p.candidate.faults().iter().map(|t| t.digest()).collect();
+    let fault_root = merkle_root(&fault_digests[..]);
     if fault_root != p.candidate.header().faultroot {
         return Err(ConsensusError::InvalidBlock);
     }
diff --git a/node-data/src/ledger/faults.rs b/node-data/src/ledger/faults.rs
index f9948d39f1..4d0faa65d7 100644
--- a/node-data/src/ledger/faults.rs
+++ b/node-data/src/ledger/faults.rs
@@ -81,8 +81,14 @@ impl From<BlsSigError> for InvalidFault {
 }
 
 impl Fault {
-    /// Hash the serialized form
-    pub fn hash(&self) -> [u8; 32] {
+    // TODO: change to HEIGHT|TYPE|PROV_KEY once faults collection is
+    // implemented
+    pub fn id(&self) -> [u8; 32] {
+        self.digest()
+    }
+
+    /// Digest the serialized form
+    pub fn digest(&self) -> [u8; 32] {
         let mut b = vec![];
         self.write(&mut b).expect("Write to a vec shall not fail");
         sha3::Sha3_256::digest(&b[..]).into()
diff --git a/node-data/src/ledger/transaction.rs b/node-data/src/ledger/transaction.rs
index e4d13aa49e..c7b4901a7a 100644
--- a/node-data/src/ledger/transaction.rs
+++ b/node-data/src/ledger/transaction.rs
@@ -55,14 +55,16 @@ pub struct SpentTransaction {
 }
 
 impl Transaction {
-    /// Computes the hash of the transaction.
+    /// Computes the hash digest of the entire transaction data.
     ///
-    /// This method returns the hash of the entire
+    /// This method returns the Sha3 256 digest of the entire
     /// transaction in its serialized form
     ///
+    /// The digest hash is currently only being used in the merkle tree.
+    ///
     /// ### Returns
     /// An array of 32 bytes representing the hash of the transaction.
-    pub fn hash(&self) -> [u8; 32] {
+    pub fn digest(&self) -> [u8; 32] {
         sha3::Sha3_256::digest(self.inner.to_var_bytes()).into()
     }
 
diff --git a/node/src/database/rocksdb.rs b/node/src/database/rocksdb.rs
index 10d3ffdc89..f43190a168 100644
--- a/node/src/database/rocksdb.rs
+++ b/node/src/database/rocksdb.rs
@@ -318,7 +318,7 @@ impl<'db, DB: DBAccess> Ledger for DBTransaction<'db, DB> {
             LightBlock {
                 header: header.clone(),
                 transactions_ids: txs.iter().map(|t| t.inner.id()).collect(),
-                faults_ids: faults.iter().map(|f| f.hash()).collect(),
+                faults_ids: faults.iter().map(|f| f.id()).collect(),
             }
             .write(&mut buf)?;
 
@@ -349,7 +349,7 @@ impl<'db, DB: DBAccess> Ledger for DBTransaction<'db, DB> {
             for f in faults {
                 let mut d = vec![];
                 f.write(&mut d)?;
-                self.put_cf(cf, f.hash(), d)?;
+                self.put_cf(cf, f.id(), d)?;
             }
         }
         self.store_block_label(header.height, &header.hash, label)?;
@@ -410,7 +410,7 @@ impl<'db, DB: DBAccess> Ledger for DBTransaction<'db, DB> {
             self.inner.delete_cf(self.ledger_txs_cf, tx.id())?;
         }
         for f in b.faults() {
-            self.inner.delete_cf(self.ledger_faults_cf, f.hash())?;
+            self.inner.delete_cf(self.ledger_faults_cf, f.id())?;
         }
 
         self.inner.delete_cf(self.ledger_cf, b.header().hash)?;
@@ -1206,7 +1206,7 @@ impl node_data::Serializable for LightBlock {
         let len = self.faults_ids.len() as u32;
         w.write_all(&len.to_le_bytes())?;
 
-        // Write faults hashes
+        // Write faults id
         for f_id in &self.faults_ids {
             w.write_all(f_id)?;
         }
@@ -1236,7 +1236,7 @@ impl node_data::Serializable for LightBlock {
         // Read faults count
         let len = Self::read_u32_le(r)?;
 
-        // Read faults hashes
+        // Read faults ids
         let mut faults_ids = vec![];
         for _ in 0..len {
             let mut f_id = [0u8; 32];
@@ -1299,10 +1299,7 @@ mod tests {
                 // Assert all faults are fully fetched from ledger as
                 // well.
                 for pos in 0..b.faults().len() {
-                    assert_eq!(
-                        db_blk.faults()[pos].hash(),
-                        b.faults()[pos].hash()
-                    );
+                    assert_eq!(db_blk.faults()[pos].id(), b.faults()[pos].id());
                 }
             });
 


Test patch: