fix: estimate_gas doesn't reflect changes in mirror-node #2409

victor-yanev · 2024-04-25T11:35:50Z

Description:
This PR updates the fallback calculations of estimateGas (in cases where the request to mirror node fails):

for contract create tx we reuse the Precheck.transactionIntrinsicGasCost method
for contract call tx we have a new constant TX_CONTRACT_CALL_AVERAGE_GAS: 500_000 which we use instead of using TX_DEFAULT_GAS: 400_000
- Note: 500_000 is an informed guess which adds a small buffer on top of the default gas of 400_000
- Couldn't come up with an exact value which might be "correct" most of the times and the 400_000 which is set for TX_DEFAULT_GAS is definitely not enough for contract calls and in some scenarios the method would return insufficient amount of gas as an estimation.
- We can watch out for related bugs and update this to a different value later if we find that it's not sufficient again or if it's giving too large of an estimation

Related issue(s):

Fixes #2379

Notes for reviewer:

Checklist

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

Signed-off-by: Victor Yanev <[email protected]>

github-actions · 2024-04-25T11:56:43Z

Acceptance Tests

    20 files   301 suites 34m 42s ⏱️
  602 tests   598 ✔️   3 💤 1 ❌
1 026 runs 1 011 ✔️ 11 💤 4 ❌

Results for commit 436558f.

♻️ This comment has been updated with latest results.

Signed-off-by: Victor Yanev <[email protected]>

…t-changes-in-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/eth.ts

github-actions · 2024-04-29T10:46:59Z

Tests

    2 files 157 suites 14s ⏱️
852 tests 851 ✔️ 1 💤 0 ❌
864 runs 863 ✔️ 1 💤 0 ❌

Results for commit 436558f.

♻️ This comment has been updated with latest results.

Signed-off-by: Victor Yanev <[email protected]>

packages/relay/src/lib/eth.ts

packages/relay/src/lib/precheck.ts

konstantinabl

Amazing work! Few nits

konstantinabl · 2024-05-08T14:15:54Z

If possible fix some of the SonarCloud issues

Signed-off-by: Victor Yanev <[email protected]>

sonarqubecloud · 2024-05-09T14:50:13Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/eth.ts # packages/relay/tests/lib/eth/eth_call.spec.ts # packages/relay/tests/lib/eth/eth_estimateGas.spec.ts

Signed-off-by: Victor Yanev <[email protected]>

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/precheck.ts

ebadiere

Please resolve the merge issue.

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/clients/mirrorNodeClient.ts

sonarqubecloud · 2024-06-24T10:41:07Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Nana-EC

LG

* fix: estimate_gas doesn't reflect changes in mirror-node Signed-off-by: Victor Yanev <[email protected]> * fix: estimate_gas doesn't reflect changes in mirror-node Signed-off-by: Victor Yanev <[email protected]> * fix: tests in eth_estimateGas.spec.ts Signed-off-by: Victor Yanev <[email protected]> * Merge branch 'refs/heads/main' into 2379-estimate_gas-does-not-reflect-changes-in-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/eth.ts * chore: simplify isNonZeroValue condition Signed-off-by: Victor Yanev <[email protected]> * chore: add docs + fix compilation errors Signed-off-by: Victor Yanev <[email protected]> * chore: fix remaining sonar issues Signed-off-by: Victor Yanev <[email protected]> * Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-in-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/eth.ts # packages/relay/tests/lib/eth/eth_call.spec.ts # packages/relay/tests/lib/eth/eth_estimateGas.spec.ts * chore: revert changes to EthImpl#call Signed-off-by: Victor Yanev <[email protected]> * Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-in-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/precheck.ts * Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-in-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/clients/mirrorNodeClient.ts --------- Signed-off-by: Victor Yanev <[email protected]> Signed-off-by: ebadiere <[email protected]>

fix: estimate_gas doesn't reflect changes in mirror-node

b80f73a

Signed-off-by: Victor Yanev <[email protected]>

victor-yanev added the limechain label Apr 25, 2024

victor-yanev self-assigned this Apr 25, 2024

victor-yanev added 4 commits April 25, 2024 17:28

fix: estimate_gas doesn't reflect changes in mirror-node

83a1af9

Signed-off-by: Victor Yanev <[email protected]>

fix: tests in eth_estimateGas.spec.ts

348ad87

Signed-off-by: Victor Yanev <[email protected]>

Merge branch 'refs/heads/main' into 2379-estimate_gas-does-not-reflec…

f65e832

…t-changes-in-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/eth.ts

Merge branch 'refs/heads/main' into 2379-estimate_gas-does-not-reflec…

d79b7ed

…t-changes-in-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/eth.ts

chore: simplify isNonZeroValue condition

b14ecf7

Signed-off-by: Victor Yanev <[email protected]>

victor-yanev marked this pull request as ready for review April 29, 2024 14:49

victor-yanev requested review from AlfredoG87, ebadiere, lukelee-sl and a team as code owners April 29, 2024 14:49

victor-yanev added the bug Something isn't working label May 2, 2024

victor-yanev requested review from konstantinabl, natanasow and Ivo-Yankov May 8, 2024 13:52

konstantinabl reviewed May 8, 2024

View reviewed changes

packages/relay/src/lib/eth.ts Show resolved Hide resolved

konstantinabl reviewed May 8, 2024

View reviewed changes

packages/relay/src/lib/eth.ts Show resolved Hide resolved

konstantinabl reviewed May 8, 2024

View reviewed changes

packages/relay/src/lib/precheck.ts Show resolved Hide resolved

konstantinabl reviewed May 8, 2024

View reviewed changes

victor-yanev added 2 commits May 9, 2024 17:26

chore: add docs + fix compilation errors

495bbd8

Signed-off-by: Victor Yanev <[email protected]>

chore: fix remaining sonar issues

9d8e21d

Signed-off-by: Victor Yanev <[email protected]>

Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-i…

37dc063

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/eth.ts # packages/relay/tests/lib/eth/eth_call.spec.ts # packages/relay/tests/lib/eth/eth_estimateGas.spec.ts

victor-yanev requested review from Nana-EC and a team as code owners June 10, 2024 12:02

victor-yanev requested review from a team and georgi-l95 as code owners June 10, 2024 12:02

victor-yanev requested review from matteriben and jeromy-cannon June 10, 2024 12:02

victor-yanev added 2 commits June 10, 2024 15:06

Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-i…

38c7262

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/eth.ts # packages/relay/tests/lib/eth/eth_call.spec.ts # packages/relay/tests/lib/eth/eth_estimateGas.spec.ts

chore: revert changes to EthImpl#call

e16a5ca

Signed-off-by: Victor Yanev <[email protected]>

victor-yanev added this to the 0.50.0 milestone Jun 11, 2024

victor-yanev added 2 commits June 11, 2024 16:57

Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-i…

e6b6adf

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/precheck.ts

Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-i…

4b03396

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/precheck.ts

quiet-node modified the milestones: 0.50.0, 0.51.0 Jun 17, 2024

ebadiere reviewed Jun 21, 2024

View reviewed changes

victor-yanev added 2 commits June 24, 2024 13:30

Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-i…

68cd008

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/clients/mirrorNodeClient.ts

Merge branch 'main' into 2379-estimate_gas-does-not-reflect-changes-i…

436558f

…n-mirror-node Signed-off-by: Victor Yanev <[email protected]> # Conflicts: # packages/relay/src/lib/clients/mirrorNodeClient.ts

victor-yanev requested review from ebadiere and konstantinabl June 24, 2024 10:44

ebadiere modified the milestones: 0.51.0, 0.52.0 Jul 1, 2024

georgi-l95 approved these changes Jul 8, 2024

View reviewed changes

Nana-EC approved these changes Jul 8, 2024

View reviewed changes

victor-yanev merged commit f458e38 into main Jul 9, 2024
37 checks passed

victor-yanev deleted the 2379-estimate_gas-does-not-reflect-changes-in-mirror-node branch July 9, 2024 10:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: estimate_gas doesn't reflect changes in mirror-node #2409

fix: estimate_gas doesn't reflect changes in mirror-node #2409

victor-yanev commented Apr 25, 2024 •

edited

Loading

github-actions bot commented Apr 25, 2024 •

edited

Loading

github-actions bot commented Apr 29, 2024 •

edited

Loading

konstantinabl left a comment

konstantinabl commented May 8, 2024

sonarqubecloud bot commented May 9, 2024

ebadiere left a comment

sonarqubecloud bot commented Jun 24, 2024

Nana-EC left a comment

fix: estimate_gas doesn't reflect changes in mirror-node #2409

fix: estimate_gas doesn't reflect changes in mirror-node #2409

Conversation

victor-yanev commented Apr 25, 2024 • edited Loading

github-actions bot commented Apr 25, 2024 • edited Loading

Acceptance Tests

github-actions bot commented Apr 29, 2024 • edited Loading

Tests

konstantinabl left a comment

Choose a reason for hiding this comment

konstantinabl commented May 8, 2024

sonarqubecloud bot commented May 9, 2024

Quality Gate passed

ebadiere left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 24, 2024

Quality Gate passed

Nana-EC left a comment

Choose a reason for hiding this comment

victor-yanev commented Apr 25, 2024 •

edited

Loading

github-actions bot commented Apr 25, 2024 •

edited

Loading

github-actions bot commented Apr 29, 2024 •

edited

Loading