Skip to content

Commit

Permalink
Cherry-pick mismatched running hash fix to 0.41 (#8444)
Browse files Browse the repository at this point in the history 
Signed-off-by: Matt Hess <[email protected]>
  • Loading branch information
@mhess-swl
mhess-swl authored Sep 8, 2023
1 parent c68df6a commit ce61cab
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 34 deletions.
1 change: 0 additions & 1 deletion ...ode/hedera-mono-service/src/main/java/com/hedera/node/app/service/mono/ServicesState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@ private ServicesApp internalInit(
// that without a dynamic address book, this MUST be a no-op during reconnect.
app.stakeStartupHelper().doRestartHousekeeping(addressBook(), stakingInfo());
if (isUpgrade) {
dualState.setFreezeTime(null);
networkCtx().discardPreparedUpgradeMeta();
if (deployedVersion.hasMigrationRecordsFrom(deserializedVersion)) {
networkCtx().markMigrationRecordsNotYetStreamed();
Expand Down
16 changes: 14 additions & 2 deletions ...mono-service/src/main/java/com/hedera/node/app/service/mono/state/logic/BlockManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import com.hedera.node.app.service.evm.contracts.execution.HederaBlockValues;
import com.hedera.node.app.service.mono.context.properties.BootstrapProperties;
import com.hedera.node.app.service.mono.state.DualStateAccessor;
import com.hedera.node.app.service.mono.state.merkle.MerkleNetworkContext;
import com.hedera.node.app.service.mono.stream.RecordsRunningHashLeaf;
import com.swirlds.common.crypto.RunningHash;
Expand All @@ -50,6 +51,7 @@ public class BlockManager {

private final long blockPeriodMs;
private final boolean logEveryTransaction;
private final DualStateAccessor dualStateAccessor;
private final Supplier<MerkleNetworkContext> networkCtx;
private final Supplier<RecordsRunningHashLeaf> runningHashLeaf;

Expand All @@ -65,12 +67,14 @@ public class BlockManager {
public BlockManager(
final BootstrapProperties bootstrapProperties,
final Supplier<MerkleNetworkContext> networkCtx,
final Supplier<RecordsRunningHashLeaf> runningHashLeaf) {
final Supplier<RecordsRunningHashLeaf> runningHashLeaf,
final DualStateAccessor dualStateAccessor) {
this.networkCtx = networkCtx;
this.runningHashLeaf = runningHashLeaf;
this.blockPeriodMs =
bootstrapProperties.getLongProperty(HEDERA_RECORD_STREAM_LOG_PERIOD) * SECONDS_TO_MILLISECONDS;
this.logEveryTransaction = bootstrapProperties.getBooleanProperty(HEDERA_RECORD_STREAM_LOG_EVERY_TRANSACTION);
this.dualStateAccessor = dualStateAccessor;
}

/** Clears all provisional block metadata for the current transaction. */
Expand Down Expand Up @@ -189,7 +193,15 @@ private void computeProvisionalBlockMeta(final Instant now) {
private boolean willCreateNewBlock(@NonNull final Instant timestamp) {
final var curNetworkCtx = networkCtx.get();
final var firstBlockTime = curNetworkCtx.firstConsTimeOfCurrentBlock();
return firstBlockTime == null || !inSamePeriod(firstBlockTime, timestamp);
final var dualState = dualStateAccessor.getDualState();
final var isFirstTransactionAfterFreezeRestart =
dualState.getFreezeTime() != null && dualState.getFreezeTime().equals(dualState.getLastFrozenTime());
if (isFirstTransactionAfterFreezeRestart) {
dualState.setFreezeTime(null);
}
return firstBlockTime == null
|| isFirstTransactionAfterFreezeRestart
|| !inSamePeriod(firstBlockTime, timestamp);
}

private boolean inSamePeriod(@NonNull final Instant then, @NonNull final Instant now) {
Expand Down
3 changes: 1 addition & 2 deletions ...service/src/main/java/com/hedera/node/app/service/mono/stream/RecordsRunningHashLeaf.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ public void serialize(final SerializableDataOutputStream out) throws IOException

@Override
public void deserialize(final SerializableDataInputStream in, final int version) throws IOException {
runningHash = new RunningHash();
runningHash.setHash(in.readSerializable());
runningHash = new RunningHash(in.readSerializable());

if (version >= RELEASE_0280_VERSION) {
resetMinusHashes(false);
Expand Down
28 changes: 0 additions & 28 deletions ...hedera-mono-service/src/test/java/com/hedera/node/app/service/mono/ServicesStateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,6 @@ void nonGenesisInitDoesNotClearPreparedUpgradeIfSameVersion() {
subject.setChild(StateChildIndices.NETWORK_CTX, networkContext);
subject.setChild(StateChildIndices.ACCOUNTS, accounts);

final var when = Instant.ofEpochSecond(1_234_567L, 890);
given(dualState.getFreezeTime()).willReturn(when);
given(dualState.getLastFrozenTime()).willReturn(when);

given(app.hashLogger()).willReturn(hashLogger);
given(app.initializationFlow()).willReturn(initFlow);
given(app.dualStateAccessor()).willReturn(dualStateAccessor);
Expand All @@ -627,7 +623,6 @@ void nonGenesisInitDoesNotClearPreparedUpgradeIfSameVersion() {
subject.init(platform, dualState, RESTART, currentVersion);

verify(networkContext, never()).discardPreparedUpgradeMeta();
verify(dualState, never()).setFreezeTime(null);
}

@Test
Expand All @@ -646,10 +641,6 @@ void nonGenesisInitWithBuildDoesntRunMigrations() {
subject.setChild(StateChildIndices.NETWORK_CTX, networkContext);
subject.setChild(StateChildIndices.ACCOUNTS, accounts);

final var when = Instant.ofEpochSecond(1_234_567L, 890);
given(dualState.getFreezeTime()).willReturn(when);
given(dualState.getLastFrozenTime()).willReturn(when);

given(app.hashLogger()).willReturn(hashLogger);
given(app.initializationFlow()).willReturn(initFlow);
given(app.dualStateAccessor()).willReturn(dualStateAccessor);
Expand All @@ -664,7 +655,6 @@ void nonGenesisInitWithBuildDoesntRunMigrations() {
subject.init(platform, dualState, RESTART, configVersion);

verify(networkContext, never()).discardPreparedUpgradeMeta();
verify(dualState, never()).setFreezeTime(null);
}

@Test
Expand All @@ -675,8 +665,6 @@ void nonGenesisInitClearsPreparedUpgradeIfDeployedIsLaterVersion() {
subject.setChild(StateChildIndices.ACCOUNTS, accounts);

final var when = Instant.ofEpochSecond(1_234_567L, 890);
given(dualState.getFreezeTime()).willReturn(when);
given(dualState.getLastFrozenTime()).willReturn(when);
given(app.workingState()).willReturn(workingState);

given(app.hashLogger()).willReturn(hashLogger);
Expand All @@ -692,7 +680,6 @@ void nonGenesisInitClearsPreparedUpgradeIfDeployedIsLaterVersion() {
subject.init(platform, dualState, RESTART, justPriorVersion);

verify(networkContext).discardPreparedUpgradeMeta();
verify(dualState).setFreezeTime(null);
unmockMigrators();
}

Expand All @@ -707,10 +694,6 @@ void nonGenesisInitWithOldVersionMarksMigrationRecordsNotStreamed() {
subject.setChild(StateChildIndices.ACCOUNTS, accounts);
subject.setDeserializedStateVersion(StateVersions.RELEASE_0310_VERSION);

final var when = Instant.ofEpochSecond(1_234_567L, 890);
given(dualState.getFreezeTime()).willReturn(when);
given(dualState.getLastFrozenTime()).willReturn(when);

given(app.hashLogger()).willReturn(hashLogger);
given(app.initializationFlow()).willReturn(initFlow);
given(app.dualStateAccessor()).willReturn(dualStateAccessor);
Expand All @@ -725,7 +708,6 @@ void nonGenesisInitWithOldVersionMarksMigrationRecordsNotStreamed() {

verify(networkContext).discardPreparedUpgradeMeta();
verify(networkContext).markMigrationRecordsNotYetStreamed();
verify(dualState).setFreezeTime(null);

unmockMigrators();
}
Expand Down Expand Up @@ -777,10 +759,6 @@ void nonGenesisInitConsolidatesRecords() {
subject.setChild(StateChildIndices.CONTRACT_STORAGE, vmap);
subject.setChild(StateChildIndices.PAYER_RECORDS_OR_CONSOLIDATED_FCQ, mmap);

final var when = Instant.ofEpochSecond(1_234_567L, 890);
given(dualState.getFreezeTime()).willReturn(when);
given(dualState.getLastFrozenTime()).willReturn(when);

given(app.hashLogger()).willReturn(hashLogger);
given(app.initializationFlow()).willReturn(initFlow);
given(app.dualStateAccessor()).willReturn(dualStateAccessor);
Expand Down Expand Up @@ -821,8 +799,6 @@ void nonGenesisInitHandlesNftMigration() {
subject.setChild(StateChildIndices.CONTRACT_STORAGE, vmap);

final var when = Instant.ofEpochSecond(1_234_567L, 890);
given(dualState.getFreezeTime()).willReturn(when);
given(dualState.getLastFrozenTime()).willReturn(when);

given(app.hashLogger()).willReturn(hashLogger);
given(app.initializationFlow()).willReturn(initFlow);
Expand Down Expand Up @@ -873,10 +849,6 @@ void nonGenesisInitHandlesTokenRelMigrationToDisk() {
subject.setChild(StateChildIndices.STORAGE, vmap);
subject.setChild(StateChildIndices.CONTRACT_STORAGE, vmap);

final var when = Instant.ofEpochSecond(1_234_567L, 890);
given(dualState.getFreezeTime()).willReturn(when);
given(dualState.getLastFrozenTime()).willReturn(when);

given(app.hashLogger()).willReturn(hashLogger);
given(app.initializationFlow()).willReturn(initFlow);
given(app.dualStateAccessor()).willReturn(dualStateAccessor);
Expand Down
52 changes: 51 additions & 1 deletion ...-service/src/test/java/com/hedera/node/app/service/mono/state/logic/BlockManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,19 @@
import static org.mockito.Mockito.verify;

import com.hedera.node.app.service.mono.context.properties.BootstrapProperties;
import com.hedera.node.app.service.mono.state.DualStateAccessor;
import com.hedera.node.app.service.mono.state.merkle.MerkleNetworkContext;
import com.hedera.node.app.service.mono.stream.RecordsRunningHashLeaf;
import com.hedera.test.utils.TxnUtils;
import com.swirlds.common.crypto.Hash;
import com.swirlds.common.crypto.RunningHash;
import com.swirlds.common.system.SwirldDualState;
import java.time.Instant;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
Expand All @@ -57,13 +60,20 @@ class BlockManagerTest {
@Mock
private RecordsRunningHashLeaf runningHashLeaf;

private DualStateAccessor dualStateAccessor;

@Mock
private SwirldDualState dualState;

private BlockManager subject;

@BeforeEach
void setUp() {
given(bootstrapProperties.getLongProperty(HEDERA_RECORD_STREAM_LOG_PERIOD))
.willReturn(blockPeriodSecs);
subject = new BlockManager(bootstrapProperties, () -> networkContext, () -> runningHashLeaf);
dualStateAccessor = new DualStateAccessor();
dualStateAccessor.setDualState(dualState);
subject = new BlockManager(bootstrapProperties, () -> networkContext, () -> runningHashLeaf, dualStateAccessor);
}

@Test
Expand Down Expand Up @@ -160,6 +170,46 @@ void knowsIfNewBlockNowIsTheTimestampAndNumberIncrements() throws InterruptedExc
assertEquals(anotherTime.getEpochSecond(), values.getTimestamp());
}

@Test
void computesNewBlockIfIsFirstTransactionAfterFreezeRestart() throws InterruptedException {
given(networkContext.firstConsTimeOfCurrentBlock()).willReturn(aTime);
given(runningHashLeaf.currentRunningHash()).willReturn(aFullBlockHash);
given(networkContext.getAlignmentBlockNo()).willReturn(someBlockNo);
given(dualState.getLastFrozenTime()).willReturn(aTime);
given(dualState.getFreezeTime()).willReturn(aTime);

final var values = subject.computeBlockValues(someTime, gasLimit);

assertEquals(someBlockNo + 1, values.getNumber());
verify(dualState).setFreezeTime(null);
}

@Test
void computesSameBlockIfPastAndCurrentFreezeTimeAreNull() {
given(networkContext.firstConsTimeOfCurrentBlock()).willReturn(aTime);
given(networkContext.getAlignmentBlockNo()).willReturn(someBlockNo);
given(dualState.getFreezeTime()).willReturn(null);
Mockito.lenient().when(dualState.getLastFrozenTime()).thenReturn(null);

final var values = subject.computeBlockValues(someTime, gasLimit);

assertEquals(someBlockNo, values.getNumber());
verify(dualState, never()).setFreezeTime(null);
}

@Test
void computesSameBlockIfPastAndCurrentFreezeTimeDontMatch() {
given(networkContext.firstConsTimeOfCurrentBlock()).willReturn(aTime);
given(networkContext.getAlignmentBlockNo()).willReturn(someBlockNo);
given(dualState.getLastFrozenTime()).willReturn(aTime);
given(dualState.getFreezeTime()).willReturn(anotherTime);

final var values = subject.computeBlockValues(someTime, gasLimit);

assertEquals(someBlockNo, values.getNumber());
verify(dualState, never()).setFreezeTime(null);
}

@Test
void knowsIfBlockIsSameThenNetworkCtxApplies() {
given(networkContext.firstConsTimeOfCurrentBlock()).willReturn(aTime);
Expand Down

0 comments on commit ce61cab

Please sign in to comment.