fix: Consider input bytes when calculating gas cost #10379

stoqnkpL · 2023-12-08T10:04:26Z

Description:
Take into consideration the input bytes' size from the evm transaction when calculating the intrinsic gas cost.

Related issue(s):

Fixes #9988

Notes for reviewer:

Checklist

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

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

github-actions · 2023-12-08T11:03:05Z

Node: Unit Test Results

    2 293 files ±0     2 293 suites ±0 47m 25s ⏱️ -18s
118 429 tests +1 118 395 ✔️ +1 34 💤 ±0 0 ❌ ±0
126 850 runs +1 126 816 ✔️ +1 34 💤 ±0 0 ❌ ±0

Results for commit e056434. ± Comparison against base commit d0e5885.

♻️ This comment has been updated with latest results.

github-actions · 2023-12-08T11:05:16Z

Node: HAPI Test (Token) Results

189 tests ±0 189 ✔️ ±0 19m 19s ⏱️ +39s
  13 suites ±0     0 💤 ±0
  13 files ±0     0 ❌ ±0

Results for commit e056434. ± Comparison against base commit d0e5885.

♻️ This comment has been updated with latest results.

codecov · 2023-12-08T11:05:17Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d0e5885) 62.97% compared to head (e056434) 62.97%.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10379      +/-   ##
=============================================
- Coverage      62.97%   62.97%   -0.01%     
- Complexity     30794    30795       +1     
=============================================
  Files           3356     3356              
  Lines         135216   135219       +3     
  Branches       14076    14076              
=============================================
- Hits           85152    85149       -3     
- Misses         46728    46733       +5     
- Partials        3336     3337       +1

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

github-actions · 2023-12-08T11:07:38Z

Node: HAPI Test (Crypto) Results

211 tests ±0 201 ✔️ ±0 22m 2s ⏱️ + 4m 45s
  22 suites ±0   10 💤 ±0
  22 files ±0     0 ❌ ±0

Results for commit e056434. ± Comparison against base commit d0e5885.

♻️ This comment has been updated with latest results.

github-actions · 2023-12-08T11:09:16Z

Node: E2E Test Results

    1 files ±    0     1 suites ±0 22m 35s ⏱️ + 22m 35s
311 tests +310 311 ✔️ +311 0 💤 ±0 0 ❌ - 1
333 runs +332 333 ✔️ +333 0 💤 ±0 0 ❌ - 1

Results for commit e056434. ± Comparison against base commit d0e5885.

This pull request removes 1 and adds 311 tests. Note that renamed tests count towards both.

EndToEndTests ‑ initializationError

EndToEndTests ‑ ADDRESS_BOOK_CONTROLCanUpdateADDRESS_BOOK
EndToEndTests ‑ ADDRESS_BOOK_CONTROLCanUpdateNODE_DETAILS
EndToEndTests ‑ AccountsGetPayerRecordsIfSoConfigured
EndToEndTests ‑ Acct57CanMakeSmallChanges
EndToEndTests ‑ Acct57CantMakeLargeChanges
EndToEndTests ‑ AddingSignaturesToExecutedTxFails
EndToEndTests ‑ AddingSignaturesToExecutedTxFailsWithLongTermEnabled
EndToEndTests ‑ AddingSignaturesToNonExistingTxFails
EndToEndTests ‑ AddingSignaturesToNonExistingTxFailsWithLongTermEnabled
EndToEndTests ‑ AddressAliasIdFuzzing
…

♻️ This comment has been updated with latest results.

github-actions · 2023-12-08T11:11:45Z

Node: HAPI Test (Time Consuming) Results

21 tests ±0   9 ✔️ ±0 25m 11s ⏱️ +38s
  2 suites ±0 12 💤 ±0
  2 files ±0   0 ❌ ±0

Results for commit e056434. ± Comparison against base commit d0e5885.

♻️ This comment has been updated with latest results.

github-actions · 2023-12-08T11:14:20Z

Node: HAPI Test (Misc) Results

420 tests ±0 318 ✔️ +1 29m 0s ⏱️ + 5m 2s
  74 suites ±0 102 💤 ±0
  74 files ±0     0 ❌ - 1

Results for commit e056434. ± Comparison against base commit d0e5885.

♻️ This comment has been updated with latest results.

github-actions · 2023-12-08T11:14:58Z

Node: Integration Test Results

279 tests ±0 279 ✔️ ±0 28m 8s ⏱️ -17s
    5 suites ±0     0 💤 ±0
    5 files ±0     0 ❌ ±0

Results for commit e056434. ± Comparison against base commit d0e5885.

♻️ This comment has been updated with latest results.

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

github-actions · 2023-12-08T11:39:33Z

Node: HAPI Test (Smart Contract) Results

397 tests ±0 356 ✔️ ±0 49m 40s ⏱️ + 3m 53s
  55 suites ±0   41 💤 ±0
  55 files ±0     0 ❌ ±0

Results for commit e056434. ± Comparison against base commit d0e5885.

♻️ This comment has been updated with latest results.

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Nana-EC

LG
One question

.../src/main/java/com/hedera/node/app/service/evm/contracts/execution/HederaEvmTxProcessor.java

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

…ut-bytes-for-gas-cost Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Nana-EC

LG

Is there a way for us to describe the gas calculation so test readers can understand why the new numbers are correct e.g. 21k + x + y etc

Maybe not in this PR if it's a lot of overhead but something to consider to improve.
Maybe not as part of this PR

IvanKavaldzhiev

LGTM

lukelee-sl · 2023-12-11T15:31:37Z

Should we consider doing the same thing here for modularized smart contract service? Or in a separate pr?
https://github.com/hashgraph/hedera-services/blob/develop/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/gas/CustomGasCharging.java#L117

tinker-michaelj

LGTM, tyvm @stoqnkpL !

david-bakin-sl

LGTM.

...c/main/java/com/hedera/services/bdd/suites/contract/precompile/AssociatePrecompileSuite.java

...s/src/main/java/com/hedera/services/bdd/suites/contract/precompile/ContractMintHTSSuite.java

...ts/src/main/java/com/hedera/services/bdd/suites/contract/traceability/TraceabilitySuite.java

stoqnkpL · 2023-12-11T16:27:10Z

Should we consider doing the same thing here for modularized smart contract service? Or in a separate pr? https://github.com/hashgraph/hedera-services/blob/develop/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/gas/CustomGasCharging.java#L117

it's already resolved as part of this PR. Or do you think there's more logic that should be updated?

lukelee-sl · 2023-12-11T16:31:57Z

Should we consider doing the same thing here for modularized smart contract service? Or in a separate pr? https://github.com/hashgraph/hedera-services/blob/develop/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/gas/CustomGasCharging.java#L117

it's already resolved as part of this PR. Or do you think there's more logic that should be updated?

No I missed that. Thanks for doing this!

lukelee-sl

LGTM

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Consider input bytes when calculating gas cost - mono service fix

b9a8751

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

stoqnkpL added Bug An error that causes the feature to behave differently than what was expected based on design. Limechain Work planned for the LimeChain team labels Dec 8, 2023

stoqnkpL added this to the v0.45 milestone Dec 8, 2023

stoqnkpL self-assigned this Dec 8, 2023

stoqnkpL requested review from a team as code owners December 8, 2023 10:04

stoqnkpL requested review from acuarica, jasperpotts, kimbor, IvanKavaldzhiev, tinker-michaelj, Nana-EC, lukelee-sl and bilyana-gospodinova December 8, 2023 10:04

stoqnkpL changed the title ~~Consider input bytes when calculating gas cost - mono service fix~~ Fix: Consider input bytes when calculating gas cost - mono service fix Dec 8, 2023

Spotless

809f008

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

stoqnkpL changed the title ~~Fix: Consider input bytes when calculating gas cost - mono service fix~~ fix: Consider input bytes when calculating gas cost - mono service fix Dec 8, 2023

Add unit test

0d476fb

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

stoqnkpL changed the title ~~fix: Consider input bytes when calculating gas cost - mono service fix~~ fix: Consider input bytes when calculating gas cost Dec 8, 2023

Consider input bytes when calculating gas cost - modularised service fix

c8a9297

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

stoqnkpL added 2 commits December 8, 2023 13:54

Provide a higher gas limit for contract creations in test suites

cdc82cf

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Fix EETs after intrinsic gas cost increase

b3942c2

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Nana-EC reviewed Dec 8, 2023

View reviewed changes

.../src/main/java/com/hedera/node/app/service/evm/contracts/execution/HederaEvmTxProcessor.java Show resolved Hide resolved

stoqnkpL added 4 commits December 8, 2023 16:49

Fix more tests after intrinsic gas cost increase

ce3023e

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Merge remote-tracking branch 'origin/develop' into 09988-consider-inp…

c823ed2

…ut-bytes-for-gas-cost Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Merge branch 'dev' into 09988-consider-input-bytes-for-gas-cost

c88bbf6

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Fix expected traceability suite sidecar records

e056434

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Nana-EC requested review from david-bakin-sl and shemnon December 9, 2023 15:38

Nana-EC approved these changes Dec 9, 2023

View reviewed changes

IvanKavaldzhiev approved these changes Dec 11, 2023

View reviewed changes

tinker-michaelj approved these changes Dec 11, 2023

View reviewed changes

david-bakin-sl approved these changes Dec 11, 2023

View reviewed changes

stoqnkpL merged commit 23a28ea into develop Dec 11, 2023
29 checks passed

stoqnkpL deleted the 09988-consider-input-bytes-for-gas-cost branch December 11, 2023 16:29

lukelee-sl reviewed Dec 11, 2023

View reviewed changes

stoqnkpL added a commit that referenced this pull request Dec 11, 2023

fix: Consider input bytes when calculating gas cost (#10379)

6b5ef96

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

stoqnkpL mentioned this pull request Dec 11, 2023

fix: Consider input bytes when calculating gas cost (#10379) #10420

Merged

2 tasks

iwsimon pushed a commit that referenced this pull request Dec 12, 2023

fix: Consider input bytes when calculating gas cost (#10379) (#10420)

733d861

Signed-off-by: Stoyan Panayotov <stoyan.panayotov@limechain.tech>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Consider input bytes when calculating gas cost #10379

fix: Consider input bytes when calculating gas cost #10379

stoqnkpL commented Dec 8, 2023

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

codecov bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

Nana-EC left a comment

Nana-EC left a comment

IvanKavaldzhiev left a comment

lukelee-sl commented Dec 11, 2023

tinker-michaelj left a comment

david-bakin-sl left a comment

stoqnkpL commented Dec 11, 2023

lukelee-sl commented Dec 11, 2023

lukelee-sl left a comment

fix: Consider input bytes when calculating gas cost #10379

fix: Consider input bytes when calculating gas cost #10379

Conversation

stoqnkpL commented Dec 8, 2023

github-actions bot commented Dec 8, 2023 • edited

Node: Unit Test Results

github-actions bot commented Dec 8, 2023 • edited

Node: HAPI Test (Token) Results

codecov bot commented Dec 8, 2023 • edited

Codecov Report

github-actions bot commented Dec 8, 2023 • edited

Node: HAPI Test (Crypto) Results

github-actions bot commented Dec 8, 2023 • edited

Node: E2E Test Results

github-actions bot commented Dec 8, 2023 • edited

Node: HAPI Test (Time Consuming) Results

github-actions bot commented Dec 8, 2023 • edited

Node: HAPI Test (Misc) Results

github-actions bot commented Dec 8, 2023 • edited

Node: Integration Test Results

github-actions bot commented Dec 8, 2023 • edited

Node: HAPI Test (Smart Contract) Results

Nana-EC left a comment

Choose a reason for hiding this comment

Nana-EC left a comment

Choose a reason for hiding this comment

IvanKavaldzhiev left a comment

Choose a reason for hiding this comment

lukelee-sl commented Dec 11, 2023

tinker-michaelj left a comment

Choose a reason for hiding this comment

david-bakin-sl left a comment

Choose a reason for hiding this comment

stoqnkpL commented Dec 11, 2023

lukelee-sl commented Dec 11, 2023

lukelee-sl left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

codecov bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited