Merge pull request #5813 from IntersectMBO/js/remove-cli-args

Remove consensus internal options from cli flags

Author: jasagredo

cardano-node/ChangeLog.md

@@ -3,6 +3,12 @@
 ## Next version
 
 - Use p2p network stack by default, warn when using the legacy network stack.
+- Deprecate some CLI flags corresponding to low-level consensus options. They are 
+  still accepted but a warning is emitted on startup on stderr suggesting to set 
+  them in the configuration file instead:
+  - `--mempool-capacity-override` and `--no-mempool-capacity-override` can be set in the configuration file via the key `MempoolCapacityBytesOverride`.
+  - `--snapshot-interval` can be set in the configuration file via the key `SnapshotInterval`.
+  - `--num-of-disk-snapshots` can be set in the configuration file via the key `NumOfDiskSnapshots`.
 
 ## 8.2.1 -- August 2023
 

cardano-node/app/cardano-node.hs

@@ -1,24 +1,27 @@
 {-# LANGUAGE OverloadedStrings #-}
 {-# LANGUAGE RankNTypes #-}
 {-# LANGUAGE TemplateHaskell #-}
+{-# LANGUAGE GADTs #-}
 
 import qualified Cardano.Crypto.Init as Crypto
 import           Cardano.Git.Rev (gitRev)
-import           Cardano.Node.Configuration.POM (PartialNodeConfiguration)
+import           Cardano.Node.Configuration.POM (PartialNodeConfiguration(..))
 import           Cardano.Node.Handlers.TopLevel
 import           Cardano.Node.Parsers (nodeCLIParser, parserHelpHeader, parserHelpOptions,
                    renderHelpDoc)
 import           Cardano.Node.Run (runNode)
 import           Cardano.Node.Tracing.Documentation (TraceDocumentationCmd (..),
                    parseTraceDocumentationCmd, runTraceDocumentationCmd)
 
+import           Data.Monoid (Last (getLast))
 import qualified Data.Text as Text
 import qualified Data.Text.IO as Text
 import           Data.Version (showVersion)
 import           Options.Applicative
 import qualified Options.Applicative as Opt
 import           Options.Applicative.Help ((<$$>))
 import           System.Info (arch, compilerName, compilerVersion, os)
+import           System.IO (hPutStrLn, stderr)
 
 import           Paths_cardano_node (version)
 
@@ -30,13 +33,27 @@ main = do
     cmd <- Opt.customExecParser p opts
 
     case cmd of
-      RunCmd args -> runNode args
+      RunCmd args -> do
+        warnIfSet args pncMaybeMempoolCapacityOverride "mempool-capacity-override" "MempoolCapacityBytesOverride"
+        warnIfSet args pncNumOfDiskSnapshots "num-of-disk-snapshots" "NumOfDiskSnapshots"
+        warnIfSet args pncSnapshotInterval "snapshot-interval" "SnapshotInterval"
+        runNode args
       TraceDocumentation tdc -> runTraceDocumentationCmd tdc
       VersionCmd  -> runVersionCommand
 
     where
       p = Opt.prefs Opt.showHelpOnEmpty
 
+      warnIfSet :: PartialNodeConfiguration -> (PartialNodeConfiguration -> Last a) -> String -> String -> IO ()
+      warnIfSet args f name key = 
+          maybe 
+            (pure ()) 
+            (\_ -> hPutStrLn stderr $ "WARNING: Option --" ++ name ++ " was set via CLI flags.\ 
+            \ This CLI flag will be removed in upcoming node releases.\
+            \ Please, set this configuration option in the configuration file instead with key " ++ key ++ ".") 
+        $ getLast
+        $ f args
+        
       opts :: Opt.ParserInfo Command
       opts =
         Opt.info (fmap RunCmd nodeCLIParser

cardano-node/src/Cardano/Node/Parsers.hs

@@ -51,8 +51,10 @@ nodeRunParser = do
   -- Filepaths
   topFp <- lastOption parseTopologyFile
   dbFp <- lastOption parseDbPath
+  validate <- lastOption parseValidateDB
   socketFp <- lastOption $ parseSocketPath "Path to a cardano-node socket"
   traceForwardSocket <- lastOption parseTracerSocketMode
+  nodeConfigFp <- lastOption parseConfigFile
 
   -- Protocol files
   byronCertFile   <- optional parseByronDelegationCert
@@ -68,15 +70,13 @@ nodeRunParser = do
   nIPv6Address <- lastOption parseHostIPv6Addr
   nPortNumber  <- lastOption parsePort
 
-  -- NodeConfiguration filepath
-  nodeConfigFp <- lastOption parseConfigFile
-  numOfDiskSnapshots <- lastOption parseNumOfDiskSnapshots
-  snapshotInterval   <- lastOption parseSnapshotInterval
-
-  validate <- lastOption parseValidateDB
+  -- Shutdown
   shutdownIPC <- lastOption parseShutdownIPC
   shutdownOnLimit <- lastOption parseShutdownOn
 
+  -- Hidden options (to be removed eventually)
+  numOfDiskSnapshots <- lastOption parseNumOfDiskSnapshots
+  snapshotInterval   <- lastOption parseSnapshotInterval
   maybeMempoolCapacityOverride <- lastOption parseMempoolCapacityOverride
 
   pure $ PartialNodeConfiguration
@@ -215,13 +215,13 @@ parseMempoolCapacityOverride = parseOverride <|> parseNoOverride
       Opt.option (auto @Word32)
         (  long "mempool-capacity-override"
         <> metavar "BYTES"
-        <> help "The number of bytes"
+        <> help "[DEPRECATED: Set it in config file with key MempoolCapacityBytesOverride] The number of bytes"
         )
     parseNoOverride :: Parser MempoolCapacityBytesOverride
     parseNoOverride =
       flag' NoMempoolCapacityBytesOverride
         (  long "no-mempool-capacity-override"
-        <> help "The port number"
+        <> help "[DEPRECATED: Set it in config file] Don't override mempool capacity"
         )
 
 parseDbPath :: Parser FilePath
@@ -336,7 +336,7 @@ parseNumOfDiskSnapshots = fmap RequestedNumOfDiskSnapshots parseNum
   parseNum = Opt.option auto
     ( long "num-of-disk-snapshots"
         <> metavar "NUMOFDISKSNAPSHOTS"
-        <> help "Number of ledger snapshots stored on disk."
+        <> help "[DEPRECATED: Set it in config file with key NumOfDiskSnapshots] Number of ledger snapshots stored on disk."
     )
 
 -- TODO revisit because it sucks
@@ -346,7 +346,7 @@ parseSnapshotInterval = fmap (RequestedSnapshotInterval . secondsToDiffTime) par
   parseDifftime = Opt.option auto
     ( long "snapshot-interval"
         <> metavar "SNAPSHOTINTERVAL"
-        <> help "Snapshot Interval (in seconds)"
+        <> help "[DEPRECATED: Set it in config file with key SnapshotInterval] Snapshot Interval (in seconds)"
     )
 
 -- | Produce just the brief help header for a given CLI option parser,

cardano-node/src/Cardano/Node/Tracing/Tracers/ChainDB.hs

@@ -422,7 +422,7 @@ instance ( LogFormatting (Header blk)
       "Added request to queue to reprocess blocks postponed by LoE."
   forHuman ChainDB.PoppedReprocessLoEBlocksFromQueue =
       "Poppped request from queue to reprocess blocks postponed by LoE."
-  forHuman ChainDB.ChainSelectionLoEDebug {} =
+  forHuman ChainDB.ChainSelectionLoEDebug{} =
       "ChainDB LoE debug event"
   forMachine dtal (ChainDB.IgnoreBlockOlderThanK pt) =
       mconcat [ "kind" .= String "IgnoreBlockOlderThanK"