services/horizon/internal: Deprecate captive-core-config-append-path

exp/services/captivecore/main.go

@@ -53,7 +53,7 @@ func main() {
 			ConfigKey:   &binaryPath,
 		},
 		&config.ConfigOption{
-			Name:        "captive-core-config-append-path",
+			Name:        "captive-core-config-path",
 			OptType:     types.String,
 			FlagDefault: "",
 			Required:    true,
@@ -124,6 +124,7 @@ func main() {
 
 			captiveCoreTomlParams.HistoryArchiveURLs = historyArchiveURLs
 			captiveCoreTomlParams.NetworkPassphrase = networkPassphrase
+			captiveCoreTomlParams.Strict = true
 			captiveCoreToml, err := ledgerbackend.NewCaptiveCoreTomlFromFile(configPath, captiveCoreTomlParams)
 			if err != nil {
 				logger.WithError(err).Fatal("Invalid captive core toml")

ingest/ledgerbackend/testdata/invalid-captive-core-field.cfg

@@ -0,0 +1,12 @@
+# CATCHUP_RECENT is not supported by captive core
+CATCHUP_RECENT=100
+
+[[HOME_DOMAINS]]
+HOME_DOMAIN="testnet.stellar.org"
+QUALITY="MEDIUM"
+
+[[VALIDATORS]]
+NAME="sdf_testnet_1"
+HOME_DOMAIN="testnet.stellar.org"
+PUBLIC_KEY="GDKXE2OZMJIPOSLNA6N6F2BVCI3O777I2OOC4BV7VOYUEHYX7RTRYA7Y"
+ADDRESS="localhost:123"

ingest/ledgerbackend/toml.go

@@ -212,26 +212,19 @@ func unmarshalTreeNode(t *toml.Tree, key string, dest interface{}) error {
 	return tree.Unmarshal(dest)
 }
 
-func (c *CaptiveCoreToml) unmarshal(data []byte) error {
-	var body captiveCoreTomlValues
+func (c *CaptiveCoreToml) unmarshal(data []byte, strict bool) error {
 	quorumSetEntries := map[string]QuorumSet{}
 	historyEntries := map[string]History{}
 	// The toml library has trouble with nested tables so we need to flatten all nested
 	// QUORUM_SET and HISTORY tables as a workaround.
 	// In Marshal() we apply the inverse process to unflatten the nested tables.
 	flattened, tablePlaceholders := flattenTables(string(data), []string{"QUORUM_SET", "HISTORY"})
 
-	data = []byte(flattened)
 	tree, err := toml.Load(flattened)
 	if err != nil {
 		return err
 	}
 
-	err = toml.NewDecoder(bytes.NewReader(data)).Decode(&body)
-	if err != nil {
-		return err
-	}
-
 	for _, key := range tree.Keys() {
 		originalKey, ok := tablePlaceholders.get(key)
 		if !ok {
@@ -252,6 +245,24 @@ func (c *CaptiveCoreToml) unmarshal(data []byte) error {
 			}
 			historyEntries[key] = h
 		}
+		if err = tree.Delete(key); err != nil {
+			return err
+		}
+	}
+
+	var body captiveCoreTomlValues
+	if withoutPlaceHolders, err := tree.Marshal(); err != nil {
+		return err
+	} else if err = toml.NewDecoder(bytes.NewReader(withoutPlaceHolders)).Strict(strict).Decode(&body); err != nil {
+		if message := err.Error(); strings.HasPrefix(message, "undecoded keys") {
+			return fmt.Errorf(strings.Replace(
+				message,
+				"undecoded keys",
+				"these fields are not supported by captive core",
+				1,
+			))
+		}
+		return err
 	}
 
 	c.tree = tree
@@ -277,6 +288,8 @@ type CaptiveCoreTomlParams struct {
 	// LogPath is the (optional) path in which to store Core logs, passed along
 	// to Stellar Core's LOG_FILE_PATH.
 	LogPath *string
+	// Strict is a flag which, if enabled, rejects Stellar Core toml fields which are not supported by captive core.
+	Strict bool
 }
 
 // NewCaptiveCoreTomlFromFile constructs a new CaptiveCoreToml instance by merging configuration
@@ -288,7 +301,7 @@ func NewCaptiveCoreTomlFromFile(configPath string, params CaptiveCoreTomlParams)
 		return nil, errors.Wrap(err, "could not load toml path")
 	}
 
-	if err = captiveCoreToml.unmarshal(data); err != nil {
+	if err = captiveCoreToml.unmarshal(data, params.Strict); err != nil {
 		return nil, errors.Wrap(err, "could not unmarshal captive core toml")
 	}
 
@@ -330,7 +343,7 @@ func (c *CaptiveCoreToml) clone() (*CaptiveCoreToml, error) {
 		return nil, errors.Wrap(err, "could not marshal toml")
 	}
 	var cloned CaptiveCoreToml
-	if err = cloned.unmarshal(data); err != nil {
+	if err = cloned.unmarshal(data, false); err != nil {
 		return nil, errors.Wrap(err, "could not unmarshal captive core toml")
 	}
 	return &cloned, nil

ingest/ledgerbackend/toml_test.go

@@ -176,6 +176,15 @@ func TestCaptiveCoreTomlValidation(t *testing.T) {
 			expectedError: "invalid captive core toml: found invalid validator entry which is missing " +
 				"a QUALITY value: sdf_testnet_2",
 		},
+		{
+			name:              "field not supported by captive core",
+			networkPassphrase: "Public Global Stellar Network ; September 2015",
+			appendPath:        filepath.Join("testdata", "invalid-captive-core-field.cfg"),
+			httpPort:          nil,
+			peerPort:          nil,
+			logPath:           nil,
+			expectedError:     "could not unmarshal captive core toml: these fields are not supported by captive core: [\"CATCHUP_RECENT\"]",
+		},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {
 			params := CaptiveCoreTomlParams{
@@ -184,6 +193,7 @@ func TestCaptiveCoreTomlValidation(t *testing.T) {
 				HTTPPort:           testCase.httpPort,
 				PeerPort:           testCase.peerPort,
 				LogPath:            testCase.logPath,
+				Strict:             true,
 			}
 			_, err := NewCaptiveCoreTomlFromFile(testCase.appendPath, params)
 			assert.EqualError(t, err, testCase.expectedError)
@@ -228,6 +238,15 @@ func TestGenerateConfig(t *testing.T) {
 			peerPort:     newUint(12345),
 			logPath:      nil,
 		},
+		{
+			name:         "online config with unsupported field in appendix",
+			mode:         stellarCoreRunnerModeOnline,
+			appendPath:   filepath.Join("testdata", "invalid-captive-core-field.cfg"),
+			expectedPath: filepath.Join("testdata", "expected-online-core.cfg"),
+			httpPort:     newUint(6789),
+			peerPort:     newUint(12345),
+			logPath:      nil,
+		},
 		{
 			name:         "online config with no peer port",
 			mode:         stellarCoreRunnerModeOnline,
@@ -274,6 +293,7 @@ func TestGenerateConfig(t *testing.T) {
 				HTTPPort:           testCase.httpPort,
 				PeerPort:           testCase.peerPort,
 				LogPath:            testCase.logPath,
+				Strict:             false,
 			}
 			if testCase.appendPath != "" {
 				captiveCoreToml, err = NewCaptiveCoreTomlFromFile(testCase.appendPath, params)

services/horizon/CHANGELOG.md

@@ -15,7 +15,9 @@ 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)).
+* Fix bug in `horizon db reingest range` command which required the `--ingest` flag to be set ([3625](https://github.com/stellar/go/pull/3625)).
+
+* Deprecate `--captive-core-config-append-path` in favor of `--captive-core-config-path`. The difference between the two flags is that `--captive-core-config-path` will validate the configuration file to reject any fields which are not supported by captive core ([3629](https://github.com/stellar/go/pull/3629)).
 
 * Add Multiplexed Account details to API responses (additional `_muxed` and `_muxed_id` optional fields following what's described in [SEP 23](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0023.md#horizon-api-changes)):
   * Transactions: `account_muxed`, `account_muxed_id`, `fee_account` and `fee_account_muxed`.

services/horizon/internal/flags.go

@@ -28,8 +28,11 @@ const (
 	StellarCoreURLFlagName = "stellar-core-url"
 	// StellarCoreBinaryPathName is the command line flag for configuring the path to the stellar core binary
 	StellarCoreBinaryPathName = "stellar-core-binary-path"
-	// CaptiveCoreConfigAppendPathName is the command line flag for configuring the path to the captive core additional configuration
-	CaptiveCoreConfigAppendPathName = "captive-core-config-append-path"
+	// captiveCoreConfigAppendPathName is the command line flag for configuring the path to the captive core additional configuration
+	// Note captiveCoreConfigAppendPathName is deprecated in favor of CaptiveCoreConfigPathName
+	captiveCoreConfigAppendPathName = "captive-core-config-append-path"
+	// CaptiveCoreConfigPathName is the command line flag for configuring the path to the captive core configuration file
+	CaptiveCoreConfigPathName = "captive-core-config-path"
 
 	captiveCoreMigrationHint = "If you are migrating from Horizon 1.x.y read the Migration Guide here: https://github.com/stellar/go/blob/master/services/horizon/internal/docs/captive_core.md"
 )
@@ -118,12 +121,39 @@ func Flags() (*Config, support.ConfigOptions) {
 			ConfigKey:   &config.RemoteCaptiveCoreURL,
 		},
 		&support.ConfigOption{
-			Name:        CaptiveCoreConfigAppendPathName,
+			Name:        captiveCoreConfigAppendPathName,
 			OptType:     types.String,
 			FlagDefault: "",
 			Required:    false,
-			Usage:       "path to additional configuration for the Stellar Core configuration file used by captive core. It must, at least, include enough details to define a quorum set",
-			ConfigKey:   &config.CaptiveCoreConfigPath,
+			Usage:       "DEPRECATED in favor of " + CaptiveCoreConfigPathName,
+			CustomSetValue: func(opt *support.ConfigOption) {
+				if val := viper.GetString(opt.Name); val != "" {
+					if viper.GetString(CaptiveCoreConfigPathName) != "" {
+						stdLog.Printf(
+							"both --%s and --%s are set. %s is deprecated so %s will be used instead",
+							captiveCoreConfigAppendPathName,
+							CaptiveCoreConfigPathName,
+							captiveCoreConfigAppendPathName,
+							CaptiveCoreConfigPathName,
+						)
+					} else {
+						config.CaptiveCoreConfigPath = val
+					}
+				}
+			},
+		},
+		&support.ConfigOption{
+			Name:        CaptiveCoreConfigPathName,
+			OptType:     types.String,
+			FlagDefault: "",
+			Required:    false,
+			Usage:       "path to the configuration file used by captive core. It must, at least, include enough details to define a quorum set. Any fields in the configuration file which are not supported by captive core will be rejected.",
+			CustomSetValue: func(opt *support.ConfigOption) {
+				if val := viper.GetString(opt.Name); val != "" {
+					config.CaptiveCoreConfigPath = val
+					config.CaptiveCoreTomlParams.Strict = true
+				}
+			},
 		},
 		&support.ConfigOption{
 			Name:        "enable-captive-core-ingestion",
@@ -489,9 +519,6 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 				}
 			}
 
-			// When running live ingestion a config file is required too
-			validateBothOrNeither(StellarCoreBinaryPathName, CaptiveCoreConfigAppendPathName)
-
 			// NOTE: If both of these are set (regardless of user- or PATH-supplied
 			//       defaults for the binary path), the Remote Captive Core URL
 			//       takes precedence.
@@ -503,7 +530,7 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 			if config.RemoteCaptiveCoreURL == "" && (binaryPath == "" || config.CaptiveCoreConfigPath == "") {
 				if options.RequireCaptiveCoreConfig {
 					stdLog.Fatalf("Invalid config: captive core requires that both --%s and --%s are set. %s",
-						StellarCoreBinaryPathName, CaptiveCoreConfigAppendPathName, captiveCoreMigrationHint)
+						StellarCoreBinaryPathName, CaptiveCoreConfigPathName, captiveCoreMigrationHint)
 				} else {
 					var err error
 					config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs
@@ -532,8 +559,12 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 		}
 	} else {
 		if config.EnableCaptiveCoreIngestion && (config.CaptiveCoreBinaryPath != "" || config.CaptiveCoreConfigPath != "") {
+			captiveCoreConfigFlag := captiveCoreConfigAppendPathName
+			if viper.GetString(CaptiveCoreConfigPathName) != "" {
+				captiveCoreConfigFlag = CaptiveCoreConfigPathName
+			}
 			stdLog.Fatalf("Invalid config: one or more captive core params passed (--%s or --%s) but --ingest not set. "+captiveCoreMigrationHint,
-				StellarCoreBinaryPathName, CaptiveCoreConfigAppendPathName)
+				StellarCoreBinaryPathName, captiveCoreConfigFlag)
 		}
 		if config.StellarCoreDatabaseURL != "" {
 			stdLog.Fatalf("Invalid config: --%s passed but --ingest not set. ", StellarCoreDBURLFlagName)

services/horizon/internal/integration/db_test.go

@@ -98,7 +98,7 @@ func TestReingestDB(t *testing.T) {
 		horizonConfig.StellarCoreDatabaseURL,
 		"--stellar-core-binary-path",
 		horizonConfig.CaptiveCoreBinaryPath,
-		"--captive-core-config-append-path",
+		"--captive-core-config-path",
 		horizonConfig.CaptiveCoreConfigPath,
 		"--enable-captive-core-ingestion=" + strconv.FormatBool(horizonConfig.EnableCaptiveCoreIngestion),
 		"--network-passphrase",

services/horizon/internal/test/integration/integration.go

@@ -213,7 +213,7 @@ of accounts, subscribe to event streams, and more.`,
 
 		"--stellar-core-binary-path",
 		captiveCoreBinaryPath,
-		"--captive-core-config-append-path",
+		"--captive-core-config-path",
 		captiveCoreConfigPath,
 
 		"--captive-core-http-port",