clients/horizonclient: support dynamic effects #1217

poliha · 2019-05-01T08:03:20Z

If you're making a doc PR or something tiny where the below is irrelevant, just delete this template and use a short description.

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

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

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 monly a patch change. The PR is targeted at the next release branch if it's not a patch change.

Summary

This PR adds support for providing dynamic effects responses. This PR closes #1022 .

Goal and scope

The aim is for the horizonclient package to be able to decode effects returned by the horizon server into its concrete structs. Doing this will provide access to fields that are specific to the effects returned.
Before this PR, users only have assess to the Base fields(Account, PagingToken, Type...)

Summary of changes

Added Effect interface and EffectsPage
Added custom unmarshal method for effects page
Added a list of effect types.
Updated tests to use the new effects response

Known limitations & issues

Well not really a limitation, but I had to copy the EffectTypes from the internal db package. See note in protocols/horizon/effects
Users that want to access fields of the concrete effect type will need to first know the type using the GetType() method, then use type assertion on the returned effect. I have added examples and tests showing how to do this.

ire-and-curses

Just a few comments.

ire-and-curses · 2019-05-02T17:58:02Z

protocols/horizon/effects/main.go

 "time"

 "github.com/stellar/go/protocols/horizon/base"
 "github.com/stellar/go/support/render/hal"
 )

+// Peter 30-04-2019: this is copied from the history package "github.com/stellar/go/services/horizon/internal/db2/history"
+// Could not import this because internal package imports must share the same path prefix as the importer.
+// Maybe this should be housed here and imported into internal packages?


Yeah let's make an issue to discuss moving this out of internal. It definitely shouldn't be duplicated so would be good to get agreement and then clean up in a later PR

ire-and-curses · 2019-05-02T17:58:44Z

protocols/horizon/effects/main.go

@@ -135,6 +268,181 @@ type Trade struct {
 BoughtAssetIssuer string `json:"bought_asset_issuer,omitempty"`
 }

+// Effect contains methods that is implemented by all effect types


typo "is" -> "are"

ire-and-curses · 2019-05-02T17:59:37Z

protocols/horizon/effects/main.go

+ return b.Account
+}
+
+// EffectsPage contains page of effects returned by Horizon.


Do we have any effects doc we could link to on the website? No worries if not

* update effects in protocols/horizon * update client interface * update tests * update changelogs * fix typo

* horizonclient/txnbuild README fixes (#1210) * fix client links in top-level readme * update clients README * mark old client deprecated in godoc short summary * fix code of conduct, standardise example * fix code of conduct/contributing links * txnbuild: enables multiple signatures (#1198) This PR enables multiple signatures on a transaction in the new Go SDK. It also lets every `Operation` type have a different source account than its `Transaction`. These changes are intertwined. Without multiple signatures, every operation in a transaction must share the transaction's source account. Differing source accounts are the most common use case for multiple signatures, and they also test it with greatest completeness. * root repo changelog links to sub-projects (#1214) * keystore: add update-keys endpoints to spec (#1215) We need an endpoint to update the encrypted seed when users forget their passwords. * move keystore to exp to fix build (#1223) * Add minimal files to fix build (#1225) * exp/ticker: Orderbook data support (#1193) * exp/ticker: update partial market query to also output base/counter asset types * exp/ticker: add functions to scrape orderbook data * exp/ticker: update orderbookstats param names to match market standards * exp/ticker: create orderbookstats database model and migrations * exp/ticker: consolidate upsert logic and implement orderbookstats upsert * exp/ticker: create CLI command to ingest orderbooks * exp/ticker: update horizon SDK path * exp/ticker: add orderbook stats to market.json generator * exp/ticker: add separate query to retrieve relevant markets * exp/ticker: add orderbook stats to graphql interface * exp/ticker: add Docker support for orderbook ingestion * exp/ticker: fix partial aggregated market query + add tests * exp/ticker: fix globall aggregated market query + add tests * exp/ticker: update ticker binary link and fix crontab comment * exp/ticker: fix issue that would enable inf values to be stored in db * exp/ticker: ensure only store valid orderbook entries are stored * exp/ticker: format gql/static/bindata.go to prevent CI errors * exp/ticker: add updated bindata with pg 9.5-compatible migrations * exp/ticker: ensure markets with 0 orderbook entries can be handled by tickerdb * exp/ticker: remove unnecessary commas from graphql schema * exp/ticker: consolidate bid and ask positions on field names * exp/ticker: simplify logic for creating orderbook requests * exp/ticker: fix how bid and ask volumes are calculated * exp/ticker: allow negative spread values * exp/ticker: fix close_time fallback on markets query * exp/ticker: add page size limit to orderbook requests * Make PR template a bit more helpful. (#1238) * Make PR template a bit more helpful. Clean up some typos, and point to an example docs folder in the go repo rather than just the external docs hosted on our site. * Fix typo. * Agh, formatting. * horizonclient: update Fund method (#1213) * update client.Fund * update change log * fix space * horizonclient: add more documentation (#1226) * Remove old stream method * more comments * fix format * split comment * horizonclient: Set default HTTP client (#1228) * set default HTTP client * changes from review * changelog for txnbuild 1.1 (#1231) * horizonclient/txnbuild README fixes (#1210) * fix client links in top-level readme * update clients README * mark old client deprecated in godoc short summary * fix code of conduct, standardise example * fix code of conduct/contributing links * txnbuild: enables multiple signatures (#1198) This PR enables multiple signatures on a transaction in the new Go SDK. It also lets every `Operation` type have a different source account than its `Transaction`. These changes are intertwined. Without multiple signatures, every operation in a transaction must share the transaction's source account. Differing source accounts are the most common use case for multiple signatures, and they also test it with greatest completeness. * root repo changelog links to sub-projects (#1214) * keystore: add update-keys endpoints to spec (#1215) We need an endpoint to update the encrypted seed when users forget their passwords. * move keystore to exp to fix build (#1223) * Add minimal files to fix build (#1225) * changelog for txnbuild 1.1 * update change log (#1233) * horizonclient: add version (#1229) * add package version * fix go fmt * clients/horizonclient: support dynamic effects (#1217) * update effects in protocols/horizon * update client interface * update tests * update changelogs * fix typo

* update effects in protocols/horizon * update client interface * update tests * update changelogs * fix typo

poliha added 4 commits May 1, 2019 08:38

update effects in protocols/horizon

2a22f7d

update client interface

b388175

update tests

2965f99

update changelogs

95a8e82

poliha requested review from bartekn and ire-and-curses May 1, 2019 08:03

poliha added the horizonclient tag for new horizon client located in clients/horizonclient label May 1, 2019

poliha requested review from nikhilsaraf and tomquisel May 2, 2019 13:05

ire-and-curses approved these changes May 2, 2019

View reviewed changes

poliha and others added 2 commits May 2, 2019 19:33

fix typo

ca62dd2

Merge branch 'release-horizonclient-v1.1.0' into horizonclient-effects

aa2ae1f

ire-and-curses merged commit 0f63acf into release-horizonclient-v1.1.0 May 2, 2019

ire-and-curses deleted the horizonclient-effects branch May 2, 2019 20:31

poliha added a commit that referenced this pull request May 3, 2019

clients/horizonclient: support dynamic effects (#1217)

66da335

* update effects in protocols/horizon * update client interface * update tests * update changelogs * fix typo

bartekn pushed a commit that referenced this pull request May 6, 2019

clients/horizonclient: support dynamic effects (#1217)

8b597e5

* update effects in protocols/horizon * update client interface * update tests * update changelogs * fix typo

This was referenced May 6, 2019

exp/clients/horizon: return dynamic effects #1022

Closed

EffectsPage struct doesn't have links #1202

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clients/horizonclient: support dynamic effects #1217

clients/horizonclient: support dynamic effects #1217

poliha commented May 1, 2019 •

edited

Loading

ire-and-curses left a comment

ire-and-curses May 2, 2019

ire-and-curses May 2, 2019

ire-and-curses May 2, 2019

clients/horizonclient: support dynamic effects #1217

clients/horizonclient: support dynamic effects #1217

Conversation

poliha commented May 1, 2019 • edited Loading

PR Structure

Thoroughness

Release planning

Summary

Goal and scope

Summary of changes

Known limitations & issues

ire-and-curses left a comment

Choose a reason for hiding this comment

ire-and-curses May 2, 2019

Choose a reason for hiding this comment

ire-and-curses May 2, 2019

Choose a reason for hiding this comment

ire-and-curses May 2, 2019

Choose a reason for hiding this comment

poliha commented May 1, 2019 •

edited

Loading