only access results() of tx results with code txFAILED, move skip mod…

…e to ifdef BUILDTEST
stellar · Nov 22, 2024 · c451463 · c451463
1 parent 3c2e52b
commit c451463
Show file tree

Hide file tree

Showing 13 changed files with 49 additions and 4 deletions.
diff --git a/src/catchup/ApplyCheckpointWork.cpp b/src/catchup/ApplyCheckpointWork.cpp
@@ -96,6 +96,7 @@ ApplyCheckpointWork::openInputFiles()
     mTxIn.open(ti.localPath_nogz());
     mTxHistoryEntry = TransactionHistoryEntry();
     mHeaderHistoryEntry = LedgerHeaderHistoryEntry();
+#ifdef BUILD_TESTS
     if (mApp.getConfig().CATCHUP_SKIP_KNOWN_RESULTS_FOR_TESTING)
     {
         mTxResultIn = std::make_optional<XDRInputFileStream>();
@@ -107,6 +108,7 @@ ApplyCheckpointWork::openInputFiles()
         mTxHistoryResultEntry =
             std::make_optional<TransactionHistoryResultEntry>();
     }
+#endif
     mFilesOpen = true;
 }
 
@@ -152,6 +154,7 @@ ApplyCheckpointWork::getCurrentTxSet()
     return TxSetXDRFrame::makeEmpty(lm.getLastClosedLedgerHeader());
 }
 
+#ifdef BUILD_TESTS
 std::optional<TransactionResultSet>
 ApplyCheckpointWork::getCurrentTxResultSet()
 {
@@ -187,6 +190,7 @@ ApplyCheckpointWork::getCurrentTxResultSet()
     CLOG_DEBUG(History, "No txresultset for ledger {}", seq);
     return std::nullopt;
 }
+#endif // BUILD_TESTS
 
 std::shared_ptr<LedgerCloseData>
 ApplyCheckpointWork::getNextLedgerCloseData()
@@ -267,10 +271,12 @@ ApplyCheckpointWork::getNextLedgerCloseData()
                txset->sizeTxTotal());
 
     std::optional<TransactionResultSet> txres = std::nullopt;
+#ifdef BUILD_TESTS
     if (mApp.getConfig().CATCHUP_SKIP_KNOWN_RESULTS_FOR_TESTING)
     {
         txres = getCurrentTxResultSet();
     }
+#endif
 
     // We've verified the ledgerHeader (in the "trusted part of history"
     // sense) in CATCHUP_VERIFY phase; we now need to check that the

diff --git a/src/catchup/ApplyCheckpointWork.h b/src/catchup/ApplyCheckpointWork.h
@@ -52,9 +52,11 @@ class ApplyCheckpointWork : public BasicWork
 
     XDRInputFileStream mHdrIn;
     XDRInputFileStream mTxIn;
-    std::optional<XDRInputFileStream> mTxResultIn;
     TransactionHistoryEntry mTxHistoryEntry;
+#ifdef BUILD_TESTS
+    std::optional<XDRInputFileStream> mTxResultIn;
     std::optional<TransactionHistoryResultEntry> mTxHistoryResultEntry;
+#endif // BUILD_TESTS
     LedgerHeaderHistoryEntry mHeaderHistoryEntry;
     OnFailureCallback mOnFailure;
 
@@ -63,7 +65,9 @@ class ApplyCheckpointWork : public BasicWork
     std::shared_ptr<ConditionalWork> mConditionalWork;
 
     TxSetXDRFrameConstPtr getCurrentTxSet();
+#ifdef BUILD_TESTS
     std::optional<TransactionResultSet> getCurrentTxResultSet();
+#endif // BUILD_TESTS
     void openInputFiles();
 
     std::shared_ptr<LedgerCloseData> getNextLedgerCloseData();

diff --git a/src/catchup/DownloadApplyTxsWork.cpp b/src/catchup/DownloadApplyTxsWork.cpp
@@ -81,7 +81,7 @@ DownloadApplyTxsWork::yieldMoreWork()
     std::vector<std::shared_ptr<BasicWork>> seq{getAndUnzip};
     std::vector<FileTransferInfo> filesToTransfer{ft};
     std::vector<std::shared_ptr<BasicWork>> optionalDownloads;
-
+#ifdef BUILD_TESTS
     if (mApp.getConfig().CATCHUP_SKIP_KNOWN_RESULTS_FOR_TESTING)
     {
         CLOG_INFO(History,
@@ -128,6 +128,7 @@ DownloadApplyTxsWork::yieldMoreWork()
 
         filesToTransfer.push_back(resultsFile);
     }
+#endif // BUILD_TESTS
 
     auto maybeWaitForMerges = [](Application& app) {
         if (app.getConfig().CATCHUP_WAIT_MERGES_TX_APPLY_FOR_TESTING)

diff --git a/src/herder/LedgerCloseData.h b/src/herder/LedgerCloseData.h
@@ -50,11 +50,13 @@ class LedgerCloseData
     {
         return mExpectedLedgerHash;
     }
+#ifdef BUILD_TESTS
     std::optional<TransactionResultSet> const&
     getExpectedResults() const
     {
         return mExpectedResults;
     }
+#endif // BUILD_TESTS
 
     StoredDebugTransactionSet
     toXDR() const

diff --git a/src/historywork/GetAndUnzipRemoteFileWork.cpp b/src/historywork/GetAndUnzipRemoteFileWork.cpp
@@ -8,6 +8,7 @@
 #include "historywork/GetRemoteFileWork.h"
 #include "historywork/GunzipFileWork.h"
 #include "util/GlobalChecks.h"
+#include "util/Logging.h"
 #include <Tracy.hpp>
 #include <fmt/format.h>
 

diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp
@@ -1363,6 +1363,7 @@ LedgerManagerImpl::processFeesSeqNums(
         auto header = ltx.loadHeader().current();
         std::map<AccountID, SequenceNumber> accToMaxSeq;
 
+#ifdef BUILD_TESTS
         // If we have expected results, we assign them to the mutable tx results
         // here.
         std::optional<std::vector<TransactionResultPair>::const_iterator>
@@ -1373,6 +1374,7 @@ LedgerManagerImpl::processFeesSeqNums(
             expectedResultsIter =
                 std::make_optional(expectedResults->results.begin());
         }
+#endif
 
         bool mergeSeen = false;
         for (auto tx : txs)
@@ -1381,6 +1383,7 @@ LedgerManagerImpl::processFeesSeqNums(
 
             txResults.push_back(
                 tx->processFeeSeqNum(ltxTx, txSet.getTxBaseFee(tx, header)));
+#ifdef BUILD_TESTS
             if (expectedResultsIter)
             {
                 releaseAssert(*expectedResultsIter !=
@@ -1391,6 +1394,7 @@ LedgerManagerImpl::processFeesSeqNums(
                     (*expectedResultsIter)->result);
                 ++(*expectedResultsIter);
             }
+#endif // BUILD_TESTS
 
             if (protocolVersionStartsFrom(
                     ltxTx.loadHeader().current().ledgerVersion,

diff --git a/src/main/CommandLine.cpp b/src/main/CommandLine.cpp
@@ -878,9 +878,10 @@ runCatchup(CommandLineArgs const& args)
             config.RUN_STANDALONE = true;
             config.MANUAL_CLOSE = true;
             config.DISABLE_BUCKET_GC = disableBucketGC;
-
+#ifdef BUILD_TESTS
             // SET TO TRUE FOR TESTING PURPOSES, will be removed pre-merge.
             config.CATCHUP_SKIP_KNOWN_RESULTS_FOR_TESTING = true;
+#endif // BUILD_TESTS
 
             if (config.AUTOMATIC_MAINTENANCE_PERIOD.count() > 0 &&
                 config.AUTOMATIC_MAINTENANCE_COUNT > 0)

diff --git a/src/main/Config.cpp b/src/main/Config.cpp
@@ -1150,10 +1150,12 @@ Config::processConfig(std::shared_ptr<cpptoml::table> t)
                      CATCHUP_RECENT =
                          readInt<uint32_t>(item, 0, UINT32_MAX - 1);
                  }},
+#ifdef BUILD_TESTS
                 {"CATCHUP_SKIP_KNOWN_RESULTS_FOR_TESTING",
                  [&]() {
                      CATCHUP_SKIP_KNOWN_RESULTS_FOR_TESTING = readBool(item);
                  }},
+#endif // BUILD_TESTS
                 {"ARTIFICIALLY_GENERATE_LOAD_FOR_TESTING",
                  [&]() {
                      ARTIFICIALLY_GENERATE_LOAD_FOR_TESTING = readBool(item);

diff --git a/src/main/Config.h b/src/main/Config.h
@@ -203,10 +203,12 @@ class Config : public std::enable_shared_from_this<Config>
     // If you want, say, a week of history, set this to 120000.
     uint32_t CATCHUP_RECENT;
 
+#ifdef BUILD_TESTS
     // Mode for "accelerated" catchup. If set to true, the node will skip
     // application of failed transactions and will not verify signatures of
     // successful transactions.
     bool CATCHUP_SKIP_KNOWN_RESULTS_FOR_TESTING;
+#endif // BUILD_TESTS
 
     // Interval between automatic maintenance executions
     std::chrono::seconds AUTOMATIC_MAINTENANCE_PERIOD;

diff --git a/src/transactions/MutableTransactionResult.cpp b/src/transactions/MutableTransactionResult.cpp
@@ -314,6 +314,7 @@ MutableTransactionResult::isSuccess() const
     return getResult().result.code() == txSUCCESS;
 }
 
+#ifdef BUILD_TESTS
 void
 MutableTransactionResult::setReplayTransactionResult(
     TransactionResult const& replayResult)
@@ -326,6 +327,7 @@ MutableTransactionResult::getReplayTransactionResult() const
 {
     return mReplayTransactionResult;
 }
+#endif // BUILD_TESTS
 
 FeeBumpMutableTransactionResult::FeeBumpMutableTransactionResult(
     MutableTxResultPtr innerTxResult)
@@ -461,6 +463,7 @@ FeeBumpMutableTransactionResult::isSuccess() const
     return mTxResult->result.code() == txFEE_BUMP_INNER_SUCCESS;
 }
 
+#ifdef BUILD_TESTS
 void
 FeeBumpMutableTransactionResult::setReplayTransactionResult(
     TransactionResult const& replayResult)
@@ -473,4 +476,5 @@ FeeBumpMutableTransactionResult::getReplayTransactionResult() const
 {
     return mReplayTransactionResult;
 }
+#endif // BUILD_TESTS
 }
diff --git a/src/transactions/MutableTransactionResult.h b/src/transactions/MutableTransactionResult.h
@@ -99,10 +99,12 @@ class MutableTransactionResultBase : public NonMovableOrCopyable
     virtual void refundSorobanFee(int64_t feeRefund,
                                   uint32_t ledgerVersion) = 0;
     virtual bool isSuccess() const = 0;
+#ifdef BUILD_TESTS
     virtual std::optional<TransactionResult> const&
     getReplayTransactionResult() const = 0;
     virtual void
     setReplayTransactionResult(TransactionResult const& replayResult) = 0;
+#endif
 };
 
 class MutableTransactionResult : public MutableTransactionResultBase
@@ -128,10 +130,12 @@ class MutableTransactionResult : public MutableTransactionResultBase
     TransactionResult const& getResult() const override;
     TransactionResultCode getResultCode() const override;
     void setResultCode(TransactionResultCode code) override;
+#ifdef BUILD_TESTS
     void
     setReplayTransactionResult(TransactionResult const& replayResult) override;
     std::optional<TransactionResult> const&
     getReplayTransactionResult() const override;
+#endif // BUILD_TESTS
 
     OperationResult& getOpResultAt(size_t index) override;
     std::shared_ptr<SorobanTxData> getSorobanData() override;
@@ -186,9 +190,11 @@ class FeeBumpMutableTransactionResult : public MutableTransactionResultBase
     void refundSorobanFee(int64_t feeRefund, uint32_t ledgerVersion) override;
     bool isSuccess() const override;
 
+#ifdef BUILD_TESTS
     void
     setReplayTransactionResult(TransactionResult const& replayResult) override;
     std::optional<TransactionResult> const&
     getReplayTransactionResult() const override;
+#endif // BUILD_TESTS
 };
 }
diff --git a/src/transactions/SignatureChecker.h b/src/transactions/SignatureChecker.h
@@ -42,6 +42,7 @@ class SignatureChecker : public AbstractSignatureChecker
     std::vector<bool> mUsedSignatures;
 };
 
+#ifdef BUILD_TESTS
 class AlwaysValidSignatureChecker : public AbstractSignatureChecker
 {
   public:
@@ -62,4 +63,5 @@ class AlwaysValidSignatureChecker : public AbstractSignatureChecker
         return true;
     }
 };
+#endif // BUILD_TESTS
 }
diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp
@@ -1576,6 +1576,7 @@ TransactionFrame::applyOperations(AbstractSignatureChecker& signatureChecker,
                                   Hash const& sorobanBasePrngSeed) const
 {
     ZoneScoped;
+#ifdef BUILD_TESTS
     auto const& result = txResult.getReplayTransactionResult();
     if (result && result->result.code() != txSUCCESS)
     {
@@ -1584,9 +1585,14 @@ TransactionFrame::applyOperations(AbstractSignatureChecker& signatureChecker,
         CLOG_DEBUG(Tx, "Skipping replay of failed transaction: tx {}",
                    binToHex(getContentsHash()));
         txResult.setResultCode(result->result.code());
-        txResult.getResult().result.results() = result->result.results();
+        // results field is only active if code is txFAILED or txSUCCESS
+        if (result->result.code() == txFAILED)
+        {
+            txResult.getResult().result.results() = result->result.results();
+        }
         return false;
     }
+#endif
 
     auto& internalErrorCounter = app.getMetrics().NewCounter(
         {"ledger", "transaction", "internal-error"});
@@ -1800,6 +1806,7 @@ TransactionFrame::apply(AppConnector& app, AbstractLedgerTxn& ltx,
         mCachedAccountPreProtocol8.reset();
         uint32_t ledgerVersion = ltx.loadHeader().current().ledgerVersion;
         std::unique_ptr<AbstractSignatureChecker> signatureChecker;
+#ifdef BUILD_TESTS
         // If the txResult has a replay result (catchup in skip mode is
         // enabled),
         //  we do not perform signature verification.
@@ -1810,9 +1817,12 @@ TransactionFrame::apply(AppConnector& app, AbstractLedgerTxn& ltx,
         }
         else
         {
+#endif // BUILD_TESTS
             signatureChecker = std::make_unique<SignatureChecker>(
                 ledgerVersion, getContentsHash(), getSignatures(mEnvelope));
+#ifdef BUILD_TESTS
         }
+#endif // BUILD_TESTS
 
         //  when applying, a failure during tx validation means that
         //  we'll skip trying to apply operations but we'll still