Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implementation of EIP-1898 in RSKj JSON-RPC Methods #1581

Merged
merged 35 commits into from
Sep 13, 2021

Conversation

Vovchyk
Copy link
Contributor

@Vovchyk Vovchyk commented Jul 6, 2021

Description

Implement EIP-1898 in RSKj JSON-RPC Methods.

The following methods were done to support EIP-1898:

  • eth_getBalance
  • eth_getStorageAt
  • eth_getTransactionCount
  • eth_getCode
  • eth_call

Originated from #1515

Motivation and Context

More details here.

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)
  • Other information:

Code Coverage

Attached coverage.zip you can find all the code coverage reports but you can see that the changes done are 100% covered, as the following image shows.

coverage
coverage-2
coverage-3

You can find it in coverage/ns-57/sources/source-c.html in the attached coverage reports.

Demonstration

Here you can see a demonstration about some methods supporting EIP-1898.

requests

rskj-node initialization
rskj-node-init

Also you can find it in the attached requests.txt

Improvements

In case of search by blockHash, 2 searches are done (one for blockHash and other by blockNumber), this could be avoided by adding blockHash search logic in Web3InformationRetriever.

@Vovchyk Vovchyk requested a review from fedejinich July 6, 2021 12:20
@Vovchyk Vovchyk changed the title Eip1898 jrpc methods Implementation of EIP-1898 in RSKj JSON-RPC Methods Jul 6, 2021
heitorfm
heitorfm previously approved these changes Jul 13, 2021
fedejinich
fedejinich previously approved these changes Jul 15, 2021
Copy link
Contributor

@fedejinich fedejinich left a comment

Choose a reason for hiding this comment

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

lgtm!

@fedejinich fedejinich dismissed stale reviews from heitorfm and themself via 4b9d64e July 15, 2021 16:55
@fedejinich fedejinich force-pushed the eip1898-jrpc-methods branch from 8da42ef to 4b9d64e Compare July 15, 2021 16:55
@fedejinich
Copy link
Contributor

rebased for sec review

@fedejinich
Copy link
Contributor

fedejinich commented Jul 15, 2021

can I squash the story ? @Vovchyk

@joaquinlpereyra
Copy link
Contributor

LGTM. A possible improvement is already mentioned on the PR:

In case of search by blockHash, 2 searches are done (one for blockHash and other by blockNumber), this could be avoided by adding blockHash search logic in Web3InformationRetriever.

But this does not pose any risk at all. The search will be slower but does not scale with input ;)

@Vovchyk
Copy link
Contributor Author

Vovchyk commented Jul 26, 2021

pipeline: run

@Vovchyk Vovchyk force-pushed the eip1898-jrpc-methods branch from 3557150 to 0014b9e Compare July 26, 2021 15:16
@Vovchyk Vovchyk requested a review from fedejinich July 26, 2021 15:58
@Vovchyk Vovchyk force-pushed the eip1898-jrpc-methods branch from 75c2909 to 44d1783 Compare September 3, 2021 13:10
@Vovchyk
Copy link
Contributor Author

Vovchyk commented Sep 3, 2021

pipeline: run

…4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3" }
…sh": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3" }
…x<non-canonical-block-hash>" } -> return tx count at given address in specified bloc - test added
elmol and others added 20 commits September 13, 2021 12:50
… getBalanceByBlockHash to toInvokeByBlockHash as is detailed in #1515 (comment)
…creating method object to replace multiline params in getBalance tests -#1515 (review)
…non-canonical block add to chain cration in getBalance tests -#1515 (review)
@Vovchyk Vovchyk force-pushed the eip1898-jrpc-methods branch from e6e2835 to 24bbacd Compare September 13, 2021 09:50
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@Vovchyk Vovchyk requested a review from heitorfm September 13, 2021 11:32
@Vovchyk Vovchyk merged commit d26533e into master Sep 13, 2021
@Vovchyk Vovchyk deleted the eip1898-jrpc-methods branch September 13, 2021 12:47
@aeidelman aeidelman added this to the Iris v3.1.0 milestone Oct 6, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants