improve UX of topology file parsing corner cases #6309
Conversation
crocodile-dentist
force-pushed
the
edgr/ux-topology-config-file
branch
from
ef49c64 to
c4ada02
Compare
the-headless-ghost
force-pushed
the
edgr/ux-topology-config-file
branch
from
c4ada02 to
d0da45e
Compare
|
@the-headless-ghost could you squash relevant commits together? |
| when (inGenesisMode && not (useLedgerPeers ntUseLedgerPeers) && not isBlockProducer) $ | ||
| liftIO $ CT.traceWith tracer | ||
| $ NetworkConfigUpdateWarning | ||
| $ createMsg "It is recommended to use ledger peers and peer snapshot file for relay operations in Genesis mode" |
There was a problem hiding this comment.
When in genesis mode and there, and ledger peers are disabled, the node should error (this message needs to be an error, and the node needs to exit - am I right that the node used to exit in this case?).
Ping @amesgen.
There was a problem hiding this comment.
I think it can still progress if association mode is localrootsonly, and it might be useful to not have it crash for testing environments.
[edit] I think the node used to crash only when the peerSnapshotFile was given but was missing.
There was a problem hiding this comment.
Also, private relays might use LocalRootsOnly. Generally, there isn't really any benefit to using Genesis when LocalRootsOnly, but it also isn't problematic either.
We indeed should error if there is no chance that we will ever leave the PreSyncing state, ie when Network can't ever signal TrustedStateWithExternalPeers.
There was a problem hiding this comment.
My main concern was a misconfiguration Daedalus node: people will notice node not starting, but won't notice a warning so they'll need to wait for node not making progress with syncing. But maybe indeed that's too conservative.
| when (inGenesisMode && isNothing ntPeerSnapshotPath && useLedgerPeers ntUseLedgerPeers && not isBlockProducer) $ | ||
| liftIO $ CT.traceWith tracer | ||
| $ NetworkConfigUpdateWarning | ||
| $ createMsg | ||
| $ "It is recommended to specify an up-to-date ledger peer snapshot file for relay operations in Genesis mode " | ||
| <> "when the use of ledger peers is enabled." |
There was a problem hiding this comment.
If we are in genesis mode and there is no peer snapshot file, we might need to exit as well. @amesgen what do you think?
There was a problem hiding this comment.
If the node is syncing up not from a blank state, but maybe somewhat stale, it can use the peers it gets from the ledger it has. Also, this file is not necessary when the node is synced up and so I didn't think crashing the node at startup was warranted.
[edit] We can add to the warning that the node might appear stuck as it may not be able to find any peers to connect to and start syncing.
the-headless-ghost
force-pushed
the
edgr/ux-topology-config-file
branch
from
a1522c5 to
4f29e2d
Compare
cardano-node/src/Cardano/Node/Run.hs
Outdated
|
|
||
| let trace = traceWith startupTracer | ||
| traceL = liftIO . trace | ||
| nothing' = MaybeT $ pure Nothing |
There was a problem hiding this comment.
I'd use empty from Alternative instead of defining my own nothing'.
the-headless-ghost
force-pushed
the
edgr/ux-topology-config-file
branch
from
5518526 to
255f1c7
Compare
the-headless-ghost
force-pushed
the
edgr/ux-topology-config-file
branch
from
255f1c7 to
7c69071
Compare
Description
resolves #6304
Checklist