Merge remote-tracking branch 'XRPLF/develop' into ximinez/failure-invariant

* XRPLF/develop: (44 commits)
  fix: Skip bootstrap DNS lookups in standalone mode (8583)
  fix: Charge transfer fee on third-party vault withdrawals (8267)
  fix: Reject VaultClawback with a nonexistent holder (8470)
  fix: Enforce pseudo-account directory contents with an invariant (8181)
  build: Unify clang variables (8581)
  fix: Enforce asset authorization on LoanBrokerDelete (8204)
  feat: Book FixedPrecision LoanSet through AssetsDeployed (8260)
  feat: Round LoanBroker cover on the FixedPrecision grid (8258)
  feat: Use FixedPrecision accounting in Vault transactors (8460)
  refactor: Migrate handlers to rpc-spec (C) (8561)
  fix: Clamp invalid XRP amounts in Quality::ceilOut & getBookPage (8480)
  fix: Waive CanTransfer on loan default and broker delete (8231)
  refactor: Decouple LedgerHistory from TaggedCache internals (5874)
  feat: Add TransactionProposalCancel & Cosign Ticket reservation (8538)
  ci: Fix rpm installation problems (8575)
  docs: Clarify freeze on large changes 2 weeks before first RC (8574)
  refactor: Retire fixAMMv1_1 amendment (8226)
  chore: [DEPENDABOT] bump pyparsing from 3.3.2 to 3.3.3 in /cmake/scripts/codegen in the codegen-dependencies group (8572)
  fix: Allow WASM transactions in simulate only in admin mode (8568)
  fix: Detect int64 overflow in XRPAmount and MPTAmount (8453)
  ...

include/xrpl/protocol/detail/features.macro

@@ -15,6 +15,7 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
+XRPL_FEATURE(TecInvariant,                Supported::Yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(Cosign,                      Supported::No,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(SmartEscrow,                 Supported::No,  VoteBehavior::DefaultNo)
 // Requires LendingProtocolV1_1. New vaults take FixedPrecision plus cash-basis.

include/xrpl/tx/Transactor.h

@@ -11,6 +11,7 @@
 #include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/Fees.h>
 #include <xrpl/protocol/Keylet.h>
+#include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/Permissions.h>
 #include <xrpl/protocol/Rules.h>
 #include <xrpl/protocol/SField.h>
@@ -398,6 +399,10 @@ public:
         UInt256 const& ticketIndex,
         beast::Journal j);
 
+    // Interface used by processPersistentChanges and Invariants
+    static std::unordered_set<LedgerEntryType>
+    typesForResult(TER const ter);
+
 protected:
     TER
     apply();

include/xrpl/tx/invariants/InvariantCheck.h

@@ -118,6 +118,59 @@ public:
 };
 #endif
 
+/**
+ * @brief An unsuccessful transaction claiming a fee can only make a very small set of changes.
+ *
+ * 1. Reduce at most one AccountRoot or Sponsorship's XRP balance (pay a fee). (A transaction may
+ *    pay 0.)
+ * 2. Increment one AccountRoot's sequence or delete a Ticket, not both.
+ * 3. Delete expired objects, depending on the failure code.
+ *      For tecOVERSIZE and tecKILLED, ltOFFER
+ *      For tecINCOMPLETE, ltRIPPLE_STATE
+ *      For tecEXPIRED, ltNFTOKEN_OFFER or ltCREDENTIAL
+ * 4. AccountRoot owner counts may be decreased. The net change must equal to the number of deleted
+ *    objects.
+ * 5. Ticket counts may be decreased. The net change must equal to the number of deleted tickets
+ *    (which should be at most 1).
+ * 6. Modify or delete Directory Nodes, only if expired objects were deleted.
+ *
+ * Anything outside of that is bad.
+ *
+ * Collect change data for the known allowed types, and collect the info in errors_ for anything
+ * else.
+ *
+ * Note that finalize() will always return true on a `tesSUCCESS`, even if there are messages
+ * collected in errors_. The errors_ only apply if the transaction was NOT successful. It will also
+ * do additional checks based on the transaction data.
+ */
+class FailedTransaction
+{
+    struct DeletedEntry
+    {
+        SLE::const_pointer before;
+        SLE::const_pointer after;
+    };
+
+    // accountPaidFee and accountIncreasedSequence are usually the same account, but they don't have
+    // to be.
+    SLE::const_pointer accountPaidFee_;
+    SLE::const_pointer sponsorPaidFee_;
+    SLE::const_pointer accountIncreasedSequence_;
+    SLE::const_pointer deletedTicket_;
+    int netOwnerCountChange_ = 0;
+    int netTicketCountChange_ = 0;
+    std::vector<DeletedEntry> deletedObjects_;
+    std::vector<SLE::const_pointer> directorySideEffects_;
+    std::vector<std::string> errors_;
+
+public:
+    void
+    visitEntry(bool isDelete, SLE::ConstRef before, SLE::ConstRef after);
+
+    [[nodiscard]] bool
+    finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
+};
+
 /**
  * @brief Invariant: We should never charge a transaction a negative fee or a
  * fee that is larger than what the transaction itself specifies.
@@ -453,6 +506,7 @@ private:
 // additional invariant checks can be declared above and then added to this
 // tuple
 using InvariantChecks = std::tuple<
+    FailedTransaction,
     TransactionFeeCheck,
     AccountRootsNotDeleted,
     AccountRootsDeletedClean,

src/libxrpl/tx/Transactor.cpp

@@ -1466,6 +1466,26 @@ Transactor::trapTransaction(UInt256 txHash) const
     JLOG(j_.debug()) << "Transaction trapped: " << txHash;
 }
 
+std::unordered_set<LedgerEntryType>
+Transactor::typesForResult(TER const ter)
+{
+    std::unordered_set<LedgerEntryType> types;
+    if ((ter == tecOVERSIZE) || (ter == tecKILLED))
+    {
+        types.insert(ltOFFER);
+    }
+    else if (ter == tecINCOMPLETE)
+    {
+        types.insert(ltRIPPLE_STATE);
+    }
+    else if (ter == tecEXPIRED)
+    {
+        types.insert(ltNFTOKEN_OFFER);
+        types.insert(ltCREDENTIAL);
+    }
+    return types;
+}
+
 std::tuple<TER, XRPAmount, bool>
 Transactor::processPersistentChanges(TER result, XRPAmount fee)
 {
@@ -1476,24 +1496,6 @@ Transactor::processPersistentChanges(TER result, XRPAmount fee)
     //        should be used, making it possible to do more useful work
     //        when transactions fail with a `tec` code.
 
-    auto const typesForResult = [](TER const ter) {
-        std::unordered_set<LedgerEntryType> types;
-        if ((ter == tecOVERSIZE) || (ter == tecKILLED))
-        {
-            types.insert(ltOFFER);
-        }
-        else if (ter == tecINCOMPLETE)
-        {
-            types.insert(ltRIPPLE_STATE);
-        }
-        else if (ter == tecEXPIRED)
-        {
-            types.insert(ltNFTOKEN_OFFER);
-            types.insert(ltCREDENTIAL);
-        }
-        return types;
-    };
-
     // Build a list of ledger entry types to collect, based on the
     // result code. Only deleted objects of these types will be
     // re-applied after the context is reset.

src/libxrpl/tx/invariants/InvariantCheck.cpp

@@ -30,6 +30,7 @@
 #include <xrpl/protocol/TxSettings.h>
 #include <xrpl/protocol/UintTypes.h>
 #include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/tx/Transactor.h>
 #include <xrpl/tx/invariants/InvariantCheckPrivilege.h>
 
 #include <algorithm>
@@ -39,6 +40,7 @@
 #include <optional>
 #include <sstream>
 #include <string>
+#include <string_view>
 #include <vector>
 
 namespace xrpl {
@@ -89,6 +91,382 @@ ledgerEntryTypeName(SLE const& sle)
     return item->getName();
 }
 
+void
+FailedTransaction::visitEntry(bool isDelete, SLE::ConstRef before, SLE::ConstRef after)
+{
+    std::ostringstream err;
+
+    XRPL_ASSERT(after, "xrpl::FailedTransaction::visitEntry : valid after");
+
+    if (!before)
+    {
+        // Nothing can be created by a failed transaction
+        auto const name = [&after] -> std::string {
+            switch (after->getType())
+            {
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable : 4996)  // 4996 is the standard deprecation warning code
+#elifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#endif
+                // These types are deprecated, which is why they are needed here. But builds with
+                // warnings as errors (which includes CI) will fail. So to use them, deprecation
+                // warnings need to be turned off for this block only.
+                case ltNICKNAME:
+                case ltCONTRACT:
+                case ltGENERATOR_MAP:
+#ifdef _MSC_VER
+#pragma warning(pop)
+#elifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+                    return "[DEPRECATED TYPE]";
+                default:
+                    return ledgerEntryTypeName(*after);
+            }
+        };
+        err << "Unexpected ledger entry created: " << name() << ", " << after->key();
+        errors_.emplace_back(err.str());
+        return;
+    }
+
+    // Returns whether the object matched the target type. If it was, the caller may do additional
+    // checks related to that type, and should return.
+    auto validateFeePayer = [isDelete, &err, this](
+                                LedgerEntryType target,
+                                SLE::const_pointer& tracker,
+                                SLE::ConstRef before,
+                                SLE::ConstRef after,
+                                SF_AMOUNT const& field,
+                                std::string_view desc,
+                                std::function<std::string(SLE::ConstRef)> labelMaker) {
+        if (after->getType() != target)
+            return false;
+
+        auto const label = labelMaker(after);
+
+        if (isDelete)
+        {
+            err << desc << " was deleted: " << label;
+            errors_.emplace_back(err.str());
+            return true;
+        }
+
+        auto const beforeOptional = before->at(~field);
+        auto const afterOptional = after->at(~field);
+
+        auto const beforeBalance = beforeOptional.value_or(STAmount{xrpIssue()});
+        auto const afterBalance = afterOptional.value_or(STAmount{xrpIssue()});
+
+        // Check both directions because there's a separate check for "balance increased".
+        if (!canSubtract(beforeBalance, afterBalance) && !canSubtract(afterBalance, beforeBalance))
+        {
+            err << desc << " before and after balances not comparable: " << label;
+            errors_.emplace_back(err.str());
+            return true;
+        }
+
+        if (afterBalance != beforeBalance)
+        {
+            if (afterBalance > beforeBalance)
+            {
+                err << desc << " balance increased: " << label;
+                errors_.emplace_back(err.str());
+                return true;
+            }
+            if (tracker)
+            {
+                err << "Multiple " << desc << "s were charged fees: " << labelMaker(tracker)
+                    << " and " << label;
+                errors_.emplace_back(err.str());
+                return true;
+            }
+            tracker = after;
+        }
+
+        auto const* format = LedgerFormats::getInstance().findByType(target);
+        if (format == nullptr)
+        {
+            // LCOV_EXCL_START
+            UNREACHABLE("xrpl::FailedTransaction::visitEntry : object has no known ledger format");
+            return true;
+            // LCOV_EXCL_STOP
+        }
+
+        auto const allowedFields = std::to_array<SField const*>(
+            {&sfSequence,
+             &sfBalance,
+             &sfOwnerCount,
+             &sfTicketCount,
+             &sfFeeAmount,
+             &sfPreviousTxnID,
+             &sfPreviousTxnLgrSeq});
+        for (auto const& elem : format->getSOTemplate())
+        {
+            auto const& sf = elem.sField();
+
+            // No fields other than allowedFields may change
+            if (std::ranges::any_of(
+                    allowedFields, [&sf](auto const* allowedField) { return allowedField == &sf; }))
+                continue;
+
+            auto const* bField = before->peekAtPField(sf);
+            auto const* aField = after->peekAtPField(sf);
+            bool const bPresent = (bField != nullptr) && bField->getSType() != STI_NOTPRESENT;
+            bool const aPresent = (aField != nullptr) && aField->getSType() != STI_NOTPRESENT;
+            if (bPresent != aPresent || (bPresent && aPresent && *bField != *aField))
+            {
+                err << "At least one unexpected " << desc << " field modified: " << label
+                    << ", field: " << sf.getName()
+                    << ", before: " << (bPresent ? bField->getText() : "(absent)")
+                    << ", after: " << (aPresent ? aField->getText() : "(absent)");
+                errors_.emplace_back(err.str());
+                return true;
+            }
+        }
+
+        return true;
+    };
+
+    if (validateFeePayer(
+            ltACCOUNT_ROOT,
+            accountPaidFee_,
+            before,
+            after,
+            sfBalance,
+            "Account root",
+            [](SLE::ConstRef sle) { return to_string(sle->at(sfAccount)); }))
+    {
+        auto const beforeOwnerCount = before->at(~sfOwnerCount).value_or(0);
+        auto const afterOwnerCount = after->at(~sfOwnerCount).value_or(0);
+        if (afterOwnerCount != beforeOwnerCount)
+        {
+            if (afterOwnerCount > beforeOwnerCount)
+            {
+                err << "Account root OwnerCount increased: " << after->at(sfAccount);
+                errors_.emplace_back(err.str());
+            }
+            // The fields are uints, so do the math in two steps.
+            netOwnerCountChange_ += afterOwnerCount;
+            netOwnerCountChange_ -= beforeOwnerCount;
+        }
+
+        auto const beforeTicketCount = before->at(~sfTicketCount).value_or(0);
+        auto const afterTicketCount = after->at(~sfTicketCount).value_or(0);
+        if (afterTicketCount != beforeTicketCount)
+        {
+            if (afterTicketCount > beforeTicketCount)
+            {
+                err << "Account root TicketCount increased: " << after->at(sfAccount);
+                errors_.emplace_back(err.str());
+            }
+            // The fields are uints, so do the math in two steps.
+            netTicketCountChange_ += afterTicketCount;
+            netTicketCountChange_ -= beforeTicketCount;
+        }
+
+        auto const beforeSequence = before->at(sfSequence);
+        auto const afterSequence = after->at(sfSequence);
+        if (afterSequence != beforeSequence)
+        {
+            if (afterSequence < beforeSequence)
+            {
+                // This is always bad, but we're only checking failed transactions here
+                // TODO: Maybe add a "global failures" check to this Invariant, at the risk of scope
+                // creep.
+                err << "Account root sequence decreased: " << after->at(sfAccount);
+                errors_.emplace_back(err.str());
+                return;
+            }
+            if (accountIncreasedSequence_)
+            {
+                err << "Multiple Account root sequences were incremented: "
+                    << accountIncreasedSequence_->at(sfAccount) << " and " << after->at(sfAccount);
+                errors_.emplace_back(err.str());
+                return;
+            }
+            if (afterSequence != beforeSequence + 1)
+            {
+                err << "Account root sequence incremented by " << (afterSequence - beforeSequence)
+                    << ": " << after->at(sfAccount);
+                errors_.emplace_back(err.str());
+                return;
+            }
+            accountIncreasedSequence_ = after;
+        }
+        return;
+    }
+
+    if (validateFeePayer(
+            ltSPONSORSHIP,
+            sponsorPaidFee_,
+            before,
+            after,
+            sfFeeAmount,
+            "Sponsor",
+            [](SLE::ConstRef sle) {
+                std::ostringstream l;
+                l << sle->at(sfOwner) << " -> " << sle->at(sfSponsee);
+                return l.str();
+            }))
+        return;
+
+    if (after->getType() == ltTICKET)
+    {
+        if (!isDelete)
+        {
+            err << "Ticket was modified: " << after->at(sfAccount) << ", "
+                << after->at(sfTicketSequence);
+            errors_.emplace_back(err.str());
+            return;
+        }
+        if (deletedTicket_)
+        {
+            err << "Multiple tickets were deleted: " << deletedTicket_->at(sfAccount) << ", "
+                << deletedTicket_->at(sfTicketSequence) << " and " << after->at(sfAccount) << ", "
+                << after->at(sfTicketSequence);
+            errors_.emplace_back(err.str());
+            return;
+        }
+        deletedTicket_ = after;
+
+        return;
+    }
+
+    if (after->getType() == ltDIR_NODE)
+    {
+        // Directory deletions and modifications are a legal side effect of deleting objects. Track
+        // them separately.
+        directorySideEffects_.emplace_back(after);
+        return;
+    }
+
+    if (isDelete)
+    {
+        switch (after->getType())
+        {
+            case ltOFFER:
+            case ltRIPPLE_STATE:
+            case ltNFTOKEN_OFFER:
+            case ltCREDENTIAL:
+                deletedObjects_.emplace_back(before, after);
+                return;
+            default:
+                err << "Unexpected ledger entry deleted: " << ledgerEntryTypeName(*after) << ", "
+                    << after->key();
+                errors_.emplace_back(err.str());
+                return;
+        }
+    }
+
+    err << "Unexpected ledger entry modified: " << ledgerEntryTypeName(*after) << ", "
+        << after->key();
+    errors_.emplace_back(err.str());
+}
+
+bool
+FailedTransaction::finalize(
+    STTx const& tx,
+    TER const ter,
+    XRPAmount const fee,
+    ReadView const& view,
+    beast::Journal const& j)
+{
+    if (isTesSuccess(ter))
+    {
+        return true;
+    }
+
+    bool result = true;
+
+    // Log all the failures regardless of amendment status, but only return false / failure if
+    // featureTecInvariant is enabled
+    for (auto const& err : errors_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: " << err;
+        result = false;
+    }
+
+    if (accountPaidFee_ && sponsorPaidFee_)
+    {
+        // Is this legal?
+        JLOG(j.fatal()) << "Invariant failed: both account and sponsor paid fee";
+        result = false;
+    }
+
+    if (accountIncreasedSequence_ && deletedTicket_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: both account sequence increased and ticket deleted";
+        result = false;
+    }
+
+    if (tx.getSeqProxy().isTicket() && accountIncreasedSequence_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: account sequence increased by a ticket transaction";
+        result = false;
+    }
+
+    if (tx.getSeqProxy().isSeq() && deletedTicket_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: ticket deleted by a sequence transaction";
+        result = false;
+    }
+
+    auto const typesAllowedToDelete = Transactor::typesForResult(ter);
+
+    for (auto const& deleted : deletedObjects_)
+    {
+        auto const type = deleted.after->getType();
+        if (!typesAllowedToDelete.contains(type))
+        {
+            JLOG(j.fatal()) << "Invariant failed: unexpected ledger entry deleted: "
+                            << ledgerEntryTypeName(*deleted.after);
+            result = false;
+        }
+        // For offers, only allow unfunded removals
+        // (where TakerPays is unchanged)
+        if (type == ltOFFER &&
+            deleted.before->getFieldAmount(sfTakerPays) !=
+                deleted.after->getFieldAmount(sfTakerPays))
+        {
+            JLOG(j.fatal()) << "Invariant failed: funded offer deleted: "
+                            << ledgerEntryTypeName(*deleted.after);
+            result = false;
+        }
+    }
+
+    if (!deletedTicket_ && deletedObjects_.empty() && !directorySideEffects_.empty())
+    {
+        JLOG(j.fatal()) << "Invariant failed: " << directorySideEffects_.size()
+                        << " directory side effects without any deleted objects";
+        result = false;
+    }
+
+    auto const expectedTicketChange = deletedTicket_ ? -1 : 0;
+    if (expectedTicketChange != netTicketCountChange_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: ticket deletions (" << expectedTicketChange
+                        << ") does not match net ticket count change (" << netTicketCountChange_
+                        << ")";
+        result = false;
+    }
+
+    auto const expectedOwnerChanges = -deletedObjects_.size() + expectedTicketChange;
+    if (expectedOwnerChanges != netOwnerCountChange_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: total deletions (" << expectedOwnerChanges
+                        << ") does not match net owner count change (" << netOwnerCountChange_
+                        << ")";
+        result = false;
+    }
+
+    bool const enforce = view.rules().enabled(featureTecInvariant);
+    XRPL_ASSERT_IF(!result, enforce, "FailedTransaction::finalize : amendment enabled");
+    return result || !enforce;
+}
+
 void
 TransactionFeeCheck::visitEntry(bool, SLE::ConstRef, SLE::ConstRef)
 {

src/test/app/InvariantsFailedTransaction_test.cpp

@@ -0,0 +1,692 @@
+#include <test/jtx/Account.h>
+#include <test/jtx/Env.h>
+#include <test/jtx/TestHelpers.h>
+#include <test/jtx/amount.h>
+#include <test/jtx/envconfig.h>
+#include <test/jtx/offer.h>
+#include <test/jtx/sponsor.h>
+#include <test/jtx/ticket.h>
+#include <test/unit_test/SuiteJournal.h>
+
+#include <xrpl/beast/unit_test/suite.h>
+#include <xrpl/beast/utility/Journal.h>
+#include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/OpenView.h>
+#include <xrpl/protocol/Feature.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/Rules.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/STLedgerEntry.h>
+#include <xrpl/protocol/STObject.h>
+#include <xrpl/protocol/STTx.h>
+#include <xrpl/protocol/SeqProxy.h>
+#include <xrpl/protocol/TER.h>
+#include <xrpl/protocol/TxFormats.h>
+#include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/tx/ApplyContext.h>
+#include <xrpl/tx/Transactor.h>
+
+#include <cstdint>
+#include <functional>
+#include <initializer_list>
+#include <memory>
+#include <source_location>
+#include <string>
+#include <vector>
+
+namespace xrpl {
+
+// Test-only factory — not part of the public API.
+// The returned Transactor holds a raw reference to ctx; the caller must ensure
+// the ApplyContext outlives the Transactor. Implemented in applySteps.cpp
+std::unique_ptr<Transactor>
+makeTransactor(ApplyContext& ctx);
+
+}  // namespace xrpl
+
+namespace xrpl::test {
+
+/**
+ * Tests for the `FailedTransaction` invariant (see InvariantCheck.h/.cpp).
+ *
+ * That invariant only does anything on a *failed* (non-`tesSUCCESS`)
+ * transaction, so unlike the checks exercised in Invariants_test.cpp, the
+ * invariant machinery has to be primed with a `tec` result code. The
+ * `doInvariantCheck` helper below is a close relative of the one in
+ * Invariants_test.cpp, but seeds the first pass with a caller-supplied `tec`
+ * code instead of `tesSUCCESS`. It also always enables `featureTecInvariant`
+ * (via testableAmendments), which is required: `FailedTransaction::finalize`
+ * asserts that the amendment is enabled whenever it is about to return false.
+ *
+ * Every failure case is covered at least once. A "failure case" is either a
+ * string appended to `errors_` in `visitEntry`, or a place where `result` is
+ * set to false in `finalize`:
+ *
+ * visitEntry:
+ *   - "Unexpected ledger entry created"          testUnexpectedEntryCreated
+ *   - "<desc> was deleted"                       testAccountRootFeePayer,
+ *                                                testSponsorFeePayer
+ *   - "<desc> balance increased"                 testAccountRootFeePayer
+ *   - "Multiple <desc>s were charged fees"       testAccountRootFeePayer
+ *   - "Account root sequence decreased"          testAccountSequence
+ *   - "Multiple Account root sequences ..."      testAccountSequence
+ *   - "Account root sequence incremented by"     testAccountSequence
+ *   - "Ticket was modified"                      testTicket
+ *   - "Multiple tickets were deleted"            testTicket
+ *   - "Unexpected ledger entry deleted"          testUnexpectedDeleteModify
+ *   - "Unexpected ledger entry modified"         testUnexpectedDeleteModify
+ *
+ * finalize:
+ *   - any collected error                        (all of the above)
+ *   - "both account and sponsor paid fee"        testSponsorFeePayer
+ *   - "both account sequence increased and       testSeqTicketCombinations
+ *      ticket deleted"
+ *   - "account sequence increased by a ticket    testSeqTicketCombinations
+ *      transaction"
+ *   - "ticket deleted by a sequence              testSeqTicketCombinations
+ *      transaction"
+ *   - "unexpected ledger entry deleted"          testDeletedObjects
+ *   - "funded offer deleted"                     testDeletedObjects
+ *   - "directory side effects without any        testDirectorySideEffects
+ *      deleted objects"
+ *
+ * Three `errors_` sites in `visitEntry` are intentionally NOT covered because
+ * they cannot be reached through the full invariant suite:
+ *   - "before and after balances not comparable" (InvariantCheck.cpp) requires
+ *     a fee-payer's balance to change currency. Both fee-payer balances
+ *     (AccountRoot.Balance and Sponsorship.FeeAmount) are always XRP, so making
+ *     them non-comparable means making one side non-XRP, which causes
+ *     XRPNotCreated::visitEntry to throw from STAmount::xrp() before finalize
+ *     ever runs, so the message can never be logged.
+ *   - the `format == nullptr` branch is marked UNREACHABLE / LCOV_EXCL: an
+ *     AccountRoot / Sponsorship always has a ledger format.
+ *   - the "field modified" loop is dead code (guarded by an unconditional
+ *     `break`).
+ */
+class InvariantsFailedTransaction_test : public beast::unit_test::Suite
+{
+    // Runs additional (valid) transactions on the ledger after the two funded
+    // accounts are created, but before the ledger is closed and the precheck
+    // runs. Used to place real objects (tickets, checks, offers, sponsorships)
+    // into the ledger.
+    using Preclose = std::function<
+        bool(test::jtx::Account const& a, test::jtx::Account const& b, test::jtx::Env& env)>;
+
+    // Manipulates the ApplyContext's view to simulate the changes a failing
+    // transaction might (incorrectly) leave behind.
+    using Precheck = std::function<
+        bool(test::jtx::Account const& a, test::jtx::Account const& b, ApplyContext& ac)>;
+
+    static FeatureBitset
+    defaultAmendments()
+    {
+        // testableAmendments() includes featureTecInvariant and featureSponsor,
+        // both of which are needed here.
+        return xrpl::test::jtx::testableAmendments() | fixCleanup3_1_3 | fixCleanup3_2_0;
+    }
+
+    test::jtx::Env
+    makeEnv()
+    {
+        return {
+            *this, test::jtx::envconfig(), defaultAmendments(), nullptr, beast::Severity::Disabled};
+    }
+
+    /**
+     * Run a single FailedTransaction invariant test case.
+     *
+     * @param expectLogs Messages that should appear in the log output.
+     * @param preclose See "Preclose" above. Runs valid transactions to set up
+     *  the ledger before the precheck. Use the overload without this parameter
+     *  when no setup is needed.
+     * @param precheck Manipulates the view to create the state to be detected.
+     * @param startTer The `tec` code the simulated transaction "failed" with.
+     *  This is fed into the first pass of the invariant checker. The invariant
+     *  only inspects failed transactions, so this must be a `tec` (claim) code.
+     * @param tx A mock transaction. Only its type and sequence/ticket fields
+     *  matter to this invariant.
+     * @param fee The fee the simulated transaction paid.
+     */
+    void
+    doInvariantCheck(
+        std::vector<std::string> const& expectLogs,
+        Preclose const& preclose,
+        Precheck const& precheck,
+        TER const startTer,
+        STTx const& tx = STTx{ttACCOUNT_SET, [](STObject&) {}},
+        XRPAmount const fee = XRPAmount{},
+        std::source_location const& loc = std::source_location::current())
+    {
+        using namespace test::jtx;
+
+        Env env = makeEnv();
+
+        Account const a1{"A1"};
+        Account const a2{"A2"};
+        env.fund(XRP(1000), a1, a2);
+        if (preclose)
+            BEAST_EXPECT(preclose(a1, a2, env));
+        env.close();
+
+        OpenView ov{*env.current()};
+        StreamSink sink{beast::Severity::Warning};
+        beast::Journal const jlog{sink};
+        ApplyContext ac{env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog};
+
+        // Invariants normally run in the Transaction's "apply" (operator())
+        // context, and can always access global Rules.
+        CurrentTransactionRulesGuard const rulesGuard(ov.rules());
+
+        BEAST_EXPECT(precheck(a1, a2, ac));
+
+        auto transactor = makeTransactor(ac);
+        if (!BEAST_EXPECT(transactor))
+            return;
+
+        // The invariant checker is invoked twice, exactly as it would be for a
+        // real transaction that claims a fee: the first pass sees the "real"
+        // failure code and, upon detecting a broken invariant, returns
+        // tecINVARIANT_FAILED. The second pass (charging only the fee) then
+        // sees that and escalates to tefINVARIANT_FAILED.
+        std::initializer_list<TER> const ters = {tecINVARIANT_FAILED, tefINVARIANT_FAILED};
+
+        TER terActual = startTer;
+        for (TER const& terExpect : ters)
+        {
+            terActual =
+                transactor->checkInvariants(terActual, fee, Transactor::InvariantScope::Full);
+            expect(
+                terExpect == terActual,
+                "expected: " + transToken(terExpect) + " got: " + transToken(terActual),
+                loc.file_name(),
+                loc.line());
+            auto const messages = sink.messages().str();
+
+            if (!isTesSuccess(terActual))
+            {
+                expect(
+                    messages.starts_with("Invariant failed:") ||
+                        messages.starts_with("Transaction caused an exception"),
+                    messages,
+                    loc.file_name(),
+                    loc.line());
+            }
+
+            // std::cerr << messages << '\n';
+            for (auto const& m : expectLogs)
+            {
+                expect(messages.contains(m), m, loc.file_name(), loc.line());
+            }
+        }
+    }
+
+    // Convenience overload for cases that need no `preclose` setup.
+    void
+    doInvariantCheck(
+        std::vector<std::string> const& expectLogs,
+        Precheck const& precheck,
+        TER const startTer,
+        STTx const& tx = STTx{ttACCOUNT_SET, [](STObject&) {}},
+        XRPAmount const fee = XRPAmount{},
+        std::source_location const& loc = std::source_location::current())
+    {
+        doInvariantCheck(expectLogs, Preclose{}, precheck, startTer, tx, fee, loc);
+    }
+
+    void
+    testUnexpectedEntryCreated()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction created a ledger entry";
+
+        // A failed transaction must not create any new ledger entries.
+        doInvariantCheck(
+            {{"Unexpected ledger entry created: Check"}},
+            [](Account const& a1, Account const&, ApplyContext& ac) {
+                // Insert a brand new (valid type) ledger entry.
+                auto const sleNew =
+                    std::make_shared<SLE>(keylet::check(a1.id(), SeqProxy::rawSequence(1)));
+                ac.view().insert(sleNew);
+                return true;
+            },
+            tecFAILED_PROCESSING);
+    }
+
+    void
+    testAccountRootFeePayer()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction account root fee payer";
+
+        // The only account root balance change allowed by a failed transaction
+        // is a single decrease (paying the fee).
+
+        // An account root was deleted.
+        doInvariantCheck(
+            {{"Account root was deleted:"}},
+            [](Account const& a1, Account const&, ApplyContext& ac) {
+                auto const sle = ac.view().peek(keylet::account(a1.id()));
+                if (!sle)
+                    return false;
+                ac.view().erase(sle);
+                return true;
+            },
+            tecFAILED_PROCESSING);
+
+        // An account root's balance increased.
+        doInvariantCheck(
+            {{"Account root balance increased:"}},
+            [](Account const& a1, Account const&, ApplyContext& ac) {
+                auto const sle = ac.view().peek(keylet::account(a1.id()));
+                if (!sle)
+                    return false;
+                sle->setFieldAmount(sfBalance, sle->getFieldAmount(sfBalance) + STAmount{500});
+                ac.view().update(sle);
+                return true;
+            },
+            tecFAILED_PROCESSING);
+
+        // More than one account root paid a fee (both balances decreased).
+        doInvariantCheck(
+            {{"Multiple Account roots were charged fees:"}},
+            [](Account const& a1, Account const& a2, ApplyContext& ac) {
+                for (auto const& acct : {a1.id(), a2.id()})
+                {
+                    auto const sle = ac.view().peek(keylet::account(acct));
+                    if (!sle)
+                        return false;
+                    sle->setFieldAmount(sfBalance, sle->getFieldAmount(sfBalance) - STAmount{500});
+                    ac.view().update(sle);
+                }
+                return true;
+            },
+            tecFAILED_PROCESSING);
+    }
+
+    void
+    testAccountSequence()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction account sequence changes";
+
+        // A failed transaction may only ever increment a single account root's
+        // sequence, and by exactly one.
+
+        // Sequence decreased.
+        doInvariantCheck(
+            {{"Account root sequence decreased:"}},
+            [](Account const& a1, Account const&, ApplyContext& ac) {
+                auto const sle = ac.view().peek(keylet::account(a1.id()));
+                if (!sle)
+                    return false;
+                sle->setFieldU32(sfSequence, sle->getFieldU32(sfSequence) - 1);
+                ac.view().update(sle);
+                return true;
+            },
+            tecFAILED_PROCESSING);
+
+        // More than one account root's sequence was incremented.
+        doInvariantCheck(
+            {{"Multiple Account root sequences were incremented:"}},
+            [](Account const& a1, Account const& a2, ApplyContext& ac) {
+                for (auto const& acct : {a1.id(), a2.id()})
+                {
+                    auto const sle = ac.view().peek(keylet::account(acct));
+                    if (!sle)
+                        return false;
+                    sle->setFieldU32(sfSequence, sle->getFieldU32(sfSequence) + 1);
+                    ac.view().update(sle);
+                }
+                return true;
+            },
+            tecFAILED_PROCESSING);
+
+        // A single account root's sequence was incremented by more than one.
+        doInvariantCheck(
+            {{"Account root sequence incremented by 2:"}},
+            [](Account const& a1, Account const&, ApplyContext& ac) {
+                auto const sle = ac.view().peek(keylet::account(a1.id()));
+                if (!sle)
+                    return false;
+                sle->setFieldU32(sfSequence, sle->getFieldU32(sfSequence) + 2);
+                ac.view().update(sle);
+                return true;
+            },
+            tecFAILED_PROCESSING);
+    }
+
+    void
+    testTicket()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction ticket changes";
+
+        // A failed transaction may delete at most one ticket, and may not
+        // otherwise modify a ticket.
+
+        // A ticket was modified (rather than only deleted).
+        {
+            std::uint32_t ticketSeq = 0;
+            doInvariantCheck(
+                {{"Ticket was modified:"}},
+                [&ticketSeq](Account const& a1, Account const&, Env& env) {
+                    ticketSeq = env.seq(a1) + 1;
+                    env(ticket::create(a1, 1));
+                    return true;
+                },
+                [&ticketSeq](Account const& a1, Account const&, ApplyContext& ac) {
+                    auto const sle =
+                        ac.view().peek(keylet::ticket(a1.id(), SeqProxy::rawTicket(ticketSeq)));
+                    if (!sle)
+                        return false;
+                    ac.view().update(sle);
+                    return true;
+                },
+                tecFAILED_PROCESSING);
+        }
+
+        // More than one ticket was deleted.
+        {
+            std::uint32_t ticketSeq = 0;
+            doInvariantCheck(
+                {{"Multiple tickets were deleted:"}},
+                [&ticketSeq](Account const& a1, Account const&, Env& env) {
+                    ticketSeq = env.seq(a1) + 1;
+                    env(ticket::create(a1, 2));
+                    return true;
+                },
+                [&ticketSeq](Account const& a1, Account const&, ApplyContext& ac) {
+                    for (std::uint32_t i = 0; i < 2; ++i)
+                    {
+                        auto const sle = ac.view().peek(
+                            keylet::ticket(a1.id(), SeqProxy::rawTicket(ticketSeq + i)));
+                        if (!sle)
+                            return false;
+                        ac.view().erase(sle);
+                    }
+                    return true;
+                },
+                tecFAILED_PROCESSING);
+        }
+    }
+
+    void
+    testUnexpectedDeleteModify()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction touched an unexpected entry";
+
+        auto const makeCheck = [](std::uint32_t& checkSeq) {
+            return [&checkSeq](Account const& a1, Account const& a2, Env& env) {
+                checkSeq = env.seq(a1);
+                env(check::create(a1, a2, XRP(1)));
+                return true;
+            };
+        };
+
+        // An entry of an unexpected type was deleted. A failed transaction may
+        // only delete tickets and (depending on the failure code) a small set
+        // of expired objects; a Check is neither.
+        {
+            std::uint32_t checkSeq = 0;
+            doInvariantCheck(
+                {{"Unexpected ledger entry deleted: Check"}},
+                makeCheck(checkSeq),
+                [&checkSeq](Account const& a1, Account const&, ApplyContext& ac) {
+                    auto const sle =
+                        ac.view().peek(keylet::check(a1.id(), SeqProxy::rawSequence(checkSeq)));
+                    if (!sle)
+                        return false;
+                    ac.view().erase(sle);
+                    return true;
+                },
+                tecFAILED_PROCESSING);
+        }
+
+        // An entry of an unexpected type was modified.
+        {
+            std::uint32_t checkSeq = 0;
+            doInvariantCheck(
+                {{"Unexpected ledger entry modified: Check"}},
+                makeCheck(checkSeq),
+                [&checkSeq](Account const& a1, Account const&, ApplyContext& ac) {
+                    auto const sle =
+                        ac.view().peek(keylet::check(a1.id(), SeqProxy::rawSequence(checkSeq)));
+                    if (!sle)
+                        return false;
+                    ac.view().update(sle);
+                    return true;
+                },
+                tecFAILED_PROCESSING);
+        }
+    }
+
+    void
+    testSponsorFeePayer()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction sponsor fee payer";
+
+        // Create a sponsorship (a1 sponsors a2's fees) with a fee pool.
+        auto const makeSponsorship = [](Account const& a1, Account const& a2, Env& env) {
+            env(sponsor::set_fee(a1, 0, XRP(10), XRP(20)), sponsor::SponseeAcc(a2));
+            return static_cast<bool>(env.le(keylet::sponsorship(a1.id(), a2.id())));
+        };
+
+        // A sponsorship was deleted. This exercises the shared fee-payer
+        // "was deleted" path for the Sponsorship type.
+        doInvariantCheck(
+            {{"Sponsor was deleted:"}},
+            makeSponsorship,
+            [](Account const& a1, Account const& a2, ApplyContext& ac) {
+                auto const sle = ac.view().peek(keylet::sponsorship(a1.id(), a2.id()));
+                if (!sle)
+                    return false;
+                ac.view().erase(sle);
+                return true;
+            },
+            tecFAILED_PROCESSING);
+
+        // Both an account root and a sponsorship paid a fee. Only one payer is
+        // ever allowed.
+        doInvariantCheck(
+            {{"both account and sponsor paid fee"}},
+            makeSponsorship,
+            [](Account const& a1, Account const& a2, ApplyContext& ac) {
+                // The account root pays a fee (balance decreases).
+                auto const acct = ac.view().peek(keylet::account(a1.id()));
+                if (!acct)
+                    return false;
+                acct->setFieldAmount(sfBalance, acct->getFieldAmount(sfBalance) - STAmount{500});
+                ac.view().update(acct);
+
+                // The sponsorship also pays a fee (its FeeAmount decreases).
+                auto const spon = ac.view().peek(keylet::sponsorship(a1.id(), a2.id()));
+                if (!spon || !spon->isFieldPresent(sfFeeAmount))
+                    return false;
+                spon->setFieldAmount(
+                    sfFeeAmount, spon->getFieldAmount(sfFeeAmount) - STAmount{500});
+                ac.view().update(spon);
+                return true;
+            },
+            tecFAILED_PROCESSING);
+    }
+
+    void
+    testSeqTicketCombinations()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction sequence / ticket combinations";
+
+        auto const bumpSequence = [](Account const& a1, Account const&, ApplyContext& ac) {
+            auto const sle = ac.view().peek(keylet::account(a1.id()));
+            if (!sle)
+                return false;
+            sle->setFieldU32(sfSequence, sle->getFieldU32(sfSequence) + 1);
+            ac.view().update(sle);
+            return true;
+        };
+
+        // A transaction may increment an account sequence OR delete a ticket,
+        // but not both.
+        {
+            std::uint32_t ticketSeq = 0;
+            doInvariantCheck(
+                {{"both account sequence increased and ticket deleted"}},
+                [&ticketSeq](Account const& a1, Account const&, Env& env) {
+                    ticketSeq = env.seq(a1) + 1;
+                    env(ticket::create(a1, 1));
+                    return true;
+                },
+                [&ticketSeq, &bumpSequence](
+                    Account const& a1, Account const& a2, ApplyContext& ac) {
+                    if (!bumpSequence(a1, a2, ac))
+                        return false;
+                    auto const sle =
+                        ac.view().peek(keylet::ticket(a1.id(), SeqProxy::rawTicket(ticketSeq)));
+                    if (!sle)
+                        return false;
+                    ac.view().erase(sle);
+                    return true;
+                },
+                tecFAILED_PROCESSING);
+        }
+
+        // A ticket transaction must not also increment the account sequence.
+        doInvariantCheck(
+            {{"account sequence increased by a ticket transaction"}},
+            bumpSequence,
+            tecFAILED_PROCESSING,
+            STTx{ttACCOUNT_SET, [](STObject& tx) {
+                     tx.setFieldU32(sfSequence, 0);
+                     tx.setFieldU32(sfTicketSequence, 1);
+                 }});
+
+        // A sequence (non-ticket) transaction must not delete a ticket.
+        {
+            std::uint32_t ticketSeq = 0;
+            doInvariantCheck(
+                {{"ticket deleted by a sequence transaction"}},
+                [&ticketSeq](Account const& a1, Account const&, Env& env) {
+                    ticketSeq = env.seq(a1) + 1;
+                    env(ticket::create(a1, 1));
+                    return true;
+                },
+                [&ticketSeq](Account const& a1, Account const&, ApplyContext& ac) {
+                    auto const sle =
+                        ac.view().peek(keylet::ticket(a1.id(), SeqProxy::rawTicket(ticketSeq)));
+                    if (!sle)
+                        return false;
+                    ac.view().erase(sle);
+                    return true;
+                },
+                tecFAILED_PROCESSING,
+                STTx{ttACCOUNT_SET, [](STObject& tx) { tx.setFieldU32(sfSequence, 1); }});
+        }
+    }
+
+    void
+    testDeletedObjects()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction deleted objects";
+
+        auto const makeOffer = [](std::uint32_t& offerSeq) {
+            return [&offerSeq](Account const& a1, Account const& a2, Env& env) {
+                offerSeq = env.seq(a1);
+                env(offer(a1, a2["USD"](10), XRP(10)));
+                return static_cast<bool>(
+                    env.le(keylet::offer(a1.id(), SeqProxy::rawSequence(offerSeq))));
+            };
+        };
+
+        // An object was deleted that is not allowed to be deleted for this
+        // failure code. tecEXPIRED allows deleting NFTokenOffers and
+        // Credentials, but not Offers.
+        {
+            std::uint32_t offerSeq = 0;
+            doInvariantCheck(
+                {{"unexpected ledger entry deleted: Offer"}},
+                makeOffer(offerSeq),
+                [&offerSeq](Account const& a1, Account const&, ApplyContext& ac) {
+                    auto const sle =
+                        ac.view().peek(keylet::offer(a1.id(), SeqProxy::rawSequence(offerSeq)));
+                    if (!sle)
+                        return false;
+                    ac.view().erase(sle);
+                    return true;
+                },
+                tecEXPIRED);
+        }
+
+        // A *funded* offer was deleted. tecKILLED allows deleting offers, but
+        // only unfunded ones (where TakerPays is unchanged).
+        {
+            std::uint32_t offerSeq = 0;
+            doInvariantCheck(
+                {{"funded offer deleted: Offer"}},
+                makeOffer(offerSeq),
+                [&offerSeq](Account const& a1, Account const& a2, ApplyContext& ac) {
+                    auto const sle =
+                        ac.view().peek(keylet::offer(a1.id(), SeqProxy::rawSequence(offerSeq)));
+                    if (!sle)
+                        return false;
+                    // Partially consume the offer (change TakerPays) so it
+                    // looks funded, then delete it.
+                    sle->setFieldAmount(sfTakerPays, a2["USD"](5));
+                    ac.view().update(sle);
+                    ac.view().erase(sle);
+                    return true;
+                },
+                tecKILLED);
+        }
+    }
+
+    void
+    testDirectorySideEffects()
+    {
+        using namespace test::jtx;
+        testcase << "failed transaction directory side effects";
+
+        // Directory modifications/deletions are only permitted as a side
+        // effect of deleting expired objects. Touching a directory without
+        // deleting any tracked object is not allowed.
+        std::uint32_t checkSeq = 0;
+        doInvariantCheck(
+            {{"directory side effects without any deleted objects"}},
+            [&checkSeq](Account const& a1, Account const& a2, Env& env) {
+                // Give a1 an owner directory to touch.
+                checkSeq = env.seq(a1);
+                env(check::create(a1, a2, XRP(1)));
+                return true;
+            },
+            [](Account const& a1, Account const&, ApplyContext& ac) {
+                auto const sle = ac.view().peek(keylet::ownerDir(a1.id()));
+                if (!sle)
+                    return false;
+                ac.view().update(sle);
+                return true;
+            },
+            tecFAILED_PROCESSING);
+    }
+
+public:
+    void
+    run() override
+    {
+        testUnexpectedEntryCreated();
+        testAccountRootFeePayer();
+        testAccountSequence();
+        testTicket();
+        testUnexpectedDeleteModify();
+        testSponsorFeePayer();
+        testSeqTicketCombinations();
+        testDeletedObjects();
+        testDirectorySideEffects();
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(InvariantsFailedTransaction, app, xrpl);
+
+}  // namespace xrpl::test

src/test/app/invariants/InvariantsAMM_test.cpp

@@ -177,16 +177,13 @@ class InvariantsAMM_test : public InvariantsBase
             return true;
         };
 
-        auto const test = [&](auto const txType,
-                              auto&& update,
-                              bool isMPT,
-                              TER error = tecINVARIANT_FAILED) {
+        auto test = [&](auto const txType, auto&& update, bool isMPT) {
             doInvariantCheck(
                 {{"AMM"}},
                 [&](Account const&, Account const&, ApplyContext& ac) { return update(ac, isMPT); },
                 XRPAmount{},
                 STTx{txType, [&](STObject& tx) {}},
-                {tecINVARIANT_FAILED, error},
+                {tecINVARIANT_FAILED, tefINVARIANT_FAILED},
                 [&](Account const&, Account const&, Env& env) {
                     env.fund(XRP(1'000), gw);
                     poolAsset = [&] -> PrettyAsset {
@@ -208,18 +205,15 @@ class InvariantsAMM_test : public InvariantsBase
 
         for (bool const isMPT : {false, true})
         {
-            // Under fixCleanup3_4_0 the MPT balance invariants also fire on the
-            // second pass, so both IOU and MPT pools now escalate to tef.
-            auto const error = TER(tefINVARIANT_FAILED);
             for (auto const txType : {ttAMM_CREATE, ttAMM_DEPOSIT, ttAMM_CLAWBACK, ttAMM_WITHDRAW})
             {
-                test(txType, deleteAMMAccount, isMPT, tefINVARIANT_FAILED);
+                test(txType, deleteAMMAccount, isMPT);
                 test(txType, updateLPTokensBadAmount, isMPT);
                 test(txType, updateLPTokensBadBalance, isMPT);
             }
             for (auto const txType : {ttAMM_BID, ttAMM_VOTE})
             {
-                test(txType, updateAMMPool, isMPT, error);
+                test(txType, updateAMMPool, isMPT);
                 test(txType, updateLPTokensBadAmount, isMPT);
                 test(txType, updateLPTokensBadBalance, isMPT);
             }

src/test/app/invariants/InvariantsMPT_test.cpp

@@ -1496,6 +1496,12 @@ class InvariantsMPT_test : public InvariantsBase
             return true;
         };
 
+        // Most of these tests use default Env, so fixTecInvaraint will always be enabled.
+        // If it's not, the invariant will usually assert.
+        bool const fixTecEnabled = true;
+        std::initializer_list<TER> const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED};
+        std::initializer_list<TER> const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED};
+
         // badDelete
         doInvariantCheck(
             {"MPToken deleted with encrypted fields while COA > 0"},
@@ -1526,7 +1532,7 @@ class InvariantsMPT_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseConfidential);
 
         doInvariantCheck(
@@ -1544,7 +1550,7 @@ class InvariantsMPT_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseConfidential);
 
         // requiresPrivacyFlag
@@ -1579,7 +1585,7 @@ class InvariantsMPT_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseNoPrivacy);
 
         // badCOA
@@ -1596,7 +1602,7 @@ class InvariantsMPT_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttMPTOKEN_ISSUANCE_SET, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseConfidential);
 
         // Conservation Violation
@@ -1616,7 +1622,7 @@ class InvariantsMPT_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseConfidential);
 
         // Send/MergeInbox must not change OutstandingAmount (coaDelta == 0)
@@ -1637,7 +1643,7 @@ class InvariantsMPT_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttCONFIDENTIAL_MPT_SEND, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseConfidential);
 
         // Send/MergeInbox and zero-COA-delta confidential transactions must not
@@ -1682,7 +1688,7 @@ class InvariantsMPT_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseConfidential);
 
         // Skipping Deleted MPTs (Issuance deleted)

src/test/app/invariants/InvariantsMisc_test.cpp

@@ -251,7 +251,7 @@ class InvariantsMisc_test : public InvariantsBase
             STTx{ttACCOUNT_DELETE, [](STObject& tx) {}});
 
         doInvariantCheck(
-            Env{*this, FeatureBitset{featureSponsor}},
+            Env{*this, FeatureBitset{featureSponsor, featureTecInvariant}},
             {{"account deletion left behind a sponsorship field"}},
             [&](Account const& a1, Account const& a2, ApplyContext& ac) {
                 auto const sleA1 = ac.view().peek(keylet::account(a1.id()));

src/test/app/invariants/InvariantsPermissioned_test.cpp

@@ -56,7 +56,9 @@ class InvariantsPermissioned_test : public InvariantsBase
     {
         using namespace test::jtx;
 
-        bool const fixEnabled = features[fixCleanup3_1_3];
+        bool const fix313Enabled = features[fixCleanup3_1_3];
+        bool const fixTecEnabled = features[featureTecInvariant];
+        bool const fixEnabled = fix313Enabled || fixTecEnabled;
         std::initializer_list<TER> const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED};
         std::initializer_list<TER> const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED};
 
@@ -261,7 +263,7 @@ class InvariantsPermissioned_test : public InvariantsBase
             testcase << "PermissionedDomain set 2 domains ";
             doInvariantCheck(
                 makeEnv(features),
-                fixEnabled ? badMoreThan1 : emptyV,
+                fix313Enabled ? badMoreThan1 : emptyV,
                 [](Account const& a1, Account const& a2, ApplyContext& ac) {
                     createPermissionedDomain(ac, a1, a2);
                     createPermissionedDomain(ac, a1, a2, 2, 11);
@@ -269,7 +271,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                 },
                 XRPAmount{},
                 STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}},
-                fixEnabled ? failTers : goodTers);
+                fix313Enabled ? failTers : goodTers);
         }
 
         {
@@ -290,7 +292,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                 std::move(env1),
                 a1,
                 a2,
-                fixEnabled ? badMoreThan1 : emptyV,
+                fix313Enabled ? badMoreThan1 : emptyV,
                 [&pd1, &pd2](Account const&, Account const&, ApplyContext& ac) {
                     auto const sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1});
                     auto const sle2 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd2});
@@ -300,18 +302,18 @@ class InvariantsPermissioned_test : public InvariantsBase
                 },
                 XRPAmount{},
                 STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}},
-                fixEnabled ? failTers : goodTers);
+                fix313Enabled ? failTers : goodTers);
         }
 
         {
             testcase << "PermissionedDomain set 0 domains ";
             doInvariantCheck(
                 makeEnv(features),
-                fixEnabled ? badNoDomains : emptyV,
+                fix313Enabled ? badNoDomains : emptyV,
                 [](Account const&, Account const&, ApplyContext&) { return true; },
                 XRPAmount{},
                 STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}},
-                fixEnabled ? badTers : goodTers);
+                fix313Enabled ? badTers : goodTers);
         }
 
         {
@@ -332,11 +334,11 @@ class InvariantsPermissioned_test : public InvariantsBase
                 std::move(env1),
                 a1,
                 a2,
-                fixEnabled ? badNoDomains : emptyV,
+                fix313Enabled ? badNoDomains : emptyV,
                 [](Account const&, Account const&, ApplyContext&) { return true; },
                 XRPAmount{},
                 STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}},
-                fixEnabled ? badTers : goodTers);
+                fix313Enabled ? badTers : goodTers);
         }
 
         {
@@ -356,7 +358,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                 std::move(env1),
                 a1,
                 a2,
-                fixEnabled ? badDeleted : emptyV,
+                fix313Enabled ? badDeleted : emptyV,
                 [&pd1](Account const&, Account const&, ApplyContext& ac) {
                     auto const sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1});
                     ac.view().erase(sle1);
@@ -364,28 +366,28 @@ class InvariantsPermissioned_test : public InvariantsBase
                 },
                 XRPAmount{},
                 STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}},
-                fixEnabled ? failTers : goodTers);
+                fix313Enabled ? failTers : goodTers);
         }
 
         {
             testcase << "PermissionedDomain del, create domain ";
             doInvariantCheck(
                 makeEnv(features),
-                fixEnabled ? badNotDeleted : emptyV,
+                fix313Enabled ? badNotDeleted : emptyV,
                 [](Account const& a1, Account const& a2, ApplyContext& ac) {
                     createPermissionedDomain(ac, a1, a2);
                     return true;
                 },
                 XRPAmount{},
                 STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}},
-                fixEnabled ? failTers : goodTers);
+                fix313Enabled ? failTers : goodTers);
         }
 
         {
             testcase << "PermissionedDomain invalid tx";
 
             doInvariantCheck(
-                fixEnabled ? badTx : emptyV,
+                fix313Enabled ? badTx : emptyV,
                 [&](Account const& a1, Account const& a2, ApplyContext& ac) {
                     createPermissionedDomain(ac, a1, a2);
                     return true;
@@ -401,7 +403,13 @@ class InvariantsPermissioned_test : public InvariantsBase
     {
         using namespace test::jtx;
 
-        bool const fixEnabled = features[fixCleanup3_1_3];
+        bool const fix313Enabled = features[fixCleanup3_1_3];
+        bool const fixTecEnabled = features[featureTecInvariant];
+        bool const fixEnabled = fix313Enabled || fixTecEnabled;
+
+        std::initializer_list<TER> const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED};
+        std::initializer_list<TER> const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED};
+        std::initializer_list<TER> const goodTers = {tesSUCCESS, tesSUCCESS};
 
         testcase << "PermissionedDEX" + std::string(fixEnabled ? " fix" : "");
 
@@ -429,7 +437,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                     tx.setFieldAmount(sfTakerPays, a1["USD"](10));
                     tx.setFieldAmount(sfTakerGets, XRP(1));
                 }},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
+            fixTecEnabled ? failTers : badTers);
 
         // missing domain ID in offer object
         doInvariantCheck(
@@ -451,7 +459,7 @@ class InvariantsPermissioned_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttOFFER_CREATE, [&](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
+            fixTecEnabled ? failTers : badTers);
 
         // more than one entry in sfAdditionalBooks
         {
@@ -488,7 +496,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                 },
                 XRPAmount{},
                 STTx{ttOFFER_CREATE, [&](STObject&) {}},
-                {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
+                fixTecEnabled ? failTers : badTers);
         }
 
         // empty sfAdditionalBooks (size 0)
@@ -503,12 +511,20 @@ class InvariantsPermissioned_test : public InvariantsBase
             [[maybe_unused]] auto [seq1, pd1] = createPermissionedDomainEnv(env1, a1, a2);
             env1.close();
 
+            auto const expectedTers = std::invoke([&] {
+                if (!fix313Enabled)
+                    return goodTers;
+                if (fixTecEnabled)
+                    return failTers;
+                return badTers;
+            });
+
             doInvariantCheck(
                 std::move(env1),
                 a1,
                 a2,
-                fixEnabled ? std::vector<std::string>{{"hybrid offer is malformed"}}
-                           : std::vector<std::string>{},
+                fix313Enabled ? std::vector<std::string>{{"hybrid offer is malformed"}}
+                              : std::vector<std::string>{},
                 [&pd1](Account const& a1, Account const& a2, ApplyContext& ac) {
                     Keylet const offerKey = keylet::offer(a2.id(), SeqProxy::rawSequence(10));
                     auto const sleOffer = std::make_shared<SLE>(offerKey);
@@ -525,8 +541,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                 },
                 XRPAmount{},
                 STTx{ttOFFER_CREATE, [&](STObject&) {}},
-                fixEnabled ? std::initializer_list<TER>{tecINVARIANT_FAILED, tecINVARIANT_FAILED}
-                           : std::initializer_list<TER>{tesSUCCESS, tesSUCCESS});
+                expectedTers);
         }
 
         // hybrid offer missing sfAdditionalBooks
@@ -559,7 +574,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                 },
                 XRPAmount{},
                 STTx{ttOFFER_CREATE, [&](STObject&) {}},
-                {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
+                fixTecEnabled ? failTers : badTers);
         }
 
         {
@@ -597,7 +612,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                         tx.setFieldAmount(sfTakerPays, a1["USD"](10));
                         tx.setFieldAmount(sfTakerGets, XRP(1));
                     }},
-                {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
+                fixTecEnabled ? failTers : badTers);
         }
 
         {
@@ -634,7 +649,7 @@ class InvariantsPermissioned_test : public InvariantsBase
                         tx.setFieldAmount(sfTakerPays, a1["USD"](10));
                         tx.setFieldAmount(sfTakerGets, XRP(1));
                     }},
-                {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
+                fixTecEnabled ? failTers : badTers);
         }
     }
 
@@ -946,8 +961,13 @@ class InvariantsPermissioned_test : public InvariantsBase
     void
     run() override
     {
-        testPermissionedDomainInvariants(all_);
-        testPermissionedDomainInvariants(all_ - fixCleanup3_1_3);
+        testPermissionedDomainInvariants(all_ | fixCleanup3_1_3 | featureTecInvariant);
+        testPermissionedDomainInvariants((all_ - fixCleanup3_1_3) | featureTecInvariant);
+        // Can't test without featureTecInvariant, because the invariant will assert
+        // testPermissionedDomainInvariants((all_ | fixCleanup3_1_3) -
+        //  featureTecInvariant );
+        // testPermissionedDomainInvariants((all_ - fixCleanup3_1_3) -
+        //  featureTecInvariant);
         testPermissionedDEX(all_);
         testPermissionedDEX(all_ - fixCleanup3_1_3);
         testPermissionedDEXDeletedOfferFallback();

src/test/app/invariants/InvariantsVault_test.cpp

@@ -279,6 +279,12 @@ class InvariantsVault_test : public InvariantsBase
             return std::pair{vaultKeylet, brokerKeylet};
         };
 
+        // Most of these tests use default Env, so fixTecInvaraint will always be enabled.
+        // If it's not, the invariant will usually assert.
+        bool const fixTecEnabled = true;
+        std::initializer_list<TER> const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED};
+        std::initializer_list<TER> const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED};
+
         testcase << "Vault general checks";
         doInvariantCheck(
             {"vault deletion succeeded without deleting a vault"},
@@ -292,7 +298,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_DELETE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -335,7 +341,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttPAYMENT, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -358,7 +364,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttPAYMENT, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
+            fixTecEnabled ? failTers : badTers);
 
         doInvariantCheck(
             {
@@ -442,7 +448,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_CREATE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED});
+            fixTecEnabled ? failTers : badTers);
 
         doInvariantCheck(
             {
@@ -516,7 +522,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [](STObject& tx) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp,
             TxAccount::A2);
 
@@ -628,7 +634,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_WITHDRAW, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -765,7 +771,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [](STObject& tx) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp);
 
         doInvariantCheck(
@@ -782,7 +788,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [](STObject& tx) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp);
 
         doInvariantCheck(
@@ -799,7 +805,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [](STObject& tx) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp);
 
         doInvariantCheck(
@@ -816,7 +822,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [](STObject& tx) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp,
             TxAccount::A2);
 
@@ -853,7 +859,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttLOAN_MANAGE, [](STObject& tx) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp,
             TxAccount::A2);
 
@@ -887,7 +893,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [](STObject& tx) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp,
             TxAccount::A2);
 
@@ -915,7 +921,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [&](STObject& tx) { tx[sfAssetsMaximum] = vaultCap; }},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) -> bool {
                 env.fund(XRP(1000), a3, a4);
                 Vault const vault{env};
@@ -939,7 +945,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_SET, [](STObject& tx) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp,
             TxAccount::A2);
 
@@ -1075,7 +1081,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttLOAN_MANAGE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp);
 
         // Loan flags may only change under the transaction types that own
@@ -1265,7 +1271,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttLOAN_PAY, [](STObject& tx) { tx.setFieldAmount(sfAmount, XRPAmount(200)); }},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp);
 
         // ttLOAN_PAY success post-conditions. A loan left with payments still
@@ -1880,7 +1886,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_CREATE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -1906,7 +1912,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_CREATE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -1933,7 +1939,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_CREATE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -2040,7 +2046,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_CREATE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -2071,7 +2077,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_CREATE, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const& a2, Env& env) {
                 Vault const vault{env};
                 auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -2251,7 +2257,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_DEPOSIT, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp);
 
         doInvariantCheck(
@@ -2458,7 +2464,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_WITHDRAW, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp);
 
         // Almost identical to the really convoluted test for deposit, where the
@@ -2727,7 +2733,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_CLAWBACK, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseXrp);
 
         // Not the same as above check: attempt to clawback MPT by bad account
@@ -2740,7 +2746,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttVAULT_CLAWBACK, [&](STObject& tx) { tx[sfAccount] = a4.id(); }},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseMpt);
 
         doInvariantCheck(
@@ -3024,7 +3030,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttLOAN_SET, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             precloseClosedEnded(/*advanceBySub=*/-1, /*doDeposit=*/false));
 
         testcase << "Vault loan set - closed-ended final payment past "
@@ -3074,7 +3080,7 @@ class InvariantsVault_test : public InvariantsBase
             },
             XRPAmount{},
             STTx{ttLOAN_SET, [](STObject&) {}},
-            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            fixTecEnabled ? failTers : badTers,
             [&](Account const& a1, Account const&, Env& env) -> bool {
                 auto const sub = env.now().time_since_epoch().count() + 60;
                 auto const red = sub + kMinInvestmentPeriod + 1'000'000;
@@ -3227,6 +3233,10 @@ class InvariantsVault_test : public InvariantsBase
             if (!withFix)
                 amendments = amendments - fixCleanup3_4_0;
 
+            bool const fixTecEnabled = amendments[featureTecInvariant];
+            std::initializer_list<TER> const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED};
+            std::initializer_list<TER> const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED};
+
             // Variant 1: L = (T - A) * 2. Fires under both settings.
             {
                 Keylet vaultKeylet = keylet::vault(UInt256{});
@@ -3258,7 +3268,7 @@ class InvariantsVault_test : public InvariantsBase
                         [&vaultKeylet](STObject& tx) {
                             tx.setFieldH256(sfVaultID, vaultKeylet.key);
                         }},
-                    {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+                    fixTecEnabled ? failTers : badTers,
                     preclose,
                     TxAccount::A1);
             }
@@ -3299,7 +3309,7 @@ class InvariantsVault_test : public InvariantsBase
                         [&vaultKeylet](STObject& tx) {
                             tx.setFieldH256(sfVaultID, vaultKeylet.key);
                         }},
-                    {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+                    fixTecEnabled ? failTers : badTers,
                     preclose,
                     TxAccount::A1);
             }