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

BugFix: Add CoinSupported configuration #295

Conversation

aeharvlee
Copy link

@aeharvlee aeharvlee commented Apr 4, 2022

Currently every check:construction tries to fetch coins but not every rosetta implementations support coin endpoint (e.g. rosetta-ethereum).

This PR Fixes #294

Motivation

  • It can solve current problem (always trying to fetch account/coins).

Solution

  • Add configuration for indicating whether rosetta-implementation implements coins feature or not.

Currently every check:construction tries to fetch coins but
not every rosetta implementations support coin endpoint (e.g. rosetta-ethereum).
@aeharvlee
Copy link
Author

@racbc @nasmithan

Hi reviewers :) PTAL this PR and related issue.
Thanks

Copy link
Contributor

@shiatcb shiatcb left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@cb-heimdall
Copy link

Review Error for shiatcb @ 2022-04-05 14:52:43 UTC
User must have write permissions to review

@shrimalmadhur
Copy link
Contributor

@aeharvlee can you sign the commits. That's a required step before I can merge. Thank you!

@shiatcb
Copy link
Contributor

shiatcb commented Apr 5, 2022

@aeharvlee I made a copy to your PR so we can quickly merge and release: #297 (don't worry, I linked to this original PR and @ your name). Please close this one. Thanks in advance!

@shrimalmadhur
Copy link
Contributor

Closing this due to #297 . Thanks @aeharvlee for the PR, sorry we had to push it fast due to an urgent blocker. Appreciate the contribution though :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Request acocunt/coins always failed when using account-based rosetta implementation
4 participants