Feature Request: Add toml file specifically for captive core

[tamirms]

What problem does your feature solve?

The configuration space for the captive core append path file is not fully documented. Operators don't know what is possible to configure in captive core so they try to copy parts of the stellar-core.cfg file from their stand-alone stellar core instances into the captive core configuration. However, it's not clear which fields can and can't be set, which forces operators to go through a trial and error process of trying different combinations of fields until they get their setup to work.

What would you like to see?

We should create a restricted toml file just for captive core which is a subset of stellar-core.cfg.

The restricted toml file should be documented with an example cfg file just like the one in the stellar core repo: https://github.com/stellar/stellar-core/blob/master/docs/stellar-core_example.cfg

We should strive to make sure that configuration is not duplicated between horizon command line flags and the captive core toml file.

[ire-and-curses]

Let's discuss this. What are the pros and cons?

[Shaptic]

One of the cons we should definitely keep in mind is that by pulling in a TOML parser, we're now exposing a potential attack vector to malicious user input. I'm not sure we should worry about that much, given that if you can control the config file, you can control everything else about Horizon anyway, but it's still worth keeping in mind, especially given our recent discussions about security.

[tamirms]

If we have a separate toml file for captive core we should consider allowing LOG_FILE_PATH to be configured. If LOG_FILE_PATH is set we will store captive core logs in that location and remove captive core output from the horizon log stream. If LOG_FILE_PATH is not set then we can default to the current behavior of including captive core logs in the horizon log stream.

One point of duplication we need to figure out is how to deal with the configuration of history archive urls. History archive urls are part of horizon configuration and stellar core configuration. Ideally we should not have to configure both. But the difficulty is that stellar core's configuration is more expressive than horizon. It allows for specifying how core should obtain the history archives (e.g. curl -sf %s/{0} -o {1} or aws s3 cp s3://history.stellar.org/{0} {1})

[Shaptic]

After the discussion with ops, I think this has progressed into a far bigger, arguably meta-issue.

Should we push to make Horizon configured by toml files entirely, just like Stellar Core? Then, it will simply contain a subsection related to Captive Core configurations (while the rest will be a tomlification replacing Horizon's existing CLI / env params entirely). While this subsection may "look" just like Core's, its validity will be controlled by us entirely. This encourages our don't worry about Core's existence abstraction intended by Horizon 2.0 and Captive Core.

This transition will subsequently encompass all of the config-related issues, like #3437, #3438, #3441, and undoubtedly others that will pop up.

(Technically this breaking change would require releasing Horizon 3.0 which is a little odd, but that's the nature of semver.)

[bartekn]

(Technically this breaking change would require releasing Horizon 3.0 which is a little odd, but that's the nature of semver.)

This can be done in a non-breaking way (however requires some extra work): if config file param is set (--config-file maybe?) all of the other flags are ignored, if it's not set we read the flags using existing code.

Pros, cons and random thoughts:

• P: The same format (toml) is used for Stellar-Core and Horizon which are often used together.
• P: toml allows setting more complicated structures than command line flag. One example is history archives which is currently a comma separated string in Horizon.
• P: Avoids duplication in some cases like network passphrase (but sometimes impossible - read on).
• C: One time requirement for rewriting Horizon configuration to use a new format.
• C: This won't solve the usability issue: while it will be easier for users to create a new config (by copy&paste) they won't be able to use existing file without modifications.
• Some changes in Stellar-Core are required if we want to avoid duplication completely. For example: Horizon asks for plain URLs when configuring history archives, Stellar-Core requires users to set a command (like curl) that will be used to download the files. Because of this users will have to set two archive values anyway. I opened Consider processing buckets in memory in captive mode stellar-core#2942 hoping that we can parse the files in memory however it seems that Stellar-Core needs these files for bucket hash calculations after each ledger. I'm going to ask it's possible to add code that downloads the buckets instead of relying on external tool.

[Shaptic]

This can be done in a non-breaking way [...]

Good point. We can even consolidate the code to essentially make an in-memory TOML file from env/cli parameters, making it easier to drop the latter entirely in a future version and ensure we only have a single parsing / validation path. e.g.

User --> [CLI] ----+
User --> [ENV] ----|
                   |
User --> [TOML] <--+
            |
            v
         Horizon

This won't solve the usability issue: [...]

With proper parsing, we'll be able to advise users much better, e.g. these lines are ignored, this line is wrong, here's a Horizon-compatible toml, etc. which is still a big win over the current "Captive Core isn't running because of a bad config" state.

For example: Horizon asks for plain URLs when configuring history archives, Stellar-Core requires users to set a command [...]

Can we make it such that we essentially add an extra parameter to the Stellar-Core command in which we populate the URL, or would that be even more confusing? e.g. instead of the following:

HISTORY="curl -sf http://history.stellar.org/prd/core-testnet/core_testnet_001/{0} -o {1}"

We have

HISTORY_ARCHIVE_URL="http://history.stellar.org/prd/core-testnet/core_testnet_001"
HISTORY="curl -sf {0}/{1} -o {2}"

Unfortunately there are some assumptions built-in to the form of the HISTORY_ARCHIVE_URL in that case, so maybe this isn't a great approach. The cleanest approach is probably just keeping both around.