diff --git a/substrate/consensus/src/select_chain.rs b/substrate/consensus/src/select_chain.rs index e10910e2..32a409ea 100644 --- a/substrate/consensus/src/select_chain.rs +++ b/substrate/consensus/src/select_chain.rs @@ -1,3 +1,19 @@ +// SelectChain, while provided by Substrate and part of PartialComponents, isn't used by Substrate +// It's common between various block-production/finality crates, yet Substrate as a system doesn't +// rely on it, which is good, because its definition is explicitly incompatible with Tendermint +// +// leaves is supposed to return all leaves of the blockchain. While Tendermint maintains that view, +// an honest node will only build on the most recently finalized block, so it is a 'leaf' despite +// having descendants +// +// best_chain will always be this finalized block, yet Substrate explicitly defines it as one of +// the above leaves, which this finalized block is explicitly not included in. Accordingly, we +// can never provide a compatible decision +// +// Since PartialComponents expects it, an implementation which does its best is provided. It panics +// if leaves is called, yet returns the finalized chain tip for best_chain, as that's intended to +// be the header to build upon + use std::{marker::PhantomData, sync::Arc}; use async_trait::async_trait; @@ -25,21 +41,7 @@ impl> TendermintSelectChain { #[async_trait] impl> SelectChain for TendermintSelectChain { async fn leaves(&self) -> Result, Error> { - panic!("should never be called") - - // Substrate may call this at some point in the future? - // It doesn't appear to do so now, and we have the question of what to do if/when it does - // Either we return the chain tip, which is the true leaf yet breaks the documented definition, - // or we return the actual leaves, when those don't contribute value - // - // Both become risky as best_chain, which is presumably used for block building, is explicitly - // defined as one of these leaves. If it's returning the best chain, the finalized chain tip, - // then it's wrong. The real comment is that this API does not support the Tendermint model - // - // Since it appears the blockchain operations happen on the Backend's leaves, not the - // SelectChain's, leaving this as a panic for now should be optimal - // - // TODO: Triple check this isn't reachable + panic!("Substrate definition of leaves is incompatible with Tendermint") } async fn best_chain(&self) -> Result { @@ -47,7 +49,9 @@ impl> SelectChain for TendermintSelectChain { self .0 .blockchain() + // There should always be a finalized block .header(BlockId::Hash(self.0.blockchain().last_finalized().unwrap())) + // There should not be an error in retrieving it and since it's finalized, it should exist .unwrap() .unwrap(), )