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 createContractWithStakingFields() #8986

Merged
merged 6 commits into from
Oct 4, 2023

Conversation

tinker-michaelj
Copy link
Collaborator

@tinker-michaelj tinker-michaelj commented Oct 2, 2023

Description:

  • Closes Implement staking id validation logic for ContractCreate transactions #8719
    • The remaining issue with the createContractWithStakingFields() spec was that a _TO_ACCOUNT stake change scenario would cause a ConcurrentModificationException in the for loop here, since the logic inside the loop would add the "stakee" account to the modified set---and cause the iterator to detect a concurrent modification.
  • Adds a spec validating system accounts cannot be used as initcode.

Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
@tinker-michaelj tinker-michaelj requested a review from a team October 2, 2023 20:40
@tinker-michaelj tinker-michaelj requested review from a team as code owners October 2, 2023 20:40
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Node: Unit Test Results

    2 221 files      2 221 suites   1h 7m 56s ⏱️
118 027 tests 117 992 ✔️ 35 💤 0
126 318 runs  126 283 ✔️ 35 💤 0

Results for commit 463220d.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Node: E2E Test Results

    1 files      1 suites   20m 8s ⏱️
310 tests 310 ✔️ 0 💤 0
328 runs  328 ✔️ 0 💤 0

Results for commit 463220d.

♻️ This comment has been updated with latest results.

Copy link
Member

@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!

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Node: HAPI Test Results

1 181 tests   457 ✔️  35m 54s ⏱️
   156 suites  724 💤
   156 files        0

Results for commit 463220d.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Node: Integration Test Results

278 tests   278 ✔️  32m 35s ⏱️
    5 suites      0 💤
    5 files        0

Results for commit 463220d.

♻️ This comment has been updated with latest results.

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

codecov bot commented Oct 2, 2023

Codecov Report

All modified lines are covered by tests ✅

❗ No coverage uploaded for pull request base (develop@20420ce). Click here to learn what that means.

❗ Current head a896ba5 differs from pull request most recent head 463220d. Consider uploading reports for the commit 463220d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #8986   +/-   ##
==========================================
  Coverage           ?   65.09%           
  Complexity         ?    29242           
==========================================
  Files              ?     3225           
  Lines              ?   122854           
  Branches           ?    12603           
==========================================
  Hits               ?    79972           
  Misses             ?    39912           
  Partials           ?     2970           

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

Nana-EC
Nana-EC previously approved these changes Oct 2, 2023
kimbor
kimbor previously approved these changes Oct 3, 2023
iwsimon
iwsimon previously approved these changes Oct 3, 2023
Copy link
Contributor

@iwsimon iwsimon left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
@tinker-michaelj tinker-michaelj dismissed stale reviews from iwsimon, kimbor, mhess-swl, and Nana-EC via a896ba5 October 3, 2023 14:39
iwsimon
iwsimon previously approved these changes Oct 3, 2023
mhess-swl
mhess-swl previously approved these changes Oct 3, 2023
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@tinker-michaelj tinker-michaelj merged commit fb67364 into develop Oct 4, 2023
@tinker-michaelj tinker-michaelj deleted the 08719-fix-stake2account-cme branch October 4, 2023 01:05
@mhess-swl mhess-swl mentioned this pull request Oct 9, 2023
31 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement staking id validation logic for ContractCreate transactions
6 participants