services/horizon/cmd: Fix prepare range bug (#3625)

The `horizon db reingest range` command failed because it requires that the top level horizon ingest flag to be true.
In other words, `horizon --ingest db reingest range` works but `horizon db reingest range` responds with the following error:

2021/05/03 12:49:43 Invalid config: one or more captive core params passed (--stellar-core-binary-path or --captive-core-config-append-path) but --ingest not set. 

This PR fixes this bug by making all ingestion subcommands set ingest to true implicitly.

This PR also fixes the captive core config validation by only requiring the config file to be present when ingesting in online mode. It is not necessary to have a captive core config file when ingesting bounded ledger ranges.

services/horizon/CHANGELOG.md

@@ -9,6 +9,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).
 
 * Add more in-depth Prometheus metrics (count & duration) for db queries.
 
+* Fix bug in `horizon db reingest range` command which required the `--ingest` flag to be set [3625](https://github.com/stellar/go/pull/3625)).
 
 ## v2.3.0
 

services/horizon/cmd/db.go

@@ -202,16 +202,18 @@ var dbReingestRangeCmd = &cobra.Command{
 
 		argsUInt32 := make([]uint32, 2)
 		for i, arg := range args {
-			seq, err := strconv.Atoi(arg)
-			if err != nil {
+			if seq, err := strconv.Atoi(arg); err != nil {
 				cmd.Usage()
 				log.Fatalf(`Invalid sequence number "%s"`, arg)
+			} else if seq < 0 {
+				log.Fatalf("sequence number %s cannot be negative", arg)
+			} else {
+				argsUInt32[i] = uint32(seq)
 			}
-			argsUInt32[i] = uint32(seq)
 		}
 
-		horizon.ApplyFlags(config, flags)
-		err := RunDBReingestRange(argsUInt32[0], argsUInt32[1], reingestForce, parallelWorkers, *config)
+		horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true})
+		err := runDBReingestRange(argsUInt32[0], argsUInt32[1], reingestForce, parallelWorkers, *config)
 		if err != nil {
 			if errors.Cause(err) == ingest.ErrReingestRangeConflict {
 				message := `
@@ -232,7 +234,7 @@ var dbReingestRangeCmd = &cobra.Command{
 	},
 }
 
-func RunDBReingestRange(from, to uint32, reingestForce bool, parallelWorkers uint, config horizon.Config) error {
+func runDBReingestRange(from, to uint32, reingestForce bool, parallelWorkers uint, config horizon.Config) error {
 	if reingestForce && parallelWorkers > 1 {
 		return errors.New("--force is incompatible with --parallel-workers > 1")
 	}
@@ -303,7 +305,7 @@ func init() {
 
 	viper.BindPFlags(dbReingestRangeCmd.PersistentFlags())
 
-	rootCmd.AddCommand(dbCmd)
+	RootCmd.AddCommand(dbCmd)
 	dbCmd.AddCommand(
 		dbInitCmd,
 		dbMigrateCmd,

services/horizon/cmd/ingest.go

@@ -71,7 +71,7 @@ var ingestVerifyRangeCmd = &cobra.Command{
 			co.SetValue()
 		}
 
-		horizon.ApplyFlags(config, flags)
+		horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true})
 
 		if ingestVerifyDebugServerPort != 0 {
 			go func() {
@@ -170,7 +170,7 @@ var ingestStressTestCmd = &cobra.Command{
 			co.SetValue()
 		}
 
-		horizon.ApplyFlags(config, flags)
+		horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true})
 
 		horizonSession, err := db.Open("postgres", config.DatabaseURL)
 		if err != nil {
@@ -229,7 +229,7 @@ var ingestTriggerStateRebuildCmd = &cobra.Command{
 	Short: "updates a database to trigger state rebuild, state will be rebuilt by a running Horizon instance, DO NOT RUN production DB, some endpoints will be unavailable until state is rebuilt",
 	Run: func(cmd *cobra.Command, args []string) {
 		ctx := context.Background()
-		horizon.ApplyFlags(config, flags)
+		horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true})
 
 		horizonSession, err := db.Open("postgres", config.DatabaseURL)
 		if err != nil {
@@ -251,7 +251,7 @@ var ingestInitGenesisStateCmd = &cobra.Command{
 	Short: "ingests genesis state (ledger 1)",
 	Run: func(cmd *cobra.Command, args []string) {
 		ctx := context.Background()
-		horizon.ApplyFlags(config, flags)
+		horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true})
 
 		horizonSession, err := db.Open("postgres", config.DatabaseURL)
 		if err != nil {
@@ -322,7 +322,7 @@ func init() {
 
 	viper.BindPFlags(ingestVerifyRangeCmd.PersistentFlags())
 
-	rootCmd.AddCommand(ingestCmd)
+	RootCmd.AddCommand(ingestCmd)
 	ingestCmd.AddCommand(
 		ingestVerifyRangeCmd,
 		ingestStressTestCmd,

services/horizon/cmd/root.go

@@ -12,7 +12,7 @@ import (
 var (
 	config, flags = horizon.Flags()
 
-	rootCmd = &cobra.Command{
+	RootCmd = &cobra.Command{
 		Use:   "horizon",
 		Short: "client-facing api server for the Stellar network",
 		Long:  "Client-facing API server for the Stellar network. It acts as the interface between Stellar Core and applications that want to access the Stellar network. It allows you to submit transactions to the network, check the status of accounts, subscribe to event streams and more.",
@@ -23,14 +23,14 @@ var (
 )
 
 func init() {
-	err := flags.Init(rootCmd)
+	err := flags.Init(RootCmd)
 	if err != nil {
 		stdLog.Fatal(err.Error())
 	}
 }
 
 func Execute() {
-	if err := rootCmd.Execute(); err != nil {
+	if err := RootCmd.Execute(); err != nil {
 		fmt.Println(err)
 		os.Exit(1)
 	}

services/horizon/cmd/serve.go

@@ -15,5 +15,5 @@ var serveCmd = &cobra.Command{
 }
 
 func init() {
-	rootCmd.AddCommand(serveCmd)
+	RootCmd.AddCommand(serveCmd)
 }

services/horizon/cmd/version.go

@@ -19,5 +19,5 @@ var versionCmd = &cobra.Command{
 }
 
 func init() {
-	rootCmd.AddCommand(versionCmd)
+	RootCmd.AddCommand(versionCmd)
 }

services/horizon/internal/flags.go

@@ -428,7 +428,7 @@ func Flags() (*Config, support.ConfigOptions) {
 
 // NewAppFromFlags constructs a new Horizon App from the given command line flags
 func NewAppFromFlags(config *Config, flags support.ConfigOptions) *App {
-	ApplyFlags(config, flags)
+	ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false})
 	// Validate app-specific arguments
 	if config.StellarCoreURL == "" {
 		log.Fatalf("flag --%s cannot be empty", StellarCoreURLFlagName)
@@ -443,15 +443,24 @@ func NewAppFromFlags(config *Config, flags support.ConfigOptions) *App {
 	return app
 }
 
+type ApplyOptions struct {
+	AlwaysIngest             bool
+	RequireCaptiveCoreConfig bool
+}
+
 // ApplyFlags applies the command line flags on the given Config instance
-func ApplyFlags(config *Config, flags support.ConfigOptions) {
+func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOptions) {
 	// Verify required options and load the config struct
 	flags.Require()
 	flags.SetValues()
 
 	// Validate options that should be provided together
 	validateBothOrNeither("tls-cert", "tls-key")
 
+	if options.AlwaysIngest {
+		config.Ingest = true
+	}
+
 	if config.Ingest {
 		// Migrations should be checked as early as possible. Apply and check
 		// only on ingesting instances which are required to have write-access
@@ -492,11 +501,19 @@ func ApplyFlags(config *Config, flags support.ConfigOptions) {
 			}
 
 			if config.RemoteCaptiveCoreURL == "" && (binaryPath == "" || config.CaptiveCoreConfigPath == "") {
-				stdLog.Fatalf("Invalid config: captive core requires that both --%s and --%s are set. %s",
-					StellarCoreBinaryPathName, CaptiveCoreConfigAppendPathName, captiveCoreMigrationHint)
-			}
-
-			if config.RemoteCaptiveCoreURL == "" {
+				if options.RequireCaptiveCoreConfig {
+					stdLog.Fatalf("Invalid config: captive core requires that both --%s and --%s are set. %s",
+						StellarCoreBinaryPathName, CaptiveCoreConfigAppendPathName, captiveCoreMigrationHint)
+				} else {
+					var err error
+					config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs
+					config.CaptiveCoreTomlParams.NetworkPassphrase = config.NetworkPassphrase
+					config.CaptiveCoreToml, err = ledgerbackend.NewCaptiveCoreToml(config.CaptiveCoreTomlParams)
+					if err != nil {
+						stdLog.Fatalf("Invalid captive core toml file %v", err)
+					}
+				}
+			} else if config.RemoteCaptiveCoreURL == "" {
 				var err error
 				config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs
 				config.CaptiveCoreTomlParams.NetworkPassphrase = config.NetworkPassphrase

services/horizon/internal/integration/db_test.go

@@ -1,7 +1,9 @@
 package integration
 
 import (
+	"fmt"
 	"path/filepath"
+	"strconv"
 	"testing"
 	"time"
 
@@ -85,9 +87,34 @@ func TestReingestDB(t *testing.T) {
 		filepath.Dir(horizonConfig.CaptiveCoreConfigPath),
 		"captive-core-reingest-range-integration-tests.cfg",
 	)
+	horizoncmd.RootCmd.SetArgs([]string{
+		"--stellar-core-url",
+		horizonConfig.StellarCoreURL,
+		"--history-archive-urls",
+		horizonConfig.HistoryArchiveURLs[0],
+		"--db-url",
+		horizonConfig.DatabaseURL,
+		"--stellar-core-db-url",
+		horizonConfig.StellarCoreDatabaseURL,
+		"--stellar-core-binary-path",
+		horizonConfig.CaptiveCoreBinaryPath,
+		"--captive-core-config-append-path",
+		horizonConfig.CaptiveCoreConfigPath,
+		"--enable-captive-core-ingestion=" + strconv.FormatBool(horizonConfig.EnableCaptiveCoreIngestion),
+		"--network-passphrase",
+		horizonConfig.NetworkPassphrase,
+		// due to ARTIFICIALLY_ACCELERATE_TIME_FOR_TESTING
+		"--checkpoint-frequency",
+		"8",
+		"db",
+		"reingest",
+		"range",
+		"1",
+		fmt.Sprintf("%d", toLedger),
+	})
 
+	err = horizoncmd.RootCmd.Execute()
 	// Reingest into the DB
-	err = horizoncmd.RunDBReingestRange(1, toLedger, false, 1, horizonConfig)
 	tt.NoError(err)
 }