chore: 16221 swirlds-state-impl module

[imalygin]

Description:

This PR introduces swirlds-state-impl module. This module contains classes related to the implementation of swirlds-state-api.

This PR migrates classes from swirlds-state-api to swirlds-state-impl

Related issue(s):

Fixes #16221

Notes for reviewer:

PLEASE DON'T BE INTIMIDATED BY THE SIZE OF THIS PR

It does only three things:

• moves classes from one directory to another
• updates imports
• fixes module dependencies

THERE ARE NO CHANGES IN THE CODE LOGIC

Just scroll it through, it shouldn't take too long.

Checklist

• Documented (Code comments, README, etc.)
• Tested (unit, integration, etc.)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.35%. Comparing base (9b0f656) to head (e318f3e).
Report is 4 commits behind head on develop.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #16379      +/-   ##
=============================================
- Coverage      63.35%   63.35%   -0.01%     
+ Complexity     20093    20092       -1     
=============================================
  Files           2526     2526              
  Lines          93647    93647              
  Branches        9804     9804              
=============================================
- Hits           59334    59329       -5     
- Misses         30711    30719       +8     
+ Partials        3602     3599       -3     

Files with missing lines	Coverage Δ
...rvice/addressbook/impl/AddressBookServiceImpl.java	100.00% <ø> (ø)
...ddressbook/impl/schemas/V053AddressBookSchema.java	96.55% <ø> (ø)
.../main/java/com/hedera/node/app/spi/AppContext.java	33.33% <ø> (ø)
...m/hedera/node/app/spi/workflows/HandleContext.java	85.71% <ø> (ø)
...-app/src/main/java/com/hedera/node/app/Hedera.java	57.34% <ø> (ø)
.../com/hedera/node/app/blocks/BlockStreamModule.java	60.00% <ø> (ø)
...com/hedera/node/app/blocks/BlockStreamService.java	69.23% <ø> (ø)
...dera/node/app/blocks/impl/FileBlockItemWriter.java	74.64% <ø> (ø)
...ode/app/blocks/schemas/V0560BlockStreamSchema.java	100.00% <ø> (ø)
...main/java/com/hedera/node/app/fees/FeeService.java	100.00% <ø> (ø)
... and 108 more

... and 7 files with indirect coverage changes

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.01% (target: -1.00%)	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (9b0f656)	96466	62798	65.10%
Head commit (e318f3e)	96466 (+0)	62790 (-8)	65.09% (-0.01%)

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 (#16379)	0	0	∅ (not applicable)

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}

[mhess-swl]

There are a number of modules that now depend on the swirlds-state-impl module instead of remaining decoupled by depending on the api only. I assume this is in preparation for more changes coming?

[imalygin]

@mhess-swl

I assume this is in preparation for more changes coming?

Yes, there will be more changes. It's a preparation for migration of classes from swirlds-platform-core. Also, I briefly checked the dependencies on swirlds-state-impl, and all of them are justified. It's okay for the app to be dependent on the state implementation details. But it's not okay for the Block Node to be dependent on swirlds-platform-core, as it has a lot of unrelated functionality.

[artemananiev]

The changes look fine, but may be sub-optimal. swirlds-state-api module is minimal, it only contains classes and methods used to work with states, read and write. Another important aspect of working with states is in swirlds-state-impl for some reason: how to manage states, create, migrate, versioning, etc. This part is indeed not required for hypothetical "test only" implementation of state APIs, but it's still API and should be in swirlds-state-api module.

So my proposal is to move a few classes a little bit:

• swirlds-state-api module to contain all classes and methods to read from and write to states, com.swirlds.state.spi package. This is so called "basic API"
• classes related to state management should also be moved to swirlds-state-api: Schema, Service, SchemaRegistry, and so on. They can be placed in a separate package like com.swirlds.state.spi.lifecycle. Let's call it "extended API"
• swirlds-state-impl will contain classes that implement basic API above on top of merkle trees. Should the module be named like swirlds-state-impl-merkle? I am not sure
• Extended API will be implemented in hedera-app module. Hedera services classes will provide all service/schema definitions

@tinker-michaelj @jasperpotts What do you think about it?

[andrewb1269hg]

Reapproved.

[tinker-michaelj]

The changes look fine, but may be sub-optimal. swirlds-state-api module is minimal, it only contains classes and methods used to work with states, read and write. Another important aspect of working with states is in swirlds-state-impl for some reason: how to manage states, create, migrate, versioning, etc. This part is indeed not required for hypothetical "test only" implementation of state APIs, but it's still API and should be in swirlds-state-api module.

So my proposal is to move a few classes a little bit:

• swirlds-state-api module to contain all classes and methods to read from and write to states, com.swirlds.state.spi package. This is so called "basic API"
• classes related to state management should also be moved to swirlds-state-api: Schema, Service, SchemaRegistry, and so on. They can be placed in a separate package like com.swirlds.state.spi.lifecycle. Let's call it "extended API"
• swirlds-state-impl will contain classes that implement basic API above on top of merkle trees. Should the module be named like swirlds-state-impl-merkle? I am not sure
• Extended API will be implemented in hedera-app module. Hedera services classes will provide all service/schema definitions

@tinker-michaelj @jasperpotts What do you think about it?

Makes sense to me Artem; it would be nice at some point if we could extract the common logic between the OrderedServiceMigrator/MerkleSchemaRegistry and FakeServiceMigrator/FakeSchemaRegistry types and reduce duplication between the subprocess and embedded Hedera state lifecycles.

[imalygin]

it would be nice at some point if we could extract the common logic between the OrderedServiceMigrator/MerkleSchemaRegistry and FakeServiceMigrator/FakeSchemaRegistry types and reduce duplication between the subprocess and embedded Hedera state lifecycles

Thank you for the hint, Michael. I'll create follow up ticket.

[tinker-michaelj]

LGTM, ty @imalygin !

[rbarkerSL]

@imalygin this PR seems to have broken the timing-sensitive tests in hedera services

See the associated run here: https://github.com/hashgraph/hedera-services/actions/runs/11709318923