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: Implement MirrorNodeContractQuery #2073

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

0xivanov
Copy link
Contributor

@0xivanov 0xivanov commented Nov 11, 2024

Description:

A new MirrorNodeContractQuery class has been introduced to support EVM gas estimation and simulation. This query class provides two distinct types of queries:

  1. MirrorNodeContractCallQuery – Executes a contract call and returns the result, simulating the execution of the contract.
  2. MirrorNodeContractEstimateGasQuery – Estimates the gas usage for a contract call, allowing developers to understand the cost of executing a transaction.

The inputs for these queries match those of ContractExecuteTransaction, making it easy for developers to calculate the cost of a follow-up transaction.

The MirrorNodeContractQuery issues an HTTP request to the api/v1/contracts/call endpoint of the Mirror Node Web3 module, which supports both gas estimation and transient simulation of state-changing transactions. This enables accurate cost predictions and helps developers optimize their contract interactions before executing them on the network.

Related issue(s):

Fixes #2076

Notes for reviewer:

Checklist

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

Signed-off-by: Ivan Ivanov <[email protected]>
@0xivanov 0xivanov self-assigned this Nov 11, 2024
Signed-off-by: Ivan Ivanov <[email protected]>
Signed-off-by: Ivan Ivanov <[email protected]>
Signed-off-by: Ivan Ivanov <[email protected]>
@0xivanov 0xivanov changed the title feat(wip): Implement MirrorNodeContractQuery feat: Implement MirrorNodeContractQuery Nov 12, 2024
Copy link

@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.

Nice start.
Missing some fields.
Also you might be able to leverage some Mirror Node web3 module tests as references of example calls.
@kselveliev should be able to help

@0xivanov 0xivanov marked this pull request as ready for review November 14, 2024 11:55
@0xivanov 0xivanov requested a review from a team as a code owner November 14, 2024 11:55
Signed-off-by: Ivan Ivanov <[email protected]>
@0xivanov 0xivanov requested review from a team as code owners November 14, 2024 12:33
@@ -40,7 +42,7 @@
/**
* Utility class used internally by the sdk.
*/
class EntityIdHelper {
public class EntityIdHelper {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note:
Made this class public in order to access it from the tests, changed most public methods to package private

rbarkerSL
rbarkerSL previously approved these changes Nov 19, 2024
Copy link
Contributor

@rbarkerSL rbarkerSL left a comment

Choose a reason for hiding this comment

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

Approval applies to gradle/plugins/src/main/kotlin/com.hedera.gradle.java.gradle.kts

Signed-off-by: Ivan Ivanov <[email protected]>
kselveliev
kselveliev previously approved these changes Nov 19, 2024
Copy link

@kselveliev kselveliev left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 91.07143% with 10 lines in your changes missing coverage. Please review.

Project coverage is 82.70%. Comparing base (b86ee46) to head (dd5edd5).
Report is 159 commits behind head on main.

Files with missing lines Patch % Lines
.../hedera/hashgraph/sdk/MirrorNodeContractQuery.java 92.77% 3 Missing and 3 partials ⚠️
.../java/com/hedera/hashgraph/sdk/EntityIdHelper.java 86.20% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2073      +/-   ##
============================================
+ Coverage     82.20%   82.70%   +0.50%     
- Complexity     3588     3975     +387     
============================================
  Files           186      200      +14     
  Lines         11697    12825    +1128     
  Branches       1150     1263     +113     
============================================
+ Hits           9615    10607     +992     
- Misses         1604     1717     +113     
- Partials        478      501      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

* MirrorNodeContractQuery returns a result from EVM execution such as cost-free execution of read-only smart contract
* queries, gas estimation, and transient simulation of read-write operations.
*/
public class MirrorNodeContractQuery {
Copy link
Contributor

Choose a reason for hiding this comment

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

I responded to the in the Notion page, but also wanted to write a comment here that the name of the API is a bit vague to intuitively understand what it does.

This API allows the dev to estimate the cost of gas for a given contract call using the mirror node. I know you mentioned you wanted to differentiate a call to the mirror node vs consensus node. I am not sure if that matters as much as using a name that someone can easily understand what task they can achieve.

The proposed design had ContractEstimateGasQuery() which clearly indicates what this API is used for. Are there issues going with that naming convention for this API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the PR and the notion page. Introducing 2 new APIs: MirrorNodeContractCallQuery and MirrorNodeContractEstimateGasQuery

Signed-off-by: Ivan Ivanov <[email protected]>
.execute(client);

/*
* Step 5:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
* Step 5:
* Step 6:

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.

Implement MirrorNodeContractQuery
5 participants