From 33e52ae79a8b3a4c46155c583db531528322df96 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sun, 13 Nov 2022 19:45:16 -0500 Subject: [PATCH] Always produce notifications for finalized blocks via origin overrides --- .../tendermint/client/src/authority/mod.rs | 5 +-- .../tendermint/client/src/block_import.rs | 40 ++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/substrate/tendermint/client/src/authority/mod.rs b/substrate/tendermint/client/src/authority/mod.rs index 8490b2f1..0ea6be9a 100644 --- a/substrate/tendermint/client/src/authority/mod.rs +++ b/substrate/tendermint/client/src/authority/mod.rs @@ -310,10 +310,7 @@ impl Network for TendermintAuthority { *self.import.importing_block.write().unwrap() = Some(hash); queue_write.as_mut().unwrap().import_blocks( - // We do not want this block, which hasn't been confirmed, to be broadcast over the net - // Substrate will generate notifications unless it's Genesis, which this isn't, InitialSync, - // which changes telemtry behavior, or File, which is... close enough - BlockOrigin::File, + BlockOrigin::ConsensusBroadcast, // TODO: Use BlockOrigin::Own when it's our block vec![IncomingBlock { hash, header: Some(header), diff --git a/substrate/tendermint/client/src/block_import.rs b/substrate/tendermint/client/src/block_import.rs index dc285e66..2b439cb4 100644 --- a/substrate/tendermint/client/src/block_import.rs +++ b/substrate/tendermint/client/src/block_import.rs @@ -5,7 +5,7 @@ use async_trait::async_trait; use sp_api::BlockId; use sp_runtime::traits::{Header, Block}; use sp_blockchain::{BlockStatus, HeaderBackend, Backend as BlockchainBackend}; -use sp_consensus::{Error, CacheKeyId, SelectChain}; +use sp_consensus::{Error, CacheKeyId, BlockOrigin, SelectChain}; use sc_consensus::{BlockCheckParams, BlockImportParams, ImportResult, BlockImport, Verifier}; @@ -86,6 +86,44 @@ where &mut self, mut block: BlockImportParams, ) -> Result<(BlockImportParams, Option)>>), String> { + block.origin = match block.origin { + BlockOrigin::Genesis => BlockOrigin::Genesis, + BlockOrigin::NetworkBroadcast => BlockOrigin::NetworkBroadcast, + + // Re-map NetworkInitialSync to NetworkBroadcast so it still triggers notifications + // Tendermint will listen to the finality stream. If we sync a block we're running a machine + // for, it'll force the machine to move ahead. We can only do that if there actually are + // notifications + // + // Then Serai also runs data indexing code based on block addition, so ensuring it always + // emits events ensures we always perform our necessary indexing (albeit with a race + // condition since Substrate will eventually prune the block's state, potentially before + // indexing finishes when syncing) + // + // The alternative to this would be editing Substrate directly, which would be a lot less + // fragile, manually triggering the notifications (which may be possible with code intended + // for testing), writing our own notification system, or implementing lock_import_and_run + // on our end, letting us directly set the notifications, so we're not beholden to when + // Substrate decides to call notify_finalized + // + // TODO: Call lock_import_and_run on our end, which already may be needed for safety reasons + BlockOrigin::NetworkInitialSync => BlockOrigin::NetworkBroadcast, + // Also re-map File so bootstraps also trigger notifications, enabling safely using + // bootstraps + BlockOrigin::File => BlockOrigin::NetworkBroadcast, + + // We do not want this block, which hasn't been confirmed, to be broadcast over the net + // Substrate will generate notifications unless it's Genesis, which this isn't, InitialSync, + // which changes telemetry behavior, or File, which is... close enough + // + // Even if we do manually implement lock_import_and_run, Substrate will still override + // our notifications if it believes it should provide notifications. That means we *still* + // have to keep this patch, with all its fragility, unless we edit Substrate or move the + // the entire block import flow under Serai + BlockOrigin::ConsensusBroadcast => BlockOrigin::File, + BlockOrigin::Own => BlockOrigin::File, + }; + if self.check_already_in_chain(block.header.hash()) { return Ok((block, None)); }