consensus: Revert to last common ancestor

node/src/chain/acceptor.rs

@@ -25,8 +25,9 @@ use crate::chain::header_validation::Validator;
 
 #[allow(dead_code)]
 pub(crate) enum RevertTarget {
-    LastFinalizedState = 0,
-    LastEpoch = 1,
+    Commit([u8; 32]),
+    LastFinalizedState,
+    LastEpoch,
 }
 
 /// Implements block acceptance procedure. This includes block header,
@@ -407,17 +408,32 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> Acceptor<N, DB, VM> {
 
         let target_state_hash = match target {
             RevertTarget::LastFinalizedState => {
-                info!(event = "vm_revert to last finalized state");
-                let state_hash = self.vm.read().await.revert()?;
+                let vm = self.vm.read().await;
+                let base_root = vm.get_base_state_root()?;
+                let state_hash = vm.revert(base_root)?;
 
                 info!(
                     event = "vm reverted",
-                    state_root = hex::encode(state_hash)
+                    state_root = hex::encode(state_hash),
+                    is_final = "true",
                 );
 
                 anyhow::Ok(state_hash)
             }
-            _ => unimplemented!(),
+            RevertTarget::Commit(state_hash) => {
+                let vm = self.vm.read().await;
+                let state_hash = vm.revert(state_hash)?;
+                let is_final = vm.get_base_state_root()? == state_hash;
+
+                info!(
+                    event = "vm reverted",
+                    state_root = hex::encode(state_hash),
+                    is_final,
+                );
+
+                anyhow::Ok(state_hash)
+            }
+            RevertTarget::LastEpoch => unimplemented!(),
         }?;
 
         // Delete any block until we reach the target_state_hash, the

node/src/chain/fsm.rs

@@ -299,7 +299,7 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> InSyncImpl<DB, VM, N> {
             // R_B.Iteration < L_B.Iteration
             //
             // Then we fallback to N_B.PrevBlock and accept N_B
-            let local_header = acc.db.read().await.view(|t| {
+            let result = acc.db.read().await.view(|t| {
                 if let Some((prev_header, _)) =
                     t.fetch_block_header(&remote_blk.header().prev_block_hash)?
                 {
@@ -308,20 +308,23 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> InSyncImpl<DB, VM, N> {
                         if remote_blk.header().iteration
                             < l_b.header().iteration
                         {
-                            return Ok(Some(l_b.header().clone()));
+                            return Ok(Some((
+                                l_b.header().clone(),
+                                prev_header.state_hash,
+                            )));
                         }
                     }
                 }
 
                 anyhow::Ok(None)
             })?;
 
-            if let Some(local_header) = local_header {
+            if let Some((local_header, state_hash)) = result {
                 match fallback::WithContext::new(acc.deref())
                     .try_revert(
                         &local_header,
                         remote_blk.header(),
-                        RevertTarget::LastFinalizedState,
+                        RevertTarget::Commit(state_hash),
                     )
                     .await
                 {
@@ -360,11 +363,28 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> InSyncImpl<DB, VM, N> {
                 new_iter = remote_blk.header().iteration,
             );
 
+            let state_hash = acc
+                .db
+                .read()
+                .await
+                .view(|t| {
+                    let res = t
+                        .fetch_block_header(
+                            &remote_blk.header().prev_block_hash,
+                        )?
+                        .map(|(prev_header, _)| prev_header.state_hash);
+
+                    anyhow::Ok::<Option<[u8; 32]>>(res)
+                })?
+                .ok_or_else(|| {
+                    anyhow::anyhow!("could not retrieve state_hash")
+                })?;
+
             match fallback::WithContext::new(acc.deref())
                 .try_revert(
                     &local_header,
                     remote_blk.header(),
-                    RevertTarget::LastFinalizedState,
+                    RevertTarget::Commit(state_hash),
                 )
                 .await
             {

node/src/vm.rs

@@ -47,6 +47,7 @@ pub trait VMExecution: Send + Sync + 'static {
     ) -> anyhow::Result<Provisioners>;
 
     fn get_state_root(&self) -> anyhow::Result<[u8; 32]>;
+    fn get_base_state_root(&self) -> anyhow::Result<[u8; 32]>;
 
-    fn revert(&self) -> anyhow::Result<[u8; 32]>;
+    fn revert(&self, state_hash: [u8; 32]) -> anyhow::Result<[u8; 32]>;
 }

node/testbed.sh

@@ -51,7 +51,7 @@
 	  RUSK_STATE_PATH=${RUSK_STATE_PATH} cargo r --release -p rusk -- recovery-state --init $GENESIS_PATH
 	  echo "starting node $ID ..."
     echo "${KEYS_PATH}/node_$ID.keys"
-	  RUSK_STATE_PATH=${RUSK_STATE_PATH} ./target/release/rusk --kadcast-bootstrap "$BOOTSTRAP_ADDR" --kadcast-public-address "$PUBLIC_ADDR" --log-level="$LOG_LEVEL" --log-filter="dusk_consensus=debug"  --consensus-keys-path="${KEYS_PATH}/node_$ID.keys" --db-path="$NODE_FOLDER" --http-listen-addr "$WS_LISTEN_ADDR" --delay-on-resp-msg=10 > "${TEMPD}/node_${ID}.log" &
+	  RUSK_STATE_PATH=${RUSK_STATE_PATH} ./target/release/rusk --kadcast-bootstrap "$BOOTSTRAP_ADDR" --kadcast-public-address "$PUBLIC_ADDR" --log-type="json" --log-level="$LOG_LEVEL" --log-filter="dusk_consensus=debug"  --consensus-keys-path="${KEYS_PATH}/node_$ID.keys" --db-path="$NODE_FOLDER" --http-listen-addr "$WS_LISTEN_ADDR" --delay-on-resp-msg=10 > "${TEMPD}/node_${ID}.log" &
 	}
 
 	## Use ~/.cargo/bin/tokio-console --retain-for 0s http://127.0.0.1:10000 to connect console to first node

rusk/benches/block_ingestion.rs

@@ -99,7 +99,9 @@ pub fn accept_benchmark(c: &mut Criterion) {
                         )
                         .expect("Accepting transactions should succeed");
 
-                        rusk.revert().expect("Reverting should succeed");
+                        let base_root = rusk.base_root();
+                        rusk.revert(base_root)
+                            .expect("Reverting should succeed");
                     })
                 },
             );

rusk/src/lib/error.rs

@@ -49,6 +49,8 @@ pub enum Error {
     InconsistentState(VerificationOutput),
     /// Other
     Other(Box<dyn std::error::Error>),
+    /// Commit not found amongst existing commits
+    CommitNotFound([u8; 32]),
 }
 
 impl std::error::Error for Error {}
@@ -140,6 +142,9 @@ impl fmt::Display for Error {
                     "Inconsistent state verification data {verification_output}",
                 )
             }
+            Error::CommitNotFound(commit_id) => {
+                write!(f, "Commit not found, id = {}", hex::encode(commit_id),)
+            }
         }
     }
 }

rusk/src/lib/lib.rs

@@ -334,9 +334,15 @@ impl Rusk {
         Ok(())
     }
 
-    pub fn revert(&self) -> Result<[u8; 32]> {
+    pub fn revert(&self, state_hash: [u8; 32]) -> Result<[u8; 32]> {
         let mut inner = self.inner.lock();
-        inner.current_commit = inner.base_commit;
+
+        let commits = &inner.vm.commits();
+        if !commits.contains(&state_hash) {
+            return Err(Error::CommitNotFound(state_hash));
+        }
+
+        inner.current_commit = state_hash;
         Ok(inner.current_commit)
     }
 

rusk/src/lib/vm.rs

@@ -149,9 +149,13 @@ impl VMExecution for Rusk {
         Ok(self.state_root())
     }
 
-    fn revert(&self) -> anyhow::Result<[u8; 32]> {
+    fn get_base_state_root(&self) -> anyhow::Result<[u8; 32]> {
+        Ok(self.base_root())
+    }
+
+    fn revert(&self, state_hash: [u8; 32]) -> anyhow::Result<[u8; 32]> {
         let state_hash = self
-            .revert()
+            .revert(state_hash)
             .map_err(|inner| anyhow::anyhow!("Cannot revert: {inner}"))?;
 
         Ok(state_hash)

rusk/tests/rusk-state.rs

@@ -102,7 +102,8 @@ pub fn rusk_state_accepted() -> Result<()> {
         "There should be two notes in the state now"
     );
 
-    rusk.revert()?;
+    let base_root = rusk.base_root();
+    rusk.revert(base_root)?;
     let leaves = leaves_from_height(&rusk, 0)?;
 
     assert_eq!(
@@ -134,7 +135,8 @@ pub fn rusk_state_finalized() -> Result<()> {
         "There should be two notes in the state now"
     );
 
-    rusk.revert()?;
+    let base_root = rusk.base_root();
+    rusk.revert(base_root)?;
     let leaves = leaves_from_height(&rusk, 0)?;
 
     assert_eq!(

rusk/tests/services/transfer.rs

@@ -185,7 +185,8 @@ pub async fn wallet() -> Result<()> {
     assert_ne!(original_root, new_root, "Root should have changed");
 
     // Revert the state
-    rusk.revert().expect("Reverting should succeed");
+    let base_root = rusk.base_root();
+    rusk.revert(base_root).expect("Reverting should succeed");
     cache.write().unwrap().clear();
 
     // Check the state's root is back to the original one