Add ability to trigger an internal error through a test case

sisuresh · Jul 16, 2024 · 1d199ff · 1d199ff
1 parent 18c96a5
commit 1d199ff
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 19 deletions.
diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp
@@ -1644,7 +1644,6 @@ LedgerManagerImpl::applyThread(ThreadEntryMap& entryMap, Thread const& thread,
         }
         txBundle.mDelta = res.mDelta;
     }
-
     for (auto const& ttlExtension : ttlExtensions)
     {
         auto it = entryMap.find(ttlExtension.first);
@@ -1760,7 +1759,6 @@ LedgerManagerImpl::applySorobanStage(Application& app, AbstractLedgerTxn& ltx,
     }
 
     LedgerTxn ltxInner(ltx);
-
     for (auto const& thread : stage)
     {
         for (auto const& txBundle : thread)
@@ -1805,7 +1803,6 @@ LedgerManagerImpl::applySorobanStage(Application& app, AbstractLedgerTxn& ltx,
             }
         }
     }
-
     // TODO: Look into adding invariants checking for conflicting writes between
     // clusters
     for (auto const& threadEntryMap : entryMapsByThread)

diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp
@@ -1510,6 +1510,20 @@ TransactionFrame::apply(Application& app, AbstractLedgerTxn& ltx,
     return apply(app, ltx, tm, txResult, sorobanBasePrngSeed);
 }
 
+#ifdef BUILD_TESTS
+void
+maybeTriggerTestInternalError(TransactionEnvelope const& env)
+{
+    auto memo =
+        env.type() == ENVELOPE_TYPE_TX_V0 ? env.v0().tx.memo : env.v1().tx.memo;
+    if (memo.type() == MEMO_TEXT && memo.text() == "txINTERNAL_ERROR")
+    {
+        throw std::runtime_error(
+            "Intentionally triggered INTERNAL_ERROR in test");
+    }
+}
+#endif
+
 ParallelOpReturnVal
 TransactionFrame::parallelApply(
     ThreadEntryMap const& entryMap, // Must not be shared between threads!,
@@ -1562,6 +1576,10 @@ TransactionFrame::parallelApply(
                                     ledgerVersion);
         }
 
+#ifdef BUILD_TESTS
+        maybeTriggerTestInternalError(mEnvelope);
+#endif
+
         if (!fastFail && res.mSuccess)
         {
             res.mDelta = std::make_shared<LedgerTxnDelta>();
@@ -1748,6 +1766,9 @@ TransactionFrame::applyOperations(SignatureChecker& signatureChecker,
 
             bool txRes = op->apply(app, signatureChecker, ltxOp, subSeed,
                                    opResult, txResult.getSorobanData());
+#ifdef BUILD_TESTS
+            maybeTriggerTestInternalError(mEnvelope);
+#endif
 
             if (!txRes)
             {

diff --git a/src/transactions/test/InvokeHostFunctionTests.cpp b/src/transactions/test/InvokeHostFunctionTests.cpp
@@ -4296,6 +4296,7 @@ TEST_CASE("Vm instantiation tightening", "[tx][soroban]")
     }
 }
 
+// TODO: Move these tests to a new file
 TEST_CASE("parallel ttl", "[tx][soroban][parallelapply]")
 {
     auto modifyCfg = [](SorobanNetworkConfig& cfg) {
@@ -4347,11 +4348,13 @@ TEST_CASE("parallel ttl", "[tx][soroban][parallelapply]")
             client.readKeySpec("key1", ContractDataDurability::PERSISTENT));
         auto tx1 = i1.withExactNonRefundableResourceFee().createTx(&a1);
 
+        // tx2 will internal error
         auto i2 = client.getContract().prepareInvocation(
             "extend_persistent",
             {makeSymbolSCVal("key1"), makeU32SCVal(2000), makeU32SCVal(2000)},
             client.readKeySpec("key1", ContractDataDurability::PERSISTENT));
-        auto tx2 = i2.withExactNonRefundableResourceFee().createTx(&a2);
+        auto tx2 = i2.withExactNonRefundableResourceFee().createTx(
+            &a2, "txINTERNAL_ERROR");
 
         SorobanResources extendResources;
         extendResources.footprint.readOnly = {client.getContract().getDataKey(
@@ -4392,7 +4395,7 @@ TEST_CASE("parallel ttl", "[tx][soroban][parallelapply]")
         }
 
         REQUIRE(tx1->getResultCode() == txSUCCESS);
-        REQUIRE(tx2->getResultCode() == txSUCCESS);
+        REQUIRE(tx2->getResultCode() == txINTERNAL_ERROR);
         REQUIRE(tx3->getResultCode() == txSUCCESS);
         REQUIRE(tx4->getResultCode() == txSUCCESS);
 
@@ -4408,24 +4411,21 @@ TEST_CASE("parallel ttl", "[tx][soroban][parallelapply]")
 
         auto const& extensionMetaChangesTx1 =
             thread1[0].meta.getXDR().v3().operations.front().changes;
-        auto const& extensionMetaChangesTx2 =
-            thread2[0].meta.getXDR().v3().operations.front().changes;
         auto const& extensionMetaChangesTx3 =
             thread2[1].meta.getXDR().v3().operations.front().changes;
         auto const& extensionMetaChangesTx4 =
             thread1[1].meta.getXDR().v3().operations.front().changes;
 
+        // This tx hit an internal error
+        REQUIRE(thread2[0].meta.getXDR().v3().operations.empty());
+
         // Note that even though both transactions are in the same thread, they
         // did not observe the other transactions bump, and instead bumped from
         // the initial ttl.
         REQUIRE(extensionMetaChangesTx1.at(0)
                     .state()
                     .data.ttl()
                     .liveUntilLedgerSeq == expectedPersistentLiveUntilLedger);
-        REQUIRE(extensionMetaChangesTx2.at(0)
-                    .state()
-                    .data.ttl()
-                    .liveUntilLedgerSeq == expectedPersistentLiveUntilLedger);
         REQUIRE(extensionMetaChangesTx3.at(0)
                     .state()
                     .data.ttl()
@@ -4439,10 +4439,6 @@ TEST_CASE("parallel ttl", "[tx][soroban][parallelapply]")
                     .updated()
                     .data.ttl()
                     .liveUntilLedgerSeq == test.getLedgerSeq() + tx1ExtendTo);
-        REQUIRE(extensionMetaChangesTx2.at(1)
-                    .updated()
-                    .data.ttl()
-                    .liveUntilLedgerSeq == test.getLedgerSeq() + 2000);
         REQUIRE(extensionMetaChangesTx3.at(1)
                     .updated()
                     .data.ttl()

diff --git a/src/transactions/test/SorobanTxTestUtils.cpp b/src/transactions/test/SorobanTxTestUtils.cpp
@@ -320,7 +320,7 @@ sorobanTransactionFrameFromOps(Hash const& networkID, TestAccount& source,
 {
     return sorobanTransactionFrameFromOps(
         networkID, source, ops, opKeys, spec.getResources(),
-        spec.getInclusionFee(), spec.getResourceFee());
+        spec.getInclusionFee(), spec.getResourceFee(), memo);
 }
 
 SorobanInvocationSpec::SorobanInvocationSpec(SorobanResources const& resources,
@@ -600,7 +600,8 @@ TestContract::Invocation::getSpec()
 }
 
 TransactionTestFramePtr
-TestContract::Invocation::createTx(TestAccount* source)
+TestContract::Invocation::createTx(TestAccount* source,
+                                   std::optional<std::string> memo)
 {
     if (mDeduplicateFootprint)
     {
@@ -609,7 +610,7 @@ TestContract::Invocation::createTx(TestAccount* source)
     auto& acc = source ? *source : mTest.getRoot();
 
     return sorobanTransactionFrameFromOps(mTest.getApp().getNetworkID(), acc,
-                                          {mOp}, {}, mSpec);
+                                          {mOp}, {}, mSpec, memo);
 }
 
 TestContract::Invocation&

diff --git a/src/transactions/test/SorobanTxTestUtils.h b/src/transactions/test/SorobanTxTestUtils.h
@@ -186,7 +186,9 @@ class TestContract
 
         SorobanInvocationSpec getSpec();
 
-        TransactionTestFramePtr createTx(TestAccount* source = nullptr);
+        TransactionTestFramePtr
+        createTx(TestAccount* source = nullptr,
+                 std::optional<std::string> memo = std::nullopt);
         bool invoke(TestAccount* source = nullptr);
 
         SCVal getReturnValue() const;