From b14c6db8fb8657c5d541fabd7cad222c955505fe Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 10 Jun 2026 19:04:10 -0400 Subject: [PATCH] use asserts instead of checks --- include/xrpl/tx/invariants/InvariantCheck.h | 3 +- .../tx/invariants/DirectoryInvariant.cpp | 7 +- src/libxrpl/tx/invariants/InvariantCheck.cpp | 68 ++++++++++++++++--- src/libxrpl/tx/invariants/LoanInvariant.cpp | 9 ++- src/libxrpl/tx/invariants/NFTInvariant.cpp | 17 +++-- .../invariants/PermissionedDEXInvariant.cpp | 8 +++ .../PermissionedDomainInvariant.cpp | 9 +-- 7 files changed, 96 insertions(+), 25 deletions(-) diff --git a/include/xrpl/tx/invariants/InvariantCheck.h b/include/xrpl/tx/invariants/InvariantCheck.h index ce1246ab28..4ea1f051eb 100644 --- a/include/xrpl/tx/invariants/InvariantCheck.h +++ b/include/xrpl/tx/invariants/InvariantCheck.h @@ -130,8 +130,7 @@ class XRPNotCreated std::int64_t drops_ = 0; public: - static constexpr auto kRelevantLedgerEntryTypes = - VisitLedgerEntryTypes{}; + static constexpr auto kRelevantLedgerEntryTypes = VisitAllLedgerEntryTypes{}; void visitEntry(bool, SLE::const_ref, SLE::const_ref); diff --git a/src/libxrpl/tx/invariants/DirectoryInvariant.cpp b/src/libxrpl/tx/invariants/DirectoryInvariant.cpp index 1624a19830..d76b5dcbbc 100644 --- a/src/libxrpl/tx/invariants/DirectoryInvariant.cpp +++ b/src/libxrpl/tx/invariants/DirectoryInvariant.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -51,7 +52,11 @@ ValidBookDirectory::visitEntry( // Only validate newly-created directories and sfRootIndex changes; // LedgerStateFix handles legacy bad exchange-rate metadata. Skip deletions // because `after` is not guaranteed to be null. - if (badBookDirectory_ || isDelete || !after || after->getType() != ltDIR_NODE) + XRPL_ASSERT( + (!before || before->getType() == ltDIR_NODE) && (!after || after->getType() == ltDIR_NODE), + "xrpl::ValidBookDirectory::visitEntry : directory node input"); + + if (badBookDirectory_ || isDelete || !after) return; auto const rootIndex = after->getFieldH256(sfRootIndex); diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index b4a533905c..aca5619ef9 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -188,6 +188,11 @@ XRPNotCreated::finalize( void XRPBalanceChecks::visitEntry(bool, SLE::const_ref before, SLE::const_ref after) { + XRPL_ASSERT( + (!before || before->getType() == ltACCOUNT_ROOT) && + (!after || after->getType() == ltACCOUNT_ROOT), + "xrpl::XRPBalanceChecks::visitEntry : account root input"); + auto isBad = [](STAmount const& balance) { if (!balance.native()) return true; @@ -206,10 +211,10 @@ XRPBalanceChecks::visitEntry(bool, SLE::const_ref before, SLE::const_ref after) return false; }; - if (before && before->getType() == ltACCOUNT_ROOT) + if (before) bad_ |= isBad((*before)[sfBalance]); - if (after && after->getType() == ltACCOUNT_ROOT) + if (after) bad_ |= isBad((*after)[sfBalance]); } @@ -235,6 +240,10 @@ XRPBalanceChecks::finalize( void NoBadOffers::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { + XRPL_ASSERT( + (!before || before->getType() == ltOFFER) && (!after || after->getType() == ltOFFER), + "xrpl::NoBadOffers::visitEntry : offer input"); + auto isBad = [](STAmount const& pays, STAmount const& gets) { // An offer should never be negative if (pays < beast::kZero) @@ -247,10 +256,10 @@ NoBadOffers::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref aft return pays.native() && gets.native(); }; - if (before && before->getType() == ltOFFER) + if (before) bad_ |= isBad((*before)[sfTakerPays], (*before)[sfTakerGets]); - if (after && after->getType() == ltOFFER) + if (after) bad_ |= isBad((*after)[sfTakerPays], (*after)[sfTakerGets]); } @@ -276,6 +285,14 @@ NoBadOffers::finalize( void NoZeroEscrow::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { + auto const validType = [](SLE::const_ref sle) { + return !sle || sle->getType() == ltESCROW || sle->getType() == ltMPTOKEN_ISSUANCE || + sle->getType() == ltMPTOKEN; + }; + XRPL_ASSERT( + validType(before) && validType(after), + "xrpl::NoZeroEscrow::visitEntry : escrow or mpt input"); + auto isBad = [](STAmount const& amount) { // XRP case if (amount.native()) @@ -380,7 +397,11 @@ NoZeroEscrow::finalize( void AccountRootsNotDeleted::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref) { - if (isDelete && before && before->getType() == ltACCOUNT_ROOT) + XRPL_ASSERT( + !before || before->getType() == ltACCOUNT_ROOT, + "xrpl::AccountRootsNotDeleted::visitEntry : account root input"); + + if (isDelete && before) accountsDeleted_++; } @@ -430,7 +451,12 @@ AccountRootsNotDeleted::finalize( void AccountRootsDeletedClean::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { - if (isDelete && before && before->getType() == ltACCOUNT_ROOT) + XRPL_ASSERT( + (!before || before->getType() == ltACCOUNT_ROOT) && + (!after || after->getType() == ltACCOUNT_ROOT), + "xrpl::AccountRootsDeletedClean::visitEntry : account root input"); + + if (isDelete && before) accountsDeleted_.emplace_back(before, after); } @@ -601,9 +627,13 @@ LedgerEntryTypesMatch::finalize( void NoXRPTrustLines::visitEntry(bool, SLE::const_ref, SLE::const_ref after) { + XRPL_ASSERT( + !after || after->getType() == ltRIPPLE_STATE, + "xrpl::NoXRPTrustLines::visitEntry : ripple state input"); + bool const overwriteFixEnabled = isFeatureEnabled(fixCleanup3_1_3, true); - if (after && after->getType() == ltRIPPLE_STATE) + if (after) { // checking the issue directly here instead of // relying on .native() just in case native somehow @@ -641,7 +671,11 @@ NoXRPTrustLines::finalize( void NoDeepFreezeTrustLinesWithoutFreeze::visitEntry(bool, SLE::const_ref, SLE::const_ref after) { - if (after && after->getType() == ltRIPPLE_STATE) + XRPL_ASSERT( + !after || after->getType() == ltRIPPLE_STATE, + "xrpl::NoDeepFreezeTrustLinesWithoutFreeze::visitEntry : ripple state input"); + + if (after) { bool const overwriteFixEnabled = isFeatureEnabled(fixCleanup3_1_3, true); @@ -684,7 +718,12 @@ NoDeepFreezeTrustLinesWithoutFreeze::finalize( void ValidNewAccountRoot::visitEntry(bool, SLE::const_ref before, SLE::const_ref after) { - if (!before && after->getType() == ltACCOUNT_ROOT) + XRPL_ASSERT( + (!before || before->getType() == ltACCOUNT_ROOT) && + (!after || after->getType() == ltACCOUNT_ROOT), + "xrpl::ValidNewAccountRoot::visitEntry : account root input"); + + if (!before && after) { accountsCreated_++; accountSeq_ = (*after)[sfSequence]; @@ -758,6 +797,10 @@ ValidNewAccountRoot::finalize( void ValidClawback::visitEntry(bool, SLE::const_ref before, SLE::const_ref) { + XRPL_ASSERT( + !before || before->getType() == ltRIPPLE_STATE || before->getType() == ltMPTOKEN, + "xrpl::ValidClawback::visitEntry : ripple state or mptoken input"); + if (before && before->getType() == ltRIPPLE_STATE) trustlinesChanged_++; @@ -843,13 +886,18 @@ ValidClawback::finalize( void ValidPseudoAccounts::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { + XRPL_ASSERT( + (!before || before->getType() == ltACCOUNT_ROOT) && + (!after || after->getType() == ltACCOUNT_ROOT), + "xrpl::ValidPseudoAccounts::visitEntry : account root input"); + if (isDelete) { // Deletion is ignored return; } - if (after && after->getType() == ltACCOUNT_ROOT) + if (after) { bool const isPseudo = [&]() { // isPseudoAccount checks that any of the pseudo-account fields are diff --git a/src/libxrpl/tx/invariants/LoanInvariant.cpp b/src/libxrpl/tx/invariants/LoanInvariant.cpp index ce9a7c6e03..5c2e52b4d6 100644 --- a/src/libxrpl/tx/invariants/LoanInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanInvariant.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -17,10 +18,12 @@ namespace xrpl { void ValidLoan::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { - if (after && after->getType() == ltLOAN) - { + XRPL_ASSERT( + (!before || before->getType() == ltLOAN) && (!after || after->getType() == ltLOAN), + "xrpl::ValidLoan::visitEntry : loan input"); + + if (after) loans_.emplace_back(before, after); - } } bool diff --git a/src/libxrpl/tx/invariants/NFTInvariant.cpp b/src/libxrpl/tx/invariants/NFTInvariant.cpp index 52ecbcd9d1..0463b69353 100644 --- a/src/libxrpl/tx/invariants/NFTInvariant.cpp +++ b/src/libxrpl/tx/invariants/NFTInvariant.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -30,9 +31,10 @@ ValidNFTokenPage::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_re static constexpr uint256 const& kPageBits = nft::kPageMask; static constexpr uint256 kAccountBits = ~kPageBits; - if ((before && before->getType() != ltNFTOKEN_PAGE) || - (after && after->getType() != ltNFTOKEN_PAGE)) - return; + XRPL_ASSERT( + (!before || before->getType() == ltNFTOKEN_PAGE) && + (!after || after->getType() == ltNFTOKEN_PAGE), + "xrpl::ValidNFTokenPage::visitEntry : nftoken page input"); auto check = [this, isDelete](SLE::const_ref sle) { uint256 const account = sle->key() & kAccountBits; @@ -185,13 +187,18 @@ ValidNFTokenPage::finalize( void NFTokenCountTracking::visitEntry(bool, SLE::const_ref before, SLE::const_ref after) { - if (before && before->getType() == ltACCOUNT_ROOT) + XRPL_ASSERT( + (!before || before->getType() == ltACCOUNT_ROOT) && + (!after || after->getType() == ltACCOUNT_ROOT), + "xrpl::NFTokenCountTracking::visitEntry : account root input"); + + if (before) { beforeMintedTotal_ += (*before)[~sfMintedNFTokens].value_or(0); beforeBurnedTotal_ += (*before)[~sfBurnedNFTokens].value_or(0); } - if (after && after->getType() == ltACCOUNT_ROOT) + if (after) { afterMintedTotal_ += (*after)[~sfMintedNFTokens].value_or(0); afterBurnedTotal_ += (*after)[~sfBurnedNFTokens].value_or(0); diff --git a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp index 1014642b36..4c67128c3c 100644 --- a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp +++ b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -19,6 +20,13 @@ namespace xrpl { void ValidPermissionedDEX::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { + auto const validType = [](SLE::const_ref sle) { + return !sle || sle->getType() == ltDIR_NODE || sle->getType() == ltOFFER; + }; + XRPL_ASSERT( + validType(before) && validType(after), + "xrpl::ValidPermissionedDEX::visitEntry : directory node or offer input"); + if (after && after->getType() == ltDIR_NODE) { if (after->isFieldPresent(sfDomainID)) diff --git a/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp b/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp index 544a3af2dc..f3b65529d3 100644 --- a/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp +++ b/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -22,10 +23,10 @@ namespace xrpl { void ValidPermissionedDomain::visitEntry(bool isDel, SLE::const_ref before, SLE::const_ref after) { - if (before && before->getType() != ltPERMISSIONED_DOMAIN) - return; - if (after && after->getType() != ltPERMISSIONED_DOMAIN) - return; + XRPL_ASSERT( + (!before || before->getType() == ltPERMISSIONED_DOMAIN) && + (!after || after->getType() == ltPERMISSIONED_DOMAIN), + "xrpl::ValidPermissionedDomain::visitEntry : permissioned domain input"); auto check = [isDel](std::vector& sleStatus, SLE::const_ref sle) { auto const& credentials = sle->getFieldArray(sfAcceptedCredentials);