9944 Fix record cache handling to remediate "RECORD_NOT_FOUND" respon…

…ses. (#9946) Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com> Signed-off-by: Joseph Sinclair <joseph.sinclair@swirldslabs.com> Co-authored-by: Michael Tinker <michael.tinker@swirldslabs.com>
hashgraph · Nov 20, 2023 · 9350fa9 · 9350fa9
1 parent 3436b87
commit 9350fa9
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 37 deletions.
diff --git a/...edera-app/src/main/java/com/hedera/node/app/state/recordcache/DeduplicationCacheImpl.java b/...edera-app/src/main/java/com/hedera/node/app/state/recordcache/DeduplicationCacheImpl.java
@@ -39,11 +39,16 @@
 public final class DeduplicationCacheImpl implements DeduplicationCache {
     /**
      * The {@link TransactionID}s that this node has already submitted to the platform, sorted by transaction start
-     * time, such that earlier start times come first. We guard this data structure within a synchronized block.
+     * time, such that earlier start times come first.
+     * <p>
+     * Note that an ID with scheduled set is different from the same ID without scheduled set.
+     * In fact, an ID with scheduled set will always match the ID of the ScheduleCreate transaction that created
+     * the schedule, except scheduled is set.
      */
     private final Set<TransactionID> submittedTxns = new ConcurrentSkipListSet<>(
             (t1, t2) -> Comparator.comparing(TransactionID::transactionValidStartOrThrow, TIMESTAMP_COMPARATOR)
                     .thenComparing(TransactionID::accountID, ACCOUNT_ID_COMPARATOR)
+                    .thenComparing(TransactionID::scheduled)
                     .compare(t1, t2));
 
     /** Used for looking up the max transaction duration window. */

diff --git a/...-node/hedera-app/src/main/java/com/hedera/node/app/state/recordcache/RecordCacheImpl.java b/...-node/hedera-app/src/main/java/com/hedera/node/app/state/recordcache/RecordCacheImpl.java
@@ -197,7 +197,6 @@ public DuplicateCheckResult hasDuplicate(@NonNull TransactionID transactionID, l
         if (history == null) {
             return DuplicateCheckResult.NO_DUPLICATE;
         }
-
         return history.nodeIds().contains(nodeId) ? DuplicateCheckResult.SAME_NODE : DuplicateCheckResult.OTHER_NODE;
     }
 
@@ -239,8 +238,10 @@ private void addToInMemoryCache(
         history.nodeIds().add(nodeId);
 
         // Either we add this tx to the main records list if it is a user/preceding transaction, or to the child
-        // transactions list of its parent
-        final var listToAddTo = isChildTx ? history.childRecords() : history.records();
+        // transactions list of its parent.  Note that scheduled transactions are always child transactions, but
+        // never produce child *records*; instead, the scheduled transaction record is treated as
+        // a user transaction record.
+        final var listToAddTo = (isChildTx && !txId.scheduled()) ? history.childRecords() : history.records();
         listToAddTo.add(transactionRecord);
 
         // Add to the payer-to-transaction index

diff --git a/...ra-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleWorkflow.java b/...ra-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleWorkflow.java
@@ -257,7 +257,7 @@ private void handlePlatformTransaction(
             @NonNull final ConsensusTransaction platformTxn) {
         // Get the consensus timestamp. FUTURE We want this to exactly match the consensus timestamp from the hashgraph,
         // but for compatibility with the current implementation, we adjust it as follows.
-        final Instant consensusNow = platformTxn.getConsensusTimestamp().minusNanos(1000 + 3L);
+        final Instant consensusNow = platformTxn.getConsensusTimestamp().minusNanos(1000 - 3L);
 
         // handle user transaction
         handleUserTransaction(consensusNow, state, dualState, platformEvent, creator, platformTxn);

diff --git a/...dera-app/src/main/java/com/hedera/node/app/workflows/handle/record/RecordListBuilder.java b/...dera-app/src/main/java/com/hedera/node/app/workflows/handle/record/RecordListBuilder.java
@@ -23,6 +23,7 @@
 import static java.util.Objects.requireNonNull;
 
 import com.hedera.hapi.node.base.ResponseCodeEnum;
+import com.hedera.hapi.node.base.TransactionID;
 import com.hedera.node.app.spi.workflows.HandleException;
 import com.hedera.node.app.spi.workflows.record.ExternalizedRecordCustomizer;
 import com.hedera.node.app.state.SingleTransactionRecord;
@@ -376,25 +377,26 @@ public Result build() {
         // a nonce of N, where N is the number of preceding transactions.
         int count = precedingTxnRecordBuilders == null ? 0 : precedingTxnRecordBuilders.size();
         for (int i = count - 1; i >= 0; i--) {
-            final var recordBuilder = precedingTxnRecordBuilders.get(i);
+            final SingleTransactionRecordBuilderImpl recordBuilder = precedingTxnRecordBuilders.get(i);
             records.add(recordBuilder
                     .transactionID(idBuilder.nonce(i + 1).build())
                     .syncBodyIdFromRecordId()
                     .build());
         }
-
         records.add(userTxnRecord);
 
         int nextNonce = count + 1; // Initialize to be 1 more than the number of preceding items
         count = childRecordBuilders == null ? 0 : childRecordBuilders.size();
         for (int i = 0; i < count; i++) {
-            final var recordBuilder = childRecordBuilders.get(i);
-            records.add(recordBuilder
-                    .transactionID(idBuilder.nonce(nextNonce++).build())
-                    .syncBodyIdFromRecordId()
-                    .build());
+            final SingleTransactionRecordBuilderImpl recordBuilder = childRecordBuilders.get(i);
+            // Only create a new transaction ID for child records if one is not provided
+            if (recordBuilder.transactionID() == null || TransactionID.DEFAULT.equals(recordBuilder.transactionID())) {
+                recordBuilder
+                        .transactionID(idBuilder.nonce(nextNonce++).build())
+                        .syncBodyIdFromRecordId();
+            }
+            records.add(recordBuilder.build());
         }
-
         return new Result(userTxnRecord, unmodifiableList(records));
     }
 

diff --git a/...ode/hedera-app/src/test/java/com/hedera/node/app/workflows/handle/HandleWorkflowTest.java b/...ode/hedera-app/src/test/java/com/hedera/node/app/workflows/handle/HandleWorkflowTest.java
@@ -105,7 +105,7 @@
 class HandleWorkflowTest extends AppTestBase {
 
     private static final Instant CONSENSUS_NOW = Instant.parse("2000-01-01T00:00:00Z");
-    private static final Instant TX_CONSENSUS_NOW = CONSENSUS_NOW.minusNanos(1000 + 3);
+    private static final Instant TX_CONSENSUS_NOW = CONSENSUS_NOW.minusNanos(1000 - 3);
 
     private static final long CONFIG_VERSION = 11L;
 

diff --git a/...rvice/contract/impl/exec/systemcontracts/hts/grantapproval/AbstractGrantApprovalCall.java b/...rvice/contract/impl/exec/systemcontracts/hts/grantapproval/AbstractGrantApprovalCall.java
@@ -19,7 +19,6 @@
 import com.hedera.hapi.node.base.AccountID;
 import com.hedera.hapi.node.base.TokenID;
 import com.hedera.hapi.node.base.TokenType;
-import com.hedera.hapi.node.base.TransactionID;
 import com.hedera.hapi.node.token.CryptoApproveAllowanceTransactionBody;
 import com.hedera.hapi.node.token.NftAllowance;
 import com.hedera.hapi.node.token.TokenAllowance;
@@ -61,7 +60,6 @@ protected AbstractGrantApprovalCall(
 
     public TransactionBody callGrantApproval() {
         return TransactionBody.newBuilder()
-                .transactionID(TransactionID.newBuilder().accountID(senderId).build())
                 .cryptoApproveAllowance(approve(token, spender, amount, tokenType))
                 .build();
     }

diff --git a/...st-clients/src/main/java/com/hedera/services/bdd/suites/schedule/ScheduleRecordSpecs.java b/...st-clients/src/main/java/com/hedera/services/bdd/suites/schedule/ScheduleRecordSpecs.java
@@ -43,6 +43,10 @@
 import static com.hedera.services.bdd.spec.utilops.UtilVerbs.validateChargedUsdWithin;
 import static com.hedera.services.bdd.spec.utilops.UtilVerbs.withOpContext;
 import static com.hedera.services.bdd.suites.schedule.ScheduleLongTermExecutionSpecs.withAndWithoutLongTermEnabled;
+import static com.hedera.services.bdd.suites.schedule.ScheduleUtils.SCHEDULING_WHITELIST;
+import static com.hedera.services.bdd.suites.schedule.ScheduleUtils.STAKING_FEES_NODE_REWARD_PERCENTAGE;
+import static com.hedera.services.bdd.suites.schedule.ScheduleUtils.STAKING_FEES_STAKING_REWARD_PERCENTAGE;
+import static com.hedera.services.bdd.suites.schedule.ScheduleUtils.WHITELIST_MINIMUM;
 import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INSUFFICIENT_PAYER_BALANCE;
 import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INSUFFICIENT_TX_FEE;
 import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.SUCCESS;
@@ -75,9 +79,6 @@ public class ScheduleRecordSpecs extends HapiSuite {
     private static final String PAYING_SENDER = "payingSender";
     private static final String OTHER_PAYER = "otherPayer";
     private static final String SIMPLE_UPDATE = "SimpleUpdate";
-    private static final String SCHEDULING_WHITELIST = "scheduling.whitelist";
-    private static final String STAKING_FEES_NODE_REWARD_PERCENTAGE = "staking.fees.nodeRewardPercentage";
-    private static final String STAKING_FEES_STAKING_REWARD_PERCENTAGE = "staking.fees.stakingRewardPercentage";
     private static final String TRIGGER = "trigger";
     private static final String INSOLVENT_PAYER = "insolventPayer";
     private static final String SCHEDULE = "schedule";
@@ -91,14 +92,14 @@ public static void main(String... args) {
     @Override
     public List<HapiSpec> getSpecsInSuite() {
         return withAndWithoutLongTermEnabled(() -> List.of(
-                executionTimeIsAvailable(),
-                deletionTimeIsAvailable(),
                 allRecordsAreQueryable(),
-                schedulingTxnIdFieldsNotAllowed(),
                 canonicalScheduleOpsHaveExpectedUsdFees(),
                 canScheduleChunkedMessages(),
+                deletionTimeIsAvailable(),
+                executionTimeIsAvailable(),
                 noFeesChargedIfTriggeredPayerIsInsolvent(),
-                noFeesChargedIfTriggeredPayerIsUnwilling()));
+                noFeesChargedIfTriggeredPayerIsUnwilling(),
+                schedulingTxnIdFieldsNotAllowed()));
     }
 
     HapiSpec canonicalScheduleOpsHaveExpectedUsdFees() {
@@ -201,6 +202,7 @@ public HapiSpec canScheduleChunkedMessages() {
                         overridingAllOf(Map.of(
                                 STAKING_FEES_NODE_REWARD_PERCENTAGE, "10",
                                 STAKING_FEES_STAKING_REWARD_PERCENTAGE, "10")),
+                        overriding(SCHEDULING_WHITELIST, WHITELIST_MINIMUM),
                         cryptoCreate(PAYING_SENDER).balance(ONE_HUNDRED_HBARS),
                         createTopic(ofGeneralInterest))
                 .when(
@@ -313,6 +315,7 @@ public HapiSpec deletionTimeIsAvailable() {
                 .then(getScheduleInfo("ntb").wasDeletedAtConsensusTimeOf("deletion"));
     }
 
+    @HapiTest
     public HapiSpec allRecordsAreQueryable() {
         return defaultHapiSpec("AllRecordsAreQueryable")
                 .given(

diff --git a/...ode/test-clients/src/main/java/com/hedera/services/bdd/suites/schedule/ScheduleUtils.java b/...ode/test-clients/src/main/java/com/hedera/services/bdd/suites/schedule/ScheduleUtils.java
@@ -18,24 +18,56 @@
 
 import com.hedera.hapi.node.base.HederaFunctionality;
 import com.hederahashgraph.api.proto.java.SchedulableTransactionBody;
+import com.hederahashgraph.api.proto.java.SchedulableTransactionBody.Builder;
 import com.hederahashgraph.api.proto.java.TransactionBody;
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 
-public class ScheduleUtils {
-    public static final String SENDER_TXN = "senderTxn";
-    public static final String STAKING_FEES_NODE_REWARD_PERCENTAGE = "staking.fees.nodeRewardPercentage";
-    public static final String STAKING_FEES_STAKING_REWARD_PERCENTAGE = "staking.fees.stakingRewardPercentage";
-    public static final String SCHEDULING_MAX_TXN_PER_SECOND = "scheduling.maxTxnPerSecond";
-    public static final String SCHEDULING_LONG_TERM_ENABLED = "scheduling.longTermEnabled";
-    public static final String LEDGER_SCHEDULE_TX_EXPIRY_TIME_SECS = "ledger.schedule.txExpiryTimeSecs";
-    public static final String SCHEDULING_WHITELIST = "scheduling.whitelist";
+public final class ScheduleUtils {
+    static final String SENDER_TXN = "senderTxn";
+    static final String STAKING_FEES_NODE_REWARD_PERCENTAGE = "staking.fees.nodeRewardPercentage";
+    static final String STAKING_FEES_STAKING_REWARD_PERCENTAGE = "staking.fees.stakingRewardPercentage";
+    static final String SCHEDULING_MAX_TXN_PER_SECOND = "scheduling.maxTxnPerSecond";
+    static final String SCHEDULING_LONG_TERM_ENABLED = "scheduling.longTermEnabled";
+    static final String LEDGER_SCHEDULE_TX_EXPIRY_TIME_SECS = "ledger.schedule.txExpiryTimeSecs";
+    static final String SCHEDULING_WHITELIST = "scheduling.whitelist";
+    static final String PAYING_ACCOUNT = "payingAccount";
+    static final String RECEIVER = "receiver";
+    static final String SENDER = "sender";
+    static final String BASIC_XFER = "basicXfer";
+    static final String CREATE_TX = "createTx";
+    static final String SIGN_TX = "signTx";
+    static final String TRIGGERING_TXN = "triggeringTxn";
+    static final String PAYING_ACCOUNT_2 = "payingAccount2";
+    static final String FALSE = "false";
+    static final String VALID_SCHEDULE = "validSchedule";
+    static final String SUCCESS_TXN = "successTxn";
+    static final String PAYER_TXN = "payerTxn";
+    static final String WRONG_RECORD_ACCOUNT_ID = "Wrong record account ID!";
+    static final String TRANSACTION_NOT_SCHEDULED = "Transaction not scheduled!";
+    static final String WRONG_SCHEDULE_ID = "Wrong schedule ID!";
+    static final String WRONG_TRANSACTION_VALID_START = "Wrong transaction valid start!";
+    static final String WRONG_CONSENSUS_TIMESTAMP = "Wrong consensus timestamp!";
+    static final String WRONG_TRANSFER_LIST = "Wrong transfer list!";
+    static final String SIMPLE_UPDATE = "SimpleUpdate";
+    static final String PAYING_ACCOUNT_TXN = "payingAccountTxn";
+    static final String LUCKY_RECEIVER = "luckyReceiver";
+    static final String SCHEDULE_CREATE_FEE = "scheduleCreateFee";
+    static final String FAILED_XFER = "failedXfer";
+    static final String WEIRDLY_POPULAR_KEY = "weirdlyPopularKey";
+    static final String SENDER_1 = "sender1";
+    static final String SENDER_2 = "sender2";
+    static final String SENDER_3 = "sender3";
+    static final String WEIRDLY_POPULAR_KEY_TXN = "weirdlyPopularKeyTxn";
+    static final String THREE_SIG_XFER = "threeSigXfer";
+    static final String PAYER = "payer";
+
     /**
      * Whitelist containing all of the non-query type transactions so we don't hit whitelist failures
      * everywhere.  Recommended for most specs that override whitelist.
      */
-    public static final String FULL_WHITELIST =
+    static final String FULL_WHITELIST =
             """
             ConsensusCreateTopic,ConsensusDeleteTopic,ConsensusSubmitMessage,ConsensusUpdateTopic,\
             ContractAutoRenew,ContractCall,ContractCallLocal,ContractCreate,ContractDelete,\
@@ -50,17 +82,18 @@ public class ScheduleUtils {
      * A very small whitelist containing just the transactions needed for SecheduleExecutionSpecs because
      * that suite has to override the whitelist on every single spec due to some sort of ordering issue.
      */
-    public static final String WHITELIST_MINIMUM =
+    static final String WHITELIST_MINIMUM =
             "ConsensusSubmitMessage,ContractCall,CryptoCreate,CryptoTransfer,FileUpdate,SystemDelete,TokenBurn,TokenMint,Freeze";
     /**
      * A whitelist guaranteed to contain every transaction type possible.  Useful for specs that need to test scheduling
      * a transaction that shouldn't work (e.g. a query).
      */
-    public static final String WHITELIST_ALL = getWhitelistAll();
+    static final String WHITELIST_ALL = getWhitelistAll();
 
-    public static SchedulableTransactionBody fromOrdinary(TransactionBody txn) {
-        var scheduleBuilder = SchedulableTransactionBody.newBuilder();
+    private ScheduleUtils() {}
 
+    public static SchedulableTransactionBody fromOrdinary(TransactionBody txn) {
+        Builder scheduleBuilder = SchedulableTransactionBody.newBuilder();
         scheduleBuilder.setTransactionFee(txn.getTransactionFee());
         scheduleBuilder.setMemo(txn.getMemo());
 
@@ -131,7 +164,6 @@ public static SchedulableTransactionBody fromOrdinary(TransactionBody txn) {
         } else if (txn.hasCryptoApproveAllowance()) {
             scheduleBuilder.setCryptoApproveAllowance(txn.getCryptoApproveAllowance());
         }
-
         return scheduleBuilder.build();
     }