Skip to content

Commit

Permalink
services/horizon: Refactor captive-core config generation.
Browse files Browse the repository at this point in the history 
 - Rename RequireCaptiveCoreConfig to RequireCaptiveCoreFullConfig to clarify that it creates a partial captive-core config.
 - Handle RequireCaptiveCoreFullConfig correctly for all code paths, including with the network parameter.
 - Remove duplicate code.
  • Loading branch information
@urvisavla
urvisavla committed Oct 3, 2023
1 parent facabfc commit 7af23a4
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 93 deletions.
4 changes: 2 additions & 2 deletions services/horizon/cmd/db.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ var dbReingestRangeCmd = &cobra.Command{
}
}

err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true})
err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreFullConfig: false, AlwaysIngest: true})
if err != nil {
return err
}
Expand Down Expand Up @@ -369,7 +369,7 @@ var dbFillGapsCmd = &cobra.Command{
withRange = true
}

err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true})
err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreFullConfig: false, AlwaysIngest: true})
if err != nil {
return err
}
Expand Down
10 changes: 5 additions & 5 deletions services/horizon/cmd/ingest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ var ingestVerifyRangeCmd = &cobra.Command{
co.SetValue()
}

if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}); err != nil {
if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreFullConfig: false, AlwaysIngest: true}); err != nil {
return err
}

Expand Down Expand Up @@ -203,7 +203,7 @@ var ingestStressTestCmd = &cobra.Command{
co.SetValue()
}

if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}); err != nil {
if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreFullConfig: false, AlwaysIngest: true}); err != nil {
return err
}

Expand Down Expand Up @@ -267,7 +267,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",
RunE: func(cmd *cobra.Command, args []string) error {
ctx := context.Background()
if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}); err != nil {
if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreFullConfig: false, AlwaysIngest: true}); err != nil {
return err
}

Expand All @@ -291,7 +291,7 @@ var ingestInitGenesisStateCmd = &cobra.Command{
Short: "ingests genesis state (ledger 1)",
RunE: func(cmd *cobra.Command, args []string) error {
ctx := context.Background()
if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}); err != nil {
if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreFullConfig: false, AlwaysIngest: true}); err != nil {
return err
}

Expand Down Expand Up @@ -363,7 +363,7 @@ var ingestBuildStateCmd = &cobra.Command{
co.SetValue()
}

if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreConfig: false, AlwaysIngest: true}); err != nil {
if err := horizon.ApplyFlags(config, flags, horizon.ApplyOptions{RequireCaptiveCoreFullConfig: false, AlwaysIngest: true}); err != nil {
return err
}

Expand Down
128 changes: 47 additions & 81 deletions services/horizon/internal/flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,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, error) {
err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false})
err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreFullConfig: true, AlwaysIngest: false})
if err != nil {
return nil, err
}
Expand All @@ -663,8 +663,8 @@ func NewAppFromFlags(config *Config, flags support.ConfigOptions) (*App, error)
}

type ApplyOptions struct {
AlwaysIngest bool
RequireCaptiveCoreConfig bool
AlwaysIngest bool
RequireCaptiveCoreFullConfig bool
}

type networkConfig struct {
Expand Down Expand Up @@ -703,97 +703,31 @@ func getCaptiveCoreBinaryPath() (string, error) {
return result, nil
}

// loadDefaultCaptiveCoreToml loads the default Captive Core TOML based on the default config data.
func loadDefaultCaptiveCoreToml(config *Config, defaultConfigData []byte) error {

config.CaptiveCoreTomlParams.CoreBinaryPath = config.CaptiveCoreBinaryPath
config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs
config.CaptiveCoreTomlParams.NetworkPassphrase = config.NetworkPassphrase

var err error
config.CaptiveCoreToml, err = ledgerbackend.NewCaptiveCoreTomlFromData(defaultConfigData, config.CaptiveCoreTomlParams)
if err != nil {
return errors.Wrap(err, "invalid captive core toml")
}
return nil
}

// loadCaptiveCoreTomlFromFile loads the Captive Core TOML file from the specified path provided config.
func loadCaptiveCoreTomlFromFile(config *Config) error {
var err error

config.CaptiveCoreTomlParams.CoreBinaryPath = config.CaptiveCoreBinaryPath
config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs
config.CaptiveCoreTomlParams.NetworkPassphrase = config.NetworkPassphrase

config.CaptiveCoreToml, err = ledgerbackend.NewCaptiveCoreTomlFromFile(config.CaptiveCoreConfigPath, config.CaptiveCoreTomlParams)
if err != nil {
return errors.Wrap(err, "invalid captive core toml file")
}
return nil
}

// createCaptiveCoreConfigFromNetwork generates the default Captive Core configuration.
// validates the configuration settings, sets default values, and loads the Captive Core TOML file.
func createCaptiveCoreConfigFromNetwork(config *Config) error {
// getCaptiveCoreConfigFromNetworkParameter generates the default Captive Core configuration.
// validates the configuration settings, sets default values
func getCaptiveCoreConfigFromNetworkParameter(config *Config) (networkConfig, error) {
var defaultNetworkConfig networkConfig

if config.NetworkPassphrase != "" {
return fmt.Errorf("invalid config: %s parameter not allowed with the %s parameter", NetworkPassphraseFlagName, NetworkFlagName)
return defaultNetworkConfig, fmt.Errorf("invalid config: %s parameter not allowed with the %s parameter", NetworkPassphraseFlagName, NetworkFlagName)

Check failure on line 712 in services/horizon/internal/flags.go

View workflow job for this annotation

GitHub Actions / golangci 

line is 151 characters (lll)
}

if len(config.HistoryArchiveURLs) > 0 {
return fmt.Errorf("invalid config: %s parameter not allowed with the %s parameter", HistoryArchiveURLsFlagName, NetworkFlagName)
return defaultNetworkConfig, fmt.Errorf("invalid config: %s parameter not allowed with the %s parameter", HistoryArchiveURLsFlagName, NetworkFlagName)

Check failure on line 716 in services/horizon/internal/flags.go

View workflow job for this annotation

GitHub Actions / golangci 

line is 152 characters (lll)
}

var defaultNetworkConfig networkConfig
switch config.Network {
case StellarPubnet:
defaultNetworkConfig = PubnetConf
case StellarTestnet:
defaultNetworkConfig = TestnetConf
default:
return fmt.Errorf("no default configuration found for network %s", config.Network)
return defaultNetworkConfig, fmt.Errorf("no default configuration found for network %s", config.Network)
}
config.NetworkPassphrase = defaultNetworkConfig.NetworkPassphrase
config.HistoryArchiveURLs = defaultNetworkConfig.HistoryArchiveURLs

if config.CaptiveCoreConfigPath == "" {
return loadDefaultCaptiveCoreToml(config, defaultNetworkConfig.defaultConfig)
}

return loadCaptiveCoreTomlFromFile(config)
}

// createCaptiveCoreConfigFromParameters generates the Captive Core configuration.
// validates the configuration settings, sets necessary values, and loads the Captive Core TOML file.
func createCaptiveCoreConfigFromParameters(config *Config, options ApplyOptions) error {

if config.NetworkPassphrase == "" {
return fmt.Errorf("%s must be set", NetworkPassphraseFlagName)
}

if len(config.HistoryArchiveURLs) == 0 {
return fmt.Errorf("%s must be set", HistoryArchiveURLsFlagName)
}

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 {
return errors.Wrap(err, "invalid captive core toml file")
}
}

return nil
return defaultNetworkConfig, nil
}

// setCaptiveCoreConfiguration prepares configuration for the Captive Core
Expand All @@ -809,16 +743,48 @@ func setCaptiveCoreConfiguration(config *Config, options ApplyOptions) error {
}
}

var defaultNetworkConfig networkConfig
if config.Network != "" {
err := createCaptiveCoreConfigFromNetwork(config)
var err error
defaultNetworkConfig, err = getCaptiveCoreConfigFromNetworkParameter(config)
if err != nil {
return err
}
} else {
err := createCaptiveCoreConfigFromParameters(config, options)
}

if config.NetworkPassphrase == "" {
return fmt.Errorf("%s must be set", NetworkPassphraseFlagName)
}

if len(config.HistoryArchiveURLs) == 0 {
return fmt.Errorf("%s must be set", HistoryArchiveURLsFlagName)
}

config.CaptiveCoreTomlParams.CoreBinaryPath = config.CaptiveCoreBinaryPath
config.CaptiveCoreTomlParams.HistoryArchiveURLs = config.HistoryArchiveURLs
config.CaptiveCoreTomlParams.NetworkPassphrase = config.NetworkPassphrase

var err error
if options.RequireCaptiveCoreFullConfig == false {

Check failure on line 768 in services/horizon/internal/flags.go

View workflow job for this annotation

GitHub Actions / golangci 

S1002: should omit comparison to bool constant, can be simplified to `!options.RequireCaptiveCoreFullConfig` (gosimple)

Check failure on line 768 in services/horizon/internal/flags.go

View workflow job for this annotation

GitHub Actions / check (ubuntu-22.04, 1.20) 

should omit comparison to bool constant, can be simplified to !options.RequireCaptiveCoreFullConfig (S1002)
// Creates a minimal captive-core config (without quorum information), just enough to run captive core.
// This is used by certain database commands, such as `reingest and fill-gaps, to reingest historical data.
config.CaptiveCoreToml, err = ledgerbackend.NewCaptiveCoreToml(config.CaptiveCoreTomlParams)
if err != nil {
return err
return errors.Wrap(err, "invalid captive core toml file")
}
} else if config.CaptiveCoreConfigPath != "" {
config.CaptiveCoreToml, err = ledgerbackend.NewCaptiveCoreTomlFromFile(config.CaptiveCoreConfigPath, config.CaptiveCoreTomlParams)
if err != nil {
return errors.Wrap(err, "invalid captive core toml file")
}
} else if len(defaultNetworkConfig.defaultConfig) != 0 {
config.CaptiveCoreToml, err = ledgerbackend.NewCaptiveCoreTomlFromData(defaultNetworkConfig.defaultConfig, config.CaptiveCoreTomlParams)
if err != nil {
return errors.Wrap(err, "invalid captive core toml file")
}
} else {
return fmt.Errorf("invalid config: captive core requires that --%s is set or you can set the --%s "+
"parameter to use the default captive core config", CaptiveCoreConfigPathName, NetworkFlagName)
}

// If we don't supply an explicit core URL and running captive core process with the http port enabled,
Expand Down
10 changes: 5 additions & 5 deletions services/horizon/internal/flags_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func Test_createCaptiveCoreDefaultConfig(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := createCaptiveCoreConfigFromNetwork(&tt.config)
_, e := getCaptiveCoreConfigFromNetworkParameter(&tt.config)
if tt.errStr == "" {
assert.NoError(t, e)
assert.Equal(t, tt.networkPassphrase, tt.config.NetworkPassphrase)
Expand Down Expand Up @@ -128,8 +128,8 @@ func Test_createCaptiveCoreConfig(t *testing.T) {
NetworkPassphrase: PubnetConf.NetworkPassphrase,
HistoryArchiveURLs: PubnetConf.HistoryArchiveURLs,
},
errStr: fmt.Sprintf("invalid config: captive core requires that --%s is set",
CaptiveCoreConfigPathName),
errStr: fmt.Sprintf("invalid config: captive core requires that --%s is set or "+
"you can set the --%s parameter to use the default captive core config", CaptiveCoreConfigPathName, NetworkFlagName),
},
{
name: "no network specified; captive-core-config-path invalid file",
Expand Down Expand Up @@ -167,8 +167,8 @@ func Test_createCaptiveCoreConfig(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := createCaptiveCoreConfigFromParameters(&tt.config,
ApplyOptions{RequireCaptiveCoreConfig: tt.requireCaptiveCoreConfig})
e := setCaptiveCoreConfiguration(&tt.config,
ApplyOptions{RequireCaptiveCoreFullConfig: tt.requireCaptiveCoreConfig})
if tt.errStr == "" {
assert.NoError(t, e)
assert.Equal(t, tt.networkPassphrase, tt.config.CaptiveCoreTomlParams.NetworkPassphrase)
Expand Down

0 comments on commit 7af23a4

Please sign in to comment.