Fix clean state initialization

[jasagredo]

Due to the unmasking of exceptions during ChainDB initialization, it is the case right now that the exit code is different from DatabaseCorruption and therefore the withCheckedDb function considers that it has to create the clean file. Because of this, a Ctrl+C during initialization would skip the validation on the next startup.

This PR ensures that the clean file is only removed after the chainDB is initialized (and therefore potentially modified) and from that point on it can be created again.

initial status	killed when	on SIGKILL	on DatabaseCorruption	on other exception (e.g. ^C)
clean	during initialization	unclean → clean	clean → unclean	clean
clean	after initialization	unclean	unclean	clean
unclean	during initialization	unclean	unclean	clean → unclean
unclean	after initialization	unclean	unclean	clean

[dnadales]

LGTM, but it'd like to go through this over a call.

[nfrisby]

I agree that this does what we want, but I think there's a more suitable implementation; one that doesn't use a mutvar "unnecessarily".

I'm thinking something like:

llrnWithCheckedDB :: forall a. (LastShutDownWasClean -> m (a, ThisShutDownWasClean)) -> m a

Or maybe something like

llrnWithCheckedDB :: forall a. (LastShutDownWasClean -> (forall b. m b -> m b) -> m a) -> m a

Where the passes wrapper is applied to the continuation of initialization, and it's the part that removes the marker (if present) and installs the handler that writes it.

Let's chat about it.

[nfrisby]

I amended your commit with a tiny change, just renaming wrapper:

$ git show
commit ec01f675b0fe647b04e55d6c246d7bfe60bbc18b (HEAD -> jasagredo/fix-clean-state-initialization)
Author: Nicolas Frisby <nick.frisby@iohk.io>
Date:   Thu Dec 2 10:13:48 2021 -0800

    TOSQUASH: rename wrapper

diff --git a/ouroboros-consensus/src/Ouroboros/Consensus/Node.hs b/ouroboros-consensus/src/Ouroboros/Consensus/Node.hs
index f530ebe37..7c655a3bb 100644
--- a/ouroboros-consensus/src/Ouroboros/Consensus/Node.hs
+++ b/ouroboros-consensus/src/Ouroboros/Consensus/Node.hs
@@ -282,7 +282,7 @@ runWith :: forall m addrNTN addrNTC versionDataNTN versionDataNTC blk p2p.
   -> m ()
 runWith RunNodeArgs{..} LowLevelRunNodeArgs{..} =
 
-    llrnWithCheckedDB $ \(LastShutDownWasClean lastShutDownWasClean) wrapper ->
+    llrnWithCheckedDB $ \(LastShutDownWasClean lastShutDownWasClean) continueWithCleanChainDB ->
     withRegistry $ \registry -> do
       let systemStart :: SystemStart
           systemStart = getSystemStart (configBlock cfg)
@@ -315,7 +315,7 @@ runWith RunNodeArgs{..} LowLevelRunNodeArgs{..} =
       chainDB <- openChainDB registry inFuture cfg initLedger
                 llrnChainDbArgsDefaults customiseChainDbArgs'
 
-      wrapper chainDB $ do
+      continueWithCleanChainDB chainDB $ do
         btime <-
           hardForkBlockchainTime $
           llrnCustomiseHardForkBlockchainTimeArgs $

I squashed that into your commit and force-pushed it back up.

[nfrisby]

Looks great!

[nfrisby]

bors merge

[iohk-bors]

Build succeeded:

• buildkite/ouroboros-network
• ci/hydra:Cardano:ouroboros-network:required