Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge master into 1.10.0 horizon release #3117

Closed
wants to merge 6 commits into from

Conversation

tamirms
Copy link
Contributor

@tamirms tamirms commented Oct 9, 2020

No description provided.

2opremio and others added 6 commits October 6, 2020 20:14
Containers and configurations should be allowed to set
`--stellar-core-binary-path` regardless of the behavior we want
from Horizon.

1. Do not assume that a non-empty `--stellar-core-binary-path` (or
   `--remote-captive-core-url`) implies captive core should be used
   (use the pre-existing `--enable-captive-core-ingestion` as a tollgate
   instead).
2. Allow setting both `--remote-captive-core-url` and `--stellar-core-binary-path` simultaneously, associating a higher priority to the URL.
… & flexibility. (#3090)

Refactors the sponsorship (CAP-33) integration tests:
 - Each test is independently runs as a sub-test of the  "TestSponsorships" test.
 - It introduces helper functions commonly used in each tests such as
   sandwiching sponsored operations, retrieving operations and effects, and
   confirming the existence of operations via `tt.Condition`.
 - `itest.MustGetAccount()` replaces instances of a manual force-retrieval of an
   account, and account/keypair management in general is simpler.
 - Running time is ~3x faster.

All of the actual _testing_ is still the same. Investigation on using 
`testing.T.Parallel` is in progress which should further improve running time.

Closes #3075.
To do so, you need to set the
`HORIZON_INTEGRATION_ENABLE_CAPTIVE_CORE` env variable
to any value (e.g. `true`).
### What
Remove the stellar.toml domain checks that occur on application start. Essentially revert the changes relating to this specific feature that were introduced in 5e82f16.

### Why
SEP-10 v2.0.0 highlighted an existing feature that the SIGNING_KEY field in the stellar.toml be the key that the SEP-10 server uses for signing. In 5e82f16 we added the feature that the server would verify that the stellar.toml that references the SEP-10 server. This feature is inconvenient because not all SEP-10 services have a stellar.toml. It's also scope creep on what this SEP-10 server implementation is concerned with.
The integration tests require that you have a compiled horizon binary present on the machine. The tests then copy that binary into the Stellar quickstart docker container.

This PR removes this step. Instead of relying on Horizon running within docker, the test setup will create a new Horizon app and start serving requests. In the test tear down, the Horizon app is shut down.
@cla-bot cla-bot bot added the cla: yes label Oct 9, 2020
@tamirms tamirms requested a review from a team October 9, 2020 17:31
@bartekn
Copy link
Contributor

bartekn commented Oct 9, 2020

Today is the last day of 1.10.0 testing and I'm not confident we should merge it. It would be great to check if refactoring didnt' break anything and it's possible that another one: #3118 will make it into a release branch too. Are there any commits we really want in 1.10.0, aa36914 I guess? Maybe we can cherry-pick instead?

@tamirms
Copy link
Contributor Author

tamirms commented Oct 9, 2020

@bartekn good point! I will close the PR

@tamirms tamirms closed this Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants