Merge pull request #2087 from dusk-network/fix-fault-overflow

dusk-network · Aug 8, 2024 · 904ca44 · 904ca44
2 parents 29df7cc + 9a701ae
commit 904ca44
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 7 deletions.
diff --git a/node-data/src/ledger/faults.rs b/node-data/src/ledger/faults.rs
@@ -44,6 +44,8 @@ pub enum InvalidFault {
     Expired,
     #[error("Fault is from future")]
     Future,
+    #[error("Fault is from genesis block")]
+    Genesis,
     #[error("Previous hash mismatch")]
     PrevHashMismatch,
     #[error("Iteration mismatch")]
@@ -153,8 +155,12 @@ impl Fault {
             return Err(InvalidFault::Duplicated);
         }
 
+        if h1.round == 0 {
+            return Err(InvalidFault::Genesis);
+        }
+
         // Check that fault is not expired. A fault expires after an epoch
-        if h1.round < current_height - EPOCH {
+        if h1.round < current_height.saturating_sub(EPOCH) {
             return Err(InvalidFault::Expired);
         }
 
@@ -298,10 +304,10 @@ impl Serializable for Fault {
                 FaultData::read(r)?,
                 FaultData::read(r)?,
             ),
-            p => {
-                println!("{p}");
-                Err(io::Error::new(io::ErrorKind::InvalidData, "Invalid faul"))?
-            }
+            p => Err(io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("Invalid fault: {p}"),
+            ))?,
         };
         Ok(fault)
     }

diff --git a/node/src/chain/header_validation.rs b/node/src/chain/header_validation.rs
@@ -302,15 +302,17 @@ pub async fn verify_faults<DB: database::DB>(
                 let prev_header = db
                     .fetch_block_header(&fault_header.prev_block_hash)?
                     .ok_or(anyhow::anyhow!("Slashing a non accepted header"))?;
+                // No overflow here, since the header has been already validated
+                // not to be 0
                 if prev_header.height != fault_header.round - 1 {
                     anyhow::bail!("Invalid height for fault");
                 }
 
                 // FIX_ME: Instead of fetching all store faults, check the fault
                 // id directly This needs the fault id to be
                 // changed into "HEIGHT|TYPE|PROV_KEY"
-                let stored_faults =
-                    db.fetch_faults_by_block(fault_header.round - EPOCH)?;
+                let start_height = fault_header.round.saturating_sub(EPOCH);
+                let stored_faults = db.fetch_faults_by_block(start_height)?;
                 if stored_faults.iter().any(|other| f.same(other)) {
                     anyhow::bail!("Double fault detected");
                 }