Cleaned up checkValid interface

SirTyson · Jul 1, 2024 · 8c25df2 · 8c25df2
1 parent a9a97a8
commit 8c25df2
Show file tree

Hide file tree

Showing 11 changed files with 44 additions and 30 deletions.
diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp
@@ -405,9 +405,9 @@ TransactionQueue::canAdd(TransactionFrameBasePtr tx,
             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);

diff --git a/src/herder/TxSetUtils.cpp b/src/herder/TxSetUtils.cpp
@@ -192,17 +192,16 @@ TxSetUtils::getInvalidTxList(TxSetTransactions const& txs, Application& app,
                 (tx->getMinSeqAge() != 0 || tx->getMinSeqLedgerGap() != 0);
 
             // Only call checkValid if we passed the seqNum check
-            bool checkValid = false;
             MutableTxResultPtr txResult{};
             if (!minSeqCheckIsInvalid)
             {
-                std::tie(checkValid, txResult) =
+                txResult =
                     tx->checkValid(app, ltx, lastSeq, lowerBoundCloseTimeOffset,
                                    upperBoundCloseTimeOffset);
                 releaseAssertOrThrow(txResult);
             }
 
-            if (minSeqCheckIsInvalid || !checkValid)
+            if (minSeqCheckIsInvalid || !txResult->isSuccess())
             {
                 invalidTxs.emplace_back(tx);
                 iter = accountQueue->mTxs.erase(iter);

diff --git a/src/transactions/FeeBumpTransactionFrame.cpp b/src/transactions/FeeBumpTransactionFrame.cpp
@@ -153,7 +153,7 @@ FeeBumpTransactionFrame::checkSignature(SignatureChecker& signatureChecker,
     return signatureChecker.checkSignature(signers, neededWeight);
 }
 
-std::pair<bool, MutableTxResultPtr>
+MutableTxResultPtr
 FeeBumpTransactionFrame::checkValid(Application& app,
                                     AbstractLedgerTxn& ltxOuter,
                                     SequenceNumber current,
@@ -164,7 +164,7 @@ FeeBumpTransactionFrame::checkValid(Application& app,
     {
         auto txResult = createSuccessResult();
         txResult->setResultCode(txMALFORMED);
-        return {false, txResult};
+        return txResult;
     }
 
     LedgerTxn ltx(ltxOuter);
@@ -178,22 +178,22 @@ FeeBumpTransactionFrame::checkValid(Application& app,
     if (commonValid(signatureChecker, ltx, false, *txResult) !=
         ValidationType::kFullyValid)
     {
-        return {false, txResult};
+        return txResult;
     }
 
     if (!signatureChecker.checkAllSignaturesUsed())
     {
         txResult->setResultCode(txBAD_AUTH_EXTRA);
-        return {false, txResult};
+        return txResult;
     }
 
-    auto [res, innerTxResult] = mInnerTx->checkValidWithOptionallyChargedFee(
+    auto innerTxResult = mInnerTx->checkValidWithOptionallyChargedFee(
         app, ltx, current, false, lowerBoundCloseTimeOffset,
         upperBoundCloseTimeOffset);
     auto finalTxResult = createSuccessResultWithNewInnerTx(
         std::move(txResult), std::move(innerTxResult), mInnerTx);
 
-    return {res, finalTxResult};
+    return finalTxResult;
 }
 
 bool

diff --git a/src/transactions/FeeBumpTransactionFrame.h b/src/transactions/FeeBumpTransactionFrame.h
@@ -76,7 +76,7 @@ class FeeBumpTransactionFrame : public TransactionFrameBase
                           TransactionMetaFrame& meta,
                           MutableTxResultPtr txResult) const override;
 
-    std::pair<bool, MutableTxResultPtr>
+    MutableTxResultPtr
     checkValid(Application& app, AbstractLedgerTxn& ltxOuter,
                SequenceNumber current, uint64_t lowerBoundCloseTimeOffset,
                uint64_t upperBoundCloseTimeOffset) const override;

diff --git a/src/transactions/MutableTransactionResult.cpp b/src/transactions/MutableTransactionResult.cpp
@@ -305,6 +305,12 @@ MutableTransactionResult::refundSorobanFee(int64_t feeRefund,
     mTxResult->feeCharged -= feeRefund;
 }
 
+bool
+MutableTransactionResult::isSuccess() const
+{
+    return getResult().result.code() == txSUCCESS;
+}
+
 FeeBumpMutableTransactionResult::FeeBumpMutableTransactionResult(
     MutableTxResultPtr innerTxResult)
     : MutableTransactionResultBase(), mInnerTxResult(innerTxResult)
@@ -432,4 +438,10 @@ FeeBumpMutableTransactionResult::refundSorobanFee(int64_t feeRefund,
         mTxResult->feeCharged -= feeRefund;
     }
 }
+
+bool
+FeeBumpMutableTransactionResult::isSuccess() const
+{
+    return mTxResult->result.code() == txFEE_BUMP_INNER_SUCCESS;
+}
 }
diff --git a/src/transactions/MutableTransactionResult.h b/src/transactions/MutableTransactionResult.h
@@ -97,6 +97,8 @@ class MutableTransactionResultBase : public NonMovableOrCopyable
 
     virtual void refundSorobanFee(int64_t feeRefund,
                                   uint32_t ledgerVersion) = 0;
+
+    virtual bool isSuccess() const = 0;
 };
 
 class MutableTransactionResult : public MutableTransactionResultBase
@@ -128,6 +130,7 @@ class MutableTransactionResult : public MutableTransactionResultBase
     xdr::xvector<DiagnosticEvent> const& getDiagnosticEvents() const override;
 
     void refundSorobanFee(int64_t feeRefund, uint32_t ledgerVersion) override;
+    bool isSuccess() const override;
 };
 
 class FeeBumpMutableTransactionResult : public MutableTransactionResultBase
@@ -173,5 +176,6 @@ class FeeBumpMutableTransactionResult : public MutableTransactionResultBase
     xdr::xvector<DiagnosticEvent> const& getDiagnosticEvents() const override;
 
     void refundSorobanFee(int64_t feeRefund, uint32_t ledgerVersion) override;
+    bool isSuccess() const override;
 };
 }
diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp
@@ -1382,7 +1382,7 @@ TransactionFrame::removeAccountSigner(AbstractLedgerTxn& ltxOuter,
     }
 }
 
-std::pair<bool, MutableTxResultPtr>
+MutableTxResultPtr
 TransactionFrame::checkValidWithOptionallyChargedFee(
     Application& app, AbstractLedgerTxn& ltxOuter, SequenceNumber current,
     bool chargeFee, uint64_t lowerBoundCloseTimeOffset,
@@ -1395,7 +1395,7 @@ TransactionFrame::checkValidWithOptionallyChargedFee(
     {
         auto txResult = createSuccessResult();
         txResult->setInnermostResultCode(txMALFORMED);
-        return {false, txResult};
+        return txResult;
     }
 
     LedgerTxn ltx(ltxOuter);
@@ -1439,7 +1439,7 @@ TransactionFrame::checkValidWithOptionallyChargedFee(
                 // checkValid on all operations as the resulting object
                 // is only used by applications
                 txResult->setInnermostResultCode(txFAILED);
-                return {false, txResult};
+                return txResult;
             }
         }
 
@@ -1449,10 +1449,11 @@ TransactionFrame::checkValidWithOptionallyChargedFee(
             txResult->setInnermostResultCode(txBAD_AUTH_EXTRA);
         }
     }
-    return {res, txResult};
+
+    return txResult;
 }
 
-std::pair<bool, MutableTxResultPtr>
+MutableTxResultPtr
 TransactionFrame::checkValid(Application& app, AbstractLedgerTxn& ltxOuter,
                              SequenceNumber current,
                              uint64_t lowerBoundCloseTimeOffset,

diff --git a/src/transactions/TransactionFrame.h b/src/transactions/TransactionFrame.h
@@ -205,11 +205,11 @@ class TransactionFrame : public TransactionFrameBase
                                  AccountID const& accountID) const;
     bool checkExtraSigners(SignatureChecker& signatureChecker) const;
 
-    std::pair<bool, MutableTxResultPtr> checkValidWithOptionallyChargedFee(
+    MutableTxResultPtr checkValidWithOptionallyChargedFee(
         Application& app, AbstractLedgerTxn& ltxOuter, SequenceNumber current,
         bool chargeFee, uint64_t lowerBoundCloseTimeOffset,
         uint64_t upperBoundCloseTimeOffset) const;
-    std::pair<bool, MutableTxResultPtr>
+    MutableTxResultPtr
     checkValid(Application& app, AbstractLedgerTxn& ltxOuter,
                SequenceNumber current, uint64_t lowerBoundCloseTimeOffset,
                uint64_t upperBoundCloseTimeOffset) const override;

diff --git a/src/transactions/TransactionFrameBase.h b/src/transactions/TransactionFrameBase.h
@@ -44,7 +44,7 @@ class TransactionFrameBase
                        TransactionMetaFrame& meta, MutableTxResultPtr txResult,
                        Hash const& sorobanBasePrngSeed = Hash{}) const = 0;
 
-    virtual std::pair<bool, MutableTxResultPtr>
+    virtual MutableTxResultPtr
     checkValid(Application& app, AbstractLedgerTxn& ltxOuter,
                SequenceNumber current, uint64_t lowerBoundCloseTimeOffset,
                uint64_t upperBoundCloseTimeOffset) const = 0;

diff --git a/src/transactions/test/TransactionTestFrame.cpp b/src/transactions/test/TransactionTestFrame.cpp
@@ -88,17 +88,16 @@ TransactionTestFrame::apply(Application& app, AbstractLedgerTxn& ltx,
     return ret;
 }
 
-std::pair<bool, MutableTxResultPtr>
+MutableTxResultPtr
 TransactionTestFrame::checkValid(Application& app, AbstractLedgerTxn& ltxOuter,
                                  SequenceNumber current,
                                  uint64_t lowerBoundCloseTimeOffset,
                                  uint64_t upperBoundCloseTimeOffset) const
 {
-    auto result = mTransactionFrame->checkValid(app, ltxOuter, current,
-                                                lowerBoundCloseTimeOffset,
-                                                upperBoundCloseTimeOffset);
-    mTransactionTxResult = result.second;
-    return result;
+    mTransactionTxResult = mTransactionFrame->checkValid(
+        app, ltxOuter, current, lowerBoundCloseTimeOffset,
+        upperBoundCloseTimeOffset);
+    return mTransactionTxResult;
 }
 
 void
@@ -122,11 +121,10 @@ TransactionTestFrame::checkValidForTesting(Application& app,
                                            uint64_t lowerBoundCloseTimeOffset,
                                            uint64_t upperBoundCloseTimeOffset)
 {
-    bool res;
-    std::tie(res, mTransactionTxResult) =
+    mTransactionTxResult =
         checkValid(app, ltxOuter, current, lowerBoundCloseTimeOffset,
                    upperBoundCloseTimeOffset);
-    return res;
+    return mTransactionTxResult->isSuccess();
 }
 
 bool

diff --git a/src/transactions/test/TransactionTestFrame.h b/src/transactions/test/TransactionTestFrame.h
@@ -61,7 +61,7 @@ class TransactionTestFrame : public TransactionFrameBase
                TransactionMetaFrame& meta, MutableTxResultPtr txResult,
                Hash const& sorobanBasePrngSeed = Hash{}) const override;
 
-    std::pair<bool, MutableTxResultPtr>
+    MutableTxResultPtr
     checkValid(Application& app, AbstractLedgerTxn& ltxOuter,
                SequenceNumber current, uint64_t lowerBoundCloseTimeOffset,
                uint64_t upperBoundCloseTimeOffset) const override;