chore: misc HIP-423 fixes (#17008)

Signed-off-by: Michael Tinker <[email protected]>

hedera-node/hapi-utils/src/main/java/com/hedera/node/app/hapi/utils/CommonPbjConverters.java

@@ -170,6 +170,15 @@ public static <T extends Record> byte[] asBytes(@NonNull Codec<T> codec, @NonNul
         };
     }
 
+    /**
+     * Given a PBJ type, converts it to a proto type.
+     * @param pbj the PBJ type
+     * @param pbjClass the PBJ class
+     * @param protoClass the proto class
+     * @return the proto type
+     * @param <T> the PBJ type
+     * @param <R> the proto type
+     */
     public static <T extends Record, R extends GeneratedMessageV3> R pbjToProto(
             final T pbj, final Class<T> pbjClass, final Class<R> protoClass) {
         try {

hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/DispatchProcessor.java

@@ -27,7 +27,6 @@
 import static com.hedera.hapi.node.base.ResponseCodeEnum.SUCCESS;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.UNAUTHORIZED;
 import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.NODE;
-import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.PRECEDING;
 import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.USER;
 import static com.hedera.node.app.workflows.handle.HandleWorkflow.ALERT_MESSAGE;
 import static com.hedera.node.app.workflows.handle.dispatch.DispatchValidator.DuplicateStatus.DUPLICATE;
@@ -128,11 +127,7 @@ public void processDispatch(@NonNull final Dispatch dispatch) {
         }
         dispatchUsageManager.finalizeAndSaveUsage(dispatch);
         recordFinalizer.finalizeRecord(dispatch);
-        if (dispatch.txnCategory() == USER || dispatch.txnCategory() == NODE) {
-            dispatch.stack().commitTransaction(dispatch.recordBuilder());
-        } else {
-            dispatch.stack().commitFullStack();
-        }
+        dispatch.stack().commitFullStack();
     }
 
     /**
@@ -150,12 +145,7 @@ private void tryHandle(@NonNull final Dispatch dispatch, @NonNull final Validati
             dispatchUsageManager.screenForCapacity(dispatch);
             dispatcher.dispatchHandle(dispatch.handleContext());
             dispatch.recordBuilder().status(SUCCESS);
-            // Only user or preceding transactions can trigger system updates in the current system
-            if (dispatch.txnCategory() == USER
-                    || dispatch.txnCategory() == PRECEDING
-                    || dispatch.txnCategory() == NODE) {
-                handleSystemUpdates(dispatch);
-            }
+            handleSystemUpdates(dispatch);
         } catch (HandleException e) {
             // In case of a ContractCall when it reverts, the gas charged should not be rolled back
             rollback(e.shouldRollbackStack(), e.getStatus(), dispatch.stack(), dispatch.recordBuilder());

hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/dispatch/RecordFinalizer.java

@@ -18,6 +18,7 @@
 
 import static com.hedera.hapi.node.base.ResponseCodeEnum.SUCCESS;
 import static java.util.Collections.emptySet;
+import static java.util.Objects.requireNonNull;
 
 import com.hedera.hapi.node.base.AccountAmount;
 import com.hedera.hapi.node.base.AccountID;
@@ -60,15 +61,17 @@ public RecordFinalizer(final FinalizeRecordHandler recordFinalizer) {
      * and the child record finalizer is used for child and preceding transactions.
      * @param dispatch the dispatch
      */
-    public void finalizeRecord(final Dispatch dispatch) {
-        switch (dispatch.txnCategory()) {
-            case USER, SCHEDULED -> recordFinalizer.finalizeStakingRecord(
+    public void finalizeRecord(@NonNull final Dispatch dispatch) {
+        requireNonNull(dispatch);
+        if (dispatch.stack().permitsStakingRewards()) {
+            recordFinalizer.finalizeStakingRecord(
                     dispatch.finalizeContext(),
                     dispatch.txnInfo().functionality(),
                     extraRewardReceivers(
                             dispatch.txnInfo().txBody(), dispatch.txnInfo().functionality(), dispatch.recordBuilder()),
                     dispatch.handleContext().dispatchPaidRewards());
-            case CHILD, PRECEDING -> recordFinalizer.finalizeNonStakingRecord(
+        } else {
+            recordFinalizer.finalizeNonStakingRecord(
                     dispatch.finalizeContext(), dispatch.txnInfo().functionality());
         }
     }

hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/stack/SavepointStackImpl.java

@@ -37,6 +37,7 @@
 import com.hedera.node.app.blocks.impl.PairedStreamBuilder;
 import com.hedera.node.app.spi.records.RecordSource;
 import com.hedera.node.app.spi.workflows.HandleContext;
+import com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory;
 import com.hedera.node.app.spi.workflows.record.StreamBuilder;
 import com.hedera.node.app.state.ReadonlyStatesWrapper;
 import com.hedera.node.app.state.SingleTransactionRecord;
@@ -132,7 +133,7 @@ public static SavepointStackImpl newRootStack(
     public static SavepointStackImpl newChildStack(
             @NonNull final SavepointStackImpl root,
             @NonNull final StreamBuilder.ReversingBehavior reversingBehavior,
-            @NonNull final HandleContext.TransactionCategory category,
+            @NonNull final TransactionCategory category,
             @NonNull final StreamBuilder.TransactionCustomizer customizer,
             @NonNull final StreamMode streamMode) {
         return new SavepointStackImpl(root, reversingBehavior, category, customizer, streamMode);
@@ -177,7 +178,7 @@ private SavepointStackImpl(
     private SavepointStackImpl(
             @NonNull final SavepointStackImpl parent,
             @NonNull final StreamBuilder.ReversingBehavior reversingBehavior,
-            @NonNull final HandleContext.TransactionCategory category,
+            @NonNull final TransactionCategory category,
             @NonNull final StreamBuilder.TransactionCustomizer customizer,
             @NonNull final StreamMode streamMode) {
         requireNonNull(reversingBehavior);
@@ -274,6 +275,28 @@ public void rollbackFullStack() {
         setupFirstSavepoint(baseBuilder.category());
     }
 
+    /**
+     * Returns true when this stack's base builder should be finalized with staking rewards. There are
+     * two qualifying cases:
+     * <ol>
+     *     <li>The stack is for top-level transaction (either a user transaction or a triggered execution
+     *     like a expiring scheduled transaction with {@code wait_for_expiry=true}); or,</li>
+     *     <li>The stack is for executing a scheduled transaction with {@code wait_for_expiry=false}, and
+     *     whose triggering parent was a user transaction.</li>
+     * </ol>
+     * The second category is solely for backward compatibility with mono-service, and should be considered
+     * for deprecation and removal.
+     */
+    public boolean permitsStakingRewards() {
+        return builderSink != null
+                ||
+                // For backward compatibility with mono-service, we permit paying staking rewards to
+                // scheduled transactions that are exactly children of user transactions
+                (baseBuilder.category() == SCHEDULED
+                        && state instanceof SavepointStackImpl parent
+                        && parent.txnCategory() == USER);
+    }
+
     /**
      * Returns the root {@link ReadableStates} for the given service name.
      *
@@ -397,11 +420,11 @@ public <T> void forEachNonBaseBuilder(@NonNull final Class<T> builderClass, @Non
     }
 
     /**
-     * Returns the {@link HandleContext.TransactionCategory} of the transaction that created this stack.
+     * Returns the {@link TransactionCategory} of the transaction that created this stack.
      *
      * @return the transaction category
      */
-    public HandleContext.TransactionCategory txnCategory() {
+    public TransactionCategory txnCategory() {
         return baseBuilder.category();
     }
 
@@ -524,7 +547,7 @@ public HandleOutput buildHandleOutput(
         return new HandleOutput(blockRecordSource, recordSource);
     }
 
-    private void setupFirstSavepoint(@NonNull final HandleContext.TransactionCategory category) {
+    private void setupFirstSavepoint(@NonNull final TransactionCategory category) {
         if (state instanceof SavepointStackImpl parent) {
             stack.push(new FirstChildSavepoint(new WrappedState(state), parent.peek(), category));
         } else {

hedera-node/hedera-app/src/test/java/com/hedera/node/app/workflows/handle/DispatchProcessorTest.java

@@ -218,9 +218,7 @@ void waivedFeesDoesNotCharge() {
                 .willReturn(true);
         given(recordBuilder.exchangeRate(any())).willReturn(recordBuilder);
         given(dispatch.handleContext()).willReturn(context);
-        given(dispatch.txnCategory()).willReturn(USER);
         givenAuthorization();
-        given(dispatch.txnCategory()).willReturn(USER);
 
         subject.processDispatch(dispatch);
 
@@ -564,7 +562,6 @@ void happyPathFreeChildCryptoTransferAsExpected() {
         given(dispatchValidator.validationReportFor(dispatch)).willReturn(newSuccess(CREATOR_ACCOUNT_ID, PAYER));
         given(dispatch.payerId()).willReturn(PAYER_ACCOUNT_ID);
         given(dispatch.txnInfo()).willReturn(CRYPTO_TRANSFER_TXN_INFO);
-        given(dispatch.txnCategory()).willReturn(HandleContext.TransactionCategory.CHILD);
         given(dispatch.handleContext()).willReturn(context);
         givenAuthorization(CRYPTO_TRANSFER_TXN_INFO);
 
@@ -648,11 +645,7 @@ private void assertFinished() {
 
     private void assertFinished(@NonNull final IsRootStack isRootStack) {
         verify(recordFinalizer).finalizeRecord(dispatch);
-        if (isRootStack == IsRootStack.YES) {
-            verify(stack).commitTransaction(any());
-        } else {
-            verify(stack).commitFullStack();
-        }
+        verify(stack).commitFullStack();
     }
 
     private void verifyTrackedFeePayments() {

hedera-node/hedera-app/src/test/java/com/hedera/node/app/workflows/handle/dispatch/RecordFinalizerTest.java

@@ -20,17 +20,16 @@
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_TRANSACTION;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.SUCCESS;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.TOKEN_NOT_ASSOCIATED_TO_ACCOUNT;
-import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.CHILD;
 import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.USER;
 import static com.hedera.node.app.spi.workflows.record.StreamBuilder.ReversingBehavior.REVERSIBLE;
 import static com.hedera.node.app.spi.workflows.record.StreamBuilder.TransactionCustomizer.NOOP_TRANSACTION_CUSTOMIZER;
 import static com.hedera.node.app.workflows.handle.steps.HollowAccountCompletionsTest.asTxn;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -48,6 +47,7 @@
 import com.hedera.node.app.workflows.TransactionInfo;
 import com.hedera.node.app.workflows.handle.Dispatch;
 import com.hedera.node.app.workflows.handle.record.RecordStreamBuilder;
+import com.hedera.node.app.workflows.handle.stack.SavepointStackImpl;
 import com.hedera.pbj.runtime.io.buffer.Bytes;
 import java.time.Instant;
 import java.util.List;
@@ -69,6 +69,9 @@ public class RecordFinalizerTest {
     @Mock
     private Dispatch dispatch;
 
+    @Mock
+    private SavepointStackImpl stack;
+
     @Mock
     private FinalizeContext finalizeContext;
 
@@ -114,8 +117,9 @@ void setUp() {
     }
 
     @Test
-    public void testFinalizeRecordUserTransaction() {
-        when(dispatch.txnCategory()).thenReturn(USER);
+    public void finalizesStakingRecordForScheduledDispatchOfUserTxn() {
+        given(dispatch.stack()).willReturn(stack);
+        given(stack.permitsStakingRewards()).willReturn(true);
 
         when(dispatch.handleContext().dispatchPaidRewards()).thenReturn(Map.of());
 
@@ -126,12 +130,13 @@ public void testFinalizeRecordUserTransaction() {
     }
 
     @Test
-    public void testFinalizeRecordChildTransaction() {
-        when(dispatch.txnCategory()).thenReturn(CHILD);
+    public void finalizesNonStakingRecordForIneligibleDispatchStack() {
+        given(dispatch.stack()).willReturn(stack);
 
         subject.finalizeRecord(dispatch);
+
         verify(finalizeRecordHandler, never()).finalizeStakingRecord(any(), any(), any(), any());
-        verify(finalizeRecordHandler, times(1)).finalizeNonStakingRecord(any(), any());
+        verify(finalizeRecordHandler).finalizeNonStakingRecord(any(), any());
     }
 
     @Test

hedera-node/hedera-app/src/test/java/com/hedera/node/app/workflows/handle/stack/SavepointStackImplTest.java

@@ -16,14 +16,19 @@
 
 package com.hedera.node.app.workflows.handle.stack;
 
+import static com.hedera.node.app.spi.workflows.HandleContext.TransactionCategory.SCHEDULED;
+import static com.hedera.node.app.spi.workflows.record.StreamBuilder.ReversingBehavior.REVERSIBLE;
+import static com.hedera.node.app.spi.workflows.record.StreamBuilder.TransactionCustomizer.NOOP_TRANSACTION_CUSTOMIZER;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.BDDMockito.given;
 import static org.mockito.Mock.Strictness.LENIENT;
 import static org.mockito.Mockito.when;
 
 import com.hedera.node.app.blocks.impl.BoundaryStateChangeListener;
 import com.hedera.node.app.blocks.impl.KVStateChangeListener;
+import com.hedera.node.app.spi.workflows.HandleContext;
 import com.hedera.node.config.VersionedConfigImpl;
 import com.hedera.node.config.data.BlockStreamConfig;
 import com.hedera.node.config.testfixtures.HederaTestConfigBuilder;
@@ -48,7 +53,6 @@
 
 @ExtendWith(MockitoExtension.class)
 class SavepointStackImplTest extends StateTestBase {
-
     private static final String FOOD_SERVICE = "FOOD_SERVICE";
 
     private final Map<String, String> BASE_DATA = Map.of(
@@ -63,6 +67,12 @@ class SavepointStackImplTest extends StateTestBase {
     @Mock(strictness = LENIENT)
     private State baseState;
 
+    @Mock
+    private SavepointStackImpl parent;
+
+    @Mock
+    private Savepoint savepoint;
+
     @Mock
     private BoundaryStateChangeListener roundStateChangeListener;
 
@@ -82,6 +92,46 @@ void setup() {
         streamMode = config.getConfigData(BlockStreamConfig.class).streamMode();
     }
 
+    @Test
+    void topLevelPermitsStakingRewards() {
+        final var subject = SavepointStackImpl.newRootStack(
+                baseState, 3, 50, roundStateChangeListener, kvStateChangeListener, StreamMode.BOTH);
+        assertThat(subject.permitsStakingRewards()).isTrue();
+    }
+
+    @Test
+    void childDoesNotPermitStakingRewardsIfNotScheduled() {
+        given(parent.peek()).willReturn(savepoint);
+        given(savepoint.followingCapacity()).willReturn(123);
+        final var subject = SavepointStackImpl.newChildStack(
+                parent,
+                REVERSIBLE,
+                HandleContext.TransactionCategory.CHILD,
+                NOOP_TRANSACTION_CUSTOMIZER,
+                StreamMode.BOTH);
+        assertThat(subject.permitsStakingRewards()).isFalse();
+    }
+
+    @Test
+    void childDoesNotPermitStakingRewardsIfNotScheduledByUser() {
+        given(parent.peek()).willReturn(savepoint);
+        given(savepoint.followingCapacity()).willReturn(123);
+        given(parent.txnCategory()).willReturn(HandleContext.TransactionCategory.CHILD);
+        final var subject = SavepointStackImpl.newChildStack(
+                parent, REVERSIBLE, SCHEDULED, NOOP_TRANSACTION_CUSTOMIZER, StreamMode.BOTH);
+        assertThat(subject.permitsStakingRewards()).isFalse();
+    }
+
+    @Test
+    void scheduledTopLevelIfSchedulingParentIsUser() {
+        given(parent.peek()).willReturn(savepoint);
+        given(savepoint.followingCapacity()).willReturn(123);
+        given(parent.txnCategory()).willReturn(HandleContext.TransactionCategory.USER);
+        final var subject = SavepointStackImpl.newChildStack(
+                parent, REVERSIBLE, SCHEDULED, NOOP_TRANSACTION_CUSTOMIZER, StreamMode.BOTH);
+        assertThat(subject.permitsStakingRewards()).isTrue();
+    }
+
     @Test
     void testConstructor() {
         // when

hedera-node/hedera-app/src/testFixtures/java/com/hedera/node/app/fixtures/state/FakeState.java

@@ -61,6 +61,14 @@ public class FakeState implements State {
      */
     private final List<StateChangeListener> listeners = new ArrayList<>();
 
+    /**
+     * Exposes the underlying states for direct manipulation in tests.
+     * @return the states
+     */
+    public Map<String, Map<String, Object>> getStates() {
+        return states;
+    }
+
     /**
      * Adds to the service with the given name the {@link ReadableKVState} {@code states}
      */
@@ -73,8 +81,7 @@ public FakeState addService(@NonNull final String serviceName, @NonNull final Ma
         });
         // Purge any readable or writable states whose state definitions are now stale,
         // since they don't include the new data sources we just added
-        readableStates.remove(serviceName);
-        writableStates.remove(serviceName);
+        purgeStatesCaches(serviceName);
         return this;
     }
 
@@ -91,6 +98,7 @@ public void removeServiceState(@NonNull final String serviceName, @NonNull final
             v.remove(stateKey);
             return v;
         });
+        purgeStatesCaches(serviceName);
     }
 
     @NonNull
@@ -242,4 +250,9 @@ public void mapDeleteChange(@NonNull final K key) {
             }
         });
     }
+
+    private void purgeStatesCaches(@NonNull final String serviceName) {
+        readableStates.remove(serviceName);
+        writableStates.remove(serviceName);
+    }
 }