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

refactor: remove State class #16352

Draft
wants to merge 69 commits into
base: develop
Choose a base branch
from

Conversation

thenswan
Copy link
Contributor

@thenswan thenswan commented Oct 31, 2024

Description

This PR replaces usages of com.swirlds.platform.state.State with com.swirlds.platform.state.MerkleStateRoot, so that the former can be removed.
It also adds initialization code required for MerkleStateRoot to work.
For example, the following code initializes the platform state in the instance of MerkleStateRoot for further use:

     NoOpMerkleStateLifecycles.NO_OP_MERKLE_STATE_LIFECYCLES.initPlatformState(state);

Fixes #15638

Signed-off-by: Nikita Lebedev <[email protected]>
@thenswan thenswan self-assigned this Oct 31, 2024
@thenswan thenswan added this to the v0.57 milestone Oct 31, 2024
Copy link

codacy-production bot commented Oct 31, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.13% (target: -1.00%) 31.86%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (0bf45bf) 96929 63121 65.12%
Head commit (fb3011b) 96825 (-104) 62931 (-190) 64.99% (-0.13%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#16352) 204 65 31.86%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Copy link

codecov bot commented Oct 31, 2024

Codecov Report

Attention: Patch coverage is 31.86275% with 139 lines in your changes missing coverage. Please review.

Project coverage is 63.28%. Comparing base (0bf45bf) to head (fb3011b).
Report is 11 commits behind head on develop.

Files with missing lines Patch % Lines
...irlds/platform/util/NoOpMerkleStateLifecycles.java 0.00% 44 Missing ⚠️
...swirlds/demo/platform/PlatformTestingToolMain.java 0.00% 26 Missing ⚠️
...s/demo/consistency/ConsistencyTestingToolMain.java 0.00% 22 Missing ⚠️
.../demo/consistency/ConsistencyTestingToolState.java 0.00% 14 Missing ⚠️
...java/com/swirlds/demo/iss/ISSTestingToolState.java 0.00% 4 Missing ⚠️
...rlds/demo/migration/MigrationTestingToolState.java 0.00% 4 Missing ⚠️
...emo/stats/signing/StatsSigningTestingToolMain.java 0.00% 4 Missing ⚠️
...om/swirlds/demo/stress/StressTestingToolState.java 0.00% 4 Missing ⚠️
...s/demo/addressbook/AddressBookTestingToolMain.java 0.00% 3 Missing ⚠️
.../java/com/swirlds/demo/iss/ISSTestingToolMain.java 0.00% 3 Missing ⚠️
... and 5 more
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop   #16352      +/-   ##
=============================================
- Coverage      63.37%   63.28%   -0.09%     
+ Complexity     20170    20144      -26     
=============================================
  Files           2532     2536       +4     
  Lines          94110    94083      -27     
  Branches        9846     9822      -24     
=============================================
- Hits           59643    59544      -99     
- Misses         30851    30945      +94     
+ Partials        3616     3594      -22     
Files with missing lines Coverage Δ
...ra/node/app/state/merkle/MerkleSchemaRegistry.java 84.92% <100.00%> (-3.84%) ⬇️
.../demo/consistency/ConsistencyTestingToolRound.java 90.56% <100.00%> (+0.18%) ⬆️
...va/com/swirlds/demo/platform/PayloadCfgSimple.java 38.93% <100.00%> (+1.43%) ⬆️
.../com/swirlds/demo/platform/TransactionCounter.java 17.69% <ø> (+0.15%) ⬆️
...va/com/swirlds/platform/state/MerkleStateRoot.java 85.08% <100.00%> (+0.49%) ⬆️
...main/java/com/swirlds/state/merkle/StateUtils.java 97.95% <100.00%> (+1.88%) ⬆️
.../swirlds/state/merkle/singleton/SingletonNode.java 100.00% <ø> (ø)
...wirlds/demo/platform/PlatformTestingToolState.java 0.00% <0.00%> (-5.10%) ⬇️
.../demo/addressbook/AddressBookTestingToolState.java 0.00% <0.00%> (ø)
...mo/stats/signing/StatsSigningTestingToolState.java 0.00% <0.00%> (ø)
... and 12 more

... and 44 files with indirect coverage changes

Impacted file tree graph

Signed-off-by: Nikita Lebedev <[email protected]>
…cleanup

# Conflicts:
#	platform-sdk/platform-apps/tests/ISSTestingTool/src/main/java/com/swirlds/demo/iss/ISSTestingToolState.java
#	platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/state/State.java
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Signed-off-by: Nikita Lebedev <[email protected]>
Copy link

github-actions bot commented Nov 5, 2024

Node: HAPI Test (Restart) Results

7 tests   2 ✅  3m 37s ⏱️
8 suites  0 💤
9 files    5 ❌
1 errors

For more details on these parsing errors and failures, see this check.

Results for commit 5a59b72.

Signed-off-by: Nikita Lebedev <[email protected]>
thenswan and others added 14 commits November 12, 2024 10:48
Signed-off-by: Nikita Lebedev <[email protected]>
…n to guarantee that it's always registered.

Signed-off-by: Ivan Malygin <[email protected]>
Signed-off-by: Ivan Malygin <[email protected]>
Signed-off-by: Ivan Malygin <[email protected]>
…cleanup

# Conflicts:
#	platform-sdk/platform-apps/tests/AddressBookTestingTool/src/main/java/com/swirlds/demo/addressbook/AddressBookTestingToolState.java
Signed-off-by: Ivan Malygin <[email protected]>
Signed-off-by: Ivan Malygin <[email protected]>
Signed-off-by: Ivan Malygin <[email protected]>
@imalygin imalygin force-pushed the 15638-merkleroot--merklestateroot--state-cleanup branch from c6bd77c to 4af0220 Compare November 13, 2024 18:31
@imalygin imalygin force-pushed the 15638-merkleroot--merklestateroot--state-cleanup branch from 7e810c8 to fa15fb6 Compare November 13, 2024 20:55
if (trigger == InitTrigger.GENESIS) {
genesisInit();
}
}

@Override
public void serialize(final SerializableDataOutputStream out) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how important this particular state implementation is, but this serialization/deserialization logic cannot be just removed. However, this class may be no longer relevant, and it doesn't make sense spend too much time fixing it

/**
* {@inheritDoc}
*/
@Override
Copy link
Member

Choose a reason for hiding this comment

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

Same here. We may want to do here something similar to this commit a53a4a0

…pl`. Added missing registration to PTT.

Signed-off-by: Ivan Malygin <[email protected]>
@imalygin imalygin force-pushed the 15638-merkleroot--merklestateroot--state-cleanup branch from f0a97e7 to 6ed59af Compare November 15, 2024 22:16
Signed-off-by: Ivan Malygin <[email protected]>
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.

Remove State class
2 participants