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

0.35 - Cherry pick precompile permission change #5572

Merged
merged 18 commits into from Mar 14, 2023

Conversation

tinker-michaelj
Copy link
Collaborator

Description:

  • Adds EET's verifying limitations on delegatecall's to precompile contracts.
  • Adds contracts.permittedDelegateCallers= whitelist to allow contracts known to be safe to bypass the default limitation.

Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
@tinker-michaelj tinker-michaelj added CI:UnitTests Initiates unit test PR checks that are required before the PR may be merged. CI:FullStackTests Initiates full stack tests PR checks that are required before the PR may be merged. CI:FinalChecks Initiates final PR checks that are required prior to PR merge. (CI:UnitTests, CI:FullStackTests) labels Mar 10, 2023
@github-actions
Copy link

github-actions bot commented Mar 10, 2023

Unit Test Results

  1 194 files    1 194 suites   47m 4s ⏱️
96 431 tests 96 430 ✔️ 1 💤 0
98 057 runs  98 056 ✔️ 1 💤 0

Results for commit b53448a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 10, 2023

Integration Test Results

9 tests   9 ✔️  1s ⏱️
1 suites  0 💤
2 files    0
1 errors

Results for commit b53448a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 10, 2023

E2E Test Results

    1 files      1 suites   17m 27s ⏱️
308 tests 308 ✔️ 0 💤 0
326 runs  326 ✔️ 0 💤 0

Results for commit b53448a.

♻️ This comment has been updated with latest results.

Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Neeharika-Sompalli
Neeharika-Sompalli previously approved these changes Mar 10, 2023
Copy link
Collaborator

@Neeharika-Sompalli Neeharika-Sompalli left a comment

Choose a reason for hiding this comment

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

Something is wrong with CI, probably re-run

@Neeharika-Sompalli Neeharika-Sompalli changed the title Cherry pick precompile permission change 0.35 - Cherry pick precompile permission change Mar 10, 2023
@Nana-EC Nana-EC added this to the v0.35.0 milestone Mar 11, 2023
Nana-EC
Nana-EC previously approved these changes Mar 11, 2023
@dimitar-dinev
Copy link
Collaborator

Something is wrong with CI, probably re-run

The latest changes and the integration/E2E test fixes from the other PR have not been picked here, that's why some of the tests are failing.

Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
@codecov
Copy link

codecov bot commented Mar 13, 2023

Codecov Report

No coverage uploaded for pull request base (release/0.35@1039779). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff               @@
##             release/0.35    #5572   +/-   ##
===============================================
  Coverage                ?   95.36%           
  Complexity              ?    16348           
===============================================
  Files                   ?     1185           
  Lines                   ?    46714           
  Branches                ?     4762           
===============================================
  Hits                    ?    44548           
  Misses                  ?     1335           
  Partials                ?      831           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@Neeharika-Sompalli Neeharika-Sompalli left a comment

Choose a reason for hiding this comment

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

Makes sense to me !

Neeharika-Sompalli
Neeharika-Sompalli previously approved these changes Mar 13, 2023
Copy link
Collaborator

@Neeharika-Sompalli Neeharika-Sompalli left a comment

Choose a reason for hiding this comment

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

LGTM ! Other than one place where we could throw or log CATASTROPHIC message. Also some E2E tests are failing. May be flaky.

Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
NOT_SUPPORTED_TXN,
CONTRACT_REVERT_EXECUTED,
recordWith().status(NOT_SUPPORTED)));
.hasCostAnswerPrecheck(INVALID_ACCOUNT_ID));
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should revisit this test case. It doesn't depend on delegate call, the bytecode just happens to have outdated HederaTokenService.sol compiled in.

Copy link
Collaborator

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

My only concern is we should fix LeakyConteactTestsSuite

lukelee-sl
lukelee-sl previously approved these changes Mar 14, 2023
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
@sonarcloud
Copy link

sonarcloud bot commented Mar 14, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Collaborator

@Neeharika-Sompalli Neeharika-Sompalli left a comment

Choose a reason for hiding this comment

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

Looks great ! Thank you @tinker-michaelj 💯

@tinker-michaelj tinker-michaelj merged commit 7560ccd into release/0.35 Mar 14, 2023
10 of 11 checks passed
@tinker-michaelj tinker-michaelj deleted the cherry-pick-precompile-permission-change branch March 14, 2023 20:14
@dimitar-dinev dimitar-dinev mentioned this pull request Mar 15, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI:FinalChecks Initiates final PR checks that are required prior to PR merge. (CI:UnitTests, CI:FullStackTests) CI:FullStackTests Initiates full stack tests PR checks that are required before the PR may be merged. CI:UnitTests Initiates unit test PR checks that are required before the PR may be merged.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants