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

getStorageAt from empty cell now returns 0x0 instead of null #1482

Merged
merged 2 commits into from
Apr 14, 2021

Conversation

ajlopez
Copy link
Contributor

@ajlopez ajlopez commented Mar 22, 2021

Description

JSON RPC method eth_getStorageAt now returns 0x0 instead of null if the storage cell to be retrieved is empty (the default value is then zero)

Motivation and Context

Returning null riases some issues with the ecosystem, ie geth console and other integration tools

How Has This Been Tested?

Adding code tests, and manually

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:

Please, we need more info about other getStorageAt compatibility. For example, our implementation returns 0x0000000000000000000000000000000000000000000000000000000000000001 when the storage cell contains the value ONE, but ganache returns 0x01. Is it an issue? Which is the value returned by geth and other implementations? Maybe they expect 0x1 as in other cases (with leading zeroes removed)

@ajlopez ajlopez force-pushed the storagenotnull branch 2 times, most recently from 17600c4 to 2777b09 Compare March 31, 2021 19:18
Vovchyk
Vovchyk previously approved these changes Apr 7, 2021
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 7, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

fedejinich
fedejinich previously approved these changes Apr 9, 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!

ajlopezrsk
ajlopezrsk previously approved these changes Apr 12, 2021
fedejinich
fedejinich previously approved these changes Apr 14, 2021
@ajlopezrsk ajlopezrsk merged commit a025b44 into rsksmart:master Apr 14, 2021
@aeidelman aeidelman added this to the Iris 3.0.0 milestone Jul 7, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants