Skip to content

Commit

Permalink
chore: 16380 MerkleStateRoot clean up (#16381)
Browse files Browse the repository at this point in the history
Signed-off-by: Ivan Malygin <[email protected]>
  • Loading branch information
imalygin authored Nov 5, 2024
1 parent ff9c427 commit 270cc68
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@
package com.swirlds.platform.state;

import static com.swirlds.logging.legacy.LogMarker.EXCEPTION;
import static com.swirlds.logging.legacy.LogMarker.STARTUP;
import static com.swirlds.platform.state.MerkleStateUtils.createInfoString;
import static com.swirlds.platform.state.service.PbjConverter.toPbjPlatformState;
import static com.swirlds.platform.state.service.schemas.V0540PlatformStateSchema.PLATFORM_STATE_KEY;
import static com.swirlds.platform.system.InitTrigger.EVENT_STREAM_RECOVERY;
import static com.swirlds.state.StateChangeListener.StateType.MAP;
Expand Down Expand Up @@ -186,16 +184,6 @@ public class MerkleStateRoot extends PartialNaryMerkleInternal
*/
private final RuntimeObjectRecord registryRecord;

/**
* If set, the platform state from a deserialized state created prior to version 0.54, and used to initialize
* the platform state as a State API singleton after migration.
*
* @deprecated since 0.54.0; this field should be removed in a future release
*/
@Nullable
@Deprecated(forRemoval = true)
private PlatformState preV054PlatformState;

/**
* Create a new instance. This constructor must be used for all creations of this class.
*
Expand All @@ -210,14 +198,6 @@ public MerkleStateRoot(
this.versionFactory = requireNonNull(versionFactory);
}

/**
* Returns the platform state found at child index 0 of a pre-0.54 state, or null if not found.
* @return the pre-0.54 platform state, or null if not found
*/
public @Nullable PlatformState getPreV054PlatformState() {
return preV054PlatformState;
}

/**
* {@inheritDoc}
* <p>
Expand Down Expand Up @@ -273,7 +253,6 @@ protected MerkleStateRoot(@NonNull final MerkleStateRoot from) {
this.lifecycles = from.lifecycles;
this.registryRecord = RuntimeObjectRegistry.createRecord(getClass());
this.versionFactory = from.versionFactory;
this.preV054PlatformState = from.preV054PlatformState;
this.listeners.addAll(from.listeners);

// Copy over the metadata
Expand Down Expand Up @@ -302,6 +281,11 @@ public int getVersion() {
return CURRENT_VERSION;
}

@Override
public int getMinimumSupportedVersion() {
return CURRENT_VERSION;
}

/**
* To be called ONLY at node shutdown. Attempts to gracefully close any virtual maps. This method is a bit of a
* hack, ideally there would be something more generic at the platform level that virtual maps could hook into
Expand Down Expand Up @@ -406,43 +390,12 @@ public void preHandle(@NonNull final Event event) {
lifecycles.onPreHandle(event, this);
}

/**
* {@inheritDoc}
*/
@Override
public MerkleNode migrate(final int version) {

if (version < CURRENT_VERSION) {
PlatformState platformState = null;
int platformStateIndex = -1;
for (int i = 0; i < getNumberOfChildren(); i++) {
if (getChild(i) instanceof PlatformState ps) {
platformState = ps;
platformStateIndex = i;
break;
}
}
if (platformState == null) {
throw new IllegalStateException("Expected MerkleStateRoot to have PlatformState as a child");
}

preV054PlatformState = platformState;
logger.info(STARTUP.getMarker(), "Found pre-0.54 PlatformState, will migrate to State API singleton");
INDEX_LOOKUP.clear();
final List<MerkleNode> newChildren = new ArrayList<>();
for (int i = 0, n = getNumberOfChildren(); i < n; i++) {
// Skip the platform state, it will be added later
if (i == platformStateIndex) {
continue;
}
final var child = getChild(i);
if (child != null) {
newChildren.add(child.copy());
}
}
addDeserializedChildren(newChildren, CURRENT_VERSION);
public MerkleNode migrate(int version) {
if (version < getMinimumSupportedVersion()) {
throw new UnsupportedOperationException("State migration from version " + version + " is not supported."
+ " The minimum supported version is " + getMinimumSupportedVersion());
}
// Always return this node, we never want to replace MerkleStateRoot node in the tree
return this;
}

Expand Down Expand Up @@ -1059,16 +1012,7 @@ private ReadablePlatformStateStore readablePlatformStateStore() {
}

private WritablePlatformStateStore writablePlatformStateStore() {
final var store = new WritablePlatformStateStore(getWritableStates(PlatformStateService.NAME), versionFactory);
if (preV054PlatformState != null) {
store.setAllFrom(preV054PlatformState);
logger.info(
STARTUP.getMarker(),
"Migrated PlatformState {} to State API singleton",
toPbjPlatformState(preV054PlatformState));
preV054PlatformState = null;
}
return store;
return new WritablePlatformStateStore(getWritableStates(PlatformStateService.NAME), versionFactory);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,8 @@ public SemanticVersion creationVersionOf(@NonNull final MerkleStateRoot root) {
}
final var index = root.findNodeIndex(NAME, PLATFORM_STATE_KEY);
if (index == -1) {
final var legacyPlatformState = requireNonNull(root.getPreV054PlatformState());
return legacyPlatformState.getCreationSoftwareVersion().getPbjSemanticVersion();
} else {
return ((SingletonNode<PlatformState>) root.getChild(index))
.getValue()
.creationSoftwareVersionOrThrow();
throw new IllegalStateException("Platform state not found in root");
}
return ((SingletonNode<PlatformState>) root.getChild(index)).getValue().creationSoftwareVersionOrThrow();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -960,13 +960,6 @@ void testUpdatePlatformStateData() {
@Nested
@DisplayName("Migrate test")
class MigrateTest {
@Test
@DisplayName("Migrate fails if the first child is not PlatformState")
void migrate_fail() {
stateRoot.putServiceStateIfAbsent(fruitMetadata, () -> fruitMerkleMap);
assertThrows(IllegalStateException.class, () -> stateRoot.migrate(CURRENT_VERSION - 1));
}

@Test
@DisplayName("If the version is current, nothing ever happens")
void migrate_currentVersion() {
Expand All @@ -980,14 +973,9 @@ void migrate_currentVersion() {
}

@Test
@DisplayName("Migrate fails if the platform state is absent")
void migrate_platform_absent() {
var node1 = mock(MerkleNode.class);
stateRoot.setChild(0, node1);
var node2 = mock(MerkleNode.class);
stateRoot.setChild(1, node2);

assertThrows(IllegalStateException.class, () -> stateRoot.migrate(CURRENT_VERSION - 1));
@DisplayName("Migration from previous versions is not supported")
void migration_not_supported() {
assertThrows(UnsupportedOperationException.class, () -> stateRoot.migrate(CURRENT_VERSION - 1));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,12 @@ void emptyRootIsAtGenesis() {
}

@Test
void rootWithNoPlatformStateAndLegacyStateAtChildZeroGetsLegacyVersion() {
void rootWithNoPlatformState_throwsException() {
given(root.getNumberOfChildren()).willReturn(1);
given(root.findNodeIndex(PlatformStateService.NAME, V0540PlatformStateSchema.PLATFORM_STATE_KEY))
.willReturn(-1);
given(root.getPreV054PlatformState()).willReturn(legacyState);
given(legacyState.getCreationSoftwareVersion()).willReturn(version);
given(version.getPbjSemanticVersion()).willReturn(SemanticVersion.DEFAULT);
assertSame(SemanticVersion.DEFAULT, PLATFORM_STATE_SERVICE.creationVersionOf(root));
}

@Test
void rootWithNoPlatformStateMustHaveLegacyStateAtChildZero() {
given(root.getNumberOfChildren()).willReturn(1);
given(root.findNodeIndex(PlatformStateService.NAME, V0540PlatformStateSchema.PLATFORM_STATE_KEY))
.willReturn(-1);
assertThrows(NullPointerException.class, () -> PLATFORM_STATE_SERVICE.creationVersionOf(root));
assertThrows(IllegalStateException.class, () -> PLATFORM_STATE_SERVICE.creationVersionOf(root));
}

@Test
Expand Down

0 comments on commit 270cc68

Please sign in to comment.