Cleaned up checkValid interface and renames

stellar · Aug 1, 2024 · eed6cad · eed6cad
1 parent 81d1d53
commit eed6cad
Show file tree

Hide file tree

Showing 70 changed files with 539 additions and 511 deletions.
diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp
@@ -53,24 +53,24 @@ std::array<const char*,
                                   "TRY_AGAIN_LATER", "FILTERED"};
 
 TransactionQueue::AddResult::AddResult(AddResultCode addCode)
-    : code(addCode), resultPayload()
+    : code(addCode), txResult()
 {
 }
 
 TransactionQueue::AddResult::AddResult(AddResultCode addCode,
-                                       TransactionResultPayloadPtr payload)
-    : code(addCode), resultPayload(payload)
+                                       MutableTxResultPtr payload)
+    : code(addCode), txResult(payload)
 {
-    releaseAssert(resultPayload);
+    releaseAssert(txResult);
 }
 
 TransactionQueue::AddResult::AddResult(AddResultCode addCode,
                                        TransactionFrameBasePtr tx,
                                        TransactionResultCode txErrorCode)
-    : code(addCode), resultPayload(tx->createResultPayload())
+    : code(addCode), txResult(tx->createSuccessResult())
 {
     releaseAssert(txErrorCode != txSUCCESS);
-    resultPayload.value()->setResultCode(txErrorCode);
+    txResult.value()->setResultCode(txErrorCode);
 }
 
 TransactionQueue::TransactionQueue(Application& app, uint32 pendingDepth,
@@ -324,7 +324,7 @@ TransactionQueue::canAdd(
             // appropriate error message
             if (tx->isSoroban())
             {
-                auto txResult = tx->createResultPayload();
+                auto txResult = tx->createSuccessResult();
                 if (!tx->checkSorobanResourceAndSetError(
                         mApp,
                         mApp.getLedgerManager()
@@ -358,8 +358,7 @@ TransactionQueue::canAdd(
                     AddResult result(
                         TransactionQueue::AddResultCode::ADD_STATUS_ERROR, tx,
                         txINSUFFICIENT_FEE);
-                    result.resultPayload.value()->getResult().feeCharged =
-                        minFee;
+                    result.txResult.value()->getResult().feeCharged = minFee;
                     return result;
                 }
 
@@ -387,12 +386,12 @@ TransactionQueue::canAdd(
         {
             AddResult result(TransactionQueue::AddResultCode::ADD_STATUS_ERROR,
                              tx, txINSUFFICIENT_FEE);
-            result.resultPayload.value()->getResult().feeCharged =
-                canAddRes.second;
+            result.txResult.value()->getResult().feeCharged = canAddRes.second;
             return result;
         }
-        return {TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER,
-                nullptr};
+        return AddResult(
+            TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER,
+            nullptr);
     }
 
     auto closeTime = mApp.getLedgerManager()
@@ -406,9 +405,9 @@ TransactionQueue::canAdd(
             mApp.getLedgerManager().getLastClosedLedgerNum() + 1;
     }
 
-    auto [isValid, txResult] = tx->checkValid(
+    auto txResult = tx->checkValid(
         mApp, ltx, 0, 0, getUpperBoundCloseTimeOffset(mApp, closeTime));
-    if (!isValid)
+    if (!txResult->isSuccess())
     {
         return AddResult(TransactionQueue::AddResultCode::ADD_STATUS_ERROR,
                          txResult);
@@ -584,8 +583,8 @@ TransactionQueue::tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf)
     // fast fail when Soroban tx is malformed
     if ((tx->isSoroban() != (c1 || c2)) || !tx->XDRProvidesValidFee())
     {
-        return {TransactionQueue::AddResultCode::ADD_STATUS_ERROR, tx,
-                txMALFORMED};
+        return AddResult(TransactionQueue::AddResultCode::ADD_STATUS_ERROR, tx,
+                         txMALFORMED);
     }
 
     AccountStates::iterator stateIter;

diff --git a/src/herder/TransactionQueue.h b/src/herder/TransactionQueue.h
@@ -75,19 +75,19 @@ class TransactionQueue
     struct AddResult
     {
         TransactionQueue::AddResultCode code;
-        std::optional<TransactionResultPayloadPtr> resultPayload;
+        std::optional<MutableTxResultPtr> txResult;
 
-        // AddResult with no resultPayload
-        AddResult(TransactionQueue::AddResultCode addCode);
+        // AddResult with no txResult
+        explicit AddResult(TransactionQueue::AddResultCode addCode);
 
         // AddResult from existing transaction result
-        AddResult(TransactionQueue::AddResultCode addCode,
-                  TransactionResultPayloadPtr payload);
+        explicit AddResult(TransactionQueue::AddResultCode addCode,
+                           MutableTxResultPtr payload);
 
-        // AddResult with error resultPayload with the specified txErrorCode
-        AddResult(TransactionQueue::AddResultCode addCode,
-                  TransactionFrameBasePtr tx,
-                  TransactionResultCode txErrorCode);
+        // AddResult with error txResult with the specified txErrorCode
+        explicit AddResult(TransactionQueue::AddResultCode addCode,
+                           TransactionFrameBasePtr tx,
+                           TransactionResultCode txErrorCode);
     };
 
     /**

diff --git a/src/herder/TxSetFrame.cpp b/src/herder/TxSetFrame.cpp
@@ -17,6 +17,7 @@
 #include "main/Application.h"
 #include "main/Config.h"
 #include "overlay/Peer.h"
+#include "transactions/MutableTransactionResult.h"
 #include "transactions/TransactionUtils.h"
 #include "util/GlobalChecks.h"
 #include "util/Logging.h"
@@ -241,14 +242,15 @@ phaseTxsAreValid(TxSetTransactions const& phase, Application& app,
         app.getLedgerManager().getLastClosedLedgerNum() + 1;
     for (auto const& tx : phase)
     {
-        if (!tx->checkValid(app, ltx, 0, lowerBoundCloseTimeOffset,
-                            upperBoundCloseTimeOffset))
+        auto txResult = tx->checkValid(app, ltx, 0, lowerBoundCloseTimeOffset,
+                                       upperBoundCloseTimeOffset);
+        if (!txResult->isSuccess())
         {
 
             CLOG_DEBUG(
                 Herder, "Got bad txSet: tx invalid tx: {} result: {}",
                 xdrToCerealString(tx->getEnvelope(), "TransactionEnvelope"),
-                tx->getResultCode());
+                txResult->getResultCode());
             return false;
         }
     }

diff --git a/src/herder/TxSetUtils.cpp b/src/herder/TxSetUtils.cpp
@@ -162,8 +162,9 @@ TxSetUtils::getInvalidTxList(TxSetTransactions const& txs, Application& app,
 
     for (auto const& tx : txs)
     {
-        if (!tx->checkValid(app, ltx, 0, lowerBoundCloseTimeOffset,
-                            upperBoundCloseTimeOffset))
+        auto txResult = tx->checkValid(app, ltx, 0, lowerBoundCloseTimeOffset,
+                                       upperBoundCloseTimeOffset);
+        if (!txResult->isSuccess())
         {
             invalidTxs.emplace_back(tx);
         }

diff --git a/src/herder/test/HerderTests.cpp b/src/herder/test/HerderTests.cpp
@@ -34,6 +34,7 @@
 #include "transactions/TransactionBridge.h"
 #include "transactions/TransactionFrame.h"
 #include "transactions/TransactionUtils.h"
+#include "transactions/test/TransactionTestFrame.h"
 #include "util/Math.h"
 #include "util/ProtocolVersion.h"
 
@@ -45,6 +46,7 @@
 #include "xdrpp/marshal.h"
 #include <algorithm>
 #include <fmt/format.h>
+#include <memory>
 #include <numeric>
 #include <optional>
 
@@ -356,7 +358,8 @@ testTxSet(uint32 protocolVersion)
         }
         SECTION("sequence gap")
         {
-            setSeqNum(std::static_pointer_cast<TransactionFrame>(txs[0]),
+            auto txPtr = std::const_pointer_cast<TransactionFrameBase>(txs[0]);
+            setSeqNum(std::static_pointer_cast<TransactionTestFrame>(txPtr),
                       txs[0]->getSeqNum() + 5);
 
             TxSetTransactions removed;