services/horizon: Add optional configuration parameter called NETWORK #4949

urvisavla · 2023-07-10T06:11:50Z

PR Checklist

PR Structure

This PR has reasonably narrow scope (if not, break it down into smaller PRs).
This PR avoids mixing refactoring changes with feature changes (split into two PRs
otherwise).
This PR's title starts with name of package that is most changed in the PR, ex.
services/friendbot, or all or doc if the changes are broad or impact many
packages.

Thoroughness

This PR adds tests for the most critical parts of the new functionality or fixes.
I've updated any docs (developer docs, .md
files, etc... affected by this change). Take a look in the docs folder for a given service,
like this one.

Release planning

I've updated the relevant CHANGELOG (here for Horizon) if
needed with deprecations, added features, breaking changes, and DB schema changes.
I've decided if this PR requires a new major/minor version according to
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.

What

The PR introduces a new optional configuration parameter called NETWORK in Horizon. This parameter allows users to specify the desired Stellar network, pubnet or testnet. When the NETWORK parameter is set, Horizon automatically adjusts the remaining configuration settings and generates the corresponding captive core config file.

If the NETWORK parameter is specified, the following fields are automatically populated based on the chosen network:
- HISTORY_ARCHIVE_URLS
- NETWORK_PASSPHRASE
- CAPTIVE_CORE_CONFIG_PATH: The default configuration file for pubnet and testnet is embedded in the Horizon binary and used to generate the default configuration.
By default, CAPTIVE_CORE_STORAGE_PATH is set to the current directory where Horizon is running.
By default, STELLAR_CORE_URL is set to http://localhost:<CAPTIVE_CORE_HTTP_PORT>
CAPTIVE_CORE_STORAGE_PATH and STELLAR_CORE_URL fields are configured by default if not explicitly specified, regardless of whether the NETWORK parameter is specified or not.
If no NETWORK parameter is specified, the following fields become mandatory and an error message is displayed if not supplied:
- HISTORY_ARCHIVE_URLS
- NETWORK_PASSPHRASE
if the NETWORK parameter is specified and HISTORY_ARCHIVE_URLS or NETWORK_PASSPHRASE is supplied, an error message is displayed.

Why

Simplified Configuration: By specifying the NETWORK parameter, users no longer need to separately set the HISTORY_ARCHIVE_URLS, NETWORK_PASSPHRASE, and CAPTIVE_CORE_CONFIG_PATH. These fields are automatically populated based on the chosen network, streamlining the configuration process.

closes #4913

Known limitations

[N/A]

Shaptic

looks really good! much easier to follow the code, too 👏 just left some minor comments below

services/horizon/internal/config.go

services/horizon/internal/flags.go

services/horizon/internal/flags_test.go

services/horizon/internal/flags.go

tamirms

looks good to me!

services/horizon/docker/docker-compose.standalone.yml

services/horizon/internal/configs/captive-core-standalone.cfg

- code refactoring - add unit tests

…ded in the horizon binary

- Added multiple HISTORY_ARCHIVE_URLS - Added comments for exported variables

Remove warning when running Horizon with default pubnet config as it is the recommended way of running.

services/horizon/internal/flags.go

sreuland

looks good, I left a question on establishing some new integration test coverage for network and some minor refactor for network constants and method names for consideration, thanks!

urvisavla · 2023-07-18T03:36:43Z

looks good, I left a question on establishing some new integration test coverage for network and some minor refactor for network constants and method names for consideration, thanks!

I've updated network constants and method names as suggested and created an issue for new integration tests #4967

* Bump golang.org/x/text from 0.3.7 to 0.3.8 Bumps [golang.org/x/text](https://github.com/golang/text) from 0.3.7 to 0.3.8. - [Release notes](https://github.com/golang/text/releases) - [Commits](golang/text@v0.3.7...v0.3.8) --- updated-dependencies: - dependency-name: golang.org/x/text dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * all: enforce simplified Golang code (#4852) * Update completed sprint on issue/pr closed (#4857) * Bump core image to latest stable release v19.10.0 * Add a simple test for asset case sorting in ascii (#4876) * services/horizon: Suppress Core timeout error (#4860) Suppress Core timeout error when ingestion state machine is in build state. * Update CHANGELOG.md for latest release (#4828) * Bump core image to latest release v19.11.0 (#4885) * services/horizon: Protect 'currentState' variable using Mutex to prevent race condition. (#4889) * services/horizon: Update default for --captive-core-use-db to true (#4877) * 4856: Update default for --captive-core-use-db to true * Update CHANGELOG.md * services/horizon: Improve error handling for when stellar-core crashes (#4893) * Parse LIMIT_TX_QUEUE_SOURCE_ACCOUNT in core config * updated changelog for 2.26.0 release notes * Pinning and updates golang and ubuntu images * services/horizon: Fix ledger endpoint url in HAL (#4928) * Goreplay middleware (#4932) * tools/goreplay-middleware: Add goreplay middleware * Fix linter errors --------- Co-authored-by: Bartek Nowotarski <bartek@nowotarski.info> * all: Fix improper use of errors.Wrap (#4926) * all: Fix improper use of errors.Wrap `errors.Wrap` method returns nil if the first argument passed is also nil. If `errors.Wrap` is copied from a condition like `if err != nil` to another one which also returns `errors.Wrap` but does not overwrite `err` before the returned value will always be `nil`. * Update services/horizon/internal/db2/history/claimable_balances.go Co-authored-by: George <Shaptic@users.noreply.github.com> --------- Co-authored-by: George <Shaptic@users.noreply.github.com> Co-authored-by: Tsachi Herman <24438559+tsachiherman@users.noreply.github.com> * fix apt repo reference to focal now (#4929) * services/horizon: Remove command line flag --remote-captive-core-url (#4940) * Small changes - 1 * Remove ingestion filtering flag * services/horizon: Fix Horizon connectivity to core in standalone docker (#4956) The default value of STELLAR_CORE_URL (localhost:11626) in standalone network mode doesn't work. We need to explictly set STELLAR_CORE_URL to http://host.docker.internal:11626, to allow Horizon to access the host container's port to connect with the core container. * Bump core image to latest release v19.12.0 (#4953) * Add new function HideFlag * Fix lint warnings * services/horizon: Add optional configuration parameter NETWORK (#4949) The PR introduces a new optional Horizon configuration parameter called NETWORK. This parameter allows users to specify the desired Stellar network, pubnet or testnet. When the NETWORK parameter is set, Horizon automatically adjusts the remaining configuration settings and generates the corresponding captive core config file. * Update flags.go * Add IsHidden variable * Update IsHidden * Remove individual IsHidden option * Change to Hidden * Add tests for ingestion-filtering cmd flag * Make changes - 1 * Make changes - 2 * Remove race condition in test * Update command_line_args_test.go * Update command_line_args_test.go * Update command_line_args_test.go * Update command_line_args_test.go * Services/horizon: Skip querying stellar-core on 127.0.0.1 when Horizon is in build state (#4977) * Resolve Horizon CI failures caused by the failure to install the right version of libc++ (#4986) * Update command_line_args_test.go Update command_line_args_test.go Remove command_line_args_test.go * Extend timeout for integration tests --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Paul Bellamy <paul@stellar.org> Co-authored-by: Mehmet <119539688+mbsdf@users.noreply.github.com> Co-authored-by: mlo <marta.lohova@gmail.com> Co-authored-by: urvisavla <urvi.savla@stellar.org> Co-authored-by: stellarsaur <126507441+stellarsaur@users.noreply.github.com> Co-authored-by: Molly Karcher <molly@stellar.org> Co-authored-by: Shawn Reuland <shawn@stellar.org> Co-authored-by: shawn <sreuland@users.noreply.github.com> Co-authored-by: Bartek Nowotarski <bartek@nowotarski.info> Co-authored-by: George <Shaptic@users.noreply.github.com> Co-authored-by: Tsachi Herman <24438559+tsachiherman@users.noreply.github.com> Co-authored-by: Aditya Vyas <aditya.vyas@stellar.org> Co-authored-by: Aditya Vyas <adityavyas17@gmail.com>

network/main.go

urvisavla force-pushed the 4913/network-config-param branch 2 times, most recently from 997de0e to 9aa7849 Compare July 10, 2023 16:40

urvisavla changed the title ~~4913/network config param~~ services/horizon: Add optional configuration parameter called NETWORK Jul 10, 2023

urvisavla marked this pull request as ready for review July 10, 2023 18:01

Shaptic reviewed Jul 10, 2023

View reviewed changes

urvisavla force-pushed the 4913/network-config-param branch from 1e6c330 to 3b6aa7a Compare July 10, 2023 21:03

tamirms reviewed Jul 11, 2023

View reviewed changes

services/horizon/internal/flags.go Outdated Show resolved Hide resolved

tamirms approved these changes Jul 11, 2023

View reviewed changes

urvisavla requested a review from sreuland July 11, 2023 21:28

urvisavla self-assigned this Jul 11, 2023

urvisavla force-pushed the 4913/network-config-param branch from af5e7ae to fd6eb9c Compare July 12, 2023 18:00

sreuland reviewed Jul 12, 2023

View reviewed changes

services/horizon/docker/docker-compose.standalone.yml Outdated Show resolved Hide resolved

sreuland reviewed Jul 12, 2023

View reviewed changes

services/horizon/internal/configs/captive-core-standalone.cfg Outdated Show resolved Hide resolved

urvisavla added 9 commits July 12, 2023 14:35

Add new NETWORK param

0460620

- add usingDefaultPubnetConfig flag

12260ee

- code refactoring - add unit tests

Remove usingDefaultPubnetConfig. Use embedded default conf only

83fd555

Remove default conf files from GH release artifacts as it'll be embed…

9f7e63c

…ded in the horizon binary

Fix unit tests

913bbd7

Update CHANGELOG.md

aec873b

Update horizon docker containers to use NETWORK config parameter

d053bbd

Addressing review comments

faa38ca

Addresing review comments

c00f1e2

- Added multiple HISTORY_ARCHIVE_URLS - Added comments for exported variables

urvisavla force-pushed the 4913/network-config-param branch from aa36af7 to 7bbc11e Compare July 12, 2023 21:35

Removed default config for standalone.

89e0ff3

Remove warning when running Horizon with default pubnet config as it is the recommended way of running.

urvisavla force-pushed the 4913/network-config-param branch from 7bbc11e to 89e0ff3 Compare July 13, 2023 00:40

Fix linter error

a3d195f

urvisavla requested review from sreuland and Shaptic July 14, 2023 16:58

sreuland reviewed Jul 17, 2023

View reviewed changes

services/horizon/internal/flags.go Outdated Show resolved Hide resolved

sreuland reviewed Jul 17, 2023

View reviewed changes

services/horizon/internal/flags.go Outdated Show resolved Hide resolved

sreuland approved these changes Jul 17, 2023

View reviewed changes

Addressing review comments

5f87c42

urvisavla mentioned this pull request Jul 18, 2023

services/horizon: Add integration tests for testing default configuration using NETWORK parameter #4967

Closed

urvisavla merged commit 95c2976 into stellar:master Jul 18, 2023

urvisavla deleted the 4913/network-config-param branch July 18, 2023 06:19

tamirms mentioned this pull request Sep 27, 2023

services/horizon/internal: Fix configuration of captive core in reingest command #5069

Merged

7 tasks

jacekn reviewed Dec 14, 2023

View reviewed changes

network/main.go Show resolved Hide resolved

sreuland mentioned this pull request Dec 15, 2023

services/horizon/cmd: spike, options on how to obtain archive server list #5142

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

services/horizon: Add optional configuration parameter called NETWORK #4949

services/horizon: Add optional configuration parameter called NETWORK #4949

urvisavla commented Jul 10, 2023 •

edited

Loading

Shaptic left a comment

tamirms left a comment

sreuland left a comment

urvisavla commented Jul 18, 2023

services/horizon: Add optional configuration parameter called NETWORK #4949

services/horizon: Add optional configuration parameter called NETWORK #4949

Conversation

urvisavla commented Jul 10, 2023 • edited Loading

PR Structure

Thoroughness

Release planning

What

Why

Known limitations

Shaptic left a comment

Choose a reason for hiding this comment

tamirms left a comment

Choose a reason for hiding this comment

sreuland left a comment

Choose a reason for hiding this comment

urvisavla commented Jul 18, 2023

urvisavla commented Jul 10, 2023 •

edited

Loading