Remove cdbFutureBlocks

docs/website/contents/for-developers/HandlingBlocksFromTheFuture.md

@@ -0,0 +1,44 @@
+#  Blocks from the future
+
+A node can receive a block whose slot is ahead of the current slot. We call such **blocks from the future**.
+
+The Praos protocol ignores chains with blocks from the future during chain selection.
+It assumes nodes have perfectly synchronized clocks, which is not realistic due to imperfections in protocols like NTP and leap seconds.
+In practice, a clock skew of up to 2 seconds is considered acceptable.
+Our implementation differentiates between blocks from the near future and those from the far future:
+
+- A block is from the near future if the onset of its slot is ahead of the wall clock, but only by at most the admissible clock skew. Despite being from the future, these blocks are assumed to potentially have been minted by honest nodes.
+- A block is from the far future if the onset of its slot is ahead of the wall clock by more than the admissible clock skew.  By assumption, these blocks cannot have been minted by an honest node.
+
+# Handling blocks from the future
+
+As of [#525](https://github.com/IntersectMBO/ouroboros-consensus/pull/525):
+
+- When receiving a header from the **near** future in `ChainSync`, an artificial delay is introduced until the header is no longer from the future.
+Only then it is validated and the corresponding block body is downloaded and added to the `ChainDB` for chain selection, where it is not considered to be from the future due to the previous artificial delay.
+- When receiving a header from the far future, we immediately disconnect from the corresponding peer.
+
+In addition, we never forge atop a block from the future (which was the case even before [#525](https://github.com/IntersectMBO/ouroboros-consensus/pull/525).
+
+### During initialization
+
+Since we now delay the headers until they are no longer from the **near future**, a caught up node will never contain blocks from the future in the `VolatileDB`, according to its own clock.
+However, there are two caveats:
+- Clock rewinds can violate this property. In particular the node [will error](https://github.com/IntersectMBO/ouroboros-consensus/blob/4488656439e78c572c3dce0f7ed2cf98f61c65bb/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/BlockchainTime/WallClock/HardFork.hs#L138-L146) when we rewind the clock by more than [20 seconds](https://github.com/IntersectMBO/ouroboros-consensus/blob/4488656439e78c572c3dce0f7ed2cf98f61c65bb/ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node.hs#L485).
+- The node clock might be set in the future relative to the rest of the nodes in the network.
+Thus, it is possible that after restarting a node with a clock set in the future, and setting the clock back so that the clock is now synchronized with the rest of the network, the blocks in the `VolatileDB` are regarded as blocks from the future.
+
+When initializing the `ChainDB` we do not check if blocks in the `VolatileDB` are from the future. This presents two inconveniences:
+
+- When the node diffuses these blocks from the **far** future, it will be disconnected from other peers.
+- The node [will not forge](https://github.com/IntersectMBO/ouroboros-consensus/blob/16fa8754be24f26eddef006c03ba945ea00e3566/ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/NodeKernel.hs#L708) a block on top of a block from the future, thus missing its chance to lead in the slot.
+
+These problems can be solved by wiping out the `VolatileDB` in this situation.
+However, note this is an extremely rare situation: the clock of the node would have to have been set quite far in the future, as shutting down a node and restarting it already takes a significant amount of time.
+
+In the future we might delete blocks from the future from the `VolatileDB` to improve the user experience and robustness of the initialization logic. For now it does not seem worthwhile to handle that rare case. (Downstream/bidirectional peers will disconnect from such a node, but only until enough time has passed that its `VolatileDB` does not contain blocks from the future anymore.)
+
+# References
+
+- [Original issue that prompted the fix](https://github.com/IntersectMBO/ouroboros-network/issues/4251)
+- [Blocks from the future (Incident report)](https://updates.cardano.intersectmbo.org/2024-09-07-incident)

docs/website/sidebars.js

@@ -50,6 +50,7 @@ const sidebars = {
         'for-developers/TechnicalReports',
         'for-developers/PreflightGuide',
         'for-developers/NodeTasks',
+        'for-developers/HandlingBlocksFromTheFuture'
       ]
     },
   ]

ouroboros-consensus-cardano/changelog.d/20240927_125151_damian.nadales_1260_remove_cdbFutureBlocks.md

@@ -0,0 +1,8 @@
+### Breaking
+
+- Remove `CheckInFuture m blk` from `openChainDB`.
+
+### Non-Breaking
+
+- Remove references to `Ouroboros.Consensus.Fragment.InFuture`.
+- Adapt the code to account for the removed `cdbFutureBlocks` and related fields.

ouroboros-consensus-cardano/src/unstable-cardano-tools/Cardano/Tools/DBAnalyser/Run.hs

@@ -18,7 +18,6 @@ import           Data.Singletons (Sing, SingI (..))
 import qualified Debug.Trace as Debug
 import           Ouroboros.Consensus.Block
 import           Ouroboros.Consensus.Config
-import qualified Ouroboros.Consensus.Fragment.InFuture as InFuture
 import           Ouroboros.Consensus.Ledger.Extended
 import qualified Ouroboros.Consensus.Ledger.SupportsMempool as LedgerSupportsMempool
                      (HasTxs)
@@ -65,7 +64,6 @@ analyse DBAnalyserConfig{analysis, confLimit, dbDir, selectDB, validation, verbo
               $ updateTracer chainDBTracer
               $ completeChainDbArgs
                   registry
-                  InFuture.dontCheck
                   cfg
                   genesisLedger
                   chunkInfo

ouroboros-consensus-cardano/src/unstable-cardano-tools/Cardano/Tools/DBSynthesizer/Run.hs

@@ -26,7 +26,6 @@ import qualified Data.Set as Set
 import           Ouroboros.Consensus.Cardano.Block
 import           Ouroboros.Consensus.Cardano.Node
 import           Ouroboros.Consensus.Config (TopLevelConfig, configStorage)
-import qualified Ouroboros.Consensus.Fragment.InFuture as InFuture (dontCheck)
 import qualified Ouroboros.Consensus.Node as Node (stdMkChainDbHasFS)
 import qualified Ouroboros.Consensus.Node.InitStorage as Node
                      (nodeImmutableDbChunkInfo)
@@ -127,7 +126,6 @@ synthesize genTxs DBSynthesizerConfig{confOptions, confShelleyGenesis, confDbDir
             dbArgs      =
              ChainDB.completeChainDbArgs
               registry
-              InFuture.dontCheck
               pInfoConfig
               pInfoInitLedger
               chunkInfo

ouroboros-consensus-diffusion/changelog.d/20240927_131047_damian.nadales_1260_remove_cdbFutureBlocks.md

@@ -0,0 +1,7 @@
+### Breaking
+
+- Remove `CheckInFuture m blk` argument from `openChainDB`.
+
+### Patch
+
+- Remove references to `Ouroboros.Consensus.Fragment.InFuture`.

ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node.hs

@@ -79,9 +79,6 @@ import           Ouroboros.Consensus.Block
 import           Ouroboros.Consensus.BlockchainTime hiding (getSystemStart)
 import           Ouroboros.Consensus.Config
 import           Ouroboros.Consensus.Config.SupportsNode
-import           Ouroboros.Consensus.Fragment.InFuture (CheckInFuture,
-                     ClockSkew)
-import qualified Ouroboros.Consensus.Fragment.InFuture as InFuture
 import           Ouroboros.Consensus.Ledger.Extended (ExtLedgerState (..))
 import           Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck
                      (HistoricityCheck)
@@ -289,7 +286,7 @@ data LowLevelRunNodeArgs m addrNTN addrNTC versionDataNTN versionDataNTC blk
     , llrnMaxCaughtUpAge :: NominalDiffTime
 
       -- | Maximum clock skew
-    , llrnMaxClockSkew :: ClockSkew
+    , llrnMaxClockSkew :: InFutureCheck.ClockSkew
 
     , llrnPublicPeerSelectionStateVar :: StrictSTM.StrictTVar m (Diffusion.PublicPeerSelectionState addrNTN)
     }
@@ -436,12 +433,6 @@ runWith RunNodeArgs{..} encAddrNtN decAddrNtN LowLevelRunNodeArgs{..} =
                            systemStart
                            (blockchainTimeTracer rnTraceConsensus)
 
-            inFuture :: CheckInFuture m blk
-            inFuture = InFuture.reference
-                         (configLedger cfg)
-                         llrnMaxClockSkew
-                         systemTime
-
         (genesisArgs, setLoEinChainDbArgs) <-
           mkGenesisNodeKernelArgs llrnGenesisConfig
 
@@ -458,7 +449,6 @@ runWith RunNodeArgs{..} encAddrNtN decAddrNtN LowLevelRunNodeArgs{..} =
 
         (chainDB, finalArgs) <- openChainDB
                      registry
-                     inFuture
                      cfg
                      initLedger
                      llrnMkImmutableHasFS
@@ -719,7 +709,6 @@ stdWithCheckedDB pb tracer databasePath networkMagic body = do
 openChainDB ::
      forall m blk. (RunNode blk, IOLike m)
   => ResourceRegistry m
-  -> CheckInFuture m blk
   -> TopLevelConfig blk
   -> ExtLedgerState blk
      -- ^ Initial ledger
@@ -732,10 +721,9 @@ openChainDB ::
   -> (Complete ChainDbArgs m blk -> Complete ChainDbArgs m blk)
       -- ^ Customise the 'ChainDbArgs'
   -> m (ChainDB m blk, Complete ChainDbArgs m blk)
-openChainDB registry inFuture cfg initLedger fsImm fsVol defArgs customiseArgs =
+openChainDB registry cfg initLedger fsImm fsVol defArgs customiseArgs =
    let args = customiseArgs $ ChainDB.completeChainDbArgs
                registry
-               inFuture
                cfg
                initLedger
                (nodeImmutableDbChunkInfo (configStorage cfg))
@@ -964,7 +952,7 @@ stdLowLevelRunNodeArgsIO RunNodeArgs{ rnProtocolInfo
           stdWithCheckedDB (Proxy @blk) srnTraceChainDB (immutableDbPath srnDatabasePath) networkMagic
       , llrnMaxCaughtUpAge = secondsToNominalDiffTime $ 20 * 60   -- 20 min
       , llrnMaxClockSkew =
-          InFuture.defaultClockSkew
+          InFutureCheck.defaultClockSkew
       , llrnPublicPeerSelectionStateVar =
           Diffusion.daPublicPeerSelectionVar srnDiffusionArguments
       }

ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/ErrorPolicy.hs

@@ -70,12 +70,7 @@ consensusErrorPolicy pb = ErrorPolicies {
               ImmutableDB.UnexpectedFailure{} -> Just shutdownNode
         , ErrorPolicy $ \(_ :: FsError) -> Just shutdownNode
 
-          -- When the system clock moved back, we have to restart the node,
-          -- because the ImmutableDB validation might have to truncate some
-          -- blocks from the future. Note that a full validation is not
-          -- required, as the default validation (most recent epoch) will keep
-          -- on truncating epochs until a block that is not from the future is
-          -- found.
+          -- When the system clock moved back, we have to restart the node.
         , ErrorPolicy $ \(_ :: SystemClockMovedBackException) -> Just shutdownNode
 
           -- Some chain DB errors are indicative of a bug in our code, others

ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/RethrowPolicy.hs

@@ -64,12 +64,7 @@ consensusRethrowPolicy pb =
             ImmutableDB.UnexpectedFailure{} -> shutdownNode)
    <> mkRethrowPolicy (\_ctx (_ :: FsError) -> shutdownNode)
 
-      -- When the system clock moved back, we have to restart the node,
-      -- because the ImmutableDB validation might have to truncate some
-      -- blocks from the future. Note that a full validation is not
-      -- required, as the default validation (most recent epoch) will keep
-      -- on truncating epochs until a block that is not from the future is
-      -- found.
+      -- When the system clock moved back, we have to restart the node.
     <> mkRethrowPolicy (\_ctx (_ :: SystemClockMovedBackException) -> shutdownNode)
 
        -- Some chain DB errors are indicative of a bug in our code, others

ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/Tracers.hs

@@ -24,13 +24,14 @@ import           Ouroboros.Consensus.Block
 import           Ouroboros.Consensus.BlockchainTime
 import           Ouroboros.Consensus.Forecast (OutsideForecastRange)
 import           Ouroboros.Consensus.Genesis.Governor (TraceGDDEvent)
+import           Ouroboros.Consensus.Ledger.Extended (ExtValidationError)
 import           Ouroboros.Consensus.Ledger.SupportsMempool
 import           Ouroboros.Consensus.Ledger.SupportsProtocol
 import           Ouroboros.Consensus.Mempool (MempoolSize, TraceEventMempool)
 import           Ouroboros.Consensus.MiniProtocol.BlockFetch.Server
                      (TraceBlockFetchServerEvent)
 import           Ouroboros.Consensus.MiniProtocol.ChainSync.Client
-                     (InvalidBlockReason, TraceChainSyncClientEvent)
+                     (TraceChainSyncClientEvent)
 import           Ouroboros.Consensus.MiniProtocol.ChainSync.Server
                      (TraceChainSyncServerEvent)
 import           Ouroboros.Consensus.MiniProtocol.LocalTxSubmission.Server
@@ -343,7 +344,7 @@ data TraceForgeEvent blk
     -- | We forged a block that is invalid according to the ledger in the
     -- ChainDB. This means there is an inconsistency between the mempool
     -- validation and the ledger validation. This is a serious error!
-  | TraceForgedInvalidBlock SlotNo blk (InvalidBlockReason blk)
+  | TraceForgedInvalidBlock SlotNo blk (ExtValidationError blk)
 
     -- | We adopted the block we produced, we also trace the transactions
     -- that were adopted.

ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/NodeKernel.hs

@@ -693,10 +693,14 @@ mkCurrentBlockContext currentSlot c = case c of
       LT -> Right $ blockContextFromPrevHeader hdr
 
       -- The block at the tip of our chain has a slot that lies in the
-      -- future. Although the chain DB does not adopt future blocks, if the
+      -- future. Although the chain DB should not contain blocks from the
+      -- future, if the volatile DB contained such blocks on startup
+      -- (due to a node clock misconfiguration) this invariant may be
+      -- violated. See: https://github.com/IntersectMBO/ouroboros-consensus/blob/main/docs/website/contents/for-developers/HandlingBlocksFromTheFuture.md#handling-blocks-from-the-future
+      -- Also note that if the
       -- system is under heavy load, it is possible (though unlikely) that
       -- one or more slots have passed after @currentSlot@ that we got from
-      -- @onSlotChange@ and and before we queried the chain DB for the block
+      -- @onSlotChange@ and before we queried the chain DB for the block
       -- at its tip. At the moment, we simply don't produce a block if this
       -- happens.
 

ouroboros-consensus-diffusion/src/unstable-diffusion-testlib/Test/ThreadNet/Network.hs

@@ -65,7 +65,6 @@ import qualified Network.TypedProtocol.Codec as Codec
 import           Ouroboros.Consensus.Block
 import           Ouroboros.Consensus.BlockchainTime
 import           Ouroboros.Consensus.Config
-import qualified Ouroboros.Consensus.Fragment.InFuture as InFuture
 import           Ouroboros.Consensus.Ledger.Abstract
 import           Ouroboros.Consensus.Ledger.Extended
 import           Ouroboros.Consensus.Ledger.Inspect
@@ -699,8 +698,7 @@ runThreadNetwork systemTime ThreadNetworkArgs
 
         void $ addTxs mempool txs
 
-    mkArgs :: OracularClock m
-           -> ResourceRegistry m
+    mkArgs :: ResourceRegistry m
            -> TopLevelConfig blk
            -> ExtLedgerState blk
            -> Tracer m (RealPoint blk, ExtValidationError blk)
@@ -716,7 +714,7 @@ runThreadNetwork systemTime ThreadNetworkArgs
            -> CoreNodeId
            -> ChainDbArgs Identity m blk
     mkArgs
-      clock registry
+      registry
       cfg initLedger
       invalidTracer addTracer selTracer updatesTracer pipeliningTracer
       nodeDBs _coreNodeId =
@@ -740,11 +738,8 @@ runThreadNetwork systemTime ThreadNetworkArgs
                       LedgerDB.lgrTracer = TraceSnapshotEvent >$< tr
                     }
                 , cdbsArgs = (cdbsArgs args) {
-                      cdbsCheckInFuture  = InFuture.reference (configLedger cfg)
-                                        InFuture.defaultClockSkew
-                                        (OracularClock.finiteSystemTime clock)
-                      -- TODO: Vary cdbsGcDelay, cdbsGcInterval, cdbsBlockToAddSize
-                    , cdbsGcDelay = 0
+                     -- TODO: Vary cdbsGcDelay, cdbsGcInterval, cdbsBlockToAddSize
+                      cdbsGcDelay = 0
                     , cdbsTracer = instrumentationTracer <> nullDebugTracer
                     }
                 }
@@ -826,7 +821,7 @@ runThreadNetwork systemTime ThreadNetworkArgs
           headerAddTracer  = wrapTracer $ nodeEventsHeaderAdds nodeInfoEvents
           pipeliningTracer = nodeEventsPipelining nodeInfoEvents
       let chainDbArgs = mkArgs
-            clock registry
+            registry
             pInfoConfig pInfoInitLedger
             invalidTracer
             addTracer
@@ -1004,7 +999,7 @@ runThreadNetwork systemTime ThreadNetworkArgs
             , initChainDB             = nodeInitChainDB
             , chainSyncFutureCheck    =
                   InFutureCheck.realHeaderInFutureCheck
-                    InFuture.defaultClockSkew
+                    InFutureCheck.defaultClockSkew
                     (OracularClock.finiteSystemTime clock)
             , chainSyncHistoricityCheck = \_getGsmState -> HistoricityCheck.noCheck
             , blockFetchSize          = estimateBlockSize

ouroboros-consensus/bench/ChainSync-client-bench/Main.hs

@@ -17,13 +17,14 @@ import           Network.TypedProtocol.Driver.Simple
 import           Ouroboros.Consensus.Block
 import           Ouroboros.Consensus.BlockchainTime
 import           Ouroboros.Consensus.Config
-import           Ouroboros.Consensus.Fragment.InFuture (clockSkewInSeconds)
 import qualified Ouroboros.Consensus.HardFork.History as HardFork
 import qualified Ouroboros.Consensus.HeaderStateHistory as HeaderStateHistory
 import qualified Ouroboros.Consensus.HeaderValidation as HV
 import qualified Ouroboros.Consensus.Ledger.Extended as Extended
 import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client as CSClient
 import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck as HistoricityCheck
+import           Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck
+                     (clockSkewInSeconds)
 import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck as InFutureCheck
 import           Ouroboros.Consensus.MiniProtocol.ChainSync.Server
                      (chainSyncServerForFollower)
@@ -158,7 +159,7 @@ oneBenchRun
 -- | No invalid blocks in this benchmark
 invalidBlock ::
     WithFingerprint
-        (HeaderHash blk -> Maybe (ChainDB.InvalidBlockReason blk))
+        (HeaderHash blk -> Maybe (Extended.ExtValidationError blk))
 invalidBlock =
     WithFingerprint isInvalidBlock fp
   where

ouroboros-consensus/changelog.d/20240927_125606_damian.nadales_1260_remove_cdbFutureBlocks.md

@@ -0,0 +1,11 @@
+### Breaking
+
+- Remove `cdbFutureBlocks` from `ChainDbEnv`.
+- Remove `BlockInTheFuture`, `ChainSelectionForFutureBlock`, `CandidateContainsFutureBlocks`, and `CandidateContainsFutureBlocksExceedingClockSkew` from `TraceAddBlockEvent`.
+- Remove `cdbCheckInFuture` from `CBD`.
+- Remove `cdbsCheckInFuture` from `ChainDbSpecificArgs`.
+- Remove `CheckInFuture m blk` argument from `completeChainDbArgs`.
+- Remove `CheckInFuture m blk` argument from `initialChainSelection`.
+- Remove `cdbsCheckInFuture` from `ChainDbSpecificArgs`.
+- Delete module `Ouroboros.Consensus.Fragment.InFuture`. `ClockSkew` functions live now in `Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck`.
+* Remove ``InvalidBlockReason`, since it was now simply wrapping up `ExtValidationError`.

ouroboros-consensus/ouroboros-consensus.cabal

@@ -90,7 +90,6 @@ library
     Ouroboros.Consensus.Config.SupportsNode
     Ouroboros.Consensus.Forecast
     Ouroboros.Consensus.Fragment.Diff
-    Ouroboros.Consensus.Fragment.InFuture
     Ouroboros.Consensus.Fragment.Validated
     Ouroboros.Consensus.Fragment.ValidatedDiff
     Ouroboros.Consensus.Genesis.Governor