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

Feat: Add RSK Support #1742

Merged
merged 3 commits into from
Apr 15, 2019
Merged

Feat: Add RSK Support #1742

merged 3 commits into from
Apr 15, 2019

Conversation

zachdaniel
Copy link
Contributor

@zachdaniel zachdaniel commented Apr 11, 2019

Motivation

  • Some application changes are required to support RSK.

Changelog

Enhancements

  • Add an RSK variant, to support the configuration changes necessary, and also not call the methods that RSK doesn't support.
  • A few small changes throughout, to account for slight differences in RSK responses.
  • I added an entry to CHANGELOG.md with this PR
  • If I added new functionality, I added tests covering it.
  • If I fixed a bug, I added a regression test to prevent the bug from silently reappearing again.
  • I checked whether I should update the docs and did so if necessary

I added tests to cover the incrementing of the rate limit, but I think testing the actual rate limit + wait combo itself would just introduce flaky tests at the expense of testing pretty straightforward code. I can rearchitect it to be better for that kind of testing, but that would probably take more time than it is worth.

@ghost ghost assigned zachdaniel Apr 11, 2019
@ghost ghost added the in progress label Apr 11, 2019
@zachdaniel zachdaniel force-pushed the rsk-support branch 2 times, most recently from b48fd5e to 60e8ae9 Compare April 11, 2019 20:59
@coveralls
Copy link

coveralls commented Apr 11, 2019

Pull Request Test Coverage Report for Build 282dfcf3-8667-40c3-85d7-1acb0ebc30c1

  • 25 of 34 (73.53%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 83.075%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/ethereum_jsonrpc/lib/ethereum_jsonrpc.ex 2 4 50.0%
apps/ethereum_jsonrpc/lib/ethereum_jsonrpc/application.ex 6 8 75.0%
apps/ethereum_jsonrpc/lib/ethereum_jsonrpc/request_coordinator.ex 17 22 77.27%
Totals Coverage Status
Change from base Build cb096331-e245-4f18-ae21-d76596e543dc: -0.2%
Covered Lines: 4403
Relevant Lines: 5300

💛 - Coveralls

Copy link
Contributor

@goodsoft goodsoft left a comment

Choose a reason for hiding this comment

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

Some questions before I approve:

apps/ethereum_jsonrpc/config/config.exs Show resolved Hide resolved
apps/ethereum_jsonrpc/lib/ethereum_jsonrpc.ex Outdated Show resolved Hide resolved
apps/ethereum_jsonrpc/lib/rsk.ex Outdated Show resolved Hide resolved
@zachdaniel zachdaniel force-pushed the rsk-support branch 4 times, most recently from 406579e to abc9a86 Compare April 12, 2019 16:30
@ghost ghost added the in progress label Apr 15, 2019
@zachdaniel zachdaniel merged commit d27f306 into master Apr 15, 2019
@ghost ghost removed the in progress label Apr 15, 2019
@vbaranov vbaranov deleted the rsk-support branch May 28, 2019 13:24
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.

4 participants