Skip to content

Commit

Permalink
Use the DualState from handleRound instead of from initialization for…
Browse files Browse the repository at this point in the history
… the DualStateUpdateFacility (#8516)

Signed-off-by: Richard Bair <[email protected]>
  • Loading branch information
rbair23 authored Sep 12, 2023
1 parent f9532a3 commit 58b207f
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,13 @@
import com.hedera.node.app.service.util.impl.UtilServiceImpl;
import com.hedera.node.app.services.ServicesRegistryImpl;
import com.hedera.node.app.spi.HapiUtils;
import com.hedera.node.app.spi.Service;
import com.hedera.node.app.state.HederaState;
import com.hedera.node.app.state.merkle.MerkleHederaState;
import com.hedera.node.app.state.merkle.MerkleSchemaRegistry;
import com.hedera.node.app.state.recordcache.RecordCacheService;
import com.hedera.node.app.throttle.ThrottleManager;
import com.hedera.node.app.version.HederaSoftwareVersion;
import com.hedera.node.app.workflows.dispatcher.ReadableStoreFactory;
import com.hedera.node.app.workflows.handle.DualStateUpdateFacility;
import com.hedera.node.app.workflows.handle.SystemFileUpdateFacility;
import com.hedera.node.config.ConfigProvider;
import com.hedera.node.config.Utils;
Expand Down Expand Up @@ -123,17 +121,6 @@ public final class Hedera implements SwirldMain {
private static final Logger logger = LogManager.getLogger(Hedera.class);
// FUTURE: This should come from configuration, not be hardcoded.
public static final int MAX_SIGNED_TXN_SIZE = 6144;

/**
* Defines the registration information for a service.
*
* @param name The name of the service.
* @param service The service implementation itself.
* @param registry The {@link MerkleSchemaRegistry} with which the service registers its schemas.
*/
private record ServiceRegistration(
@NonNull String name, @NonNull Service service, @NonNull MerkleSchemaRegistry registry) {}

/** The registry of all known services */
private final ServicesRegistryImpl servicesRegistry;
/** The current version of THIS software */
Expand Down Expand Up @@ -342,8 +329,8 @@ private void onStateInitialized(
// here. This is intentional so as to avoid forgetting to handle a new trigger.
try {
switch (trigger) {
case GENESIS -> genesis(state, dualState);
case RESTART -> restart(state, dualState, deserializedVersion);
case GENESIS -> genesis(state);
case RESTART -> restart(state, deserializedVersion);
case RECONNECT -> reconnect();
// We exited from this method early if we were recovering from an event stream.
case EVENT_STREAM_RECOVERY -> throw new RuntimeException("Should never be reached");
Expand Down Expand Up @@ -584,7 +571,7 @@ private void onPreHandle(@NonNull final Event event, @NonNull final HederaState
private void onHandleConsensusRound(
@NonNull final Round round, @NonNull final SwirldDualState dualState, @NonNull final HederaState state) {
daggerApp.workingStateAccessor().setHederaState(state);
daggerApp.handleWorkflow().handleRound(state, round);
daggerApp.handleWorkflow().handleRound(state, dualState, round);
}

/*==================================================================================================================
Expand Down Expand Up @@ -612,7 +599,7 @@ public void shutdownGrpcServer() {
=================================================================================================================*/

/** Implements the code flow for initializing the state of a new Hedera node with NO SAVED STATE. */
private void genesis(@NonNull final MerkleHederaState state, @NonNull final SwirldDualState dualState) {
private void genesis(@NonNull final MerkleHederaState state) {
logger.debug("Genesis Initialization");

// Initialize the configuration from disk (genesis case). We must do this BEFORE we run migration, because
Expand All @@ -634,7 +621,7 @@ private void genesis(@NonNull final MerkleHederaState state, @NonNull final Swir
onMigrate(state, null);

// Now that we have the state created, we are ready to create the dependency graph with Dagger
initializeDagger(state, dualState, GENESIS);
initializeDagger(state, GENESIS);

// And now that the entire dependency graph has been initialized, and we have config, and all migration has
// been completed, we are prepared to initialize in-memory data structures. These specifically are loaded
Expand All @@ -652,9 +639,7 @@ private void genesis(@NonNull final MerkleHederaState state, @NonNull final Swir

/** Initialize flow for when a node has been restarted. This means it was started from a saved state. */
private void restart(
@NonNull final MerkleHederaState state,
@NonNull final SwirldDualState dualState,
@Nullable final HederaSoftwareVersion deserializedVersion) {
@NonNull final MerkleHederaState state, @Nullable final HederaSoftwareVersion deserializedVersion) {
logger.debug("Restart Initialization");

// The deserialized version can ONLY be null if we are in genesis, otherwise something is wrong with the state
Expand All @@ -681,7 +666,7 @@ private void restart(
onMigrate(state, deserializedVersion);

// Now that we have the state created, we are ready to create the dependency graph with Dagger
initializeDagger(state, dualState, RESTART);
initializeDagger(state, RESTART);

// And now that the entire dependency graph has been initialized, and we have config, and all migration has
// been completed, we are prepared to initialize in-memory data structures. These specifically are loaded
Expand All @@ -708,10 +693,7 @@ private void reconnect() {
*
=================================================================================================================*/

private void initializeDagger(
@NonNull final MerkleHederaState state,
@NonNull final SwirldDualState dualState,
@NonNull final InitTrigger trigger) {
private void initializeDagger(@NonNull final MerkleHederaState state, @NonNull final InitTrigger trigger) {
logger.debug("Initializing dagger");
final var selfId = platform.getSelfId();
if (daggerApp == null) {
Expand All @@ -726,7 +708,6 @@ private void initializeDagger(
.exchangeRateManager(exchangeRateManager)
.systemFileUpdateFacility(
new SystemFileUpdateFacility(configProvider, throttleManager, exchangeRateManager))
.dualStateUpdateFacility(new DualStateUpdateFacility(dualState))
.self(SelfNodeInfoImpl.of(nodeAddress, version))
.initialHash(initialHash)
.platform(platform)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ public interface HederaInjectionComponent {

ThrottleManager throttleManager();

DualStateUpdateFacility dualStateUpdateFacility();

@Component.Builder
interface Builder {

Expand Down Expand Up @@ -148,9 +150,6 @@ interface Builder {
@BindsInstance
Builder systemFileUpdateFacility(SystemFileUpdateFacility systemFileUpdateFacility);

@BindsInstance
Builder dualStateUpdateFacility(DualStateUpdateFacility dualStateUpdateFacility);

@BindsInstance
Builder exchangeRateManager(ExchangeRateManager exchangeRateManager);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,24 @@
import com.swirlds.common.system.DualState;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.time.Instant;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

/**
* Simple facility that notifies interested parties when the freeze state is updated.
*/
@Singleton
public class DualStateUpdateFacility {
private final DualState dualState;
private static final Logger logger = LogManager.getLogger(DualStateUpdateFacility.class);

/**
* Creates a new instance of this class.
*
* @param dualState the configuration provider
*/
public DualStateUpdateFacility(@NonNull final DualState dualState) {
this.dualState = requireNonNull(dualState, "configProvider must not be null");
@Inject
public DualStateUpdateFacility() {
// For dagger
}

/**
Expand All @@ -55,22 +59,28 @@ public DualStateUpdateFacility(@NonNull final DualState dualState) {
* @param state the current state
* @param txBody the transaction body
*/
public void handleTxBody(@NonNull final HederaState state, @NonNull final TransactionBody txBody) {
public void handleTxBody(
@NonNull final HederaState state,
@NonNull final DualState dualState,
@NonNull final TransactionBody txBody) {
requireNonNull(state, "state must not be null");
requireNonNull(txBody, "txBody must not be null");

if (txBody.hasFreeze()) {
final FreezeType freezeType = txBody.freezeOrThrow().freezeType();
if (freezeType == FREEZE_UPGRADE || freezeType == FREEZE_ONLY) {
logger.info("Transaction freeze of type {} detected", freezeType);
// copy freeze state to dual state
final ReadableStates states = state.createReadableStates(FreezeService.NAME);
final ReadableSingletonState<Timestamp> freezeTime =
states.getSingleton(FreezeServiceImpl.FREEZE_TIME_KEY);
requireNonNull(freezeTime.get());
final Instant freezeTimeInstant = Instant.ofEpochSecond(
freezeTime.get().seconds(), freezeTime.get().nanos());
logger.info("Freeze time will be {}", freezeTimeInstant);
dualState.setFreezeTime(freezeTimeInstant);
} else if (freezeType == FREEZE_ABORT) {
logger.info("Aborting freeze");
// copy freeze state (which is null) to dual state
// we just set dual state to null
dualState.setFreezeTime(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import com.hedera.node.config.data.HederaConfig;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import com.swirlds.common.system.Round;
import com.swirlds.common.system.SwirldDualState;
import com.swirlds.common.system.events.ConsensusEvent;
import com.swirlds.common.system.transaction.ConsensusTransaction;
import com.swirlds.config.api.Configuration;
Expand Down Expand Up @@ -162,7 +163,8 @@ public HandleWorkflow(
* @param state the writable {@link HederaState} that this round will work on
* @param round the next {@link Round} that needs to be processed
*/
public void handleRound(@NonNull final HederaState state, @NonNull final Round round) {
public void handleRound(
@NonNull final HederaState state, @NonNull final SwirldDualState dualState, @NonNull final Round round) {
// Keep track of whether any user transactions were handled. If so, then we will need to close the round
// with the block record manager.
final var userTransactionsHandled = new AtomicBoolean(false);
Expand All @@ -187,7 +189,7 @@ public void handleRound(@NonNull final HederaState state, @NonNull final Round r
// skip system transactions
if (!platformTxn.isSystem()) {
userTransactionsHandled.set(true);
handlePlatformTransaction(state, event, creator, platformTxn);
handlePlatformTransaction(state, dualState, event, creator, platformTxn);
}
} catch (final Exception e) {
logger.fatal(
Expand All @@ -209,6 +211,7 @@ public void handleRound(@NonNull final HederaState state, @NonNull final Round r

private void handlePlatformTransaction(
@NonNull final HederaState state,
@NonNull final SwirldDualState dualState,
@NonNull final ConsensusEvent platformEvent,
@NonNull final NodeInfo creator,
@NonNull final ConsensusTransaction platformTxn) {
Expand All @@ -217,7 +220,7 @@ private void handlePlatformTransaction(
final Instant consensusNow = platformTxn.getConsensusTimestamp();

// handle user transaction
handleUserTransaction(consensusNow, state, platformEvent, creator, platformTxn);
handleUserTransaction(consensusNow, state, dualState, platformEvent, creator, platformTxn);

// TODO: handle long scheduled transactions

Expand All @@ -228,6 +231,7 @@ private void handlePlatformTransaction(
private void handleUserTransaction(
@NonNull final Instant consensusNow,
@NonNull final HederaState state,
@NonNull final SwirldDualState dualState,
@NonNull final ConsensusEvent platformEvent,
@NonNull final NodeInfo creator,
@NonNull final ConsensusTransaction platformTxn) {
Expand Down Expand Up @@ -346,7 +350,7 @@ private void handleUserTransaction(
systemFileUpdateFacility.handleTxBody(stack, txBody);

// Notify if dual state was updated
dualStateUpdateFacility.handleTxBody(stack, txBody);
dualStateUpdateFacility.handleTxBody(stack, dualState, txBody);

} catch (final HandleException e) {
rollback(e.getStatus(), stack, recordListBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import com.hedera.node.app.state.recordcache.RecordCacheService;
import com.hedera.node.app.throttle.ThrottleManager;
import com.hedera.node.app.version.HederaSoftwareVersion;
import com.hedera.node.app.workflows.handle.DualStateUpdateFacility;
import com.hedera.node.app.workflows.handle.SystemFileUpdateFacility;
import com.hedera.node.config.testfixtures.HederaTestConfigBuilder;
import com.swirlds.common.context.PlatformContext;
Expand Down Expand Up @@ -101,7 +100,6 @@ void setUp() {
.configuration(configProvider)
.systemFileUpdateFacility(
new SystemFileUpdateFacility(configProvider, throttleManager, exchangeRateManager))
.dualStateUpdateFacility(new DualStateUpdateFacility(dualState))
.throttleManager(throttleManager)
.self(selfNodeInfo)
.initialHash(new Hash())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ void setUp() {
.when(dualState)
.setFreezeTime(any(Instant.class));

subject = new DualStateUpdateFacility(dualState);
subject = new DualStateUpdateFacility();
}

@SuppressWarnings("ConstantConditions")
Expand All @@ -82,9 +82,10 @@ void testMethodsWithInvalidArguments() {
final var txBody = simpleCryptoTransfer().body();

// then
assertThatThrownBy(() -> new DualStateUpdateFacility(null)).isInstanceOf(NullPointerException.class);
assertThatThrownBy(() -> subject.handleTxBody(null, txBody)).isInstanceOf(NullPointerException.class);
assertThatThrownBy(() -> subject.handleTxBody(state, null)).isInstanceOf(NullPointerException.class);
assertThatThrownBy(() -> subject.handleTxBody(null, dualState, txBody))
.isInstanceOf(NullPointerException.class);
assertThatThrownBy(() -> subject.handleTxBody(state, null, txBody)).isInstanceOf(NullPointerException.class);
assertThatThrownBy(() -> subject.handleTxBody(state, dualState, null)).isInstanceOf(NullPointerException.class);
}

@Test
Expand All @@ -95,7 +96,8 @@ void testCryptoTransferShouldBeNoOp() {
.build();

// then
Assertions.assertThatCode(() -> subject.handleTxBody(state, txBody)).doesNotThrowAnyException();
Assertions.assertThatCode(() -> subject.handleTxBody(state, dualState, txBody))
.doesNotThrowAnyException();
}

@Test
Expand All @@ -107,7 +109,7 @@ void testFreezeUpgrade() {
.freeze(FreezeTransactionBody.newBuilder().freezeType(FREEZE_UPGRADE));

// when
subject.handleTxBody(state, txBody.build());
subject.handleTxBody(state, dualState, txBody.build());

// then
assertEquals(freezeTime.seconds(), dualState.getFreezeTime().getEpochSecond());
Expand Down
Loading

0 comments on commit 58b207f

Please sign in to comment.