From b2b240e9d5a283b52f52048baccadc39f8d53e98 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 24 Jun 2026 11:12:17 +0200 Subject: [PATCH] refactor: extract amendmentCombinations; document vault-share freeze checks - Move amendmentCombinations algorithm to jtx::amendmentCombinations in test/jtx/utility.h+cpp; it was copy-pasted into both AMM_test and AMMMPT_test. AMM_test retains a one-liner wrapper that seeds from its local testableAmendments() (strips SAV/Lending). - Document the isVaultPseudoAccountFrozen sub-step in the Doxygen for checkWithdrawFreeze and checkDepositFreeze; the MPT-only transitive freeze check occurs between the listed steps 1 and 2 in each function. - Expand the IgnoreFreeze comment in VaultWithdraw::doApply to explain why preclaim validation makes the flag safe to use post-fix330. --- include/xrpl/ledger/helpers/TokenHelpers.h | 9 ++++++--- .../tx/transactors/vault/VaultWithdraw.cpp | 5 ++++- src/test/app/AMM_test.cpp | 13 +++---------- src/test/jtx/impl/utility.cpp | 14 ++++++++++++++ src/test/jtx/utility.h | 16 ++++++++++++++++ 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/include/xrpl/ledger/helpers/TokenHelpers.h b/include/xrpl/ledger/helpers/TokenHelpers.h index 661867c336..16d47b06fc 100644 --- a/include/xrpl/ledger/helpers/TokenHelpers.h +++ b/include/xrpl/ledger/helpers/TokenHelpers.h @@ -72,9 +72,6 @@ isIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& [[nodiscard]] TER checkIndividualFrozen(ReadView const& view, AccountID const& account, Asset const& asset); -[[nodiscard]] TER -checkIndividualDeepFrozen(ReadView const& view, AccountID const& account, Asset const& asset); - /** * isFrozen check is recursive for MPT shares in a vault, descending to * assets in the vault, up to maxAssetCheckDepth recursion depth. This is @@ -149,6 +146,9 @@ checkDeepFrozen(ReadView const& view, AccountID const& account, Asset const& ass * Otherwise checks, in order: * 1. checkFrozen(sourceAcct, asset) — the pseudo-account's trustline / * global freeze must not block sending. + * 1a. For MPT assets: isVaultPseudoAccountFrozen(sourceAcct, asset) — + * the pseudo-account's vault share must not be transitively frozen + * via its underlying asset. * 2. checkFrozen(submitterAcct, asset) — skipped when submitter == dst * (self-withdrawal); a regular freeze should not prevent recovering * one's own funds. @@ -176,6 +176,9 @@ checkWithdrawFreeze( * Checks, in order: * 1. checkFrozen(srcAcct, asset) — the depositor must not be frozen * (global or individual) for the asset. + * 1a. For MPT assets: isVaultPseudoAccountFrozen(dstAcct, asset) — + * the pseudo-account's vault share must not be transitively frozen + * via its underlying asset. * 2. checkFrozen(dstAcct, asset) — the pseudo-account must not be * frozen for the asset. Unlike regular accounts, pseudo-accounts * cannot receive deposits under a regular freeze because the diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 438706ffea..528ffa5090 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -272,7 +272,10 @@ VaultWithdraw::doApply() return tecPATH_DRY; } - // Post-fixCleanup3_3_0: preclaim already handles freeze checks + // Post-fixCleanup3_3_0: preclaim already validated all freeze conditions + // (checkWithdrawFreeze), so IgnoreFreeze avoids a redundant check that + // would incorrectly return zero for vault pseudo-accounts whose shares + // are frozen via a transitively frozen underlying asset. auto const freezeHandling = view().rules().enabled(fixCleanup3_3_0) ? FreezeHandling::IgnoreFreeze : FreezeHandling::ZeroIfFrozen; diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 0863e7cb83..987ef2df74 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -86,19 +87,11 @@ private: return jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol; } - // All 2^N permutations of testableAmendments() with each subset of the - // given features excluded. + // Seed from the local testableAmendments() which strips SAV and Lending. static std::vector amendmentCombinations(std::initializer_list features) { - std::vector result{testableAmendments()}; - for (auto const& f : features) - { - auto const n = result.size(); - for (std::size_t i = 0; i < n; ++i) - result.push_back(result[i] - f); - } - return result; + return jtx::amendmentCombinations(features, testableAmendments()); } void diff --git a/src/test/jtx/impl/utility.cpp b/src/test/jtx/impl/utility.cpp index 9256242417..b21e2b72cb 100644 --- a/src/test/jtx/impl/utility.cpp +++ b/src/test/jtx/impl/utility.cpp @@ -1,6 +1,7 @@ #include #include +#include #include @@ -101,4 +102,17 @@ cmdToJSONRPC(std::vector const& args, beast::Journal j, unsigned in return jv; } +std::vector +amendmentCombinations(std::initializer_list features, FeatureBitset seed) +{ + std::vector result{seed}; + for (auto const& f : features) + { + auto const n = result.size(); + for (std::size_t i = 0; i < n; ++i) + result.push_back(result[i] - f); + } + return result; +} + } // namespace xrpl::test::jtx diff --git a/src/test/jtx/utility.h b/src/test/jtx/utility.h index 535e14cf1d..8dbc34de63 100644 --- a/src/test/jtx/utility.h +++ b/src/test/jtx/utility.h @@ -1,12 +1,16 @@ #pragma once #include +#include #include #include +#include #include +#include #include +#include namespace xrpl::test::jtx { @@ -51,4 +55,16 @@ fillSeq(json::Value& jv, ReadView const& view); json::Value cmdToJSONRPC(std::vector const& args, beast::Journal j, unsigned int apiVersion); +/** + * Returns all 2^N permutations of a seed FeatureBitset with each subset of + * the given features excluded. The seed is included as the first element. + * + * Useful for running a test over every combination of optional amendments + * so that each case is exercised both with and without each feature. + */ +std::vector +amendmentCombinations( + std::initializer_list features, + FeatureBitset seed = testableAmendments()); + } // namespace xrpl::test::jtx