Update SelectChain documentation

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<B: Block, Be: Backend<B>> TendermintSelectChain<B, Be> {
 #[async_trait]
 impl<B: Block, Be: Backend<B>> SelectChain<B> for TendermintSelectChain<B, Be> {
   async fn leaves(&self) -> Result<Vec<B::Hash>, Error> {
-    panic!("should never be called")
+    panic!("Substrate definition of leaves is incompatible with Tendermint")
-
-    // 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
   }
 
   async fn best_chain(&self) -> Result<B::Header, Error> {
@@ -47,7 +49,9 @@ impl<B: Block, Be: Backend<B>> SelectChain<B> for TendermintSelectChain<B, Be> {
       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(),
     )