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

exp/client/horizon: support fee_stats and account offers #1003

Merged
merged 2 commits into from
Mar 14, 2019

Conversation

poliha
Copy link
Contributor

@poliha poliha commented Mar 13, 2019

This PR adds support for getting fee_stats and getting offers made by an account from horizon.
This closes #954

Copy link
Contributor

@bartekn bartekn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just minor comments.


// AccountOffers returns information about offers made by an account on the SDEX
// See https://www.stellar.org/developers/horizon/reference/endpoints/offers-for-account.html
func (c *Client) AccountOffers(request OfferRequest) (offers hProtocol.OffersPage, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change the name to Offers. I think we will add /offers endpoint in a future so this will be confusing and we have ForAccount field in OfferRequest anyway.

"p80_accepted_fee": "1225",
"p90_accepted_fee": "1225",
"p95_accepted_fee": "1225",
"p99_accepted_fee": "8000"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this response to have a different value for each field? Currently, typos like p10 instead of p20 won't be caught.

func (or OfferRequest) BuildUrl() (endpoint string, err error) {
endpoint = fmt.Sprintf(
"accounts/%s/offers",
or.ForAccount,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check if ForAccount is empty and error in such case (because /offers is not there yet).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already checked in client.AccountOffers()

@poliha poliha merged commit c434c13 into master Mar 14, 2019
@poliha poliha deleted the exp-horizonclient-feestats branch March 14, 2019 15:06
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.

exp/clients/horizon: get dynamic fees
2 participants