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

fix: ERC approve and remove scenarios #10325

Merged
merged 35 commits into from Dec 21, 2023

Conversation

mustafauzunn
Copy link
Collaborator

@mustafauzunn mustafauzunn commented Dec 6, 2023

Description:

Fix ERC approve and remove scenarios

Related issue(s):

Fixes #10188

Notes for reviewer:

Checklist

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

Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
@mustafauzunn mustafauzunn added the Limechain Work planned for the LimeChain team label Dec 6, 2023
@mustafauzunn mustafauzunn self-assigned this Dec 6, 2023
Copy link

github-actions bot commented Dec 6, 2023

Node: Unit Test Results

    2 293 files  ±0      2 293 suites  ±0   54m 0s ⏱️ - 11m 26s
118 681 tests +4  118 647 ✔️ +4  34 💤 ±0  0 ±0 
127 098 runs  +4  127 064 ✔️ +4  34 💤 ±0  0 ±0 

Results for commit 0344f31. ± Comparison against base commit c81d6f3.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

Node: HAPI Test (Token) Results

189 tests  ±0   189 ✔️ ±0   16m 15s ⏱️ -41s
  13 suites ±0       0 💤 ±0 
  13 files   ±0       0 ±0 

Results for commit 0344f31. ± Comparison against base commit c81d6f3.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

Node: HAPI Test (Crypto) Results

211 tests  ±0   204 ✔️ ±0   18m 22s ⏱️ + 1m 38s
  22 suites ±0       7 💤 ±0 
  22 files   ±0       0 ±0 

Results for commit 0344f31. ± Comparison against base commit c81d6f3.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

Node: HAPI Test (Time Consuming) Results

21 tests  ±0     9 ✔️ ±0   24m 45s ⏱️ -1s
  2 suites ±0   12 💤 ±0 
  2 files   ±0     0 ±0 

Results for commit 0344f31. ± Comparison against base commit c81d6f3.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

Node: E2E Test Results

    1 files  ±    0      1 suites  ±0   24m 16s ⏱️ + 24m 16s
311 tests +310  311 ✔️ +311  0 💤 ±0  0  - 1 
333 runs  +332  333 ✔️ +333  0 💤 ±0  0  - 1 

Results for commit 0344f31. ± Comparison against base commit c81d6f3.

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.

Copy link

github-actions bot commented Dec 6, 2023

Node: HAPI Test (Misc) Results

420 tests  ±0   333 ✔️ +1   29m 38s ⏱️ -30s
  74 suites ±0     87 💤 ±0 
  74 files   ±0       0  - 1 

Results for commit 0344f31. ± Comparison against base commit c81d6f3.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

Node: Integration Test Results

278 tests  ±0   278 ✔️ ±0   26m 41s ⏱️ +2s
    5 suites ±0       0 💤 ±0 
    5 files   ±0       0 ±0 

Results for commit 0344f31. ± Comparison against base commit c81d6f3.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

Node: HAPI Test (Smart Contract) Results

399 tests  ±0   374 ✔️ +1   46m 58s ⏱️ + 1m 28s
  55 suites ±0     25 💤  - 1 
  55 files   ±0       0 ±0 

Results for commit 0344f31. ± Comparison against base commit c81d6f3.

♻️ This comment has been updated with latest results.

Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (c81d6f3) 63.04% compared to head (0344f31) 63.03%.
Report is 5 commits behind head on develop.

Files Patch % Lines
...s/hts/grantapproval/AbstractGrantApprovalCall.java 58.53% 13 Missing and 4 partials ⚠️
...en/impl/handlers/CryptoDeleteAllowanceHandler.java 66.66% 0 Missing and 3 partials ⚠️
...tracts/hts/grantapproval/ERCGrantApprovalCall.java 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10325      +/-   ##
=============================================
- Coverage      63.04%   63.03%   -0.01%     
- Complexity     30838    30851      +13     
=============================================
  Files           3363     3363              
  Lines         135487   135549      +62     
  Branches       14096    14104       +8     
=============================================
+ Hits           85416    85450      +34     
- Misses         46714    46731      +17     
- Partials        3357     3368      +11     

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

Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
@mustafauzunn mustafauzunn marked this pull request as ready for review December 15, 2023 16:07
mustafauzunn and others added 6 commits December 18, 2023 14:25
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
# Conflicts:
#	hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/utils/SystemContractUtils.java
Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

There is one if condition we need to remove because it would break an HTS CryptoDeleteAllowance edge case.

But overall with all checks passing, LGTM! tyvm @mustafauzunn

tinker-michaelj and others added 3 commits December 19, 2023 14:06
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
…narios' into 10188-fix-approve-and-remove-scenarios
Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

LGTM, ty @mustafauzunn !

Copy link
Contributor

@stoyanov-st stoyanov-st left a comment

Choose a reason for hiding this comment

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

LGTM

@mustafauzunn mustafauzunn merged commit f10afe6 into develop Dec 21, 2023
38 checks passed
@mustafauzunn mustafauzunn deleted the 10188-fix-approve-and-remove-scenarios branch December 21, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Limechain Work planned for the LimeChain team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERCPrecompileSuite.someErc721ApproveAndRemoveScenariosPass()
3 participants