refactor(blockchain): improve chain command AX

Co-Authored-By: Charon <charon@lethean.io>

chain_commands.go

@@ -32,16 +32,17 @@ const defaultChainSeed = "seeds.lthn.io:36942"
 const chainCommandPath = AppName + " " + ChainCommandName
 const chainExplorerCommandPath = chainCommandPath + " " + chainExplorerCommandName
 const chainSyncCommandPath = chainCommandPath + " " + chainSyncCommandName
-const chainCommandShort = "Sync and inspect the blockchain"
-const chainCommandLong = "Sync the blockchain from peers or inspect it in a terminal UI."
-const chainExplorerCommandShort = "Inspect the chain in a terminal UI"
-const chainExplorerCommandLong = "Browse blocks and transactions in a terminal UI while syncing."
-const chainSyncCommandShort = "Sync from peers"
-const chainSyncCommandLong = "Sync the blockchain from P2P peers in the foreground by default. Use --daemon to run in the background or --stop to stop a running background sync process."
-const chainExplorerExample = chainExplorerCommandPath + " --data-dir ~/.lethean/chain"
-const chainSyncExample = chainSyncCommandPath + "\n" +
-	chainSyncCommandPath + " --daemon\n" +
-	chainSyncCommandPath + " --stop"
+const chainCommandShort = "Sync, inspect, and manage the blockchain"
+const chainCommandLong = "Sync from peers, inspect the chain in a terminal UI, or manage a background sync."
+const chainExplorerCommandShort = "Open the chain explorer"
+const chainExplorerCommandLong = "Browse blocks and transactions in a terminal UI while the node keeps syncing."
+const chainSyncCommandShort = "Start or stop chain sync"
+const chainSyncCommandLong = "Start syncing from peers in the foreground by default. Use --daemon to keep syncing in the background or --stop to stop a running background sync."
+const chainExplorerExample = "Open the explorer:\n  " + chainExplorerCommandPath + " --data-dir ~/.lethean/chain\n\n" +
+	"Open the explorer on testnet:\n  " + chainExplorerCommandPath + " --testnet"
+const chainSyncExample = "Start a foreground sync:\n  " + chainSyncCommandPath + "\n\n" +
+	"Keep syncing in the background:\n  " + chainSyncCommandPath + " --daemon\n\n" +
+	"Stop the background sync:\n  " + chainSyncCommandPath + " --stop"
 const chainCommandExample = chainExplorerExample + "\n" + chainSyncExample
 
 // AddChainCommands registers the `chain` command group on a Cobra root.
@@ -69,9 +70,9 @@ func AddChainCommands(root *cobra.Command) {
 		Args:    cobra.NoArgs,
 	}
 
-	chainCmd.PersistentFlags().StringVar(&dataDir, "data-dir", defaultChainDataDirPath(), "blockchain data directory")
-	chainCmd.PersistentFlags().StringVar(&seed, "seed", defaultChainSeed, "seed peer address (host:port)")
-	chainCmd.PersistentFlags().BoolVar(&testnet, "testnet", false, "use testnet")
+	chainCmd.PersistentFlags().StringVar(&dataDir, "data-dir", defaultChainDataDirPath(), "store chain data in this directory")
+	chainCmd.PersistentFlags().StringVar(&seed, "seed", defaultChainSeed, "connect to this seed peer first (host:port)")
+	chainCmd.PersistentFlags().BoolVar(&testnet, "testnet", false, "use the Lethean testnet")
 
 	chainCmd.AddCommand(
 		newChainExplorerCommand(&dataDir, &seed, &testnet),
@@ -113,6 +114,13 @@ func validateChainDataDir(dataDir string) error {
 	return nil
 }
 
+func validateChainSyncModeSelection(runAsDaemon, stopDaemon bool) error {
+	if runAsDaemon && stopDaemon {
+		return fmt.Errorf("blockchain: choose either --daemon to start background sync or --stop to stop it")
+	}
+	return nil
+}
+
 func validateChainRuntimeInputs(dataDir, seed string) error {
 	if err := validateChainDataDir(dataDir); err != nil {
 		return err

commands_test.go

@@ -94,12 +94,12 @@ func TestChainCommandName_Good(t *testing.T) {
 }
 
 func TestChainCommandLabels_Good(t *testing.T) {
-	assert.Equal(t, "Sync and inspect the blockchain", chainCommandShort)
-	assert.Equal(t, "Sync the blockchain from peers or inspect it in a terminal UI.", chainCommandLong)
-	assert.Equal(t, "Inspect the chain in a terminal UI", chainExplorerCommandShort)
-	assert.Equal(t, "Browse blocks and transactions in a terminal UI while syncing.", chainExplorerCommandLong)
-	assert.Equal(t, "Sync from peers", chainSyncCommandShort)
-	assert.Equal(t, "Sync the blockchain from P2P peers in the foreground by default. Use --daemon to run in the background or --stop to stop a running background sync process.", chainSyncCommandLong)
+	assert.Equal(t, "Sync, inspect, and manage the blockchain", chainCommandShort)
+	assert.Equal(t, "Sync from peers, inspect the chain in a terminal UI, or manage a background sync.", chainCommandLong)
+	assert.Equal(t, "Open the chain explorer", chainExplorerCommandShort)
+	assert.Equal(t, "Browse blocks and transactions in a terminal UI while the node keeps syncing.", chainExplorerCommandLong)
+	assert.Equal(t, "Start or stop chain sync", chainSyncCommandShort)
+	assert.Equal(t, "Start syncing from peers in the foreground by default. Use --daemon to keep syncing in the background or --stop to stop a running background sync.", chainSyncCommandLong)
 }
 
 func TestValidateChainRuntimeInputs_Good(t *testing.T) {
@@ -107,6 +107,11 @@ func TestValidateChainRuntimeInputs_Good(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestValidateChainSyncModeSelection_Good(t *testing.T) {
+	err := validateChainSyncModeSelection(true, false)
+	require.NoError(t, err)
+}
+
 func TestAddChainCommands_Bad_RejectsUnexpectedArgs(t *testing.T) {
 	root := &cobra.Command{Use: "test"}
 	AddChainCommands(root)
@@ -128,7 +133,7 @@ func TestNewChainSyncCommand_Bad_RejectsConflictingFlags(t *testing.T) {
 
 	err := cmd.RunE(cmd, nil)
 	require.Error(t, err)
-	assert.EqualError(t, err, "blockchain: --daemon and --stop are mutually exclusive")
+	assert.EqualError(t, err, "blockchain: choose either --daemon to start background sync or --stop to stop it")
 }
 
 func TestNewChainSyncCommand_Bad_RejectsEmptyDataDirOnStop(t *testing.T) {
@@ -165,6 +170,33 @@ func TestValidateChainRuntimeInputs_Bad_RejectsNonNumericSeedPort(t *testing.T)
 	require.EqualError(t, err, `blockchain: invalid --seed "seeds.lthn.io:http": expected host:port with a numeric port`)
 }
 
+func TestValidateChainSyncModeSelection_Bad_RejectsConflictingFlags(t *testing.T) {
+	err := validateChainSyncModeSelection(true, true)
+	require.EqualError(t, err, "blockchain: choose either --daemon to start background sync or --stop to stop it")
+}
+
+func TestAddChainCommands_Good_PersistentFlagHelp(t *testing.T) {
+	root := &cobra.Command{Use: "test"}
+	AddChainCommands(root)
+
+	chainCmd, _, _ := root.Find([]string{ChainCommandName})
+
+	assert.Equal(t, "store chain data in this directory", chainCmd.PersistentFlags().Lookup("data-dir").Usage)
+	assert.Equal(t, "connect to this seed peer first (host:port)", chainCmd.PersistentFlags().Lookup("seed").Usage)
+	assert.Equal(t, "use the Lethean testnet", chainCmd.PersistentFlags().Lookup("testnet").Usage)
+}
+
+func TestNewChainSyncCommand_Good_FlagHelp(t *testing.T) {
+	dataDir := ""
+	seed := ""
+	testnet := false
+
+	cmd := newChainSyncCommand(&dataDir, &seed, &testnet)
+
+	assert.Equal(t, "keep syncing in the background", cmd.Flags().Lookup("daemon").Usage)
+	assert.Equal(t, "stop a background sync started with --daemon", cmd.Flags().Lookup("stop").Usage)
+}
+
 func TestRunChainSyncOnce_Bad_RespectsCancelledContext(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()

sync_command.go

@@ -47,8 +47,8 @@ func newChainSyncCommand(dataDir, seed *string, testnet *bool) *cobra.Command {
 		Example: chainSyncExample,
 		Args:    cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if runAsDaemon && stopDaemon {
-				return fmt.Errorf("blockchain: --daemon and --stop are mutually exclusive")
+			if err := validateChainSyncModeSelection(runAsDaemon, stopDaemon); err != nil {
+				return err
 			}
 			if stopDaemon {
 				return stopChainSyncDaemon(*dataDir)
@@ -60,8 +60,8 @@ func newChainSyncCommand(dataDir, seed *string, testnet *bool) *cobra.Command {
 		},
 	}
 
-	cmd.Flags().BoolVar(&runAsDaemon, "daemon", false, "run synchronisation in the background")
-	cmd.Flags().BoolVar(&stopDaemon, "stop", false, "stop the background sync process")
+	cmd.Flags().BoolVar(&runAsDaemon, "daemon", false, "keep syncing in the background")
+	cmd.Flags().BoolVar(&stopDaemon, "stop", false, "stop a background sync started with --daemon")
 
 	return cmd
 }
@@ -87,7 +87,7 @@ func runChainSyncForeground(dataDir, seed string, testnet bool) error {
 	ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
 	defer cancel()
 
-	log.Printf("Starting %s in foreground...", chainSyncDisplayName)
+	log.Printf("Starting %s with data dir %s and seed %s.", chainSyncDisplayName, dataDir, resolvedSeed)
 	runChainSyncLoop(ctx, blockchain, &chainConfig, hardForks, resolvedSeed)
 	log.Printf("%s stopped.", chainSyncDisplayName)
 	return nil
@@ -131,7 +131,7 @@ func runChainSyncDaemon(dataDir, seed string, testnet bool) error {
 	defer cancel()
 
 	syncDaemon.SetReady(true)
-	log.Printf("Background sync started for %s.", chainSyncDisplayName)
+	log.Printf("Background sync started for %s with data dir %s and seed %s. Stop it with %s --stop --data-dir %s.", chainSyncDisplayName, dataDir, resolvedSeed, chainSyncCommandPath, dataDir)
 
 	var wg sync.WaitGroup
 	wg.Add(1)
@@ -165,6 +165,6 @@ func stopChainSyncDaemon(dataDir string) error {
 		return coreerr.E("stopChainSyncDaemon", fmt.Sprintf("signal process %d", pid), err)
 	}
 
-	log.Printf("Sent SIGTERM to background sync process for %s (PID %d)", chainSyncDisplayName, pid)
+	log.Printf("Stopping background sync for %s (PID %d).", chainSyncDisplayName, pid)
 	return nil
 }