Send NewTip event on block invalidation and resetting failure flags

Author: ImplOfAnImpl

blockprod/src/detail/job_manager/mod.rs

@@ -289,7 +289,7 @@ impl JobManager {
                                         height: _,
                                         is_initial_block_download: _,
                                     } => {
-                                        _ = chainstate_sender.send(block_id.into()).log_err_pfx(
+                                        _ = chainstate_sender.send(block_id).log_err_pfx(
                                             "Chainstate subscriber failed to send new tip",
                                         );
                                     }

chainstate/src/detail/block_invalidation/mod.rs

@@ -328,32 +328,46 @@ impl<'a, S: BlockchainStorage, V: TransactionVerificationStrategy> BlockInvalida
 pub enum BlockInvalidatorError {
     #[error("Block storage error: {0}")]
     StorageError(#[from] chainstate_storage::Error),
+
     #[error("The block {0} is too deep to invalidate")]
     BlockTooDeepToInvalidate(Id<Block>),
+
     #[error("Error manipulating best chain candidates: {0}")]
     BestChainCandidatesError(#[from] BestChainCandidatesError),
+
     #[error("Error disconnecting blocks until block {disconnect_until}: {error}")]
     BlocksDisconnectionError {
         disconnect_until: Id<GenBlock>,
         error: Box<BlockError>,
     },
+
     #[error("Error updating block status for block {0}: {1}")]
     BlockStatusUpdateError(Id<Block>, Box<BlockError>),
+
     #[error("Generic error during reorg: {0}")]
     GenericReorgError(Box<BlockError>),
+
     #[error("Failed to commit to the DB after {0} attempts: {1}, context: {2}")]
     DbCommitError(usize, chainstate_storage::Error, DbCommittingContext),
 
     #[error("Failed to obtain best block index: {0}")]
     BlockIndicesForBranchQueryError(PropertyQueryError),
+
     #[error("Failed to determine if the block {0} is in mainchain: {1}")]
     IsBlockInMainChainQueryError(Id<GenBlock>, PropertyQueryError),
+
     #[error("Failed to obtain the minimum height with allowed reorgs: {0}")]
     MinHeightForReorgQueryError(PropertyQueryError),
+
     #[error("Failed to obtain best block index: {0}")]
     BestBlockIndexQueryError(PropertyQueryError),
+
+    #[error("Failed to obtain best block id: {0}")]
+    BestBlockIdQueryError(PropertyQueryError),
+
     #[error("Failed to obtain block index for block {0}: {1}")]
     BlockIndexQueryError(Id<GenBlock>, PropertyQueryError),
+
     #[error("Error deleting index for block {0}: {1}")]
     DelBlockIndexError(Id<Block>, BlockError),
 }

chainstate/src/detail/mod.rs

@@ -270,7 +270,7 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
 
     fn broadcast_new_tip_event(
         &mut self,
-        best_block_index: &BlockIndex,
+        best_block_index: GenBlockIndexRef<'_>,
         is_initial_block_download: bool,
     ) {
         let event = ChainstateEvent::NewTip {
@@ -623,7 +623,10 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
 
         if let Some(best_block_index) = &best_block_index_opt {
             self.update_initial_block_download_flag(GenBlockIndexRef::Block(best_block_index));
-            self.broadcast_new_tip_event(best_block_index, self.is_initial_block_download());
+            self.broadcast_new_tip_event(
+                GenBlockIndexRef::Block(best_block_index),
+                self.is_initial_block_download(),
+            );
 
             let compact_target = match best_block_index.block_header().consensus_data() {
                 ConsensusData::None => Compact::from(Uint256::ZERO),
@@ -707,15 +710,60 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
 
     #[log_error]
     pub fn invalidate_block(&mut self, block_id: &Id<Block>) -> Result<(), BlockInvalidatorError> {
+        let prev_best_block_id = self
+            .make_db_tx_ro()?
+            .get_best_block_id()
+            .map_err(BlockInvalidatorError::BestBlockIdQueryError)?;
+
         let result = BlockInvalidator::new(self)
             .invalidate_block(block_id, block_invalidation::IsExplicit::Yes);
         // Note: we don't ignore the result of check_consistency even though we may already have
         // an error to return (if the checks are enabled but couldn't be done for some reason,
         // we don't want to miss this).
         self.check_consistency()?;
+
+        let new_best_block_index = self
+            .make_db_tx_ro()?
+            .get_best_block_index()
+            .map_err(BlockInvalidatorError::BestBlockIndexQueryError)?;
+
+        if new_best_block_index.block_id() != prev_best_block_id {
+            self.broadcast_new_tip_event(
+                new_best_block_index.as_ref(),
+                self.is_initial_block_download(),
+            );
+        }
+
         result
     }
 
+    #[log_error]
+    pub fn reset_block_failure_flags(
+        &mut self,
+        block_id: &Id<Block>,
+    ) -> Result<(), BlockInvalidatorError> {
+        let prev_best_block_id = self
+            .make_db_tx_ro()?
+            .get_best_block_id()
+            .map_err(BlockInvalidatorError::BestBlockIdQueryError)?;
+
+        BlockInvalidator::new(self).reset_block_failure_flags(block_id)?;
+
+        let new_best_block_index = self
+            .make_db_tx_ro()?
+            .get_best_block_index()
+            .map_err(BlockInvalidatorError::BestBlockIndexQueryError)?;
+
+        if new_best_block_index.block_id() != prev_best_block_id {
+            self.broadcast_new_tip_event(
+                new_best_block_index.as_ref(),
+                self.is_initial_block_download(),
+            );
+        }
+
+        Ok(())
+    }
+
     #[log_error]
     fn create_pool_in_storage(
         &self,

chainstate/src/interface/chainstate_interface_impl.rs

@@ -23,7 +23,6 @@ use crate::{
     detail::{
         self,
         block_checking::BlockChecker,
-        block_invalidation::BlockInvalidator,
         bootstrap::{export_bootstrap_stream, import_bootstrap_stream},
         calculate_median_time_past,
         tx_verification_strategy::TransactionVerificationStrategy,
@@ -108,7 +107,7 @@ where
 
     #[tracing::instrument(skip_all, fields(id = %block_id))]
     fn reset_block_failure_flags(&mut self, block_id: &Id<Block>) -> Result<(), ChainstateError> {
-        BlockInvalidator::new(&mut self.chainstate)
+        self.chainstate
             .reset_block_failure_flags(block_id)
             .map_err(ChainstateError::BlockInvalidatorError)
     }

chainstate/src/lib.rs

@@ -52,7 +52,7 @@ pub use tx_verifier;
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum ChainstateEvent {
     NewTip {
-        id: Id<Block>,
+        id: Id<GenBlock>,
         height: BlockHeight,
         is_initial_block_download: bool,
     },

chainstate/src/rpc/types/event.rs

@@ -14,7 +14,7 @@
 // limitations under the License.
 
 use common::{
-    chain::Block,
+    chain::GenBlock,
     primitives::{BlockHeight, Id},
 };
 
@@ -23,7 +23,10 @@ use crate::ChainstateEvent;
 #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, rpc::description::HasValueHint)]
 #[serde(tag = "type", content = "content")]
 pub enum RpcEvent {
-    NewTip { id: Id<Block>, height: BlockHeight },
+    NewTip {
+        id: Id<GenBlock>,
+        height: BlockHeight,
+    },
 }
 
 impl RpcEvent {