From c969af7735a78c450fcfead2cd9afd0e7ccdddb9 Mon Sep 17 00:00:00 2001 From: goshawk-3 Date: Wed, 17 Jan 2024 09:23:14 +0200 Subject: [PATCH 1/5] node: Allow fallback to lower-round blocks --- node/src/chain/fsm.rs | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/node/src/chain/fsm.rs b/node/src/chain/fsm.rs index 15b707d5bc..22f81d4c7b 100644 --- a/node/src/chain/fsm.rs +++ b/node/src/chain/fsm.rs @@ -9,6 +9,7 @@ use crate::chain::fallback; use crate::database; use crate::{vm, Network}; +use crate::database::Ledger; use node_data::ledger::{to_str, Block, Label}; use node_data::message::payload::{GetBlocks, Inv}; use node_data::message::Message; @@ -273,6 +274,46 @@ impl InSyncImpl { let curr_hash = acc.get_curr_hash().await; if height < tip_height { + let exists = acc + .db + .read() + .await + .view(|t| t.get_block_exists(&blk.header().hash))?; + + if exists { + // Already exists in local state + return Ok(None); + } + + // If our local chain has a block L_B with ConsensusState not Final, + // and we receive a block N_B such that: + // + // N_B.PrevBlock == L_B.PrevBlock + // N_B.Iteration < L_B.Iteration + // + // Then we fallback to N_B.PrevBlock and accept N_B + acc.db.read().await.view(|t| { + if let Some((header, _)) = + t.fetch_block_header(&blk.header().prev_block_hash)? + { + let l_b_height = header.height + 1; + if let Some(Label::Final) = + t.fetch_block_label_by_height(l_b_height)? + { + // L_B is already final + return Ok(()); + } + + if let Some(l_b) = t.fetch_block_by_height(l_b_height)? { + if blk.header().iteration < l_b.header().iteration { + // TODO: Trigger fallback to l_b_height - 1 + } + } + } + + anyhow::Ok(()) + })?; + return Ok(None); } From fcb523af00c6dac5c8d22a9807b6e6cfd3bd4a9b Mon Sep 17 00:00:00 2001 From: goshawk-3 Date: Thu, 18 Jan 2024 10:40:28 +0200 Subject: [PATCH 2/5] node: Enable fallback for blocks different from the Tip --- node/src/chain/fallback.rs | 126 +++++++++++++++++++++---------------- 1 file changed, 73 insertions(+), 53 deletions(-) diff --git a/node/src/chain/fallback.rs b/node/src/chain/fallback.rs index 3609b64cbc..f8d72da706 100644 --- a/node/src/chain/fallback.rs +++ b/node/src/chain/fallback.rs @@ -5,7 +5,10 @@ // Copyright (c) DUSK NETWORK. All rights reserved. use anyhow::{anyhow, Result}; -use node_data::ledger::Block; +use dusk_consensus::user::provisioners::ContextProvisioners; +use node_data::ledger; +use node_data::ledger::Header; +use std::cmp::Ordering; use tracing::info; use crate::{ @@ -33,67 +36,84 @@ impl<'a, N: Network, DB: database::DB, VM: vm::VMExecution> Self { acc } } - pub(crate) async fn try_execute_fallback(&self, blk: &Block) -> Result<()> { - self.sanity_checks(blk).await?; + /// + pub(crate) async fn try_execute_fallback( + &self, + local: &Header, + remote: &Header, + ) -> Result<()> { + self.verify_header(local, remote).await?; self.acc.try_revert(RevertTarget::LastFinalizedState).await } - /// Performs a serias of checks to securely allow fallback execution. - async fn sanity_checks(&self, blk: &Block) -> Result<()> { - let acc = self.acc; - - let curr_height = acc.get_curr_height().await; - let curr_iteration = acc.get_curr_iteration().await; - - if curr_height < 1 { - return Err(anyhow!("cannot fallback over genesis block")); - } - - if blk.header().iteration > curr_iteration { - return Err(anyhow!("iteration is higher than current")); - } - - if blk.header().iteration == curr_iteration { - // This may happen only if: - // - // we have more than one winner blocks per a single iteration, same - // round. - - // An invalid block was received. - return Err(anyhow!("iteration is equal to the current")); - } + /// Verifies if a block of header `local` can be replaced with a block with + /// header `remote` + async fn verify_header( + &self, + local: &Header, + remote: &Header, + ) -> Result<()> { + match (local.height, remote.iteration.cmp(&local.iteration)) { + (0, _) => Err(anyhow!("cannot fallback over genesis block")), + (_, Ordering::Greater) => Err(anyhow!( + "iteration {:?} is higher than the current {:?}", + remote.iteration, + local.iteration + )), + (_, Ordering::Equal) => Err(anyhow!( + "iteration is equal to the current {:?}", + local.iteration + )), + _ => Ok(()), + }?; + + let (prev_header, prev_prev_header) = + self.acc.db.read().await.view(|t| { + let (prev_block_header, _) = t + .fetch_block_header(&local.prev_block_hash)? + .expect("block must exist"); + + let (prev_prev_block_header, _) = t + .fetch_block_header(&prev_block_header.prev_block_hash)? + .expect("block must exist"); + + Ok::<(ledger::Header, ledger::Header), anyhow::Error>(( + prev_block_header, + prev_prev_block_header, + )) + })?; info!( - event = "starting fallback", - height = curr_height, - iter = curr_iteration, - target_iter = blk.header().iteration, + event = "execute fallback checks", + height = local.height, + iter = local.iteration, + target_iter = remote.iteration, ); - let prev_block_height = curr_height - 1; - let prev_block = acc.db.read().await.view(|v| { - Ledger::fetch_block_by_height(&v, prev_block_height)? - .ok_or_else(|| anyhow::anyhow!("could not fetch block")) - })?; - - info!( - event = "fallback checking block", - height = curr_height, - iter = curr_iteration, - target_iter = blk.header().iteration, - ); - - // Validate Header/Certificate of the new block upon previous block and - // provisioners. - - // In an edge case, this may fail on performing fallback between two - // epochs. - let provisioners_list = acc.provisioners_list.read().await; - acceptor::verify_block_header( + let provisioners_list = self + .acc + .vm + .read() + .await + .get_provisioners(prev_header.state_hash)?; + + let prev_provisioners_list = self + .acc + .vm + .read() + .await + .get_provisioners(prev_prev_header.state_hash)?; + + let mut provisioners_list = ContextProvisioners::new(provisioners_list); + provisioners_list.set_previous(prev_provisioners_list); + + // Ensure header of the new block is valid according to prev_block + // header + let _ = acceptor::verify_block_header( self.acc.db.clone(), - prev_block.header(), + &prev_header, &provisioners_list, - blk.header(), + remote, ) .await?; From 30094319c4434b422151c3fdb41b1f282b1babc5 Mon Sep 17 00:00:00 2001 From: goshawk-3 Date: Thu, 18 Jan 2024 10:50:00 +0200 Subject: [PATCH 3/5] node: Run the fallback procedure initiated by block from previous height --- node/src/chain/acceptor.rs | 5 ++ node/src/chain/fsm.rs | 103 ++++++++++++++++++++++++------------- 2 files changed, 71 insertions(+), 37 deletions(-) diff --git a/node/src/chain/acceptor.rs b/node/src/chain/acceptor.rs index 9bf8d751e3..180b90e745 100644 --- a/node/src/chain/acceptor.rs +++ b/node/src/chain/acceptor.rs @@ -493,6 +493,11 @@ impl Acceptor { self.mrb.read().await.inner().header().height } + /// Returns chain tip header + pub(crate) async fn header(&self) -> ledger::Header { + self.mrb.read().await.inner().header().clone() + } + pub(crate) async fn get_curr_hash(&self) -> [u8; 32] { self.mrb.read().await.inner().header().hash } diff --git a/node/src/chain/fsm.rs b/node/src/chain/fsm.rs index 22f81d4c7b..5fc153dc36 100644 --- a/node/src/chain/fsm.rs +++ b/node/src/chain/fsm.rs @@ -264,27 +264,34 @@ impl InSyncImpl { async fn on_event( &mut self, - blk: &Block, + remote_blk: &Block, msg: &Message, ) -> anyhow::Result> { let mut acc = self.acc.write().await; - let height = blk.header().height; - let tip_height = acc.get_curr_height().await; - let iter = acc.get_curr_iteration().await; - let curr_hash = acc.get_curr_hash().await; + let local_header = acc.header().await; + let remote_height = remote_blk.header().height; - if height < tip_height { + if remote_height < local_header.height { + // Ensure that the block does not exist in the local state let exists = acc .db .read() .await - .view(|t| t.get_block_exists(&blk.header().hash))?; + .view(|t| t.get_block_exists(&remote_blk.header().hash))?; if exists { // Already exists in local state return Ok(None); } + // Ensure that the block height is higher than the last finalized + // TODO: Retrieve the block from memory + if remote_height + <= acc.get_latest_final_block().await?.header().height + { + return Ok(None); + } + // If our local chain has a block L_B with ConsensusState not Final, // and we receive a block N_B such that: // @@ -292,33 +299,51 @@ impl InSyncImpl { // N_B.Iteration < L_B.Iteration // // Then we fallback to N_B.PrevBlock and accept N_B - acc.db.read().await.view(|t| { - if let Some((header, _)) = - t.fetch_block_header(&blk.header().prev_block_hash)? + let header = acc.db.read().await.view(|t| { + if let Some((prev_header, _)) = + t.fetch_block_header(&remote_blk.header().prev_block_hash)? { - let l_b_height = header.height + 1; - if let Some(Label::Final) = - t.fetch_block_label_by_height(l_b_height)? - { - // L_B is already final - return Ok(()); - } - + let l_b_height = prev_header.height + 1; if let Some(l_b) = t.fetch_block_by_height(l_b_height)? { - if blk.header().iteration < l_b.header().iteration { - // TODO: Trigger fallback to l_b_height - 1 + if remote_blk.header().iteration + < l_b.header().iteration + { + return Ok(Some(l_b.header().clone())); } } } - anyhow::Ok(()) + anyhow::Ok(None) })?; + if let Some(header) = header { + match fallback::WithContext::new(acc.deref()) + .try_execute_fallback(&header, remote_blk.header()) + .await + { + Ok(_) => { + if remote_height == acc.get_curr_height().await + 1 { + acc.try_accept_block(remote_blk, true).await?; + return Ok(None); + } + } + Err(e) => { + error!( + event = "fallback failed", + height = local_header.height, + remote_height, + err = format!("{:?}", e) + ); + return Ok(None); + } + } + } + return Ok(None); } - if height == tip_height { - if blk.header().hash == curr_hash { + if remote_height == local_header.height { + if remote_blk.header().hash == local_header.hash { // Duplicated block. // Node has already accepted it. return Ok(None); @@ -326,13 +351,13 @@ impl InSyncImpl { info!( event = "entering fallback", - height = tip_height, - iter = iter, - new_iter = blk.header().iteration, + height = local_header.height, + iter = local_header.height, + new_iter = remote_blk.header().iteration, ); match fallback::WithContext::new(acc.deref()) - .try_execute_fallback(blk) + .try_execute_fallback(&local_header, remote_blk.header()) .await { Err(e) => { @@ -347,15 +372,18 @@ impl InSyncImpl { // Blacklist the old-block hash so that if it's again // sent then this node does not try to accept it. - self.blacklisted_blocks.write().await.insert(curr_hash); + self.blacklisted_blocks + .write() + .await + .insert(local_header.hash); - if height == acc.get_curr_height().await + 1 { + if remote_height == acc.get_curr_height().await + 1 { // If we have fallback-ed to previous block only, then // accepting the new block would be enough to continue // in in_Sync mode instead of switching to Out-Of-Sync // mode. - acc.try_accept_block(blk, true).await?; + acc.try_accept_block(remote_blk, true).await?; return Ok(None); } @@ -363,7 +391,7 @@ impl InSyncImpl { // sync-up procedure to download all missing blocks from the // main chain. if let Some(metadata) = &msg.metadata { - let res = (blk.clone(), metadata.src_addr); + let res = (remote_blk.clone(), metadata.src_addr); return Ok(Some(res)); } else { return Ok(None); @@ -373,8 +401,8 @@ impl InSyncImpl { } // Try accepting consecutive block - if height == tip_height + 1 { - let label = acc.try_accept_block(blk, true).await?; + if remote_height == local_header.height + 1 { + let label = acc.try_accept_block(remote_blk, true).await?; // On first final block accepted while we're inSync, clear // blacklisted blocks @@ -387,7 +415,7 @@ impl InSyncImpl { if let Some(metadata) = &msg.metadata { if let Some(presync) = &mut self.presync { if metadata.src_addr == presync.peer_addr - && height == presync.start_height() + 1 + && remote_height == presync.start_height() + 1 { let res = (presync.target_blk.clone(), presync.peer_addr); @@ -413,12 +441,13 @@ impl InSyncImpl { if self.presync.is_none() { self.presync = Some(PresyncInfo::new( metadata.src_addr, - blk.clone(), - tip_height, + remote_blk.clone(), + local_header.height, )); } - self.request_block(tip_height + 1, metadata.src_addr).await; + self.request_block(local_header.height + 1, metadata.src_addr) + .await; } Ok(None) From bc039f5d2825a561417f2641a1ccf6ad42d1a2b1 Mon Sep 17 00:00:00 2001 From: goshawk-3 Date: Thu, 18 Jan 2024 10:54:43 +0200 Subject: [PATCH 4/5] node: Make revert target explicit --- node/src/chain/fallback.rs | 8 +++++--- node/src/chain/fsm.rs | 14 +++++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/node/src/chain/fallback.rs b/node/src/chain/fallback.rs index f8d72da706..0da001c45a 100644 --- a/node/src/chain/fallback.rs +++ b/node/src/chain/fallback.rs @@ -36,14 +36,16 @@ impl<'a, N: Network, DB: database::DB, VM: vm::VMExecution> Self { acc } } - /// - pub(crate) async fn try_execute_fallback( + /// Makes an attempt to revert to the specified Target, if remote header is + /// fully valid + pub(crate) async fn try_revert( &self, local: &Header, remote: &Header, + revert_target: RevertTarget, ) -> Result<()> { self.verify_header(local, remote).await?; - self.acc.try_revert(RevertTarget::LastFinalizedState).await + self.acc.try_revert(revert_target).await } /// Verifies if a block of header `local` can be replaced with a block with diff --git a/node/src/chain/fsm.rs b/node/src/chain/fsm.rs index 5fc153dc36..8867f0c9f1 100644 --- a/node/src/chain/fsm.rs +++ b/node/src/chain/fsm.rs @@ -4,7 +4,7 @@ // // Copyright (c) DUSK NETWORK. All rights reserved. -use super::acceptor::Acceptor; +use super::acceptor::{Acceptor, RevertTarget}; use crate::chain::fallback; use crate::database; use crate::{vm, Network}; @@ -318,7 +318,11 @@ impl InSyncImpl { if let Some(header) = header { match fallback::WithContext::new(acc.deref()) - .try_execute_fallback(&header, remote_blk.header()) + .try_revert( + &header, + remote_blk.header(), + RevertTarget::LastFinalizedState, + ) .await { Ok(_) => { @@ -357,7 +361,11 @@ impl InSyncImpl { ); match fallback::WithContext::new(acc.deref()) - .try_execute_fallback(&local_header, remote_blk.header()) + .try_revert( + &local_header, + remote_blk.header(), + RevertTarget::LastFinalizedState, + ) .await { Err(e) => { From 172375c1696500791c502f16ed33f8005b289d21 Mon Sep 17 00:00:00 2001 From: goshawk-3 Date: Thu, 18 Jan 2024 13:45:07 +0200 Subject: [PATCH 5/5] node: Address PR comments --- node/src/chain/acceptor.rs | 15 +++++++-------- node/src/chain/fallback.rs | 6 +++--- node/src/chain/fsm.rs | 20 ++++++++++---------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/node/src/chain/acceptor.rs b/node/src/chain/acceptor.rs index 180b90e745..a3e1eb2a4c 100644 --- a/node/src/chain/acceptor.rs +++ b/node/src/chain/acceptor.rs @@ -494,7 +494,7 @@ impl Acceptor { } /// Returns chain tip header - pub(crate) async fn header(&self) -> ledger::Header { + pub(crate) async fn tip_header(&self) -> ledger::Header { self.mrb.read().await.inner().header().clone() } @@ -547,15 +547,14 @@ impl Acceptor { } } -/// Performs full verification of block header (blk_header) against -/// local/current state. +/// Performs full verification of block header against prev_block header where +/// prev_block is usually the blockchain tip pub(crate) async fn verify_block_header( db: Arc>, - mrb: &ledger::Header, + prev_block: &ledger::Header, provisioners: &ContextProvisioners, - candidate_header: &ledger::Header, + header: &ledger::Header, ) -> anyhow::Result { - let validator = Validator::new(db, mrb, provisioners); - - validator.execute_checks(candidate_header, false).await + let validator = Validator::new(db, prev_block, provisioners); + validator.execute_checks(header, false).await } diff --git a/node/src/chain/fallback.rs b/node/src/chain/fallback.rs index 0da001c45a..b4b3484924 100644 --- a/node/src/chain/fallback.rs +++ b/node/src/chain/fallback.rs @@ -48,8 +48,8 @@ impl<'a, N: Network, DB: database::DB, VM: vm::VMExecution> self.acc.try_revert(revert_target).await } - /// Verifies if a block of header `local` can be replaced with a block with - /// header `remote` + /// Verifies if a block with header `local` can be replaced with a block + /// with header `remote` async fn verify_header( &self, local: &Header, @@ -65,7 +65,7 @@ impl<'a, N: Network, DB: database::DB, VM: vm::VMExecution> (_, Ordering::Equal) => Err(anyhow!( "iteration is equal to the current {:?}", local.iteration - )), + )), // TODO: This may be a slashing condition _ => Ok(()), }?; diff --git a/node/src/chain/fsm.rs b/node/src/chain/fsm.rs index 8867f0c9f1..d566a3ba7c 100644 --- a/node/src/chain/fsm.rs +++ b/node/src/chain/fsm.rs @@ -268,7 +268,7 @@ impl InSyncImpl { msg: &Message, ) -> anyhow::Result> { let mut acc = self.acc.write().await; - let local_header = acc.header().await; + let local_header = acc.tip_header().await; let remote_height = remote_blk.header().height; if remote_height < local_header.height { @@ -293,18 +293,18 @@ impl InSyncImpl { } // If our local chain has a block L_B with ConsensusState not Final, - // and we receive a block N_B such that: + // and we receive a block R_B such that: // - // N_B.PrevBlock == L_B.PrevBlock - // N_B.Iteration < L_B.Iteration + // R_B.PrevBlock == L_B.PrevBlock + // R_B.Iteration < L_B.Iteration // // Then we fallback to N_B.PrevBlock and accept N_B - let header = acc.db.read().await.view(|t| { + let local_header = acc.db.read().await.view(|t| { if let Some((prev_header, _)) = t.fetch_block_header(&remote_blk.header().prev_block_hash)? { - let l_b_height = prev_header.height + 1; - if let Some(l_b) = t.fetch_block_by_height(l_b_height)? { + let local_height = prev_header.height + 1; + if let Some(l_b) = t.fetch_block_by_height(local_height)? { if remote_blk.header().iteration < l_b.header().iteration { @@ -316,10 +316,10 @@ impl InSyncImpl { anyhow::Ok(None) })?; - if let Some(header) = header { + if let Some(local_header) = local_header { match fallback::WithContext::new(acc.deref()) .try_revert( - &header, + &local_header, remote_blk.header(), RevertTarget::LastFinalizedState, ) @@ -356,7 +356,7 @@ impl InSyncImpl { info!( event = "entering fallback", height = local_header.height, - iter = local_header.height, + iter = local_header.iteration, new_iter = remote_blk.header().iteration, );