Skip to content

Commit 23485cf

Browse files
johnalotoskijohnalotoski
authored
Merge pull request #6309 from IntersectMBO/edgr/ux-topology-config-file
improve UX of topology file parsing corner cases
2 parents 7927f6f + 7c69071 commit 23485cf

File tree

5 files changed

+140
-36
lines changed

5 files changed

+140
-36
lines changed

‎cardano-node/src/Cardano/Node/Configuration/TopologyP2P.hs‎

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,15 @@ import Ouroboros.Network.PeerSelection.State.LocalRootPeers (HotValenc
4545
WarmValency (..))
4646

4747
import Control.Applicative (Alternative (..))
48-
import Control.Exception.Safe (Exception (..), IOException, handleAny)
48+
import Control.Exception.Safe (Exception (..), IOException, try)
4949
import Control.Monad
5050
import Control.Monad.IO.Class
5151
import qualified "contra-tracer" Control.Tracer as CT
5252
import Data.Aeson
5353
import Data.Bifunctor (first)
5454
import qualified Data.ByteString as BS
5555
import qualified Data.ByteString.Lazy.Char8 as LBS
56+
import Data.Maybe (isJust, isNothing)
5657
import Data.Text (Text)
5758
import qualified Data.Text as Text
5859
import Data.Word (Word64)
@@ -232,26 +233,63 @@ instance ToJSON adr => ToJSON (NetworkTopology adr) where
232233
readTopologyFile :: ()
233234
=> forall adr. FromJSON adr
234235
=> NodeConfiguration -> CT.Tracer IO (StartupTrace blk) -> IO (Either Text (NetworkTopology adr))
235-
readTopologyFile NodeConfiguration{ncTopologyFile=TopologyFile topologyFilePath, ncConsensusMode} tracer = runExceptT $ do
236+
readTopologyFile NodeConfiguration{ncTopologyFile=TopologyFile topologyFilePath, ncConsensusMode, ncProtocolFiles} tracer = runExceptT $ do
236237
bs <- handleIOExceptionsLiftWith handler $ BS.readFile topologyFilePath
237-
topology@RealNodeTopology{ntUseBootstrapPeers} <-
238+
topology@RealNodeTopology{ntUseLedgerPeers, ntUseBootstrapPeers, ntPeerSnapshotPath} <-
238239
liftEither . first handlerJSON $
239240
eitherDecode $ LBS.fromStrict bs
240241

241242
unless (isValidTrustedPeerConfiguration topology) $
242243
throwError handlerBootstrap
243244

245+
when (isBlockProducer && useLedgerPeers ntUseLedgerPeers) $
246+
liftIO $ CT.traceWith tracer
247+
$ NetworkConfigUpdateWarning
248+
$ createMsg "Use of ledger peers is not recommended for BP operation"
249+
250+
when (isJust ntPeerSnapshotPath && not (useLedgerPeers ntUseLedgerPeers) && isBlockProducer) $
251+
liftIO $ CT.traceWith tracer
252+
$ NetworkConfigUpdateInfo
253+
$ createMsg "Ledger peers and peer snapshot, although specified in the topology file, are disabled in line with recommended BP operation"
254+
255+
when (inPraosMode && isJust ntPeerSnapshotPath && not (useLedgerPeers ntUseLedgerPeers)) $
256+
if isBlockProducer
257+
then liftIO $ CT.traceWith tracer
258+
$ NetworkConfigUpdateWarning
259+
$ createMsg
260+
$ "Peer snapshot file specified but topology disables ledger peers - ignoring file. "
261+
<> "To turn off this message remove peerSnapshotFile from the topology file."
262+
else liftIO $ CT.traceWith tracer
263+
$ NetworkConfigUpdateWarning
264+
$ createMsg
265+
$ "Peer snapshot file specified but topology disables ledger peers - ignoring file. "
266+
<> "To turn off this message enable the use of ledger peers or remove peerSnapshotFile from the topology file."
267+
268+
269+
when (inGenesisMode && not (useLedgerPeers ntUseLedgerPeers) && not isBlockProducer) $
270+
liftIO $ CT.traceWith tracer
271+
$ NetworkConfigUpdateWarning
272+
$ createMsg "It is recommended to use ledger peers and peer snapshot file for relay operations in Genesis mode"
273+
274+
when (inGenesisMode && isNothing ntPeerSnapshotPath && useLedgerPeers ntUseLedgerPeers && not isBlockProducer) $
275+
liftIO $ CT.traceWith tracer
276+
$ NetworkConfigUpdateWarning
277+
$ createMsg
278+
$ "It is recommended to specify an up-to-date ledger peer snapshot file for relay operations in Genesis mode "
279+
<> "when the use of ledger peers is enabled."
280+
244281
-- make all relative paths in the topology file relative to the topology file location
245282
adjustFilePaths (takeDirectory topologyFilePath </>) <$>
246283
if isGenesisCompatible ncConsensusMode ntUseBootstrapPeers
247284
then pure topology
248285
else do
249-
liftIO $ CT.traceWith tracer $ NetworkConfigUpdateError genesisIncompatible
286+
liftIO $ CT.traceWith tracer $ NetworkConfigUpdateWarning genesisIncompatible
250287
pure $ topology{ntUseBootstrapPeers = DontUseBootstrapPeers}
251288
where
289+
createMsg msg =
290+
"Cardano.Node.Configuration.Topology.readTopologyFile: " <> msg
252291
handler :: IOException -> Text
253-
handler e = Text.pack $ "Cardano.Node.Configuration.Topology.readTopologyFile: "
254-
++ displayException e
292+
handler = Text.pack . createMsg . displayException
255293
handlerJSON :: String -> Text
256294
handlerJSON err = mconcat
257295
[ "Is your topology file formatted correctly? "
@@ -263,8 +301,8 @@ readTopologyFile NodeConfiguration{ncTopologyFile=TopologyFile topologyFilePath,
263301
, Text.pack err
264302
]
265303
genesisIncompatible
266-
= Text.pack $ "Cardano.Node.Configuration.Topology.readTopologyFile: "
267-
<> "Bootstrap peers (field 'bootstrapPeers') are not compatible "
304+
= Text.pack . createMsg $
305+
"Bootstrap peers (field 'bootstrapPeers') are not compatible "
268306
<> "with Genesis syncing mode, reverting to 'DontUseBootstrapPeers'. "
269307
<> "Big ledger peers will be leveraged for decentralized syncing - it "
270308
<> "is recommened to provide an up-to-date big ledger peer snapshot file "
@@ -277,8 +315,13 @@ readTopologyFile NodeConfiguration{ncTopologyFile=TopologyFile topologyFilePath,
277315
, "in bootstrap mode. Make sure you provide at least one bootstrap peer "
278316
, "source. "
279317
]
318+
useLedgerPeers DontUseLedgerPeers = False
319+
useLedgerPeers _ = True
280320
isGenesisCompatible GenesisMode UseBootstrapPeers{} = False
281321
isGenesisCompatible _ _ = True
322+
inPraosMode = ncConsensusMode == PraosMode
323+
inGenesisMode = ncConsensusMode == GenesisMode
324+
isBlockProducer = hasProtocolFile ncProtocolFiles
282325

283326
readTopologyFileOrError :: ()
284327
=> forall adr. FromJSON adr
@@ -289,12 +332,18 @@ readTopologyFileOrError nc tr =
289332
<> Text.unpack err)
290333
pure
291334

292-
readPeerSnapshotFile :: PeerSnapshotFile -> IO LedgerPeerSnapshot
293-
readPeerSnapshotFile (PeerSnapshotFile peerSnapshotFile) =
294-
handleException $
295-
either error pure =<< eitherDecodeFileStrict peerSnapshotFile
335+
readPeerSnapshotFile :: PeerSnapshotFile -> IO (Either Text LedgerPeerSnapshot)
336+
readPeerSnapshotFile (PeerSnapshotFile file) = do
337+
content <- first renderException <$> try (BS.readFile file)
338+
return $ first handler $ content >>= eitherDecodeStrict
296339
where
297-
handleException = handleAny $ \e -> error $ "Cardano.Node.Configuration.TopologyP2P.readPeerSnapshotFile: " <> displayException e
340+
renderException :: IOException -> String
341+
renderException = displayException
342+
343+
handler :: String -> Text
344+
handler msg =
345+
Text.pack
346+
$ "Cardano.Node.Configuration.TopologyP2P.readPeerSnapshotFile: " <> msg
298347

299348
--
300349
-- Checking for chance of progress in bootstrap phase

‎cardano-node/src/Cardano/Node/Run.hs‎

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ import Ouroboros.Consensus.Util.Orphans ()
8484
import Cardano.Network.PeerSelection.Bootstrap (UseBootstrapPeers (..))
8585
import Cardano.Network.PeerSelection.PeerTrustable (PeerTrustable)
8686
import Cardano.Network.Types (NumberOfBigLedgerPeers (..))
87+
import Cardano.Network.ConsensusMode (ConsensusMode (..))
8788
import qualified Ouroboros.Cardano.PeerSelection.PeerSelectionActions as Cardano
8889
import Ouroboros.Cardano.PeerSelection.Churn (peerChurnGovernor)
8990
import Ouroboros.Cardano.Network.Types (ChurnMode (..))
@@ -124,15 +125,17 @@ import Ouroboros.Network.Protocol.ChainSync.Codec
124125
import Ouroboros.Network.Subscription (DnsSubscriptionTarget (..),
125126
IPSubscriptionTarget (..))
126127

128+
import Control.Applicative (empty)
127129
import Control.Concurrent (killThread, mkWeakThreadId, myThreadId, getNumCapabilities)
128130
import Control.Concurrent.Class.MonadSTM.Strict
129131
import Control.Exception (try, Exception, IOException)
130132
import qualified Control.Exception as Exception
131-
import Control.Monad (forM, forM_, unless, void, when)
133+
import Control.Monad (forM, forM_, unless, void, when, join)
132134
import Control.Monad.Class.MonadThrow (MonadThrow (..))
133135
import Control.Monad.IO.Class (MonadIO (..))
134136
import Control.Monad.Trans.Except (ExceptT, runExceptT)
135-
import Control.Monad.Trans.Except.Extra (left)
137+
import Control.Monad.Trans.Except.Extra (left, hushM)
138+
import Control.Monad.Trans.Maybe (MaybeT(runMaybeT, MaybeT), hoistMaybe)
136139
import "contra-tracer" Control.Tracer
137140
import Data.Bits
138141
import Data.Either (partitionEithers)
@@ -486,13 +489,21 @@ handleSimpleNode blockType runP p2pMode tracers nc onKernel = do
486489
publicRoots
487490
ntUseLedgerPeers
488491
ntPeerSnapshotPath
492+
case ncPeerSharing nc of
493+
PeerSharingEnabled
494+
| hasProtocolFile (ncProtocolFiles nc) ->
495+
traceWith (startupTracer tracers) . NetworkConfigUpdateWarning . Text.pack $
496+
"Mainnet block producers may not meet the Praos performance guarantees "
497+
<> "and host IP address will be leaked since peer sharing is enabled."
498+
_otherwise -> pure ()
489499
localRootsVar <- newTVarIO localRoots
490500
publicRootsVar <- newTVarIO publicRoots
491501
useLedgerVar <- newTVarIO ntUseLedgerPeers
492502
useBootstrapVar <- newTVarIO ntUseBootstrapPeers
493503
ledgerPeerSnapshotPathVar <- newTVarIO ntPeerSnapshotPath
494504
ledgerPeerSnapshotVar <- newTVarIO =<< updateLedgerPeerSnapshot
495505
(startupTracer tracers)
506+
nc
496507
(readTVar ledgerPeerSnapshotPathVar)
497508
(readTVar useLedgerVar)
498509
(const . pure $ ())
@@ -534,6 +545,7 @@ handleSimpleNode blockType runP p2pMode tracers nc onKernel = do
534545
ledgerPeerSnapshotPathVar
535546
void $ updateLedgerPeerSnapshot
536547
(startupTracer tracers)
548+
nc
537549
(readTVar ledgerPeerSnapshotPathVar)
538550
(readTVar useLedgerVar)
539551
(writeTVar ledgerPeerSnapshotVar)
@@ -763,6 +775,7 @@ installP2PSigHUPHandler startupTracer blockType nc nodeKernel localRootsVar publ
763775
useLedgerVar useBootstrapPeersVar ledgerPeerSnapshotPathVar
764776
void $ updateLedgerPeerSnapshot
765777
startupTracer
778+
nc
766779
(readTVar ledgerPeerSnapshotPathVar)
767780
(readTVar useLedgerVar)
768781
(writeTVar ledgerPeerSnapshotVar)
@@ -854,7 +867,7 @@ updateTopologyConfiguration :: Tracer IO (StartupTrace blk)
854867
updateTopologyConfiguration startupTracer nc localRootsVar publicRootsVar useLedgerVar
855868
useBootsrapPeersVar ledgerPeerSnapshotPathVar = do
856869
traceWith startupTracer NetworkConfigUpdate
857-
result <- try $ readTopologyFileOrError nc startupTracer
870+
result <- try $ TopologyP2P.readTopologyFileOrError nc startupTracer
858871
case result of
859872
Left (FatalError err) ->
860873
traceWith startupTracer
@@ -876,31 +889,45 @@ updateTopologyConfiguration startupTracer nc localRootsVar publicRootsVar useLed
876889
#endif
877890

878891
updateLedgerPeerSnapshot :: Tracer IO (StartupTrace blk)
892+
-> NodeConfiguration
879893
-> STM IO (Maybe PeerSnapshotFile)
880894
-> STM IO UseLedgerPeers
881895
-> (Maybe LedgerPeerSnapshot -> STM IO ())
882896
-> IO (Maybe LedgerPeerSnapshot)
883-
updateLedgerPeerSnapshot startupTracer readLedgerPeerPath readUseLedgerVar writeVar = do
884-
mPeerSnapshotFile <- atomically readLedgerPeerPath
885-
mLedgerPeerSnapshot <- forM mPeerSnapshotFile $ \f -> do
886-
lps@(LedgerPeerSnapshot (wOrigin, _)) <- readPeerSnapshotFile f
887-
useLedgerPeers <- atomically readUseLedgerVar
897+
updateLedgerPeerSnapshot startupTracer (NodeConfiguration {ncConsensusMode}) readLedgerPeerPath readUseLedgerVar writeVar = do
898+
(mPeerSnapshotFile, useLedgerPeers)
899+
<- atomically $ (,) <$> readLedgerPeerPath <*> readUseLedgerVar
900+
901+
let trace = traceWith startupTracer
902+
traceL = liftIO . trace
903+
904+
mLedgerPeerSnapshot <- runMaybeT $ do
888905
case useLedgerPeers of
889-
DontUseLedgerPeers ->
890-
traceWith startupTracer (LedgerPeerSnapshotLoaded . Left $ (useLedgerPeers, wOrigin))
891-
UseLedgerPeers afterSlot
892-
| Always <- afterSlot ->
893-
traceWith startupTracer (LedgerPeerSnapshotLoaded . Right $ wOrigin)
894-
| After slotNo <- afterSlot ->
895-
case wOrigin of
896-
Origin -> error "Unsupported big ledger peer snapshot file: taken at Origin"
897-
At slotNo' | slotNo' >= slotNo ->
898-
traceWith startupTracer (LedgerPeerSnapshotLoaded . Right $ wOrigin)
899-
_otherwise ->
900-
traceWith startupTracer (LedgerPeerSnapshotLoaded . Left $ (useLedgerPeers, wOrigin))
901-
return lps
902-
atomically . writeVar $ mLedgerPeerSnapshot
903-
pure mLedgerPeerSnapshot
906+
DontUseLedgerPeers -> empty
907+
UseLedgerPeers afterSlot -> do
908+
eSnapshot
909+
<- liftIO . readPeerSnapshotFile =<< hoistMaybe mPeerSnapshotFile
910+
lps@(LedgerPeerSnapshot (wOrigin, _)) <-
911+
case ncConsensusMode of
912+
GenesisMode ->
913+
MaybeT $ hushM eSnapshot (trace . NetworkConfigUpdateError)
914+
PraosMode ->
915+
MaybeT $ hushM eSnapshot (trace . NetworkConfigUpdateWarning)
916+
case afterSlot of
917+
Always -> do
918+
traceL $ LedgerPeerSnapshotLoaded . Right $ wOrigin
919+
return lps
920+
After ledgerSlotNo
921+
| fileSlot >= ledgerSlotNo -> do
922+
traceL $ LedgerPeerSnapshotLoaded . Right $ wOrigin
923+
pure lps
924+
| otherwise -> do
925+
traceL $ LedgerPeerSnapshotLoaded . Left $ (useLedgerPeers, wOrigin)
926+
empty
927+
where
928+
fileSlot = case wOrigin of; Origin -> 0; At slot -> slot
929+
930+
mLedgerPeerSnapshot <$ atomically (writeVar mLedgerPeerSnapshot)
904931

905932
--------------------------------------------------------------------------------
906933
-- Helper functions

‎cardano-node/src/Cardano/Node/Startup.hs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,14 @@ data StartupTrace blk =
108108
--
109109
| NetworkConfigUpdateError Text
110110

111+
-- | Log network configuration update warning.
112+
--
113+
| NetworkConfigUpdateWarning Text
114+
115+
-- | Log network configuration update info.
116+
--
117+
| NetworkConfigUpdateInfo Text
118+
111119
-- | Log peer-to-peer network configuration, either on startup or when its
112120
-- updated.
113121
--

‎cardano-node/src/Cardano/Node/Tracing/Tracers/Startup.hs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,12 @@ instance ( Show (BlockNodeToNodeVersion blk)
233233
forMachine _dtal (NetworkConfigUpdateError err) =
234234
mconcat [ "kind" .= String "NetworkConfigUpdateError"
235235
, "error" .= String err ]
236+
forMachine _dtal (NetworkConfigUpdateWarning msg) =
237+
mconcat [ "kind" .= String "NetworkConfigUpdateWarning"
238+
, "message" .= String msg ]
239+
forMachine _dtal (NetworkConfigUpdateInfo msg) =
240+
mconcat [ "kind" .= String "NetworkConfigUpdateInfo"
241+
, "message" .= String msg ]
236242
forMachine _dtal (NetworkConfig localRoots publicRoots useLedgerPeers peerSnapshotFileMaybe) =
237243
mconcat [ "kind" .= String "NetworkConfig"
238244
, "localRoots" .= toJSON localRoots
@@ -333,6 +339,10 @@ instance MetaTrace (StartupTrace blk) where
333339
Namespace [] ["NetworkConfigUpdateUnsupported"]
334340
namespaceFor NetworkConfigUpdateError {} =
335341
Namespace [] ["NetworkConfigUpdateError"]
342+
namespaceFor NetworkConfigUpdateWarning {} =
343+
Namespace [] ["NetworkConfigUpdateWarning"]
344+
namespaceFor NetworkConfigUpdateInfo {} =
345+
Namespace [] ["NetworkConfigUpdateInfo"]
336346
namespaceFor NetworkConfig {} =
337347
Namespace [] ["NetworkConfig"]
338348
namespaceFor NonP2PWarning {} =
@@ -354,6 +364,8 @@ instance MetaTrace (StartupTrace blk) where
354364

355365
severityFor (Namespace _ ["SocketConfigError"]) _ = Just Error
356366
severityFor (Namespace _ ["NetworkConfigUpdate"]) _ = Just Notice
367+
severityFor (Namespace _ ["NetworkConfigUpdateInfo"]) _ = Just Info
368+
severityFor (Namespace _ ["NetworkConfigUpdateWarning"]) _ = Just Warning
357369
severityFor (Namespace _ ["NetworkConfigUpdateError"]) _ = Just Error
358370
severityFor (Namespace _ ["NetworkConfigUpdateUnsupported"]) _ = Just Warning
359371
severityFor (Namespace _ ["NonP2PWarning"]) _ = Just Warning
@@ -387,6 +399,10 @@ instance MetaTrace (StartupTrace blk) where
387399
""
388400
documentFor (Namespace [] ["NetworkConfigUpdateUnsupported"]) = Just
389401
""
402+
documentFor (Namespace [] ["NetworkConfigUpdateInfo"]) = Just
403+
""
404+
documentFor (Namespace [] ["NetworkConfigUpdateWarning"]) = Just
405+
""
390406
documentFor (Namespace [] ["NetworkConfigUpdateError"]) = Just
391407
""
392408
documentFor (Namespace [] ["NetworkConfig"]) = Just
@@ -539,6 +555,8 @@ ppStartupInfoTrace NetworkConfigUpdate = "Performing topology configuration upda
539555
ppStartupInfoTrace NetworkConfigUpdateUnsupported =
540556
"Network topology reconfiguration is not supported in non-p2p mode"
541557
ppStartupInfoTrace (NetworkConfigUpdateError err) = err
558+
ppStartupInfoTrace (NetworkConfigUpdateWarning msg) = msg
559+
ppStartupInfoTrace (NetworkConfigUpdateInfo msg) = msg
542560
ppStartupInfoTrace (NetworkConfig localRoots publicRoots useLedgerPeers peerSnapshotFile) =
543561
pack
544562
$ intercalate "\n"

‎cardano-node/src/Cardano/Tracing/Startup.hs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ instance HasSeverityAnnotation (StartupTrace blk) where
2727
getSeverityAnnotation (StartupSocketConfigError _) = Error
2828
getSeverityAnnotation NetworkConfigUpdate = Notice
2929
getSeverityAnnotation (NetworkConfigUpdateError _) = Error
30+
getSeverityAnnotation (NetworkConfigUpdateWarning _) = Warning
31+
getSeverityAnnotation (NetworkConfigUpdateInfo _) = Info
3032
getSeverityAnnotation NetworkConfigUpdateUnsupported = Warning
3133
getSeverityAnnotation NonP2PWarning = Warning
3234
getSeverityAnnotation WarningDevelopmentNodeToNodeVersions {} = Warning

0 commit comments

Comments
 (0)