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 horizon v0.24.1 release branch #2048

Closed
wants to merge 31 commits into from

Conversation

tamirms
Copy link
Contributor

@tamirms tamirms commented Dec 12, 2019

In particular, I'd like to make sure #2047 is included in the horizon v0.24.1 release so we can test out the publish docker circleci job

leighmcculloch and others added 30 commits November 20, 2019 14:20
Update Federation's CHANGELOG to move features that were released in [Federation v0.2.1] into their own section, separate from unreleased features.

It looks like when we released [Federation v0.2.1] we forgot to update the CHANGELOG.

I'm making these changes to prepare the next release of Federation and discovered features in the unreleased section of its changelog that have been released.

[Federation v0.2.1]: https://github.com/stellar/go/releases/tag/federation-v0.2.1
…1.1 (#1959)

Update the Keystore CHANGELOG to signal that the current revision is v1.2.0.

Also, I re-ordered the versions so they are in descending order, like our other changelogs.

Why:
An important fix was made to Keystore and we should signal that the current version is a usable version of Keystore.

Known limitations:
It appears we haven't been tagging releases or releasing binaries of Keystore at the moment because our only deployment of it is continuously deployed commit-by-commit irrespective of versions. I think given the two fixes are noteworthy it's worth signaling them as v1.2.0 to encourage someone who might be happen to checking out the application to use a new version oft he application but I don't want to address the tagging or releasing in this release especially as this service is still really experimental and we're not using it as we had intended.
Set version in CHANGELOG for Friendbot v0.0.2.

Also added version for the initial release of [Friendbot v0.0.1] that was missing.

To release Friendbot v0.0.2.

[Friendbot v0.0.1]: https://github.com/stellar/go/releases/tag/friendbot-v0.0.1
Set version on CHANGELOG for Federation v0.3.0.

To release v0.3.0 of Federation.
Set version for the next release of Ticker, v1.2.0.

To release pending unreleased changes to Ticker.
…1966)

Update CHANGELOG of Bridge and Compliance with change introduced in 12f6d43.

To release Bridge and Compliance with the fix in the shared library.
#1947)

Removes unnecessary `panic`s that stop execution of the app and changes
the temp results folder name.

Very often developers run `horizon-cmp` over the night. It's important
that it continues running when unexpected scenarios occur. This commit
also changes the temp results folder name to be more readable (date vs
unix timestamp).
Change the beginning of the README to include the Stellar logo, and Stellar's mission.

Why:

At GitHub Universe one of the talks I attended talked about how creating an inviting and engaging README makes a huge difference to communicating to developers who are coming across the project. The [dev.to README] was given as an example of a great README that goes a long way to engage poeple who visit the project. I think there's probably a lot of things we can do to make our README more alive to someone visiting it, and adding our logo and including our mission is just one small thing.

This PR is a quick proof of concept intended to feel out do we want to make our README sparkle a little? Do we want to do something like this consistently across our major projects like stellar-core, stellar-protocol, js-stellar-sdk, and this repo?

Known limitations:

This change does nothing to improve the content of our READMEs which we could definitely do, but I think making a change like this is one small thing we can do to engage developers who visit our projects and to tell them what we're all here for.

[dev.to README]: https://github.com/thepracticaldev/dev.to/blob/master/README.md
Link the Stellar logo in the README to stellar.org.

I intended to do this when I added the logo in #1948 and forgot.
…1968)

* Initial commit: add current state processor, with debugging.

* Remove debugging code.

* Update main script flags.

* Remove comments about custom structs.

* Change account state mutator to factory, update processor, clean up flags.

* Tests - many done, a few still left, end of week commit.

* Actually committing tests.

* Add all tests.

* Rename files and factory.

* Pass static check.

* Split schema factory, add TODOs from review.
* Update horizon-cmp README.

* Update tools/horizon-cmp/README.md

Co-Authored-By: Bartek Nowotarski <[email protected]>
* Initial commit.

* Remove internal panics.
* Initial commit.

* Add default.

* Modify final message.

* Change function return value from error to boolean.
Change the link to the Compliance Protocol to be the new developers
guide link.

The old link resolves to a 404. Looks like the URL changed at some point
and we didn't update it here.
Merge release-horizon-v0.24.0 into master
* Initial commit.

* Refactor into one large success table test.

* Move account entry test around.

* Add failure testing outline, change makeAccountID.

* Add TODO for error cases.

* Move address definition to processor, remove unused tooling.

* Fix variable shadowing.
Also, do not override LATEST_LEDGER if the variable is already set
Add `keypair.ParseFull` function that behaves the same as
`keypair.Parse`, but returns a `keypair.Full` instead of a `keypair.KP`.

A couple times in my own coding I've found that my code knows that I
have a seed, not an address, and I need to get a `keypair.Full`. The
path to doing that is inconvenient because I first need to call
`keypair.Parse` to get back a `keypair.KP`, and then I need to cast that
to a `keypair.Full`.

The `keypair.Parse` function first attempts to decode as an address,
then as a seed. In cases where we know we have a seed it would be
convenient to be able to parse just expecting a full key.

Co-Authored-By: Adolfo Builes <[email protected]>
…2042)

Adds a `ConfigOptions` type that is a slice of `ConfigOptions`, and wraps up initializing and setting the values of all the options. Also updates Horizon to use those new functions.

Why:
I had a go of using the `ConfigOption` type and following the same pattern in Horizon in some new code and I lost a lot of time to a silly mistake when defining the slice of options that caused none of the options to ever be set (I forgot to use a pointer type). I think this code will help prevent that, but also this code reduces the repetitiveness of using this code in another application.

I think it's great that we have some code to help us setup flags and environment variables in a common format. I think this is one small thing we can do to improve this code.
Move the call to `viper.BindPFlags` from our services that use the `support/config` package, into the config package `Init` function.

Why:
The `support/config` package does a good job of wrapping up cobra and viper functionality so that an application only needs to define one set of config and by calling a few functions can have that config provided by flags and environment variables in a consistent manner. While the package does most of the work to configure all the moving parts, it doesn't call the BindPFlags function and relies on the app to call this. It's really easy to miss that the app has to do this one thing, and I lost a good amount of time trying to understand why none of my configs were being populated. The package owns setting viper up, and so it should own this part of the process too.

We're only using this on Horizon right now but this package looks like a good starting point for us to have a common pattern to configure our services with environment variables and flags. On some of our services this doesn't matter because the configs are all non-sensitive values, but as we write services that need keys making use of flags and environment variables consistently will be ideal.

@bartekn originally proposed this change here #834 (comment) and made the change in 75b8658. Before the PR was merged @ire-and-curses reverted the change in 2a8354b because the first commit had broken the setting of values, but as it turned out only some of the code needed to be reverted.
Because CircleCI does not build tags by default, any child jobs that rely on a job that only builds on tags will also require the tag filter. In this case, the hold and publish_latest_horizon_docker_image jobs will need the filter for /^horizon-v.*/ tags as well, otherwise, they will not be picked up for running.
@tamirms tamirms requested a review from bartekn December 12, 2019 18:17
@cla-bot cla-bot bot added the cla: yes label Dec 12, 2019
@bartekn
Copy link
Contributor

bartekn commented Dec 12, 2019

@tamirms can you cherry-pick this specific PR commit: #2047 and create a new PR? We're going to release 0.24.1 early next week so I'd like to make a set of changes minimal (and there are a lot of files included in Horizon that has changed recently in master).

Add a `FromAddress()` function to all `KP` implementers that returns the public key, or address only, version of the keypair.

I have an application that will use a Stellar account seed/key, and some of the handlers in that application need the full key, and some only need the address for verification. It's common to use variable types in code to communicate intent and to limit the code that is given a private key to reduce the risk in a bug accidentally exposing the key.

It's not convenient to load a full keypair and then to split it into its public and private components. The developer is required to get the address string, then reparse that address string. The full keypair should know how to get the address-only version.

This is common in the stdlib's crypto classes where private key types provide direct access to the public key.
@tamirms
Copy link
Contributor Author

tamirms commented Dec 12, 2019

closing this PR in favor of #2049

@tamirms tamirms closed this Dec 12, 2019
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.

6 participants