Merge pull request #3738 from dusk-network/refactor_state_transition_part2

Refactor State Transition (Part 2)

Author: fed-franz

consensus/src/errors.rs

@@ -69,10 +69,10 @@ impl From<BlsSigError> for ConsensusError {
 
 #[derive(Debug, Error)]
 pub enum OperationError {
-    #[error("VST failed: {0}")]
-    FailedVST(StateTransitionError),
-    #[error("EST failed: {0}")]
-    FailedEST(StateTransitionError),
+    #[error("State transition creation failed: {0}")]
+    FailedTransitionCreation(StateTransitionError),
+    #[error("State transition verification failed: {0}")]
+    FailedTransitionVerification(StateTransitionError),
     #[error("Invalid header: {0}")]
     InvalidHeader(HeaderError),
     #[error("Unable to update metrics: {0}")]
@@ -111,19 +111,6 @@ pub enum HeaderError {
     #[error("Invalid Failed Iterations: {0}")]
     InvalidFailedIterations(FailedIterationError),
 
-    #[error(
-        "mismatch, event_bloom: {}, candidate_event_bloom: {}",
-        hex::encode(.0.as_ref()),
-        hex::encode(.1.as_ref())
-    )]
-    EventBloomMismatch(Box<[u8; 256]>, Box<[u8; 256]>),
-    #[error(
-        "mismatch, state_hash: {}, candidate_state_hash: {}",
-        hex::encode(.0),
-        hex::encode(.1)
-    )]
-    StateRootMismatch([u8; 32], [u8; 32]),
-
     #[error("Generic error in header verification: {0}")]
     Generic(&'static str),
 
@@ -139,17 +126,33 @@ pub enum StateTransitionError {
     InvalidSlash(io::Error),
     #[error("Invalid generator: {0:?}")]
     InvalidGenerator(dusk_bytes::Error),
+    #[error("Session instantiation failed: {0}")]
+    SessionError(String),
     #[error("Execution failed: {0}")]
     ExecutionError(String),
-    #[error("Verification failed: {0}")]
-    VerificationError(String),
+    #[error(
+      "State root mismatch. transition result: {}, block header: {}",
+      hex::encode(.0),
+      hex::encode(.1)
+    )]
+    StateRootMismatch([u8; 32], [u8; 32]),
+    #[error(
+      "Event bloom mismatch. transition result: {}, block header: {}",
+      hex::encode(.0.as_ref()),
+      hex::encode(.1.as_ref())
+    )]
+    EventBloomMismatch(Box<[u8; 256]>, Box<[u8; 256]>),
+    #[error("Failed to persist state: {0}")]
+    PersistenceError(String),
 }
 
 impl OperationError {
     pub fn must_vote(&self) -> bool {
         match self {
             Self::InvalidHeader(e) => e.must_vote(),
-            Self::FailedVST(StateTransitionError::TipChanged) => false,
+            Self::FailedTransitionVerification(
+                StateTransitionError::TipChanged,
+            ) => false,
             _ => true,
         }
     }
@@ -171,8 +174,6 @@ impl HeaderError {
             HeaderError::InvalidSeed(_) => true,
             HeaderError::InvalidAttestation(_) => true,
             HeaderError::InvalidFailedIterations(_) => true,
-            HeaderError::EventBloomMismatch(_, _) => true,
-            HeaderError::StateRootMismatch(_, _) => true,
 
             HeaderError::Generic(..) => false,
         }

consensus/src/operations.rs

@@ -8,9 +8,7 @@ use std::fmt;
 use std::time::Duration;
 
 use node_data::bls::{PublicKey, PublicKeyBytes};
-use node_data::ledger::{
-    Block, Fault, Header, Slash, SpentTransaction, Transaction,
-};
+use node_data::ledger::{Block, Fault, Header, Slash, SpentTransaction};
 use node_data::StepName;
 
 use crate::errors::*;
@@ -61,7 +59,7 @@ pub trait Operations: Send + Sync {
         &self,
         candidate_header: &Header,
         expected_generator: &PublicKeyBytes,
-    ) -> Result<(u8, Vec<Voter>, Vec<Voter>), HeaderError>;
+    ) -> Result<Vec<Voter>, HeaderError>;
 
     async fn validate_faults(
         &self,
@@ -74,19 +72,12 @@ pub trait Operations: Send + Sync {
         prev_state: StateRoot,
         blk: &Block,
         cert_voters: &[Voter],
-    ) -> Result<StateTransitionResult, OperationError>;
+    ) -> Result<(), OperationError>;
 
-    async fn new_state_transition(
+    async fn generate_state_transition(
         &self,
         transition_data: StateTransitionData,
-    ) -> Result<
-        (
-            Vec<SpentTransaction>,
-            StateTransitionResult,
-            Vec<Transaction>,
-        ),
-        OperationError,
-    >;
+    ) -> Result<(Vec<SpentTransaction>, StateTransitionResult), OperationError>;
 
     async fn add_step_elapsed_time(
         &self,

consensus/src/proposal/block_generator.rs

@@ -117,8 +117,10 @@ impl<T: Operations> Generator<T> {
         };
 
         // Compute a valid state transition for the block
-        let (executed_txs, transition_result, _) =
-            self.executor.new_state_transition(transition_data).await?;
+        let (executed_txs, transition_result) = self
+            .executor
+            .generate_state_transition(transition_data)
+            .await?;
 
         blk_header.state_hash = transition_result.state_root;
         blk_header.event_bloom = transition_result.event_bloom;

consensus/src/proposal/step.rs

@@ -113,7 +113,7 @@ impl<T: Operations + 'static, D: Database> ProposalStep<T, D> {
                 }
 
                 Err(err) => match err {
-                    OperationError::FailedEST(
+                    OperationError::FailedTransitionCreation(
                         StateTransitionError::TipChanged,
                     ) => {
                         warn!(

consensus/src/validation/step.rs

@@ -7,7 +7,7 @@
 use std::sync::Arc;
 
 use node_data::bls::PublicKeyBytes;
-use node_data::ledger::{to_str, Block, Header};
+use node_data::ledger::{to_str, Block};
 use node_data::message::payload::{Validation, Vote};
 use node_data::message::{
     AsyncQueue, ConsensusHeader, Message, Payload, SignInfo, SignedStepMessage,
@@ -18,10 +18,10 @@ use tracing::{debug, error, info, warn, Instrument};
 
 use crate::commons::{Database, RoundUpdate};
 use crate::config::is_emergency_iter;
-use crate::errors::{HeaderError, OperationError};
+use crate::errors::OperationError;
 use crate::execution_ctx::ExecutionCtx;
 use crate::msg_handler::StepOutcome;
-use crate::operations::{Operations, StateRoot, StateTransitionResult};
+use crate::operations::{Operations, StateRoot};
 use crate::validation::handler;
 
 pub struct ValidationStep<T, D: Database> {
@@ -89,7 +89,7 @@ impl<T: Operations + 'static, D: Database> ValidationStep<T, D> {
         let header = candidate.header();
         let candidate_hash = header.hash;
 
-        let vote = match Self::verify_candidate(
+        let vote = match Self::validate_candidate(
             candidate,
             ru.state_root(),
             executor,
@@ -119,32 +119,29 @@ impl<T: Operations + 'static, D: Database> ValidationStep<T, D> {
         Self::cast_vote(vote, ru, iteration, outbound, inbound).await;
     }
 
-    async fn verify_candidate(
+    async fn validate_candidate(
         candidate: &Block,
         prev_state: StateRoot,
         executor: Arc<T>,
         expected_generator: PublicKeyBytes,
     ) -> Result<(), OperationError> {
         let header = candidate.header();
 
-        // Verify faults
+        // Validate faults
         executor
             .validate_faults(header.height, candidate.faults())
             .await?;
 
-        // Verify candidate header
-        let (_, cert_voters, _) = executor
+        // Validate candidate header
+        let cert_voters = executor
             .validate_block_header(header, &expected_generator)
             .await?;
 
-        // Verify state transition
-        let transition_result = executor
+        // Validate state transition
+        executor
             .validate_state_transition(prev_state, candidate, &cert_voters)
             .await?;
 
-        // Verify header against state transition output
-        Self::check_header_vst(header, &transition_result)?;
-
         Ok(())
     }
 
@@ -177,28 +174,6 @@ impl<T: Operations + 'static, D: Database> ValidationStep<T, D> {
             inbound.try_send(msg);
         }
     }
-
-    fn check_header_vst(
-        header: &Header,
-        transition_result: &StateTransitionResult,
-    ) -> Result<(), HeaderError> {
-        // Check the header against `event_bloom` and `state_root` from VST
-        if transition_result.event_bloom != header.event_bloom {
-            return Err(HeaderError::EventBloomMismatch(
-                Box::new(transition_result.event_bloom),
-                Box::new(header.event_bloom),
-            ));
-        }
-
-        if transition_result.state_root != header.state_hash {
-            return Err(HeaderError::StateRootMismatch(
-                transition_result.state_root,
-                header.state_hash,
-            ));
-        }
-
-        Ok(())
-    }
 }
 
 pub fn build_validation_payload(

node/src/chain/acceptor.rs

@@ -718,12 +718,12 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> Acceptor<N, DB, VM> {
     ///
     /// * blk: the block to accept as successor of the tip
     /// * restart_consensus: `true` if the consensus loop has to be restarted
-    ///   after accepting a block, false otherwise.
+    ///   after accepting a block.
     ///
     /// # Returns
     ///
     /// * `true` if the accepted block triggered rolling finality on a previous
-    ///   block; `false` otherwise.
+    ///   block.
     pub(crate) async fn accept_block(
         &mut self,
         blk: &Block,
@@ -740,6 +740,8 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> Acceptor<N, DB, VM> {
 
         let header_verification_start = std::time::Instant::now();
         // Verify Block Header
+        // `cert_voters` and `att_voters` are the voters of the previous-block
+        // Certificate and the `blk` Attestation, respectively
         let (pni, cert_voters, att_voters) = verify_block_header(
             self.db.clone(),
             &prev_header,
@@ -767,34 +769,36 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> Acceptor<N, DB, VM> {
             let (contract_events, finality) =
                 self.db.read().await.update(|db| {
                     // Execute, verify, and persist the state_transition
-                    let (txs, transition_result, contract_events) = vm
-                        .do_accept_state_transition(
+                    let (spent_txs, contract_events) = vm
+                        .accept_state_transition(
                             prev_header.state_hash,
                             blk,
                             &cert_voters[..],
                         )?;
 
-                    for spent_tx in txs.iter() {
+                    // Add spent txs to event list
+                    for spent_tx in spent_txs.iter() {
                         events
                             .push(TransactionEvent::Executed(spent_tx).into());
                     }
-                    est_elapsed_time = start.elapsed();
 
-                    assert_eq!(header.state_hash, transition_result.state_root);
-                    assert_eq!(
-                        header.event_bloom,
-                        transition_result.event_bloom
-                    );
+                    // Stop elapsed time
+                    est_elapsed_time = start.elapsed();
 
-                    // Check if the new block triggers rolling finality
+                    // Check finality for previous blocks and the Consensus
+                    // State label for the accepted block
                     let finality =
                         self.rolling_finality::<DB>(pni, blk, db, &mut events)?;
-
                     let label = finality.0;
+
                     // Store block with spent transactions info (including
                     // GasSpent and Errors), faults, and consensus status label
-                    block_size_on_disk =
-                        db.store_block(header, &txs, blk.faults(), label)?;
+                    block_size_on_disk = db.store_block(
+                        header,
+                        &spent_txs,
+                        blk.faults(),
+                        label,
+                    )?;
 
                     Ok((contract_events, finality))
                 })?;
@@ -1531,10 +1535,13 @@ async fn broadcast<N: Network>(network: &Arc<RwLock<N>>, msg: &Message) {
     });
 }
 
-/// Performs full verification of block header against prev_block header where
-/// prev_block is usually the blockchain tip
+/// Verifies a block header against its previous block
+///
+/// # Returns
 ///
-/// Returns the number of Previous Non-Attested Iterations (PNI).
+/// * The number of Previous Non-Attested Iterations (PNI)
+/// * The list of voters in the previous-block Certificate
+/// * The list of voters in the current-block Attestation
 pub(crate) async fn verify_block_header<DB: database::DB>(
     db: Arc<RwLock<DB>>,
     prev_header: &ledger::Header,
@@ -1565,6 +1572,6 @@ pub(crate) async fn verify_block_header<DB: database::DB>(
     // Verify header validity
     let validator = Validator::new(db, prev_header, provisioners);
     validator
-        .execute_checks(header, &expected_generator, check_att)
+        .verify_block_header_fields(header, &expected_generator, check_att)
         .await
 }