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

Developer mode #668

Merged
merged 5 commits into from
Nov 9, 2022
Merged

Developer mode #668

merged 5 commits into from
Nov 9, 2022

Conversation

Ivo-Yankov
Copy link
Collaborator

@Ivo-Yankov Ivo-Yankov commented Nov 2, 2022

Description:

Adds a developer mode to the relay. Currently it only changes the behaviour of eth_getTransactionByHash, but in the future it can be used for enabling special settings in the local node, which will make it run faster.

The following code is a standard way to assert for the revert message of a contract call when using hardhat with its internal evm or any Ethereum JsonRpcRelay:

await expect(someContract.revertWithString()).to.be.revertedWith("Some revert message");

But the same code does not work when it is configured to use the relay. The difference is in eth_estimateGas - normally it throws a JsonRpc error, but in Hedera's context that is not an option. Developer mode changes the behaviour of eth_getTransactionByHash to throw that same error after it has retrieved the error_message from the Mirror node.

Related issue(s):

Fixes #

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
@codecov-commenter
Copy link

codecov-commenter commented Nov 2, 2022

Codecov Report

Base: 70.60% // Head: 70.75% // Increases project coverage by +0.15% 🎉

Coverage data is based on head (31b8411) compared to base (1c14d9b).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #668      +/-   ##
==========================================
+ Coverage   70.60%   70.75%   +0.15%     
==========================================
  Files          16       16              
  Lines        1167     1173       +6     
  Branches      205      207       +2     
==========================================
+ Hits          824      830       +6     
  Misses        286      286              
  Partials       57       57              
Impacted Files Coverage Δ
packages/relay/src/lib/eth.ts 82.66% <100.00%> (+0.20%) ⬆️
packages/relay/src/lib/constants.ts 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
@Ivo-Yankov Ivo-Yankov added enhancement New feature or request limechain labels Nov 4, 2022
@Ivo-Yankov Ivo-Yankov linked an issue Nov 4, 2022 that may be closed by this pull request
@Ivo-Yankov Ivo-Yankov added the bug Something isn't working label Nov 4, 2022
@Ivo-Yankov Ivo-Yankov marked this pull request as ready for review November 4, 2022 13:20
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Changes are great,
Just need to fill in the documentation holes so it's easier understood. Especially since you have a vision for dev mode

This is my suggestion to make sure everyone understands and it's exposed to users.

  1. Add a docs/dev-mode.md file as part of your PR
    a. It should have an initial summary with your vision for dev mode.
    b. It should have a sub section that has the above summary focused on ethersjs revert error work around.
  2. Add a link under the main README where you set the DEV_MODE = false that points to the dev-mode.md for more details
  3. Update the https://github.com/hashgraph/hedera-json-rpc-relay/tree/main/tools#supported-tools section with a note that points to the dev mode feature for ethersjs support

README.md Show resolved Hide resolved
packages/relay/src/lib/eth.ts Show resolved Hide resolved
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 7, 2022

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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LG

@Nana-EC Nana-EC added this to the 0.12.0 milestone Nov 7, 2022
@Ivo-Yankov Ivo-Yankov merged commit b3864c1 into main Nov 9, 2022
@Ivo-Yankov Ivo-Yankov deleted the 661-revert-reason-dev-mode branch November 9, 2022 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request limechain
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Revert reason is not sent back in transaction receipt
3 participants