fix(migration): Migrate contract slots from mono- to modular- in deterministic order #10310
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Nodes added to the merkle tree _must be added in deterministic order_. Especially for the migrators, which add so many nodes they cross _commit_ boundaries. Remove the concurrent sinking of contract slots; replace with accumulating all `fromState` slots into a concurrent queue, then placing them in an array, and then sorting them. Fixes #10293 Signed-off-by: David S Bakin <117694041+david-bakin-sl@users.noreply.github.com>
david-bakin-sl
requested review from
Neeharika-Sompalli,
povolev15 and
mhess-swl
david-bakin-sl
changed the title
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #10310 +/- ##
=============================================
- Coverage 63.51% 63.51% -0.01%
- Complexity 30953 30989 +36
=============================================
Files 3337 3341 +4
Lines 134351 134558 +207
Branches 13965 13994 +29
=============================================
+ Hits 85334 85464 +130
- Misses 45661 45732 +71
- Partials 3356 3362 +6 ☔ View full report in Codecov by Sentry. |
Node: E2E Test Results 1 files ± 0 1 suites ±0 23m 29s ⏱️ + 23m 29s Results for commit fcd2182. ± Comparison against base commit ce367a4. This pull request removes 1 and adds 311 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
mhess-swl
reviewed
Dec 6, 2023
...ce/src/main/java/com/hedera/node/app/service/mono/state/migration/ContractStateMigrator.java
Outdated
Show resolved
Hide resolved
(remove misleading no-longer-needed comment) Signed-off-by: David S Bakin <117694041+david-bakin-sl@users.noreply.github.com>
mhess-swl
approved these changes
Dec 6, 2023
david-bakin-sl
deleted the
10293-contract-store-deterministic-migration
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Nodes added to the merkle tree must be added in deterministic order.
Especially for the migrators, which add so many nodes they cross commit boundaries.
Remove the concurrent sinking of contract slots; replace with accumulating all
fromStateslots into a concurrent queue, then placing them in an array, and then sorting them.Related issue(s):
Fixes #10293
Checklist