fix: add record list checkpoint, so following & preceding child records are reverted correctly #10137

MrValioBg · 2023-11-28T13:49:25Z

Description:

Add support for reverting call from specific "checkpoint". Every time a new ProxyWorldUpdater is created we make a checkpoint. When revert is called we revert both preceding & following child records from this checkpoint.

This will fix more tests in LeakyContractTestSuites and in Create2OperationSuite.

Related issue(s):

Fixes #9403
Fixes #10419
Fixes #10352

Notes for reviewer:

Todo

Verify that javadoc is ok (Code comments, README, etc.)
Tested (unit, integration, etc.)

github-actions · 2023-11-28T14:16:35Z

Node: Unit Test Results

    2 292 files +  1     2 292 suites +1 1h 3m 30s ⏱️ + 10m 32s
118 440 tests +11 118 406 ✔️ +11 34 💤 ±0 0 ❌ ±0
126 861 runs +11 126 827 ✔️ +11 34 💤 ±0 0 ❌ ±0

Results for commit ae79778. ± Comparison against base commit 4db7794.

♻️ This comment has been updated with latest results.

codecov · 2023-11-28T14:17:23Z

Codecov Report

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

Comparison is base (4db7794) 63.04% compared to head (ae79778) 63.02%.
Report is 7 commits behind head on develop.

Files	Patch %	Lines
...app/workflows/handle/record/RecordListBuilder.java	83.33%	1 Missing and 2 partials ⚠️
...a/node/app/workflows/handle/HandleContextImpl.java	81.81%	2 Missing ⚠️
...service/contract/impl/state/ProxyWorldUpdater.java	33.33%	2 Missing ⚠️
...app/spi/workflows/record/RecordListCheckPoint.java	0.00%	1 Missing ⚠️
...ontract/impl/exec/scope/QueryHederaOperations.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10137      +/-   ##
=============================================
- Coverage      63.04%   63.02%   -0.03%     
- Complexity     30814    30816       +2     
=============================================
  Files           3361     3362       +1     
  Lines         135336   135399      +63     
  Branches       14088    14094       +6     
=============================================
+ Hits           85320    85329       +9     
- Misses         46658    46709      +51     
- Partials        3358     3361       +3

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

github-actions · 2023-11-28T14:17:25Z

Node: HAPI Test (Crypto) Results

211 tests ±0 204 ✔️ ±0 17m 48s ⏱️ +4s
  22 suites ±0     7 💤 ±0
  22 files ±0     0 ❌ ±0

Results for commit ae79778. ± Comparison against base commit 4db7794.

♻️ This comment has been updated with latest results.

github-actions · 2023-11-28T14:19:37Z

Node: HAPI Test (Token) Results

189 tests ±0 189 ✔️ ±0 18m 34s ⏱️ +46s
  13 suites ±0     0 💤 ±0
  13 files ±0     0 ❌ ±0

Results for commit ae79778. ± Comparison against base commit 4db7794.

♻️ This comment has been updated with latest results.

github-actions · 2023-11-28T14:20:58Z

Node: HAPI Test (Misc) Results

419 tests +1 322 ✔️ +93 26m 14s ⏱️ + 7m 53s
  73 suites +1   97 💤 -   2
  73 files ±0     0 ❌ - 90

Results for commit ae79778. ± Comparison against base commit 4db7794.

♻️ This comment has been updated with latest results.

github-actions · 2023-11-28T14:22:39Z

Node: HAPI Test (Time Consuming) Results

21 tests ±0   9 ✔️ ±0 25m 1s ⏱️ -20s
  2 suites ±0 12 💤 ±0
  2 files ±0   0 ❌ ±0

Results for commit ae79778. ± Comparison against base commit 4db7794.

♻️ This comment has been updated with latest results.

github-actions · 2023-11-28T14:23:51Z

Node: E2E Test Results

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

Results for commit ae79778. ± Comparison against base commit 4db7794.

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.

github-actions · 2023-11-28T14:28:25Z

Node: Integration Test Results

279 tests ±0 279 ✔️ +1 27m 50s ⏱️ -34s
    5 suites ±0     0 💤 ±0
    5 files ±0     0 ❌ - 1

Results for commit ae79778. ± Comparison against base commit 4db7794.

♻️ This comment has been updated with latest results.

github-actions · 2023-11-28T14:43:43Z

Node: HAPI Test (Smart Contract) Results

398 tests ±0 361 ✔️ +1 45m 51s ⏱️ + 1m 37s
  55 suites ±0   37 💤 - 1
  55 files ±0     0 ❌ ±0

Results for commit ae79778. ± Comparison against base commit 4db7794.

♻️ This comment has been updated with latest results.

...ra-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleContextImpl.java

.../hedera-app/src/main/java/com/hedera/node/app/workflows/handle/record/RecordListBuilder.java

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

…ld records for given ProxyWorldUpdater Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

.../hedera-app/src/main/java/com/hedera/node/app/workflows/handle/record/RecordListBuilder.java

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

# Conflicts: # hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/contract/opcodes/Create2OperationSuite.java # hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/leaky/LeakyContractTestsSuite.java

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

tinker-michaelj

LGTM, nice work @MrValioBg !

MrValioBg added the Limechain Work planned for the LimeChain team label Nov 28, 2023

MrValioBg self-assigned this Nov 28, 2023

mustafauzunn changed the title ~~Fix more tests in LeakyContractTestSuites~~ fix: more tests in LeakyContractTestSuites Dec 1, 2023

MrValioBg commented Dec 5, 2023

View reviewed changes

...ra-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleContextImpl.java Outdated Show resolved Hide resolved

MrValioBg marked this pull request as ready for review December 5, 2023 14:13

MrValioBg requested review from a team and tinker-michaelj as code owners December 5, 2023 14:13

MrValioBg requested review from povolev15, lukelee-sl and Neeharika-Sompalli December 5, 2023 14:13

MrValioBg commented Dec 5, 2023

View reviewed changes

.../hedera-app/src/main/java/com/hedera/node/app/workflows/handle/record/RecordListBuilder.java Outdated Show resolved Hide resolved

MrValioBg marked this pull request as draft December 5, 2023 15:33

MrValioBg marked this pull request as ready for review December 6, 2023 13:01

MrValioBg marked this pull request as draft December 12, 2023 10:32

nathanklick force-pushed the develop branch from d971a0f to 6353aeb Compare December 12, 2023 14:05

MrValioBg added 4 commits December 12, 2023 17:00

feat: dispatch hollow account creation as removable child

d84bb9b

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

refactor: revert only the last child when revert operation is called.

842684a

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

chore: add documentation, rename method & format

b976479

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

chore: fix documentation & format

3ec8d09

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

MrValioBg added 6 commits December 12, 2023 17:00

refactor: fix checks

37084e1

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

refactor: fix logic

0c39961

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

chore: spotless apply

9700125

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

refactor: implement initial changes for storing checkpoint of the chi…

f7fe908

…ld records for given ProxyWorldUpdater Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

chore: spotless apply

9c3fe74

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

refactor: remove changes no longer needed after merge from develop

aaad8e1

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

MrValioBg force-pushed the 09403-fix-child-records-leaky-tests branch from a940939 to aaad8e1 Compare December 12, 2023 15:03

refactor: fix logic issues resulting in failing test

0a94661

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

MrValioBg marked this pull request as ready for review December 12, 2023 16:29

Merge branch 'develop' into 09403-fix-child-records-leaky-tests

9606c30

MrValioBg linked an issue Dec 13, 2023 that may be closed by this pull request

(mod-service) Should be able to revert suffix of preceding records #10419

Closed

fix: enable canUseAliasesInPrecompilesAndContractKeys test

c766b52

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

tinker-michaelj reviewed Dec 13, 2023

View reviewed changes

.../hedera-app/src/main/java/com/hedera/node/app/workflows/handle/record/RecordListBuilder.java Outdated Show resolved Hide resolved

fix: only revert preceding records after the first one

21cb5ba

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

MrValioBg requested a review from tinker-michaelj December 13, 2023 17:25

agadzhalov mentioned this pull request Dec 14, 2023

WP - Fix Create2Operation tests + some of precompile suites #10114

Closed

7 tasks

MrValioBg added 2 commits December 14, 2023 14:01

test: add unit tests to handle new revert logic from checkpoint

ab081cf

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

test: improve readability

a1bd215

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

MrValioBg changed the title ~~fix: more tests in LeakyContractTestSuites~~ fix: add record list checkpoint, to revert child records correctly Dec 14, 2023

MrValioBg changed the title ~~fix: add record list checkpoint, to revert child records correctly~~ fix: add record list checkpoint, so following & preceding child records are reverted correctly Dec 14, 2023

MrValioBg added 2 commits December 14, 2023 14:13

refactor: gradle spotlessApply

d3507b6

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

tests: add more unit test coverage

ae79778

Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>

tinker-michaelj approved these changes Dec 14, 2023

View reviewed changes

MrValioBg merged commit e49cc5a into develop Dec 15, 2023
37 checks passed

MrValioBg deleted the 09403-fix-child-records-leaky-tests branch December 15, 2023 09:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add record list checkpoint, so following & preceding child records are reverted correctly #10137

fix: add record list checkpoint, so following & preceding child records are reverted correctly #10137

MrValioBg commented Nov 28, 2023 •

edited by tinker-michaelj

github-actions bot commented Nov 28, 2023 •

edited

codecov bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

tinker-michaelj left a comment

fix: add record list checkpoint, so following & preceding child records are reverted correctly #10137

fix: add record list checkpoint, so following & preceding child records are reverted correctly #10137

Conversation

MrValioBg commented Nov 28, 2023 • edited by tinker-michaelj

github-actions bot commented Nov 28, 2023 • edited

Node: Unit Test Results

codecov bot commented Nov 28, 2023 • edited

Codecov Report

github-actions bot commented Nov 28, 2023 • edited

Node: HAPI Test (Crypto) Results

github-actions bot commented Nov 28, 2023 • edited

Node: HAPI Test (Token) Results

github-actions bot commented Nov 28, 2023 • edited

Node: HAPI Test (Misc) Results

github-actions bot commented Nov 28, 2023 • edited

Node: HAPI Test (Time Consuming) Results

github-actions bot commented Nov 28, 2023 • edited

Node: E2E Test Results

github-actions bot commented Nov 28, 2023 • edited

Node: Integration Test Results

github-actions bot commented Nov 28, 2023 • edited

Node: HAPI Test (Smart Contract) Results

tinker-michaelj left a comment

Choose a reason for hiding this comment

MrValioBg commented Nov 28, 2023 •

edited by tinker-michaelj

github-actions bot commented Nov 28, 2023 •

edited

codecov bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited

github-actions bot commented Nov 28, 2023 •

edited