diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index de02fed7d8c..609fcb53c48 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/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_FIX (TecInvariant, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_4_0, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(Sponsor, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(BatchV1_1, Supported::Yes, VoteBehavior::DefaultNo) diff --git a/include/xrpl/tx/Transactor.h b/include/xrpl/tx/Transactor.h index a71285f70e0..00094210656 100644 --- a/include/xrpl/tx/Transactor.h +++ b/include/xrpl/tx/Transactor.h @@ -336,6 +336,10 @@ class Transactor uint256 const& ticketIndex, beast::Journal j); + // Interface used by processPersistentChanges and Invariants + static std::unordered_set + typesForResult(TER const ter); + protected: TER apply(); diff --git a/include/xrpl/tx/invariants/InvariantCheck.h b/include/xrpl/tx/invariants/InvariantCheck.h index 1239305e792..b9a892f332e 100644 --- a/include/xrpl/tx/invariants/InvariantCheck.h +++ b/include/xrpl/tx/invariants/InvariantCheck.h @@ -113,6 +113,53 @@ class InvariantChecker_PROTOTYPE }; #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. 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_; + std::vector deletedObjects_; + std::vector directorySideEffects_; + std::vector errors_; + +public: + void + visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref 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. @@ -432,6 +479,7 @@ class ObjectHasPseudoAccount // additional invariant checks can be declared above and then added to this // tuple using InvariantChecks = std::tuple< + FailedTransaction, TransactionFeeCheck, AccountRootsNotDeleted, AccountRootsDeletedClean, diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 5fc6942e20d..216673af347 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -1426,6 +1427,26 @@ Transactor::trapTransaction(uint256 txHash) const JLOG(j_.debug()) << "Transaction trapped: " << txHash; } +std::unordered_set +Transactor::typesForResult(TER const ter) +{ + std::unordered_set 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 Transactor::processPersistentChanges(TER result, XRPAmount fee) { @@ -1436,24 +1457,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 typesForResult = [](TER const ter) { - std::unordered_set 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. @@ -1637,7 +1640,7 @@ Transactor::operator()() if (auto stream = j_.trace()) stream << "preclaim result: " << transToken(result); - bool applied = isTesSuccess(result); + bool canApply = isTesSuccess(result); auto fee = ctx_.tx.getFieldAmount(sfFee).xrp(); if (ctx_.size() > kOversizeMetaDataCap) @@ -1648,74 +1651,76 @@ Transactor::operator()() // If the TapFailHard flag is set, a tec result // must not do anything ctx_.discard(); - applied = false; + canApply = false; } else if ( (result == tecOVERSIZE) || (result == tecKILLED) || (result == tecINCOMPLETE) || (result == tecEXPIRED) || (isTecClaimHardFail(result, view().flags()))) { - std::tie(result, fee, applied) = processPersistentChanges(result, fee); + // This is and must remain the only place where `canApply` can change from false to true. + // Changing from true to false is no problem. + std::tie(result, fee, canApply) = processPersistentChanges(result, fee); } - if (applied) + ScopeExit const logger{[&] { + JLOG(j_.trace()) << (canApply ? "applied " : "not applied ") << transToken(result); + }}; + + if (!canApply) + return {result, canApply}; + + // Check invariants: if `tecINVARIANT_FAILED` is not returned, we can + // proceed to apply the tx + result = checkInvariants(result, fee); + if (result == tecINVARIANT_FAILED) { - // Check invariants: if `tecINVARIANT_FAILED` is not returned, we can - // proceed to apply the tx - result = checkInvariants(result, fee); - if (result == tecINVARIANT_FAILED) - { - // Reset to fee-claim only - auto const resetResult = reset(fee); - if (!isTesSuccess(resetResult.first)) - result = resetResult.first; - - fee = resetResult.second; - - // Check invariants again to ensure the fee claiming doesn't violate - // invariants. After reset, only protocol invariants are re-checked. - // Transaction invariants are not meaningful here — the transaction's - // effects have been rolled back. - if (isTesSuccess(result) || isTecClaim(result)) - result = ctx_.checkInvariants(result, fee); - } + // Reset to fee-claim only + auto const resetResult = reset(fee); + if (!isTesSuccess(resetResult.first)) + result = resetResult.first; - // We ran through the invariant checker, which can, in some cases, - // return a tef error code. Don't apply the transaction in that case. - if (!isTecClaim(result) && !isTesSuccess(result)) - applied = false; + fee = resetResult.second; + + // Check invariants again to ensure the fee claiming doesn't violate + // invariants. After reset, only protocol invariants are re-checked. + // Transaction invariants are not meaningful here — the transaction's + // effects have been rolled back. + if (isTesSuccess(result) || isTecClaim(result)) + result = ctx_.checkInvariants(result, fee); } + // We ran through the invariant checker, which can, in some cases, + // return a tef error code. Don't apply the transaction in that case. + if (!isTecClaim(result) && !isTesSuccess(result)) + return {result, false}; + std::optional metadata; - if (applied) - { - // Transaction succeeded fully or (retries are not allowed and the - // transaction could claim a fee) - // The transactor and invariant checkers guarantee that this will - // *never* trigger but if it, somehow, happens, don't allow a tx - // that charges a negative fee. - if (fee < beast::kZero) - Throw("fee charged is negative!"); + // Transaction succeeded fully or (retries are not allowed and the + // transaction could claim a fee) - // Charge whatever fee they specified. The fee has already been - // deducted from the balance of the account that issued the - // transaction. We just need to account for it in the ledger - // header. - if (!view().open() && fee != beast::kZero) - ctx_.destroyXRP(fee); + // The transactor and invariant checkers guarantee that this will + // *never* trigger but if it, somehow, happens, don't allow a tx + // that charges a negative fee. + if (fee < beast::kZero) + Throw("fee charged is negative!"); - // Once we call apply, we will no longer be able to look at view() - metadata = ctx_.apply(result); - } + // Charge whatever fee they specified. The fee has already been + // deducted from the balance of the account that issued the + // transaction. We just need to account for it in the ledger + // header. + if (!view().open() && fee != beast::kZero) + ctx_.destroyXRP(fee); + + // Once we call apply, we will no longer be able to look at view() + metadata = ctx_.apply(result); if ((ctx_.flags() & TapDryRun) != 0u) { - applied = false; + return {result, false, metadata}; } - JLOG(j_.trace()) << (applied ? "applied " : "not applied ") << transToken(result); - - return {result, applied, metadata}; + return {result, canApply, metadata}; } } // namespace xrpl diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index 9b997e06dd9..c77bb71b240 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -81,6 +82,320 @@ ledgerEntryTypeName(SLE const& sle) return item->getName(); } +void +FailedTransaction::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref 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()) + { + case ltNICKNAME: + case ltCONTRACT: + case ltGENERATOR_MAP: + 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::const_ref before, + SLE::const_ref after, + SF_AMOUNT const& field, + std::string_view desc, + std::function 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 : account root has no known ledger format"); + return true; + // LCOV_EXCL_STOP + } + + for (auto const& elem : format->getSOTemplate()) + { + // skip this for now + + break; + + auto const& sf = elem.sField(); + + // No fields other than sfSequence, sfBalance, or sfFeeAmount may change + if (&sf == &sfSequence || &sf == &sfBalance || &sf == &sfFeeAmount) + 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 " << 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::const_ref sle) { return to_string(sle->at(sfAccount)); })) + { + 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::const_ref 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 + // fixTecInvariant 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; + } + + bool const enforce = view.rules().enabled(fixTecInvariant); + XRPL_ASSERT_IF(!result, enforce, "FailedTransaction::finalize : amendment enabled"); + return result || !enforce; +} + void TransactionFeeCheck::visitEntry(bool, SLE::const_ref, SLE::const_ref) { diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index ffdfe6bc834..f82174ea372 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -421,7 +421,7 @@ class Invariants_test : public beast::unit_test::Suite STTx{ttACCOUNT_DELETE, [](STObject& tx) {}}); doInvariantCheck( - Env{*this, FeatureBitset{featureSponsor}}, + Env{*this, FeatureBitset{featureSponsor, fixTecInvariant}}, {{"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())); @@ -1503,7 +1503,9 @@ class Invariants_test : public beast::unit_test::Suite { using namespace test::jtx; - bool const fixEnabled = features[fixCleanup3_1_3]; + bool const fix313Enabled = features[fixCleanup3_1_3]; + bool const fixTecEnabled = features[fixTecInvariant]; + bool const fixEnabled = fix313Enabled || fixTecEnabled; std::initializer_list const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED}; std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; @@ -1705,7 +1707,7 @@ class Invariants_test : public beast::unit_test::Suite 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); @@ -1713,7 +1715,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixEnabled ? failTers : goodTers); + fix313Enabled ? failTers : goodTers); } { @@ -1734,7 +1736,7 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixEnabled ? badMoreThan1 : emptyV, + fix313Enabled ? badMoreThan1 : emptyV, [&pd1, &pd2](Account const&, Account const&, ApplyContext& ac) { auto sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1}); auto sle2 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd2}); @@ -1744,18 +1746,18 @@ class Invariants_test : public beast::unit_test::Suite }, 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); } { @@ -1776,11 +1778,11 @@ class Invariants_test : public beast::unit_test::Suite makeEnv(features), 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); } { @@ -1800,7 +1802,7 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixEnabled ? badDeleted : emptyV, + fix313Enabled ? badDeleted : emptyV, [&pd1](Account const&, Account const&, ApplyContext& ac) { auto sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1}); ac.view().erase(sle1); @@ -1808,28 +1810,28 @@ class Invariants_test : public beast::unit_test::Suite }, 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; @@ -2006,7 +2008,13 @@ class Invariants_test : public beast::unit_test::Suite { using namespace test::jtx; - bool const fixEnabled = features[fixCleanup3_1_3]; + bool const fix313Enabled = features[fixCleanup3_1_3]; + bool const fixTecEnabled = features[fixTecInvariant]; + bool const fixEnabled = fix313Enabled || fixTecEnabled; + + std::initializer_list const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED}; + std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; + std::initializer_list const goodTers = {tesSUCCESS, tesSUCCESS}; testcase << "PermissionedDEX" + std::string(fixEnabled ? " fix" : ""); @@ -2034,7 +2042,7 @@ class Invariants_test : public beast::unit_test::Suite 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( @@ -2056,7 +2064,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttOFFER_CREATE, [&](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); // more than one entry in sfAdditionalBooks { @@ -2093,7 +2101,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttOFFER_CREATE, [&](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); } // empty sfAdditionalBooks (size 0) @@ -2112,8 +2120,8 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixEnabled ? std::vector{{"hybrid offer is malformed"}} - : std::vector{}, + fix313Enabled ? std::vector{{"hybrid offer is malformed"}} + : std::vector{}, [&pd1](Account const& a1, Account const& a2, ApplyContext& ac) { Keylet const offerKey = keylet::offer(a2.id(), SeqProxy::rawSequence(10)); auto sleOffer = std::make_shared(offerKey); @@ -2130,8 +2138,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttOFFER_CREATE, [&](STObject&) {}}, - fixEnabled ? std::initializer_list{tecINVARIANT_FAILED, tecINVARIANT_FAILED} - : std::initializer_list{tesSUCCESS, tesSUCCESS}); + fix313Enabled ? (fixTecEnabled ? failTers : badTers) : goodTers); } // hybrid offer missing sfAdditionalBooks @@ -2164,7 +2171,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttOFFER_CREATE, [&](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); } { @@ -2202,7 +2209,7 @@ class Invariants_test : public beast::unit_test::Suite tx.setFieldAmount(sfTakerPays, a1["USD"](10)); tx.setFieldAmount(sfTakerGets, XRP(1)); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); } { @@ -2239,7 +2246,7 @@ class Invariants_test : public beast::unit_test::Suite tx.setFieldAmount(sfTakerPays, a1["USD"](10)); tx.setFieldAmount(sfTakerGets, XRP(1)); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); } } @@ -2923,6 +2930,12 @@ class Invariants_test : public beast::unit_test::Suite 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 const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED}; + std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; + testcase << "Vault general checks"; doInvariantCheck( {"vault deletion succeeded without deleting a vault"}, @@ -2936,7 +2949,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -2977,7 +2990,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -3000,7 +3013,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPAYMENT, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); doInvariantCheck( {"vault deleted by a wrong transaction type", @@ -3080,7 +3093,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_CREATE, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); doInvariantCheck( {"deleted vault must also delete shares", @@ -3148,7 +3161,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3257,7 +3270,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -3300,7 +3313,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3317,7 +3330,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp); doInvariantCheck( @@ -3333,7 +3346,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp); doInvariantCheck( @@ -3349,7 +3362,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp); doInvariantCheck( @@ -3364,7 +3377,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3381,7 +3394,7 @@ class Invariants_test : public beast::unit_test::Suite XRPAmount{}, STTx{ ttVAULT_DEPOSIT, [](STObject& tx) { tx.setFieldAmount(sfAmount, XRPAmount(200)); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3399,7 +3412,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttLOAN_MANAGE, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3430,7 +3443,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3444,7 +3457,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3467,7 +3480,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3488,7 +3501,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3532,7 +3545,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -3558,7 +3571,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -3585,7 +3598,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -3613,7 +3626,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -3637,7 +3650,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -3665,7 +3678,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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()}); @@ -3729,7 +3742,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_SET, [](STObject&) {}}, - {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); doInvariantCheck( {"shares issuer and vault pseudo-account must be the same", @@ -3789,7 +3802,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_CREATE, [](STObject&) {}}, - {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + fixTecEnabled ? failTers : badTers); doInvariantCheck( {"shares issuer and vault pseudo-account must be the same", "shares issuer must exist"}, @@ -3843,7 +3856,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp); doInvariantCheck( @@ -3857,7 +3870,7 @@ class Invariants_test : public beast::unit_test::Suite XRPAmount{}, STTx{ ttVAULT_DEPOSIT, [](STObject& tx) { tx.setFieldAmount(sfAmount, XRPAmount(200)); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3888,7 +3901,7 @@ class Invariants_test : public beast::unit_test::Suite tx[sfFee] = XRPAmount(100); tx[sfAccount] = a3.id(); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp); doInvariantCheck( @@ -3914,7 +3927,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3936,7 +3949,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3950,7 +3963,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3965,7 +3978,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -3983,7 +3996,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject& tx) { tx[sfAmount] = XRPAmount(5); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4007,7 +4020,7 @@ class Invariants_test : public beast::unit_test::Suite tx[sfDelegate] = a3.id(); tx[sfFee] = XRPAmount(2000); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4023,7 +4036,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_DEPOSIT, [](STObject& tx) { tx[sfAmount] = XRPAmount(10); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4038,7 +4051,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp); // Almost identical to the really convoluted test for deposit, where the @@ -4070,7 +4083,7 @@ class Invariants_test : public beast::unit_test::Suite // This commented out line causes the invariant violation. // tx[sfDestination] = A4.id(); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp); doInvariantCheck( @@ -4098,7 +4111,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4119,7 +4132,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_WITHDRAW, [&](STObject& tx) { tx.setAccountID(sfDestination, a3.id()); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4133,7 +4146,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4147,7 +4160,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4164,7 +4177,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4180,7 +4193,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_WITHDRAW, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4204,7 +4217,7 @@ class Invariants_test : public beast::unit_test::Suite tx[sfDelegate] = a3.id(); tx[sfFee] = XRPAmount(2000); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseXrp, TxAccount::A2); @@ -4267,7 +4280,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_WITHDRAW, [&](STObject& tx) { tx[sfAccount] = a3.id(); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseMpt, TxAccount::A2); @@ -4283,7 +4296,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_CLAWBACK, [&](STObject& tx) { tx[sfAccount] = a3.id(); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseMpt); // Not the same as below check: attempt to clawback XRP @@ -4295,7 +4308,7 @@ class Invariants_test : public beast::unit_test::Suite }, 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 @@ -4308,7 +4321,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttVAULT_CLAWBACK, [&](STObject& tx) { tx[sfAccount] = a4.id(); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseMpt); doInvariantCheck( @@ -4329,7 +4342,7 @@ class Invariants_test : public beast::unit_test::Suite tx[sfAccount] = a3.id(); tx[sfHolder] = a4.id(); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseMpt); doInvariantCheck( @@ -4348,7 +4361,7 @@ class Invariants_test : public beast::unit_test::Suite tx[sfAccount] = a3.id(); tx[sfHolder] = a4.id(); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseMpt); doInvariantCheck( @@ -4371,7 +4384,7 @@ class Invariants_test : public beast::unit_test::Suite tx[sfAccount] = a3.id(); tx[sfHolder] = a4.id(); }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseMpt); } @@ -4479,6 +4492,12 @@ class Invariants_test : public beast::unit_test::Suite 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 const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED}; + std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; + // Overflow/Invalid balance on payment auto testPayment = [&](std::string const& log, auto&& update) { MPTID id; @@ -4489,7 +4508,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPAYMENT, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, [&](Account const& a1, Account const& a2, Env& env) { Account const gw("gw"); env.fund(XRP(1'000), gw); @@ -5429,16 +5448,13 @@ class Invariants_test : public beast::unit_test::Suite return true; }; - auto 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 { @@ -5460,16 +5476,15 @@ class Invariants_test : public beast::unit_test::Suite for (bool const isMPT : {false, true}) { - auto const error = isMPT ? TER(tecINVARIANT_FAILED) : TER(tefINVARIANT_FAILED); for (auto 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 txType : {ttAMM_BID, ttAMM_VOTE}) { - test(txType, updateAMMPool, isMPT, error); + test(txType, updateAMMPool, isMPT); test(txType, updateLPTokensBadAmount, isMPT); test(txType, updateLPTokensBadBalance, isMPT); } @@ -5999,6 +6014,12 @@ class Invariants_test : public beast::unit_test::Suite 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 const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED}; + std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; + // badDelete doInvariantCheck( {"MPToken deleted with encrypted fields while COA > 0"}, @@ -6029,7 +6050,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseConfidential); doInvariantCheck( @@ -6047,7 +6068,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseConfidential); // requiresPrivacyFlag @@ -6080,7 +6101,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseNoPrivacy); // badCOA @@ -6097,7 +6118,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttMPTOKEN_ISSUANCE_SET, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseConfidential); // Conservation Violation @@ -6117,7 +6138,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseConfidential); // Send/MergeInbox must not change OutstandingAmount (coaDelta == 0) @@ -6136,7 +6157,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttCONFIDENTIAL_MPT_SEND, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseConfidential); // Send/MergeInbox and zero-COA-delta confidential transactions must not @@ -6154,7 +6175,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttCONFIDENTIAL_MPT_SEND, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseConfidential); // badVersion @@ -6174,7 +6195,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttMPTOKEN_AUTHORIZE, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + fixTecEnabled ? failTers : badTers, precloseConfidential); // Skipping Deleted MPTs (Issuance deleted) @@ -6235,8 +6256,12 @@ class Invariants_test : public beast::unit_test::Suite testNFTokenPageInvariants(); testAMMDeleteInvariants(defaultAmendments()); testAMMDeleteInvariants(defaultAmendments() - fixCleanup3_3_0); - testPermissionedDomainInvariants(defaultAmendments() | fixCleanup3_1_3); - testPermissionedDomainInvariants(defaultAmendments() - fixCleanup3_1_3); + testPermissionedDomainInvariants(defaultAmendments() | fixCleanup3_1_3 | fixTecInvariant); + testPermissionedDomainInvariants(defaultAmendments() - fixCleanup3_1_3 | fixTecInvariant); + // Can't test without fixTecInvariant, because the invariant will assert + // testPermissionedDomainInvariants((defaultAmendments() | fixCleanup3_1_3) - + // fixTecInvariant ); testPermissionedDomainInvariants(defaultAmendments() - + // fixCleanup3_1_3- fixTecInvariant); testPermissionedDEX(defaultAmendments() | fixCleanup3_1_3); testPermissionedDEX(defaultAmendments() - fixCleanup3_1_3); testBookDirectoryExchangeRate();