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

Validations for dispatchSyntheticTxn #9308

Merged
merged 34 commits into from Nov 8, 2023

Conversation

mustafauzunn
Copy link
Collaborator

Description:

Add checks to HandleContextImpl.dispatchSyntheticTxn() right before we call dispatchHandle()

Related issue(s):

Fixes #9213

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>
@mustafauzunn mustafauzunn requested a review from a team as a code owner October 17, 2023 11:11
@mustafauzunn mustafauzunn self-assigned this Oct 17, 2023
@mustafauzunn mustafauzunn added the Limechain Work planned for the LimeChain team label Oct 17, 2023
@github-actions
Copy link

github-actions bot commented Oct 17, 2023

Node: E2E Test Results

    1 files  ±    0      1 suites  ±0   21m 24s ⏱️ + 21m 24s
310 tests +309  310 ✔️ +310  0 💤 ±0  0  - 1 
332 runs  +331  332 ✔️ +332  0 💤 ±0  0  - 1 

Results for commit d7f7847. ± Comparison against base commit abb9bb4.

This pull request removes 1 and adds 310 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
Copy link

github-actions bot commented Oct 17, 2023

Node: Unit Test Results

    2 276 files  ±0      2 276 suites  ±0   1h 36m 19s ⏱️ + 2m 39s
118 367 tests +4  118 333 ✔️ +4  34 💤 ±0  0 ±0 
126 628 runs  +4  126 594 ✔️ +4  34 💤 ±0  0 ±0 

Results for commit d7f7847. ± Comparison against base commit abb9bb4.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #9308   +/-   ##
==========================================
  Coverage           ?   65.42%           
  Complexity         ?    29987           
==========================================
  Files              ?     3296           
  Lines              ?   125603           
  Branches           ?    13037           
==========================================
  Hits               ?    82176           
  Misses             ?    40285           
  Partials           ?     3142           

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

@github-actions
Copy link

github-actions bot commented Oct 17, 2023

Node: Integration Test Results

280 tests  ±0   280 ✔️ ±0   28m 28s ⏱️ - 4m 40s
    5 suites ±0       0 💤 ±0 
    5 files   ±0       0 ±0 

Results for commit d7f7847. ± Comparison against base commit abb9bb4.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 17, 2023

Node: HAPI Test Results

1 245 tests  +3   851 ✔️ +26   1h 25m 31s ⏱️ + 7m 34s
   166 suites +1   394 💤  - 23 
   166 files   +1       0 ±  0 

Results for commit d7f7847. ± Comparison against base commit abb9bb4.

♻️ 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.

Some unit tests and HAPI tests are failing on this PR 🤔

@mustafauzunn mustafauzunn marked this pull request as draft October 19, 2023 06:28
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
mustafauzunn and others added 4 commits October 19, 2023 15:58
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: Valentin Valkanov <valentin.valkanov@limechain.tech>
@mustafauzunn mustafauzunn marked this pull request as ready for review October 19, 2023 14:51
@sonarcloud
Copy link

sonarcloud bot commented Oct 19, 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

@mustafauzunn mustafauzunn marked this pull request as draft October 20, 2023 12:45
# Conflicts:
#	hedera-node/hedera-app/src/test/java/com/hedera/node/app/workflows/handle/HandleContextImplTest.java
@stoyanov-st stoyanov-st marked this pull request as ready for review October 30, 2023 08:44
# Conflicts:
#	hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/SolvencyPreCheck.java
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
tinker-michaelj
tinker-michaelj previously approved these changes Nov 1, 2023
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.

Since the handle workflow will ultimately assign a guaranteed-unique TransactionID to a child transaction, I think the duplicate check is superfluous. But overall this makes sense to me, ty @mustafauzunn !

@netopyr netopyr linked an issue Nov 2, 2023 that may be closed by this pull request
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
stoyanov-st and others added 5 commits November 7, 2023 10:13
# Conflicts:
#	hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleContextImpl.java
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Signed-off-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
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.

LGTM !

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.

I think we should merge and then urgently fix XTest's; c.f. #9776

@Neeharika-Sompalli Neeharika-Sompalli merged commit f593934 into develop Nov 8, 2023
16 of 17 checks passed
@Neeharika-Sompalli Neeharika-Sompalli deleted the 09213-validations-dispatchSyntheticTxn branch November 8, 2023 20:31
Ivo-Yankov pushed a commit that referenced this pull request Nov 9, 2023
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Signed-off-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
Co-authored-by: Valentin Valkanov <valentin.valkanov@limechain.tech>
Co-authored-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Co-authored-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
nickpoorman pushed a commit that referenced this pull request Nov 22, 2023
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Signed-off-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
Co-authored-by: Valentin Valkanov <valentin.valkanov@limechain.tech>
Co-authored-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
Co-authored-by: Neeharika-Sompalli <neeharika.sompalli@swirldslabs.com>
Signed-off-by: Nick Poorman <nick@swirldslabs.com>
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.

Add checks to HandleContextImpl.dispatchSyntheticTxn() Add additional checks for child transactions
7 participants