services/horizon/internal: Fix configuration of captive core in reing…

…est command (stellar#5069) * Fix configuration of captive core in reingest command * Add validation for an empty --captive-core-config-path. --------- Co-authored-by: Urvi <[email protected]>
urvisavla · Oct 3, 2023 · facabfc · facabfc
1 parent 7d95f68
commit facabfc
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 19 deletions.
diff --git a/services/horizon/CHANGELOG.md b/services/horizon/CHANGELOG.md
@@ -8,6 +8,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Breaking Changes
 - The command line flag `--remote-captive-core-url` has been removed, as remote captive core functionality is now deprecated ([4940](https://github.com/stellar/go/pull/4940)).
+- The functionality of generating default captive core configuration based on the --network-passphrase is now deprecated. Use the --network command instead ([4949](https://github.com/stellar/go/pull/4949)).
 
 ### Added
 - Added new command-line flag `--network` to specify the Stellar network (pubnet or testnet), aiming at simplifying the configuration process by automatically configuring the following parameters based on the chosen network: `--history-archive-urls`, `--network-passphrase`, and `--captive-core-config-path` ([4949](https://github.com/stellar/go/pull/4949)).

diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go
@@ -766,7 +766,7 @@ func createCaptiveCoreConfigFromNetwork(config *Config) error {
 
 // createCaptiveCoreConfigFromParameters generates the Captive Core configuration.
 // validates the configuration settings, sets necessary values, and loads the Captive Core TOML file.
-func createCaptiveCoreConfigFromParameters(config *Config) error {
+func createCaptiveCoreConfigFromParameters(config *Config, options ApplyOptions) error {
 
 	if config.NetworkPassphrase == "" {
 		return fmt.Errorf("%s must be set", NetworkPassphraseFlagName)
@@ -778,7 +778,14 @@ func createCaptiveCoreConfigFromParameters(config *Config) error {
 
 	if config.CaptiveCoreConfigPath != "" {
 		return loadCaptiveCoreTomlFromFile(config)
+	} else if options.RequireCaptiveCoreConfig {
+		return fmt.Errorf(
+			"invalid config: captive core requires that --%s is set", CaptiveCoreConfigPathName)
 	} else {
+		config.CaptiveCoreTomlParams.CoreBinaryPath = config.CaptiveCoreBinaryPath
+		config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs
+		config.CaptiveCoreTomlParams.NetworkPassphrase = config.NetworkPassphrase
+
 		var err error
 		config.CaptiveCoreToml, err = ledgerbackend.NewCaptiveCoreToml(config.CaptiveCoreTomlParams)
 		if err != nil {
@@ -790,7 +797,7 @@ func createCaptiveCoreConfigFromParameters(config *Config) error {
 }
 
 // setCaptiveCoreConfiguration prepares configuration for the Captive Core
-func setCaptiveCoreConfiguration(config *Config) error {
+func setCaptiveCoreConfiguration(config *Config, options ApplyOptions) error {
 	stdLog.Println("Preparing captive core...")
 
 	// If the user didn't specify a Stellar Core binary, we can check the
@@ -808,7 +815,7 @@ func setCaptiveCoreConfiguration(config *Config) error {
 			return err
 		}
 	} else {
-		err := createCaptiveCoreConfigFromParameters(config)
+		err := createCaptiveCoreConfigFromParameters(config, options)
 		if err != nil {
 			return err
 		}
@@ -858,7 +865,7 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 		}
 
 		if config.EnableCaptiveCoreIngestion {
-			err := setCaptiveCoreConfiguration(config)
+			err := setCaptiveCoreConfiguration(config, options)
 			if err != nil {
 				return errors.Wrap(err, "error generating captive core configuration")
 			}

diff --git a/services/horizon/internal/flags_test.go b/services/horizon/internal/flags_test.go
@@ -87,43 +87,92 @@ func Test_createCaptiveCoreConfig(t *testing.T) {
 
 	var errorMsgConfig = "%s must be set"
 	tests := []struct {
-		name               string
-		config             Config
-		networkPassphrase  string
-		historyArchiveURLs []string
-		errStr             string
+		name                     string
+		requireCaptiveCoreConfig bool
+		config                   Config
+		networkPassphrase        string
+		historyArchiveURLs       []string
+		errStr                   string
 	}{
 		{
-			name: "no network specified",
+			name:                     "no network specified; valid parameters",
+			requireCaptiveCoreConfig: true,
 			config: Config{
-				NetworkPassphrase:  "NetworkPassphrase",
-				HistoryArchiveURLs: []string{"HistoryArchiveURLs"},
+				NetworkPassphrase:     PubnetConf.NetworkPassphrase,
+				HistoryArchiveURLs:    PubnetConf.HistoryArchiveURLs,
+				CaptiveCoreConfigPath: "configs/captive-core-pubnet.cfg",
 			},
-			networkPassphrase:  "NetworkPassphrase",
-			historyArchiveURLs: []string{"HistoryArchiveURLs"},
+			networkPassphrase:  PubnetConf.NetworkPassphrase,
+			historyArchiveURLs: PubnetConf.HistoryArchiveURLs,
 		},
 		{
-			name: "no network specified; passphrase not supplied",
+			name:                     "no network specified; passphrase not supplied",
+			requireCaptiveCoreConfig: true,
 			config: Config{
 				HistoryArchiveURLs: []string{"HistoryArchiveURLs"},
 			},
 			errStr: fmt.Sprintf(errorMsgConfig, NetworkPassphraseFlagName),
 		},
 		{
-			name: "no network specified; history archive urls not supplied",
+			name:                     "no network specified; history archive urls not supplied",
+			requireCaptiveCoreConfig: true,
 			config: Config{
 				NetworkPassphrase: "NetworkPassphrase",
 			},
 			errStr: fmt.Sprintf(errorMsgConfig, HistoryArchiveURLsFlagName),
 		},
+		{
+			name:                     "no network specified; captive-core-config-path not supplied",
+			requireCaptiveCoreConfig: true,
+			config: Config{
+				NetworkPassphrase:  PubnetConf.NetworkPassphrase,
+				HistoryArchiveURLs: PubnetConf.HistoryArchiveURLs,
+			},
+			errStr: fmt.Sprintf("invalid config: captive core requires that --%s is set",
+				CaptiveCoreConfigPathName),
+		},
+		{
+			name:                     "no network specified; captive-core-config-path invalid file",
+			requireCaptiveCoreConfig: true,
+			config: Config{
+				NetworkPassphrase:     PubnetConf.NetworkPassphrase,
+				HistoryArchiveURLs:    PubnetConf.HistoryArchiveURLs,
+				CaptiveCoreConfigPath: "xyz.cfg",
+			},
+			errStr: "invalid captive core toml file: could not load toml path:" +
+				" open xyz.cfg: no such file or directory",
+		},
+		{
+			name:                     "no network specified; captive-core-config-path incorrect config",
+			requireCaptiveCoreConfig: true,
+			config: Config{
+				NetworkPassphrase:     PubnetConf.NetworkPassphrase,
+				HistoryArchiveURLs:    PubnetConf.HistoryArchiveURLs,
+				CaptiveCoreConfigPath: "configs/captive-core-testnet.cfg",
+			},
+			errStr: fmt.Sprintf("invalid captive core toml file: invalid captive core toml: "+
+				"NETWORK_PASSPHRASE in captive core config file: %s does not match Horizon "+
+				"network-passphrase flag: %s", TestnetConf.NetworkPassphrase, PubnetConf.NetworkPassphrase),
+		},
+		{
+			name:                     "no network specified; captive-core-config not required",
+			requireCaptiveCoreConfig: false,
+			config: Config{
+				NetworkPassphrase:  PubnetConf.NetworkPassphrase,
+				HistoryArchiveURLs: PubnetConf.HistoryArchiveURLs,
+			},
+			networkPassphrase:  PubnetConf.NetworkPassphrase,
+			historyArchiveURLs: PubnetConf.HistoryArchiveURLs,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			e := createCaptiveCoreConfigFromParameters(&tt.config)
+			e := createCaptiveCoreConfigFromParameters(&tt.config,
+				ApplyOptions{RequireCaptiveCoreConfig: tt.requireCaptiveCoreConfig})
 			if tt.errStr == "" {
 				assert.NoError(t, e)
-				assert.Equal(t, tt.networkPassphrase, tt.config.NetworkPassphrase)
-				assert.Equal(t, tt.historyArchiveURLs, tt.config.HistoryArchiveURLs)
+				assert.Equal(t, tt.networkPassphrase, tt.config.CaptiveCoreTomlParams.NetworkPassphrase)
+				assert.Equal(t, tt.historyArchiveURLs, tt.config.CaptiveCoreTomlParams.HistoryArchiveURLs)
 			} else {
 				require.Error(t, e)
 				assert.Equal(t, tt.errStr, e.Error())