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.

Sign up for GitHub

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

Jump to bottom

fix: HAS.isAuthorizedRaw: Choose proper gas amount for EC vs ED signature verification #14162

Merged

david-bakin-sl merged 2 commits into develop from 14156-charge-correct-gas-for-isAuthorizedRaw

Jul 9, 2024

Member

david-bakin-sl commented Jul 5, 2024

Description:

Chooses gas requirement for isAuthorizedRaw based on whether you're verifying an EC key (3000) or an ED key (1.5M).

Related issue(s):

Fixes #14156

Checklist

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

david-bakin-sl requested review from a team and tinker-michaelj as code owners

July 5, 2024 08:05

david-bakin-sl requested review from mhess-swl and MrValioBg

July 5, 2024 08:05

david-bakin-sl self-assigned this

david-bakin-sl modified the milestones: v0.50, v0.52

github-actions bot commented Jul 5, 2024 •

edited

Loading

Node: HAPI Test (Restart) Results

7 files 7 suites 8m 12s ⏱️
6 tests 6 ✅ 0 💤 0 ❌
7 runs 7 ✅ 0 💤 0 ❌

Results for commit f1c8dad.

♻️ This comment has been updated with latest results.

github-actions bot commented Jul 5, 2024 •

edited

Loading

Node: Unit Test Results

1 545 files 1 545 suites 1h 59m 8s ⏱️
115 792 tests 115 733 ✅ 59 💤 0 ❌
124 107 runs 124 048 ✅ 59 💤 0 ❌

Results for commit f1c8dad.

♻️ This comment has been updated with latest results.

github-actions bot commented Jul 5, 2024 •

edited

Loading

Node: HAPI Test (Node Death Reconnect) Results

3 tests 3 ✅ 5m 5s ⏱️
3 suites 0 💤
3 files 0 ❌

Results for commit f1c8dad.

♻️ This comment has been updated with latest results.

github-actions bot commented Jul 5, 2024 •

edited

Loading

Node: HAPI Test (Token) Results

22 files 22 suites 7m 54s ⏱️
273 tests 273 ✅ 0 💤 0 ❌
351 runs 351 ✅ 0 💤 0 ❌

Results for commit f1c8dad.

♻️ This comment has been updated with latest results.

github-actions bot commented Jul 5, 2024 •

edited

Loading

Node: HAPI Test (Crypto) Results

24 files 24 suites 12m 31s ⏱️
366 tests 366 ✅ 0 💤 0 ❌
373 runs 373 ✅ 0 💤 0 ❌

Results for commit f1c8dad.

♻️ This comment has been updated with latest results.

github-actions bot commented Jul 5, 2024 •

edited

Loading

Node: HAPI Test (Misc) Results

51 files 51 suites 21m 33s ⏱️
357 tests 357 ✅ 0 💤 0 ❌
375 runs 375 ✅ 0 💤 0 ❌

Results for commit f1c8dad.

♻️ This comment has been updated with latest results.

github-actions bot commented Jul 5, 2024 •

edited

Loading

Node: HAPI Test (Time Consuming) Results

19 tests 19 ✅ 23m 4s ⏱️
4 suites 0 💤
4 files 0 ❌

Results for commit f1c8dad.

♻️ This comment has been updated with latest results.

github-actions bot commented Jul 5, 2024 •

edited

Loading

Node: HAPI Test (Smart Contract) Results

73 files 73 suites 22m 46s ⏱️
684 tests 684 ✅ 0 💤 0 ❌
738 runs 738 ✅ 0 💤 0 ❌

Results for commit f1c8dad.

♻️ This comment has been updated with latest results.

MrValioBg reviewed

View reviewed changes

.../app/service/contract/impl/exec/systemcontracts/has/isauthorizedraw/IsAuthorizedRawCall.java Show resolved Hide resolved

david-bakin-sl force-pushed the 14156-charge-correct-gas-for-isAuthorizedRaw branch from 6f25865 to 615974d Compare

July 8, 2024 21:31

david-bakin-sl requested a review from a team as a code owner

July 8, 2024 21:31


          isAuthorizedRaw: Choose proper gas amount for EC vs ED signature veri…

dcc0266

…fication

Signed-off-by: David S Bakin <117694041+david-bakin-sl@users.noreply.github.com>

david-bakin-sl force-pushed the 14156-charge-correct-gas-for-isAuthorizedRaw branch from 615974d to dcc0266 Compare

July 8, 2024 21:50

codecov bot commented Jul 8, 2024 •

edited

Loading

Codecov Report

Attention: Patch coverage is 45.00000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 58.07%. Comparing base (9973fae) to head (f1c8dad).
Report is 116 commits behind head on develop.

Files	Patch %	Lines
...racts/has/isauthorizedraw/IsAuthorizedRawCall.java	37.50%	8 Missing and 2 partials ⚠️
...ce/contract/impl/exec/gas/CustomGasCalculator.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #14162      +/-   ##
=============================================
- Coverage      67.58%   58.07%   -9.51%     
+ Complexity     36486    22063   -14423     
=============================================
  Files           3732     2868     -864     
  Lines         149767   110371   -39396     
  Branches       15571    11280    -4291     
=============================================
- Hits          101223    64103   -37120     
+ Misses         44179    42683    -1496     
+ Partials        4365     3585     -780

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

lukelee-sl reviewed

View reviewed changes

.../app/service/contract/impl/exec/systemcontracts/has/isauthorizedraw/IsAuthorizedRawCall.java Outdated Show resolved Hide resolved


          Use CustomGasCalculator to get gas fees for isAuthorizedRaw

f1c8dad

Signed-off-by: David S Bakin <117694041+david-bakin-sl@users.noreply.github.com>

lukelee-sl approved these changes

View reviewed changes

Member

lukelee-sl left a comment

LGTM

david-bakin-sl merged commit 0d3c042 into develop

40 of 43 checks passed

david-bakin-sl deleted the 14156-charge-correct-gas-for-isAuthorizedRaw branch

July 9, 2024 01:58

netopyr added a commit that referenced this pull request


          Merge branch 'develop' into 14166-add-storeRegistry-serviceApiRegistry

af6759d

* develop:
  fix: HAS.isAuthorizedRaw: Choose proper gas amount for EC vs ED signature verification (#14162)
  test: Add hapi tests for call operations on system accounts (#13686)
  build(deps): bump com.gradle:develocity-gradle-plugin from 3.17.2 to 3.17.5 in /gradle/plugins (#13820)

david-bakin-sl added a commit that referenced this pull request


          fix: HAS.isAuthorizedRaw: Choose proper gas amount for EC vs ED signa…

1c019af

…ture verification (#14162)

Chooses gas requirement for `isAuthorizedRaw` based on whether you're verifying an EC key (3000) or an ED key (1.5M).

Fixes #14156 

Signed-off-by: David S Bakin <117694041+david-bakin-sl@users.noreply.github.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

MrValioBg MrValioBg left review comments

lukelee-sl lukelee-sl approved these changes

tinker-michaelj Awaiting requested review from tinker-michaelj tinker-michaelj is a code owner

mhess-swl Awaiting requested review from mhess-swl mhess-swl is a code owner automatically assigned from hashgraph/hedera-services