Merge branch 'develop' into ripple/wasmi

* Use conan repo for wasmi lib
* Generate lockfile

.config/cspell.config.yaml

@@ -51,7 +51,6 @@ words:
   - Btrfs
   - canonicality
   - checkme
-  - choco
   - chrono
   - citardauq
   - clawback
@@ -258,6 +257,7 @@ words:
   - venv
   - vfalco
   - vinnie
+  - wasmi
   - wextra
   - wptr
   - writeme

.github/workflows/pre-commit.yml

@@ -9,7 +9,7 @@ on:
 jobs:
   # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks.
   run-hooks:
-    uses: XRPLF/actions/.github/workflows/pre-commit.yml@5ca417783f0312ab26d6f48b85c78edf1de99bbd
+    uses: XRPLF/actions/.github/workflows/pre-commit.yml@34790936fae4c6c751f62ec8c06696f9c1a5753a
     with:
       runs_on: ubuntu-latest
       container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-a8c7be1" }'

.github/workflows/reusable-build-test-config.yml

@@ -100,9 +100,9 @@ jobs:
         uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@121d1de2775d486d46140b9a91b32d5002c08153
+        uses: XRPLF/actions/prepare-runner@2ece4ec6ab7de266859a6f053571425b2bd684b6
         with:
-          enable_ccache: ${{ inputs.ccache_enabled }}
+          disable_ccache: ${{ !inputs.ccache_enabled }}
 
       - name: Set ccache log file
         if: ${{ inputs.ccache_enabled && runner.debug == '1' }}

.github/workflows/upload-conan-deps.yml

@@ -70,9 +70,9 @@ jobs:
         uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@65da1c59e81965eeb257caa3587b9d45066fb925
+        uses: XRPLF/actions/prepare-runner@2ece4ec6ab7de266859a6f053571425b2bd684b6
         with:
-          enable_ccache: false
+          disable_ccache: true
 
       - name: Print build environment
         uses: ./.github/actions/print-env

CMakeLists.txt

@@ -116,6 +116,7 @@ find_package(date REQUIRED)
 find_package(ed25519 REQUIRED)
 find_package(nudb REQUIRED)
 find_package(secp256k1 REQUIRED)
+find_package(wasmi REQUIRED)
 find_package(xxHash REQUIRED)
 
 target_link_libraries(xrpl_libs INTERFACE

cmake/Ccache.cmake

@@ -46,12 +46,6 @@ set(CMAKE_VS_GLOBALS
         "TrackFileAccess=false"
         "UseMultiToolTask=true")
 
-# By default Visual Studio generators will use /Zi to capture debug information,
-# which is not compatible with ccache, so tell it to use /Z7 instead.
-if (MSVC)
-    foreach (var_
-        CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE
-        CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE)
-        string (REPLACE "/Zi" "/Z7" ${var_} "${${var_}}")
-    endforeach ()
-endif ()
+# By default Visual Studio generators will use /Zi, which is not compatible with
+# ccache, so tell it to use /Z7 instead.
+set(CMAKE_MSVC_DEBUG_INFORMATION_FORMAT "$<$<CONFIG:Debug,RelWithDebInfo>:Embedded>")

cmake/XrplCompiler.cmake

@@ -44,7 +44,6 @@ if (MSVC)
     # omit debug info completely under CI (not needed)
     if (is_ci)
       string (REPLACE "/Zi" " " ${var_} "${${var_}}")
-      string (REPLACE "/Z7" " " ${var_} "${${var_}}")
     endif ()
   endforeach ()
 

cmake/XrplCore.cmake

@@ -63,6 +63,7 @@ target_link_libraries(xrpl.imports.main
     Xrpl::opts
     Xrpl::syslibs
     secp256k1::secp256k1
+    wasmi::wasmi
     xrpl.libpb
     xxHash::xxhash
     $<$<BOOL:${voidstar}>:antithesis-sdk-cpp>

cmake/XrplSettings.cmake

@@ -68,21 +68,6 @@ if(is_linux)
   option(perf "Enables flags that assist with perf recording" OFF)
   option(use_gold "enables detection of gold (binutils) linker" ON)
   option(use_mold "enables detection of mold (binutils) linker" ON)
-  # Set a default value for the log flag based on the build type.
-  # This provides a sensible default (on for debug, off for release)
-  # while still allowing the user to override it for any build.
-  if(CMAKE_BUILD_TYPE STREQUAL "Debug")
-    set(TRUNCATED_LOGS_DEFAULT ON)
-  else()
-    set(TRUNCATED_LOGS_DEFAULT OFF)
-  endif()
-  option(TRUNCATED_THREAD_NAME_LOGS
-    "Show warnings about truncated thread names on Linux."
-    ${TRUNCATED_LOGS_DEFAULT}
-  )
-  if(TRUNCATED_THREAD_NAME_LOGS)
-    add_compile_definitions(TRUNCATED_THREAD_NAME_LOGS)
-  endif()
 else()
   # we are not ready to allow shared-libs on windows because it would require
   # export declarations. On macos it's more feasible, but static openssl

conan.lock

@@ -3,6 +3,7 @@
     "requires": [
         "zlib/1.3.1#b8bc2603263cf7eccbd6e17e66b0ed76%1756234269.497",
         "xxhash/0.8.3#681d36a0a6111fc56e5e45ea182c19cc%1756234289.683",
+        "wasmi/0.42.1#2a96357d4e6bf40dfe201106d849c24f%1764802092.014",
         "sqlite3/3.49.1#8631739a4c9b93bd3d6b753bac548a63%1756234266.869",
         "soci/4.0.3#a9f8d773cd33e356b5879a4b0564f287%1756234262.318",
         "snappy/1.1.10#968fef506ff261592ec30c574d4a7809%1756234314.246",

conanfile.py

@@ -35,6 +35,7 @@ class Xrpl(ConanFile):
         "openssl/3.5.4",
         "secp256k1/0.7.0",
         "soci/4.0.3",
+        "wasmi/0.42.1",
         "zlib/1.3.1",
     ]
 
@@ -210,6 +211,7 @@ class Xrpl(ConanFile):
             "soci::soci",
             "secp256k1::secp256k1",
             "sqlite3::sqlite",
+            "wasmi::wasmi",
             "xxhash::xxhash",
             "zlib::zlib",
         ]

include/xrpl/beast/core/CurrentThreadName.h

@@ -5,8 +5,6 @@
 #ifndef BEAST_CORE_CURRENT_THREAD_NAME_H_INCLUDED
 #define BEAST_CORE_CURRENT_THREAD_NAME_H_INCLUDED
 
-#include <boost/predef.h>
-
 #include <string>
 #include <string_view>
 
@@ -18,31 +16,6 @@ namespace beast {
 void
 setCurrentThreadName(std::string_view newThreadName);
 
-#if BOOST_OS_LINUX
-
-// On Linux, thread names are limited to 16 bytes including the null terminator.
-// Maximum number of characters is therefore 15.
-constexpr std::size_t maxThreadNameLength = 15;
-
-/** Sets the name of the caller thread with compile-time size checking.
-    @tparam N The size of the string literal including null terminator
-    @param newThreadName A string literal to set as the thread name
-
-    This template overload enforces that thread names are at most 16 characters
-    (including null terminator) at compile time, matching Linux's limit.
-*/
-template <std::size_t N>
-void
-setCurrentThreadName(char const (&newThreadName)[N])
-{
-    static_assert(
-        N <= maxThreadNameLength + 1,
-        "Thread name cannot exceed 15 characters");
-
-    setCurrentThreadName(std::string_view(newThreadName, N - 1));
-}
-#endif
-
 /** Returns the name of the caller thread.
 
     The name returned is the name as set by a call to setCurrentThreadName().

include/xrpl/protocol/detail/features.macro

@@ -16,7 +16,6 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
-XRPL_FIX    (BatchInnerSigs,             Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocol,            Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionDelegationV1_1,   Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (DirectoryLimit,             Supported::yes, VoteBehavior::DefaultNo)

src/libxrpl/beast/core/CurrentThreadName.cpp

@@ -1,5 +1,7 @@
 #include <xrpl/beast/core/CurrentThreadName.h>
 
+#include <boost/predef.h>
+
 #include <string>
 #include <string_view>
 
@@ -71,32 +73,12 @@ setCurrentThreadNameImpl(std::string_view name)
 #if BOOST_OS_LINUX
 #include <pthread.h>
 
-#include <iostream>
-
 namespace beast::detail {
 
 inline void
 setCurrentThreadNameImpl(std::string_view name)
 {
-    // truncate and set the thread name.
-    char boundedName[maxThreadNameLength + 1];
-    std::snprintf(
-        boundedName,
-        sizeof(boundedName),
-        "%.*s",
-        static_cast<int>(maxThreadNameLength),
-        name.data());
-
-    pthread_setname_np(pthread_self(), boundedName);
-
-#ifdef TRUNCATED_THREAD_NAME_LOGS
-    if (name.size() > maxThreadNameLength)
-    {
-        std::cerr << "WARNING: Thread name \"" << name << "\" (length "
-                  << name.size() << ") exceeds maximum of "
-                  << maxThreadNameLength << " characters on Linux.\n";
-    }
-#endif
+    pthread_setname_np(pthread_self(), name.data());
 }
 
 }  // namespace beast::detail

src/libxrpl/protocol/SecretKey.cpp

@@ -77,7 +77,7 @@ deriveDeterministicRootKey(Seed const& seed)
     std::array<std::uint8_t, 20> buf;
     std::copy(seed.begin(), seed.end(), buf.begin());
 
-    // The odds that this loop executes more than once are negligible
+    // The odds that this loop executes more than once are neglible
     // but *just* in case someone managed to generate a key that required
     // more iterations loop a few times.
     for (std::uint32_t seq = 0; seq != 128; ++seq)
@@ -137,7 +137,7 @@ private:
         std::copy(generator_.begin(), generator_.end(), buf.begin());
         copy_uint32(buf.data() + 33, seq);
 
-        // The odds that this loop executes more than once are negligible
+        // The odds that this loop executes more than once are neglible
         // but we impose a maximum limit just in case.
         for (std::uint32_t subseq = 0; subseq != 128; ++subseq)
         {

src/libxrpl/resource/ResourceManager.cpp

@@ -140,7 +140,7 @@ private:
     void
     run()
     {
-        beast::setCurrentThreadName("Resource::Mngr");
+        beast::setCurrentThreadName("Resource::Manager");
         for (;;)
         {
             logic_.periodicActivity();

src/test/app/Batch_test.cpp

@@ -148,21 +148,15 @@ class Batch_test : public beast::unit_test::suite
     void
     testEnable(FeatureBitset features)
     {
+        testcase("enabled");
+
         using namespace test::jtx;
         using namespace std::literals;
 
-        bool const withInnerSigFix = features[fixBatchInnerSigs];
-
         for (bool const withBatch : {true, false})
         {
-            testcase << "enabled: Batch "
-                     << (withBatch ? "enabled" : "disabled")
-                     << ", Inner Sig Fix: "
-                     << (withInnerSigFix ? "enabled" : "disabled");
-
             auto const amend = withBatch ? features : features - featureBatch;
-
-            test::jtx::Env env{*this, amend};
+            test::jtx::Env env{*this, envconfig(), amend};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -185,7 +179,7 @@ class Batch_test : public beast::unit_test::suite
 
             // tfInnerBatchTxn
             // If the feature is disabled, the transaction fails with
-            // temINVALID_FLAG. If the feature is enabled, the transaction fails
+            // temINVALID_FLAG If the feature is enabled, the transaction fails
             // early in checkValidity()
             {
                 auto const txResult =
@@ -211,7 +205,7 @@ class Batch_test : public beast::unit_test::suite
         //----------------------------------------------------------------------
         // preflight
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -623,7 +617,7 @@ class Batch_test : public beast::unit_test::suite
         //----------------------------------------------------------------------
         // preclaim
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -864,7 +858,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -955,7 +949,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -1193,7 +1187,7 @@ class Batch_test : public beast::unit_test::suite
 
         // Bad Fee Without Signer
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1215,7 +1209,7 @@ class Batch_test : public beast::unit_test::suite
 
         // Bad Fee With MultiSign
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1242,7 +1236,7 @@ class Batch_test : public beast::unit_test::suite
 
         // Bad Fee With MultiSign + BatchSigners
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1271,7 +1265,7 @@ class Batch_test : public beast::unit_test::suite
 
         // Bad Fee With MultiSign + BatchSigners.Signers
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1303,7 +1297,7 @@ class Batch_test : public beast::unit_test::suite
 
         // Bad Fee With BatchSigners
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1327,7 +1321,7 @@ class Batch_test : public beast::unit_test::suite
 
         // Bad Fee Dynamic Fee Calculation
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1367,7 +1361,7 @@ class Batch_test : public beast::unit_test::suite
 
         // telENV_RPC_FAILED: Batch: txns array exceeds 8 entries.
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1392,7 +1386,7 @@ class Batch_test : public beast::unit_test::suite
 
         // temARRAY_TOO_LARGE: Batch: txns array exceeds 8 entries.
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1425,7 +1419,7 @@ class Batch_test : public beast::unit_test::suite
 
         // telENV_RPC_FAILED: Batch: signers array exceeds 8 entries.
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1444,7 +1438,7 @@ class Batch_test : public beast::unit_test::suite
 
         // temARRAY_TOO_LARGE: Batch: signers array exceeds 8 entries.
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -1478,7 +1472,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -1614,7 +1608,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -1846,7 +1840,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -2068,7 +2062,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -2254,26 +2248,14 @@ class Batch_test : public beast::unit_test::suite
     }
 
     void
-    doTestInnerSubmitRPC(FeatureBitset features, bool withBatch)
+    testInnerSubmitRPC(FeatureBitset features)
     {
-        bool const withInnerSigFix = features[fixBatchInnerSigs];
-
-        std::string const testName = [&]() {
-            std::stringstream ss;
-            ss << "inner submit rpc: batch "
-               << (withBatch ? "enabled" : "disabled") << ", inner sig fix: "
-               << (withInnerSigFix ? "enabled" : "disabled") << ": ";
-            return ss.str();
-        }();
-
-        auto const amend = withBatch ? features : features - featureBatch;
+        testcase("inner submit rpc");
 
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, amend};
-        if (!BEAST_EXPECT(amend[featureBatch] == withBatch))
-            return;
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -2281,170 +2263,75 @@ class Batch_test : public beast::unit_test::suite
         env.fund(XRP(10000), alice, bob);
         env.close();
 
-        auto submitAndValidate =
-            [&](std::string caseName,
-                Slice const& slice,
-                int line,
-                std::optional<std::string> expectedEnabled = std::nullopt,
-                std::optional<std::string> expectedDisabled = std::nullopt,
-                bool expectInvalidFlag = false) {
-                testcase << testName << caseName
-                         << (expectInvalidFlag
-                                 ? " - Expected to reach tx engine!"
-                                 : "");
-                auto const jrr = env.rpc("submit", strHex(slice))[jss::result];
-                auto const expected = withBatch
-                    ? expectedEnabled.value_or(
-                          "fails local checks: Malformed: Invalid inner batch "
-                          "transaction.")
-                    : expectedDisabled.value_or(
-                          "fails local checks: Empty SigningPubKey.");
-                if (expectInvalidFlag)
-                {
-                    expect(
-                        jrr[jss::status] == "success" &&
-                            jrr[jss::engine_result] == "temINVALID_FLAG",
-                        pretty(jrr),
-                        __FILE__,
-                        line);
-                }
-                else
-                {
-                    expect(
-                        jrr[jss::status] == "error" &&
-                            jrr[jss::error] == "invalidTransaction" &&
-                            jrr[jss::error_exception] == expected,
-                        pretty(jrr),
-                        __FILE__,
-                        line);
-                }
-                env.close();
-            };
+        auto submitAndValidate = [&](Slice const& slice) {
+            auto const jrr = env.rpc("submit", strHex(slice))[jss::result];
+            BEAST_EXPECT(
+                jrr[jss::status] == "error" &&
+                jrr[jss::error] == "invalidTransaction" &&
+                jrr[jss::error_exception] ==
+                    "fails local checks: Malformed: Invalid inner batch "
+                    "transaction.");
+            env.close();
+        };
 
         // Invalid RPC Submission: TxnSignature
-        // + has `TxnSignature` field
+        // - has `TxnSignature` field
         // - has no `SigningPubKey` field
         // - has no `Signers` field
-        // + has `tfInnerBatchTxn` flag
+        // - has `tfInnerBatchTxn` flag
         {
             auto txn = batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
             txn[sfTxnSignature] = "DEADBEEF";
             STParsedJSONObject parsed("test", txn.getTxn());
             Serializer s;
             parsed.object->add(s);
-            submitAndValidate("TxnSignature set", s.slice(), __LINE__);
+            submitAndValidate(s.slice());
         }
 
         // Invalid RPC Submission: SigningPubKey
         // - has no `TxnSignature` field
-        // + has `SigningPubKey` field
+        // - has `SigningPubKey` field
         // - has no `Signers` field
-        // + has `tfInnerBatchTxn` flag
+        // - has `tfInnerBatchTxn` flag
         {
             auto txn = batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
             txn[sfSigningPubKey] = strHex(alice.pk());
             STParsedJSONObject parsed("test", txn.getTxn());
             Serializer s;
             parsed.object->add(s);
-            submitAndValidate(
-                "SigningPubKey set",
-                s.slice(),
-                __LINE__,
-                std::nullopt,
-                "fails local checks: Invalid signature.");
+            submitAndValidate(s.slice());
         }
 
         // Invalid RPC Submission: Signers
         // - has no `TxnSignature` field
-        // + has empty `SigningPubKey` field
-        // + has `Signers` field
-        // + has `tfInnerBatchTxn` flag
+        // - has empty `SigningPubKey` field
+        // - has `Signers` field
+        // - has `tfInnerBatchTxn` flag
         {
             auto txn = batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
             txn[sfSigners] = Json::arrayValue;
             STParsedJSONObject parsed("test", txn.getTxn());
             Serializer s;
             parsed.object->add(s);
-            submitAndValidate(
-                "Signers set",
-                s.slice(),
-                __LINE__,
-                std::nullopt,
-                "fails local checks: Invalid Signers array size.");
-        }
-
-        {
-            // Fully signed inner batch transaction
-            auto const txn =
-                batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
-            auto const jt = env.jt(txn.getTxn());
-
-            STParsedJSONObject parsed("test", jt.jv);
-            Serializer s;
-            parsed.object->add(s);
-            submitAndValidate(
-                "Fully signed",
-                s.slice(),
-                __LINE__,
-                std::nullopt,
-                std::nullopt,
-                !withBatch);
+            submitAndValidate(s.slice());
         }
 
         // Invalid RPC Submission: tfInnerBatchTxn
         // - has no `TxnSignature` field
-        // + has empty `SigningPubKey` field
+        // - has empty `SigningPubKey` field
         // - has no `Signers` field
-        // + has `tfInnerBatchTxn` flag
+        // - has `tfInnerBatchTxn` flag
         {
             auto txn = batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
             STParsedJSONObject parsed("test", txn.getTxn());
             Serializer s;
             parsed.object->add(s);
-            submitAndValidate(
-                "No signing fields set",
-                s.slice(),
-                __LINE__,
-                "fails local checks: Empty SigningPubKey.",
-                "fails local checks: Empty SigningPubKey.",
-                withBatch && !withInnerSigFix);
-        }
+            auto const jrr = env.rpc("submit", strHex(s.slice()))[jss::result];
+            BEAST_EXPECT(
+                jrr[jss::status] == "success" &&
+                jrr[jss::engine_result] == "temINVALID_FLAG");
 
-        // Invalid RPC Submission: tfInnerBatchTxn pseudo-transaction
-        // - has no `TxnSignature` field
-        // + has empty `SigningPubKey` field
-        // - has no `Signers` field
-        // + has `tfInnerBatchTxn` flag
-        {
-            STTx amendTx(
-                ttAMENDMENT, [seq = env.closed()->header().seq + 1](auto& obj) {
-                    obj.setAccountID(sfAccount, AccountID());
-                    obj.setFieldH256(sfAmendment, fixBatchInnerSigs);
-                    obj.setFieldU32(sfLedgerSequence, seq);
-                    obj.setFieldU32(sfFlags, tfInnerBatchTxn);
-                });
-            auto txn = batch::inner(
-                amendTx.getJson(JsonOptions::none), env.seq(alice));
-            STParsedJSONObject parsed("test", txn.getTxn());
-            Serializer s;
-            parsed.object->add(s);
-            submitAndValidate(
-                "Pseudo-transaction",
-                s.slice(),
-                __LINE__,
-                withInnerSigFix
-                    ? "fails local checks: Empty SigningPubKey."
-                    : "fails local checks: Cannot submit pseudo transactions.",
-                "fails local checks: Empty SigningPubKey.");
-        }
-    }
-
-    void
-    testInnerSubmitRPC(FeatureBitset features)
-    {
-        for (bool const withBatch : {true, false})
-        {
-            doTestInnerSubmitRPC(features, withBatch);
+            env.close();
         }
     }
 
@@ -2456,7 +2343,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -2503,7 +2390,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -2556,7 +2443,7 @@ class Batch_test : public beast::unit_test::suite
 
         // tfIndependent: account delete success
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -2597,7 +2484,7 @@ class Batch_test : public beast::unit_test::suite
 
         // tfIndependent: account delete fails
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -2642,7 +2529,7 @@ class Batch_test : public beast::unit_test::suite
 
         // tfAllOrNothing: account delete fails
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -2694,6 +2581,7 @@ class Batch_test : public beast::unit_test::suite
 
         test::jtx::Env env{
             *this,
+            envconfig(),
             features | featureSingleAssetVault | featureLendingProtocol |
                 featureMPTokensV1};
 
@@ -2888,7 +2776,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -3001,7 +2889,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -3059,7 +2947,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -3121,7 +3009,7 @@ class Batch_test : public beast::unit_test::suite
             using namespace test::jtx;
             using namespace std::literals;
 
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -3170,7 +3058,7 @@ class Batch_test : public beast::unit_test::suite
             using namespace test::jtx;
             using namespace std::literals;
 
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -3218,7 +3106,7 @@ class Batch_test : public beast::unit_test::suite
             using namespace test::jtx;
             using namespace std::literals;
 
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -3281,7 +3169,7 @@ class Batch_test : public beast::unit_test::suite
             // overwritten by the payment in the batch transaction. Because the
             // terPRE_SEQ is outside of the batch this noop transaction will ge
             // reapplied in the following ledger
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob, carol);
             env.close();
 
@@ -3328,7 +3216,7 @@ class Batch_test : public beast::unit_test::suite
             // IMPORTANT: The batch txn is applied first, then the noop txn.
             // Because of this ordering, the noop txn is not applied and is
             // overwritten by the payment in the batch transaction.
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob);
             env.close();
 
@@ -3370,7 +3258,7 @@ class Batch_test : public beast::unit_test::suite
             // IMPORTANT: The batch txn is applied first, then the noop txn.
             // Because of this ordering, the noop txn is not applied and is
             // overwritten by the payment in the batch transaction.
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob);
             env.close();
 
@@ -3407,7 +3295,7 @@ class Batch_test : public beast::unit_test::suite
 
         // Outer Batch terPRE_SEQ
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob, carol);
             env.close();
 
@@ -3465,7 +3353,7 @@ class Batch_test : public beast::unit_test::suite
             // IMPORTANT: The batch txn is applied first, then the noop txn.
             // Because of this ordering, the noop txn is not applied and is
             // overwritten by the payment in the batch transaction.
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob);
             env.close();
 
@@ -3514,7 +3402,7 @@ class Batch_test : public beast::unit_test::suite
             // IMPORTANT: The batch txn is applied first, then the noop txn.
             // Because of this ordering, the noop txn is not applied and is
             // overwritten by the payment in the batch transaction.
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob);
             env.close();
 
@@ -3576,7 +3464,7 @@ class Batch_test : public beast::unit_test::suite
             // batch will run in the close ledger process. The batch will be
             // allied and then retry this transaction in the current ledger.
 
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob);
             env.close();
 
@@ -3623,7 +3511,7 @@ class Batch_test : public beast::unit_test::suite
 
         // Create Object Before Batch Txn
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob);
             env.close();
 
@@ -3670,7 +3558,7 @@ class Batch_test : public beast::unit_test::suite
             // batch will run in the close ledger process. The batch will be
             // applied and then retry this transaction in the current ledger.
 
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             env.fund(XRP(10000), alice, bob);
             env.close();
 
@@ -3717,7 +3605,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
 
         auto const alice = Account("alice");
         auto const bob = Account("bob");
@@ -3756,7 +3644,7 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, features};
+        test::jtx::Env env{*this, envconfig()};
         XRPAmount const baseFee = env.current()->fees().base;
 
         auto const alice = Account("alice");
@@ -3837,7 +3725,6 @@ class Batch_test : public beast::unit_test::suite
                 *this,
                 makeSmallQueueConfig(
                     {{"minimum_txn_in_ledger_standalone", "2"}}),
-                features,
                 nullptr,
                 beast::severities::kError};
 
@@ -3898,7 +3785,6 @@ class Batch_test : public beast::unit_test::suite
                 *this,
                 makeSmallQueueConfig(
                     {{"minimum_txn_in_ledger_standalone", "2"}}),
-                features,
                 nullptr,
                 beast::severities::kError};
 
@@ -4006,7 +3892,7 @@ class Batch_test : public beast::unit_test::suite
 
         // delegated non atomic inner
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -4051,7 +3937,7 @@ class Batch_test : public beast::unit_test::suite
 
         // delegated atomic inner
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -4103,7 +3989,7 @@ class Batch_test : public beast::unit_test::suite
         // this also makes sure tfInnerBatchTxn won't block delegated AccountSet
         // with granular permission
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -4152,7 +4038,7 @@ class Batch_test : public beast::unit_test::suite
         // this also makes sure tfInnerBatchTxn won't block delegated
         // MPTokenIssuanceSet with granular permission
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             Account alice{"alice"};
             Account bob{"bob"};
             env.fund(XRP(100000), alice, bob);
@@ -4208,7 +4094,7 @@ class Batch_test : public beast::unit_test::suite
         // this also makes sure tfInnerBatchTxn won't block delegated TrustSet
         // with granular permission
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             Account gw{"gw"};
             Account alice{"alice"};
             Account bob{"bob"};
@@ -4248,7 +4134,7 @@ class Batch_test : public beast::unit_test::suite
 
         // inner transaction not authorized by the delegating account.
         {
-            test::jtx::Env env{*this, features};
+            test::jtx::Env env{*this, envconfig()};
             Account gw{"gw"};
             Account alice{"alice"};
             Account bob{"bob"};
@@ -4296,7 +4182,7 @@ class Batch_test : public beast::unit_test::suite
         testcase("Validate RPC response");
 
         using namespace jtx;
-        Env env(*this, features);
+        Env env(*this);
         Account const alice("alice");
         Account const bob("bob");
         env.fund(XRP(10000), alice, bob);
@@ -4373,7 +4259,7 @@ class Batch_test : public beast::unit_test::suite
     testBatchCalculateBaseFee(FeatureBitset features)
     {
         using namespace jtx;
-        Env env(*this, features);
+        Env env(*this);
         Account const alice("alice");
         Account const bob("bob");
         Account const carol("carol");
@@ -4498,7 +4384,6 @@ public:
     {
         using namespace test::jtx;
         auto const sa = testable_amendments();
-        testWithFeats(sa - fixBatchInnerSigs);
         testWithFeats(sa);
     }
 };

src/test/app/HashRouter_test.cpp

@@ -349,7 +349,7 @@ class HashRouter_test : public beast::unit_test::suite
             h.set("hold_time", "alice");
             h.set("relay_time", "bob");
             auto const setup = setup_HashRouter(cfg);
-            // The set function ignores values that don't convert, so the
+            // The set function ignores values that don't covert, so the
             // defaults are left unchanged
             BEAST_EXPECT(setup.holdTime == 300s);
             BEAST_EXPECT(setup.relayTime == 30s);

src/test/app/MPToken_test.cpp

@@ -2651,7 +2651,7 @@ class MPToken_test : public beast::unit_test::suite
         STAmount const amt3{asset3, 10'000};
 
         {
-            testcase("Test STAmount MPT arithmetic");
+            testcase("Test STAmount MPT arithmetics");
             using namespace std::string_literals;
             STAmount res = multiply(amt1, amt2, asset3);
             BEAST_EXPECT(res == amt3);
@@ -2688,7 +2688,7 @@ class MPToken_test : public beast::unit_test::suite
         }
 
         {
-            testcase("Test MPTAmount arithmetic");
+            testcase("Test MPTAmount arithmetics");
             MPTAmount mptAmt1{100};
             MPTAmount const mptAmt2{100};
             BEAST_EXPECT((mptAmt1 += mptAmt2) == MPTAmount{200});

src/test/app/MultiSign_test.cpp

@@ -708,7 +708,7 @@ public:
     void
     testHeterogeneousSigners(FeatureBitset features)
     {
-        testcase("Heterogeneous Signers");
+        testcase("Heterogenous Signers");
 
         using namespace jtx;
         Env env{*this, features};

src/test/app/Vault_test.cpp

@@ -21,6 +21,7 @@
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/STNumber.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
 #include <xrpl/protocol/XRPAmount.h>
@@ -939,6 +940,25 @@ class Vault_test : public beast::unit_test::suite
             }
         });
 
+        testCase([&](Env& env,
+                     Account const& issuer,
+                     Account const& owner,
+                     Asset const& asset,
+                     Vault& vault) {
+            testcase("clawback from self");
+
+            auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
+
+            {
+                auto tx = vault.clawback(
+                    {.issuer = issuer,
+                     .id = keylet.key,
+                     .holder = issuer,
+                     .amount = asset(10)});
+                env(tx, ter{temMALFORMED});
+            }
+        });
+
         testCase([&](Env& env,
                      Account const&,
                      Account const& owner,
@@ -1177,13 +1197,11 @@ class Vault_test : public beast::unit_test::suite
 
             auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
 
-            // Preclaim only checks for native assets.
-            if (asset.native())
             {
                 auto tx = vault.clawback(
-                    {.issuer = issuer,
+                    {.issuer = owner,
                      .id = keylet.key,
-                     .holder = owner,
+                     .holder = issuer,
                      .amount = asset(50)});
                 env(tx, ter(temMALFORMED));
             }
@@ -1906,20 +1924,8 @@ class Vault_test : public beast::unit_test::suite
             env.close();
 
             {
-                auto tx = vault.clawback({
-                    .issuer = depositor,
-                    .id = keylet.key,
-                    .holder = depositor,
-                });
-                env(tx, ter(tecNO_PERMISSION));
-            }
-
-            {
-                auto tx = vault.clawback({
-                    .issuer = owner,
-                    .id = keylet.key,
-                    .holder = depositor,
-                });
+                auto tx = vault.clawback(
+                    {.issuer = owner, .id = keylet.key, .holder = depositor});
                 env(tx, ter(tecNO_PERMISSION));
             }
         });
@@ -2371,15 +2377,6 @@ class Vault_test : public beast::unit_test::suite
                 env(tx, ter(tecNO_AUTH));
             }
 
-            {
-                // Cannot clawback if issuer is the holder
-                tx = vault.clawback(
-                    {.issuer = issuer,
-                     .id = keylet.key,
-                     .holder = issuer,
-                     .amount = asset(800)});
-                env(tx, ter(tecNO_PERMISSION));
-            }
             // Clawback works
             tx = vault.clawback(
                 {.issuer = issuer,
@@ -5246,542 +5243,6 @@ class Vault_test : public beast::unit_test::suite
         });
     }
 
-    void
-    testVaultClawbackBurnShares()
-    {
-        using namespace test::jtx;
-        using namespace loanBroker;
-        using namespace loan;
-        Env env(*this, beast::severities::kWarning);
-
-        auto const vaultAssetBalance = [&](Keylet const& vaultKeylet) {
-            auto const sleVault = env.le(vaultKeylet);
-            BEAST_EXPECT(sleVault != nullptr);
-
-            return std::make_pair(
-                sleVault->at(sfAssetsAvailable), sleVault->at(sfAssetsTotal));
-        };
-
-        auto const vaultShareBalance = [&](Keylet const& vaultKeylet) {
-            auto const sleVault = env.le(vaultKeylet);
-            BEAST_EXPECT(sleVault != nullptr);
-
-            auto const sleIssuance =
-                env.le(keylet::mptIssuance(sleVault->at(sfShareMPTID)));
-            BEAST_EXPECT(sleIssuance != nullptr);
-
-            return sleIssuance->at(sfOutstandingAmount);
-        };
-
-        auto const setupVault =
-            [&](PrettyAsset const& asset,
-                Account const& owner,
-                Account const& depositor) -> std::pair<Vault, Keylet> {
-            Vault vault{env};
-
-            auto const& [tx, vaultKeylet] =
-                vault.create({.owner = owner, .asset = asset});
-            env(tx, ter(tesSUCCESS), THISLINE);
-            env.close();
-
-            auto const& vaultSle = env.le(vaultKeylet);
-            BEAST_EXPECT(vaultSle != nullptr);
-
-            Asset share = vaultSle->at(sfShareMPTID);
-
-            env(vault.deposit(
-                    {.depositor = depositor,
-                     .id = vaultKeylet.key,
-                     .amount = asset(100)}),
-                ter(tesSUCCESS),
-                THISLINE);
-            env.close();
-
-            auto const& [availablePreDefault, totalPreDefault] =
-                vaultAssetBalance(vaultKeylet);
-            BEAST_EXPECT(availablePreDefault == totalPreDefault);
-            BEAST_EXPECT(availablePreDefault == asset(100).value());
-
-            // attempt to clawback shares while there are assets fails
-            env(vault.clawback(
-                    {.issuer = owner,
-                     .id = vaultKeylet.key,
-                     .holder = depositor,
-                     .amount = share(0).value()}),
-                ter(tecNO_PERMISSION),
-                THISLINE);
-            env.close();
-
-            auto const& sharesAvailable = vaultShareBalance(vaultKeylet);
-            auto const& brokerKeylet =
-                keylet::loanbroker(owner.id(), env.seq(owner));
-
-            env(set(owner, vaultKeylet.key), THISLINE);
-            env.close();
-
-            auto const& loanKeylet = keylet::loan(brokerKeylet.key, 1);
-
-            // Create a simple Loan for the full amount of Vault assets
-            env(set(depositor, brokerKeylet.key, asset(100).value()),
-                loan::interestRate(TenthBips32(0)),
-                gracePeriod(10),
-                paymentInterval(120),
-                paymentTotal(10),
-                sig(sfCounterpartySignature, owner),
-                fee(env.current()->fees().base * 2),
-                ter(tesSUCCESS),
-                THISLINE);
-            env.close();
-
-            // attempt to clawback shares while there assetsAvailable == 0 and
-            // assetsTotal > 0 fails
-            env(vault.clawback(
-                    {.issuer = owner,
-                     .id = vaultKeylet.key,
-                     .holder = depositor,
-                     .amount = share(0).value()}),
-                ter(tecNO_PERMISSION),
-                THISLINE);
-            env.close();
-
-            env.close(std::chrono::seconds{120 + 10});
-
-            env(manage(owner, loanKeylet.key, tfLoanDefault),
-                ter(tesSUCCESS),
-                THISLINE);
-
-            auto const& [availablePostDefault, totalPostDefault] =
-                vaultAssetBalance(vaultKeylet);
-
-            BEAST_EXPECT(availablePostDefault == totalPostDefault);
-            BEAST_EXPECT(availablePostDefault == asset(0).value());
-            BEAST_EXPECT(vaultShareBalance(vaultKeylet) == sharesAvailable);
-
-            return std::make_pair(vault, vaultKeylet);
-        };
-
-        auto const testCase = [&](PrettyAsset const& asset,
-                                  std::string const& prefix,
-                                  Account const& owner,
-                                  Account const& depositor) {
-            {
-                testcase(
-                    "VaultClawback (share) - " + prefix +
-                    " owner asset clawback fails");
-                auto [vault, vaultKeylet] = setupVault(asset, owner, depositor);
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                        .amount = asset(100).value(),
-                    }),
-                    // when asset is XRP or owner is not issuer clawback fail
-                    // when owner is issuer precision loss occurs as vault is
-                    // empty
-                    asset.native() ? ter(temMALFORMED)
-                        : asset.raw().getIssuer() != owner.id()
-                        ? ter(tecNO_PERMISSION)
-                        : ter(tecPRECISION_LOSS),
-                    THISLINE);
-                env.close();
-            }
-
-            {
-                testcase(
-                    "VaultClawback (share) - " + prefix +
-                    " owner incomplete share clawback fails");
-                auto [vault, vaultKeylet] = setupVault(asset, owner, depositor);
-                auto const& vaultSle = env.le(vaultKeylet);
-                BEAST_EXPECT(vaultSle != nullptr);
-                if (!vaultSle)
-                    return;
-                Asset share = vaultSle->at(sfShareMPTID);
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                        .amount = share(1).value(),
-                    }),
-                    ter(tecLIMIT_EXCEEDED),
-                    THISLINE);
-                env.close();
-            }
-
-            {
-                testcase(
-                    "VaultClawback (share) - " + prefix +
-                    " owner implicit complete share clawback");
-                auto [vault, vaultKeylet] = setupVault(asset, owner, depositor);
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                    }),
-                    // when owner is issuer implicit clawback fails
-                    asset.native() || asset.raw().getIssuer() != owner.id()
-                        ? ter(tesSUCCESS)
-                        : ter(tecWRONG_ASSET),
-                    THISLINE);
-                env.close();
-            }
-
-            {
-                testcase(
-                    "VaultClawback (share) - " + prefix +
-                    " owner explicit complete share clawback succeeds");
-                auto [vault, vaultKeylet] = setupVault(asset, owner, depositor);
-                auto const& vaultSle = env.le(vaultKeylet);
-                BEAST_EXPECT(vaultSle != nullptr);
-                if (!vaultSle)
-                    return;
-                Asset share = vaultSle->at(sfShareMPTID);
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                        .amount = share(vaultShareBalance(vaultKeylet)).value(),
-                    }),
-                    ter(tesSUCCESS),
-                    THISLINE);
-                env.close();
-            }
-            {
-                testcase(
-                    "VaultClawback (share) - " + prefix +
-                    " owner can clawback own shares");
-                auto [vault, vaultKeylet] = setupVault(asset, owner, owner);
-                auto const& vaultSle = env.le(vaultKeylet);
-                BEAST_EXPECT(vaultSle != nullptr);
-                if (!vaultSle)
-                    return;
-                Asset share = vaultSle->at(sfShareMPTID);
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = owner,
-                        .amount = share(vaultShareBalance(vaultKeylet)).value(),
-                    }),
-                    ter(tesSUCCESS),
-                    THISLINE);
-                env.close();
-            }
-
-            {
-                testcase(
-                    "VaultClawback (share) - " + prefix +
-                    " empty vault share clawback fails");
-                auto [vault, vaultKeylet] = setupVault(asset, owner, owner);
-                auto const& vaultSle = env.le(vaultKeylet);
-                if (BEAST_EXPECT(vaultSle != nullptr))
-                    return;
-                Asset share = vaultSle->at(sfShareMPTID);
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = owner,
-                        .amount = share(vaultShareBalance(vaultKeylet)).value(),
-                    }),
-                    ter(tesSUCCESS),
-                    THISLINE);
-
-                // Now the vault is empty, clawback again fails
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = owner,
-                    }),
-                    ter(tecNO_PERMISSION),
-                    THISLINE);
-                env.close();
-            }
-        };
-
-        Account owner{"alice"};
-        Account depositor{"bob"};
-        Account issuer{"issuer"};
-
-        env.fund(XRP(10000), issuer, owner, depositor);
-        env.close();
-
-        // Test XRP
-        PrettyAsset xrp = xrpIssue();
-        testCase(xrp, "XRP", owner, depositor);
-        testCase(xrp, "XRP (depositor is owner)", owner, owner);
-
-        // Test IOU
-        PrettyAsset IOU = issuer["IOU"];
-        env(fset(issuer, asfAllowTrustLineClawback));
-        env.close();
-
-        env.trust(IOU(1000), owner);
-        env.trust(IOU(1000), depositor);
-        env(pay(issuer, owner, IOU(100)));
-        env(pay(issuer, depositor, IOU(100)));
-        env.close();
-        testCase(IOU, "IOU", owner, depositor);
-        testCase(IOU, "IOU (owner is issuer)", issuer, depositor);
-
-        // Test MPT
-        MPTTester mptt{env, issuer, mptInitNoFund};
-        mptt.create(
-            {.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock});
-        PrettyAsset MPT = mptt.issuanceID();
-        mptt.authorize({.account = owner});
-        mptt.authorize({.account = depositor});
-        env(pay(issuer, owner, MPT(1000)));
-        env(pay(issuer, depositor, MPT(1000)));
-        env.close();
-        testCase(MPT, "MPT", owner, depositor);
-        testCase(MPT, "MPT (owner is issuer)", issuer, depositor);
-    }
-
-    void
-    testVaultClawbackAssets()
-    {
-        using namespace test::jtx;
-        using namespace loanBroker;
-        using namespace loan;
-        Env env(*this);
-
-        auto const setupVault =
-            [&](PrettyAsset const& asset,
-                Account const& owner,
-                Account const& depositor,
-                Account const& issuer) -> std::pair<Vault, Keylet> {
-            Vault vault{env};
-
-            auto const& [tx, vaultKeylet] =
-                vault.create({.owner = owner, .asset = asset});
-            env(tx, ter(tesSUCCESS), THISLINE);
-            env.close();
-
-            auto const& vaultSle = env.le(vaultKeylet);
-            BEAST_EXPECT(vaultSle != nullptr);
-            env(vault.deposit(
-                    {.depositor = depositor,
-                     .id = vaultKeylet.key,
-                     .amount = asset(100)}),
-                ter(tesSUCCESS),
-                THISLINE);
-            env.close();
-
-            return std::make_pair(vault, vaultKeylet);
-        };
-
-        auto const testCase = [&](PrettyAsset const& asset,
-                                  std::string const& prefix,
-                                  Account const& owner,
-                                  Account const& depositor,
-                                  Account const& issuer) {
-            if (asset.native())
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " issuer XRP clawback fails");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, owner, depositor, issuer);
-                // If the asset is XRP, clawback with amount fails as malfored
-                // when asset is specified.
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = issuer,
-                        .amount = asset(1).value(),
-                    }),
-                    ter(temMALFORMED),
-                    THISLINE);
-                // When asset is implicit, clawback fails as no permission.
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = issuer,
-                    }),
-                    ter(tecNO_PERMISSION),
-                    THISLINE);
-                return;
-            }
-
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " clawback for different asset fails");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, owner, depositor, issuer);
-
-                Account issuer2{"issuer2"};
-                PrettyAsset asset2 = issuer2["FOO"];
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                        .amount = asset2(1).value(),
-                    }),
-                    ter(tecWRONG_ASSET),
-                    THISLINE);
-            }
-
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " ambiguous owner/issuer asset clawback fails");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, issuer, depositor, issuer);
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = issuer,
-                    }),
-                    ter(tecWRONG_ASSET),
-                    THISLINE);
-            }
-
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " non-issuer asset clawback fails");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, owner, depositor, issuer);
-
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                    }),
-                    ter(tecNO_PERMISSION),
-                    THISLINE);
-
-                env(vault.clawback({
-                        .issuer = owner,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                        .amount = asset(1).value(),
-                    }),
-                    ter(tecNO_PERMISSION),
-                    THISLINE);
-            }
-
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " issuer clawback from self fails");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, owner, issuer, issuer);
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = issuer,
-                    }),
-                    ter(tecNO_PERMISSION),
-                    THISLINE);
-            }
-
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " issuer share clawback fails");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, owner, depositor, issuer);
-                auto const& vaultSle = env.le(vaultKeylet);
-                BEAST_EXPECT(vaultSle != nullptr);
-                if (!vaultSle)
-                    return;
-                Asset share = vaultSle->at(sfShareMPTID);
-
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                        .amount = share(1).value(),
-                    }),
-                    ter(tecNO_PERMISSION),
-                    THISLINE);
-            }
-
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " partial issuer asset clawback succeeds");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, owner, depositor, issuer);
-
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                        .amount = asset(1).value(),
-                    }),
-                    ter(tesSUCCESS),
-                    THISLINE);
-            }
-
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " full issuer asset clawback succeeds");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, owner, depositor, issuer);
-
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                        .amount = asset(100).value(),
-                    }),
-                    ter(tesSUCCESS),
-                    THISLINE);
-            }
-
-            {
-                testcase(
-                    "VaultClawback (asset) - " + prefix +
-                    " implicit full issuer asset clawback succeeds");
-                auto [vault, vaultKeylet] =
-                    setupVault(asset, owner, depositor, issuer);
-
-                env(vault.clawback({
-                        .issuer = issuer,
-                        .id = vaultKeylet.key,
-                        .holder = depositor,
-                    }),
-                    ter(tesSUCCESS),
-                    THISLINE);
-            }
-        };
-
-        Account owner{"alice"};
-        Account depositor{"bob"};
-        Account issuer{"issuer"};
-
-        env.fund(XRP(10000), issuer, owner, depositor);
-        env.close();
-
-        // Test XRP
-        PrettyAsset xrp = xrpIssue();
-        testCase(xrp, "XRP", owner, depositor, issuer);
-
-        // Test IOU
-        PrettyAsset IOU = issuer["IOU"];
-        env(fset(issuer, asfAllowTrustLineClawback));
-        env.close();
-        env.trust(IOU(1000), owner);
-        env.trust(IOU(1000), depositor);
-        env(pay(issuer, owner, IOU(1000)));
-        env(pay(issuer, depositor, IOU(1000)));
-        env.close();
-        testCase(IOU, "IOU", owner, depositor, issuer);
-
-        // Test MPT
-        MPTTester mptt{env, issuer, mptInitNoFund};
-        mptt.create(
-            {.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock});
-        PrettyAsset MPT = mptt.issuanceID();
-        mptt.authorize({.account = owner});
-        mptt.authorize({.account = depositor});
-        env(pay(issuer, depositor, MPT(1000)));
-        env.close();
-        testCase(MPT, "MPT", owner, depositor, issuer);
-    }
-
 public:
     void
     run() override
@@ -5800,8 +5261,6 @@ public:
         testScaleIOU();
         testRPC();
         testDelegate();
-        testVaultClawbackBurnShares();
-        testVaultClawbackAssets();
     }
 };
 

src/test/basics/IntrusiveShared_test.cpp

@@ -396,7 +396,7 @@ public:
                 // This checks that partialDelete has run to completion
                 // before the destructor is called. A sleep is inserted
                 // inside the partial delete to make sure the destructor is
-                // given an opportunity to run during partial delete.
+                // given an opportunity to run durring partial delete.
                 BEAST_EXPECT(cur == partiallyDeleted);
             }
             if (next == partiallyDeletedStarted)

src/test/beast/beast_CurrentThreadName_test.cpp

@@ -1,8 +1,6 @@
 #include <xrpl/beast/core/CurrentThreadName.h>
 #include <xrpl/beast/unit_test.h>
 
-#include <boost/predef/os.h>
-
 #include <thread>
 
 namespace xrpl {
@@ -39,71 +37,33 @@ private:
         if (beast::getCurrentThreadName() == myName)
             *state = 2;
     }
-#if BOOST_OS_LINUX
-    // Helper function to test a specific name.
-    // It creates a thread, sets the name, and checks if the OS-level
-    // name matches the expected (potentially truncated) name.
-    void
-    testName(std::string const& nameToSet, std::string const& expectedName)
-    {
-        std::thread t([&] {
-            beast::setCurrentThreadName(nameToSet);
-
-            // Initialize buffer to be safe.
-            char actualName[beast::maxThreadNameLength + 1] = {};
-            pthread_getname_np(pthread_self(), actualName, sizeof(actualName));
-
-            BEAST_EXPECT(std::string(actualName) == expectedName);
-        });
-        t.join();
-    }
-#endif
 
 public:
     void
     run() override
     {
-        // Make two different threads with two different names.
-        // Make sure that the expected thread names are still there
-        // when the thread exits.
-        {
-            std::atomic<bool> stop{false};
+        // Make two different threads with two different names.  Make sure
+        // that the expected thread names are still there when the thread
+        // exits.
+        std::atomic<bool> stop{false};
 
-            std::atomic<int> stateA{0};
-            std::thread tA(exerciseName, "tA", &stop, &stateA);
+        std::atomic<int> stateA{0};
+        std::thread tA(exerciseName, "tA", &stop, &stateA);
 
-            std::atomic<int> stateB{0};
-            std::thread tB(exerciseName, "tB", &stop, &stateB);
+        std::atomic<int> stateB{0};
+        std::thread tB(exerciseName, "tB", &stop, &stateB);
 
-            // Wait until both threads have set their names.
-            while (stateA == 0 || stateB == 0)
-                ;
+        // Wait until both threads have set their names.
+        while (stateA == 0 || stateB == 0)
+            ;
 
-            stop = true;
-            tA.join();
-            tB.join();
+        stop = true;
+        tA.join();
+        tB.join();
 
-            // Both threads should still have the expected name when they exit.
-            BEAST_EXPECT(stateA == 2);
-            BEAST_EXPECT(stateB == 2);
-        }
-#if BOOST_OS_LINUX
-        // On Linux, verify that thread names longer than 15 characters
-        // are truncated to 15 characters (the 16th character is reserved for
-        // the null terminator).
-        {
-            testName(
-                "123456789012345",
-                "123456789012345");  // 15 chars, no truncation
-            testName(
-                "1234567890123456", "123456789012345");  // 16 chars, truncated
-            testName(
-                "ThisIsAVeryLongThreadNameExceedingLimit",
-                "ThisIsAVeryLong");      // 39 chars, truncated
-            testName("", "");            // empty name
-            testName("short", "short");  // short name, no truncation
-        }
-#endif
+        // Both threads should still have the expected name when they exit.
+        BEAST_EXPECT(stateA == 2);
+        BEAST_EXPECT(stateB == 2);
     }
 };
 

src/xrpld/app/misc/NetworkOPs.cpp

@@ -1681,7 +1681,7 @@ NetworkOPsImp::apply(std::unique_lock<std::mutex>& batchLock)
                     // only be set if the Batch feature is enabled. If Batch is
                     // not enabled, the flag is always invalid, so don't relay
                     // it regardless.
-                    !(sttx.isFlag(tfInnerBatchTxn)))
+                    !sttx.isFlag(tfInnerBatchTxn))
                 {
                     protocol::TMTransaction tx;
                     Serializer s;

src/xrpld/app/tx/detail/InvariantCheck.cpp

@@ -95,7 +95,6 @@ hasPrivilege(STTx const& tx, Privilege priv)
     switch (tx.getTxnType())
     {
 #include <xrpl/protocol/detail/transactions.macro>
-
         // Deprecated types
         default:
             return false;
@@ -2623,7 +2622,6 @@ ValidVault::Vault::make(SLE const& from)
     self.key = from.key();
     self.asset = from.at(sfAsset);
     self.pseudoId = from.getAccountID(sfAccount);
-    self.owner = from.at(sfOwner);
     self.shareMPTID = from.getFieldH192(sfShareMPTID);
     self.assetsTotal = from.at(sfAssetsTotal);
     self.assetsAvailable = from.at(sfAssetsAvailable);
@@ -3068,10 +3066,6 @@ ValidVault::finalize(
                                    : std::nullopt;
     };
 
-    auto const vaultHoldsNoAssets = [&](Vault const& vault) {
-        return vault.assetsAvailable == 0 && vault.assetsTotal == 0;
-    };
-
     // Technically this does not need to be a lambda, but it's more
     // convenient thanks to early "return false"; the not-so-nice
     // alternatives are several layers of nested if/else or more complex
@@ -3454,56 +3448,29 @@ ValidVault::finalize(
                 if (vaultAsset.native() ||
                     vaultAsset.getIssuer() != tx[sfAccount])
                 {
-                    // The owner can use clawback to force-burn shares when the
-                    // vault is empty but there are outstanding shares
-                    if (!(beforeShares && beforeShares->sharesTotal > 0 &&
-                          vaultHoldsNoAssets(beforeVault) &&
-                          beforeVault.owner == tx[sfAccount]))
-                    {
-                        JLOG(j.fatal()) <<  //
-                            "Invariant failed: clawback may only be performed "
-                            "by the asset issuer, or by the vault owner of an "
-                            "empty vault";
-                        return false;  // That's all we can do
-                    }
+                    JLOG(j.fatal()) <<  //
+                        "Invariant failed: clawback may only be performed by "
+                        "the asset issuer";
+                    return false;  // That's all we can do
                 }
 
                 auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId);
-                if (vaultDeltaAssets)
-                {
-                    if (*vaultDeltaAssets >= zero)
-                    {
-                        JLOG(j.fatal()) <<  //
-                            "Invariant failed: clawback must decrease vault "
-                            "balance";
-                        result = false;
-                    }
 
-                    if (beforeVault.assetsTotal + *vaultDeltaAssets !=
-                        afterVault.assetsTotal)
-                    {
-                        JLOG(j.fatal()) <<  //
-                            "Invariant failed: clawback and assets outstanding "
-                            "must add up";
-                        result = false;
-                    }
-
-                    if (beforeVault.assetsAvailable + *vaultDeltaAssets !=
-                        afterVault.assetsAvailable)
-                    {
-                        JLOG(j.fatal()) <<  //
-                            "Invariant failed: clawback and assets available "
-                            "must add up";
-                        result = false;
-                    }
-                }
-                else if (!vaultHoldsNoAssets(beforeVault))
+                if (!vaultDeltaAssets)
                 {
                     JLOG(j.fatal()) <<  //
                         "Invariant failed: clawback must change vault balance";
                     return false;  // That's all we can do
                 }
 
+                if (*vaultDeltaAssets >= zero)
+                {
+                    JLOG(j.fatal()) <<  //
+                        "Invariant failed: clawback must decrease vault "
+                        "balance";
+                    result = false;
+                }
+
                 auto const accountDeltaShares = deltaShares(tx[sfHolder]);
                 if (!accountDeltaShares)
                 {
@@ -3536,6 +3503,24 @@ ValidVault::finalize(
                     result = false;
                 }
 
+                if (beforeVault.assetsTotal + *vaultDeltaAssets !=
+                    afterVault.assetsTotal)
+                {
+                    JLOG(j.fatal()) <<  //
+                        "Invariant failed: clawback and assets outstanding "
+                        "must add up";
+                    result = false;
+                }
+
+                if (beforeVault.assetsAvailable + *vaultDeltaAssets !=
+                    afterVault.assetsAvailable)
+                {
+                    JLOG(j.fatal()) <<  //
+                        "Invariant failed: clawback and assets available must "
+                        "add up";
+                    result = false;
+                }
+
                 return result;
             }
 

src/xrpld/app/tx/detail/InvariantCheck.h

@@ -861,7 +861,6 @@ class ValidVault
         uint256 key = beast::zero;
         Asset asset = {};
         AccountID pseudoId = {};
-        AccountID owner = {};
         uint192 shareMPTID = beast::zero;
         Number assetsTotal = 0;
         Number assetsAvailable = 0;

src/xrpld/app/tx/detail/Transactor.cpp

@@ -204,14 +204,8 @@ Transactor::preflight2(PreflightContext const& ctx)
         // regardless of success or failure
         return *ret;
 
-    // It should be impossible for the InnerBatchTxn flag to be set without
-    // featureBatch being enabled
-    XRPL_ASSERT_PARTS(
-        !ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatch),
-        "xrpl::Transactor::preflight2",
-        "InnerBatch flag only set if feature enabled");
     // Skip signature check on batch inner transactions
-    if (ctx.tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatch))
+    if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatch))
         return tesSUCCESS;
     // Do not add any checks after this point that are relevant for
     // batch inner transactions. They will be skipped.

src/xrpld/app/tx/detail/VaultClawback.cpp

@@ -1,17 +1,18 @@
 #include <xrpld/app/tx/detail/VaultClawback.h>
-//
+
 #include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/ledger/View.h>
 #include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/MPTIssue.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STNumber.h>
 #include <xrpl/protocol/TER.h>
-
-#include <optional>
+#include <xrpl/protocol/TxFlags.h>
 
 namespace xrpl {
+
 NotTEC
 VaultClawback::preflight(PreflightContext const& ctx)
 {
@@ -21,6 +22,15 @@ VaultClawback::preflight(PreflightContext const& ctx)
         return temMALFORMED;
     }
 
+    AccountID const issuer = ctx.tx[sfAccount];
+    AccountID const holder = ctx.tx[sfHolder];
+
+    if (issuer == holder)
+    {
+        JLOG(ctx.j.debug()) << "VaultClawback: issuer cannot be holder.";
+        return temMALFORMED;
+    }
+
     auto const amount = ctx.tx[~sfAmount];
     if (amount)
     {
@@ -32,27 +42,17 @@ VaultClawback::preflight(PreflightContext const& ctx)
             JLOG(ctx.j.debug()) << "VaultClawback: cannot clawback XRP.";
             return temMALFORMED;
         }
+        else if (amount->asset().getIssuer() != issuer)
+        {
+            JLOG(ctx.j.debug())
+                << "VaultClawback: only asset issuer can clawback.";
+            return temMALFORMED;
+        }
     }
 
     return tesSUCCESS;
 }
 
-[[nodiscard]] STAmount
-clawbackAmount(
-    std::shared_ptr<SLE const> const& vault,
-    std::optional<STAmount> const& maybeAmount,
-    AccountID const& account)
-{
-    if (maybeAmount)
-        return *maybeAmount;
-
-    Asset const share = MPTIssue{vault->at(sfShareMPTID)};
-    if (account == vault->at(sfOwner))
-        return STAmount{share};
-
-    return STAmount{vault->at(sfAsset)};
-}
-
 TER
 VaultClawback::preclaim(PreclaimContext const& ctx)
 {
@@ -60,264 +60,61 @@ VaultClawback::preclaim(PreclaimContext const& ctx)
     if (!vault)
         return tecNO_ENTRY;
 
-    Asset const vaultAsset = vault->at(sfAsset);
-    auto const account = ctx.tx[sfAccount];
-    auto const holder = ctx.tx[sfHolder];
-    auto const maybeAmount = ctx.tx[~sfAmount];
-    auto const mptIssuanceID = vault->at(sfShareMPTID);
-    auto const sleShareIssuance =
-        ctx.view.read(keylet::mptIssuance(mptIssuanceID));
-    if (!sleShareIssuance)
+    auto account = ctx.tx[sfAccount];
+    auto const issuer = ctx.view.read(keylet::account(account));
+    if (!issuer)
     {
         // LCOV_EXCL_START
-        JLOG(ctx.j.error())
-            << "VaultClawback: missing issuance of vault shares.";
+        JLOG(ctx.j.error()) << "VaultClawback: missing issuer account.";
         return tefINTERNAL;
         // LCOV_EXCL_STOP
     }
 
-    Asset const share = MPTIssue{mptIssuanceID};
-
-    // Ambiguous case: If Issuer is Owner they must specify the asset
-    if (!maybeAmount && !vaultAsset.native() &&
-        vaultAsset.getIssuer() == vault->at(sfOwner))
-    {
-        JLOG(ctx.j.debug())
-            << "VaultClawback: must specify amount when issuer is owner.";
+    Asset const vaultAsset = vault->at(sfAsset);
+    if (auto const amount = ctx.tx[~sfAmount];
+        amount && vaultAsset != amount->asset())
         return tecWRONG_ASSET;
+
+    if (vaultAsset.native())
+    {
+        JLOG(ctx.j.debug()) << "VaultClawback: cannot clawback XRP.";
+        return tecNO_PERMISSION;  // Cannot clawback XRP.
+    }
+    else if (vaultAsset.getIssuer() != account)
+    {
+        JLOG(ctx.j.debug()) << "VaultClawback: only asset issuer can clawback.";
+        return tecNO_PERMISSION;  // Only issuers can clawback.
     }
 
-    auto const amount = clawbackAmount(vault, maybeAmount, account);
-
-    // There is a special case that allows the VaultOwner to use clawback to
-    // burn shares when Vault assets total and available are zero, but
-    // shares remain. However, that case is handled in doApply() directly,
-    // so here we just enforce checks.
-    if (amount.asset() == share)
+    if (vaultAsset.holds<MPTIssue>())
     {
-        // Only the Vault Owner may clawback shares
-        if (account != vault->at(sfOwner))
+        auto const mpt = vaultAsset.get<MPTIssue>();
+        auto const mptIssue =
+            ctx.view.read(keylet::mptIssuance(mpt.getMptID()));
+        if (mptIssue == nullptr)
+            return tecOBJECT_NOT_FOUND;
+
+        std::uint32_t const issueFlags = mptIssue->getFieldU32(sfFlags);
+        if (!(issueFlags & lsfMPTCanClawback))
         {
             JLOG(ctx.j.debug())
-                << "VaultClawback: only vault owner can clawback shares.";
+                << "VaultClawback: cannot clawback MPT vault asset.";
             return tecNO_PERMISSION;
         }
-
-        auto const assetsTotal = vault->at(sfAssetsTotal);
-        auto const assetsAvailable = vault->at(sfAssetsAvailable);
-        auto const sharesTotal = sleShareIssuance->at(sfOutstandingAmount);
-
-        // Owner can clawback funds when the vault has shares but no assets
-        if (sharesTotal == 0 || (assetsTotal != 0 || assetsAvailable != 0))
+    }
+    else if (vaultAsset.holds<Issue>())
+    {
+        std::uint32_t const issuerFlags = issuer->getFieldU32(sfFlags);
+        if (!(issuerFlags & lsfAllowTrustLineClawback) ||
+            (issuerFlags & lsfNoFreeze))
         {
             JLOG(ctx.j.debug())
-                << "VaultClawback: vault owner can clawback shares only"
-                   " when vault has no assets.";
+                << "VaultClawback: cannot clawback IOU vault asset.";
             return tecNO_PERMISSION;
         }
-
-        // If amount is non-zero, the VaultOwner must burn all shares
-        if (amount != beast::zero)
-        {
-            Number const& sharesHeld = accountHolds(
-                ctx.view,
-                holder,
-                share,
-                FreezeHandling::fhIGNORE_FREEZE,
-                AuthHandling::ahIGNORE_AUTH,
-                ctx.j);
-
-            // The VaultOwner must burn all shares
-            if (amount != sharesHeld)
-            {
-                JLOG(ctx.j.debug())
-                    << "VaultClawback: vault owner must clawback all "
-                       "shares.";
-                return tecLIMIT_EXCEEDED;
-            }
-        }
-
-        return tesSUCCESS;
     }
 
-    // The asset that is being clawed back is the vault asset
-    if (amount.asset() == vaultAsset)
-    {
-        // XRP cannot be clawed back
-        if (vaultAsset.native())
-        {
-            JLOG(ctx.j.debug()) << "VaultClawback: cannot clawback XRP.";
-            return tecNO_PERMISSION;
-        }
-
-        // Only the Asset Issuer may clawback the asset
-        if (account != vaultAsset.getIssuer())
-        {
-            JLOG(ctx.j.debug())
-                << "VaultClawback: only asset issuer can clawback asset.";
-            return tecNO_PERMISSION;
-        }
-
-        // The issuer cannot clawback from itself
-        if (account == holder)
-        {
-            JLOG(ctx.j.debug())
-                << "VaultClawback: issuer cannot be the holder.";
-            return tecNO_PERMISSION;
-        }
-
-        return std::visit(
-            [&]<ValidIssueType TIss>(TIss const& issue) -> TER {
-                if constexpr (std::is_same_v<TIss, MPTIssue>)
-                {
-                    auto const mptIssue =
-                        ctx.view.read(keylet::mptIssuance(issue.getMptID()));
-                    if (mptIssue == nullptr)
-                        return tecOBJECT_NOT_FOUND;
-
-                    std::uint32_t const issueFlags =
-                        mptIssue->getFieldU32(sfFlags);
-                    if (!(issueFlags & lsfMPTCanClawback))
-                    {
-                        JLOG(ctx.j.debug()) << "VaultClawback: cannot clawback "
-                                               "MPT vault asset.";
-                        return tecNO_PERMISSION;
-                    }
-                }
-                else if constexpr (std::is_same_v<TIss, Issue>)
-                {
-                    auto const issuerSle =
-                        ctx.view.read(keylet::account(account));
-                    if (!issuerSle)
-                    {
-                        // LCOV_EXCL_START
-                        JLOG(ctx.j.error())
-                            << "VaultClawback: missing submitter account.";
-                        return tefINTERNAL;
-                        // LCOV_EXCL_STOP
-                    }
-
-                    std::uint32_t const issuerFlags =
-                        issuerSle->getFieldU32(sfFlags);
-                    if (!(issuerFlags & lsfAllowTrustLineClawback) ||
-                        (issuerFlags & lsfNoFreeze))
-                    {
-                        JLOG(ctx.j.debug()) << "VaultClawback: cannot clawback "
-                                               "IOU vault asset.";
-                        return tecNO_PERMISSION;
-                    }
-                }
-                return tesSUCCESS;
-            },
-            vaultAsset.value());
-    }
-
-    // Invalid asset
-    return tecWRONG_ASSET;
-}
-
-Expected<std::pair<STAmount, STAmount>, TER>
-VaultClawback::assetsToClawback(
-    std::shared_ptr<SLE> const& vault,
-    std::shared_ptr<SLE const> const& sleShareIssuance,
-    AccountID const& holder,
-    STAmount const& clawbackAmount)
-{
-    if (clawbackAmount.asset() != vault->at(sfAsset))
-    {
-        // preclaim should have blocked this , now it's an internal error
-        // LCOV_EXCL_START
-        JLOG(j_.error()) << "VaultClawback: asset mismatch in clawback.";
-        return Unexpected(tecINTERNAL);
-        // LCOV_EXCL_STOP
-    }
-
-    auto const assetsAvailable = vault->at(sfAssetsAvailable);
-    auto const mptIssuanceID = *vault->at(sfShareMPTID);
-    MPTIssue const share{mptIssuanceID};
-
-    if (clawbackAmount == beast::zero)
-    {
-        auto const sharesDestroyed = accountHolds(
-            view(),
-            holder,
-            share,
-            FreezeHandling::fhIGNORE_FREEZE,
-            AuthHandling::ahIGNORE_AUTH,
-            j_);
-        auto const maybeAssets =
-            sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
-        if (!maybeAssets)
-            return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-
-        return std::make_pair(*maybeAssets, sharesDestroyed);
-    }
-
-    STAmount sharesDestroyed;
-    STAmount assetsRecovered = clawbackAmount;
-    try
-    {
-        {
-            auto const maybeShares = assetsToSharesWithdraw(
-                vault, sleShareIssuance, assetsRecovered);
-            if (!maybeShares)
-                return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-            sharesDestroyed = *maybeShares;
-        }
-
-        auto const maybeAssets =
-            sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
-        if (!maybeAssets)
-            return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-        assetsRecovered = *maybeAssets;
-
-        // Clamp to maximum.
-        if (assetsRecovered > *assetsAvailable)
-        {
-            assetsRecovered = *assetsAvailable;
-            // Note, it is important to truncate the number of shares,
-            // otherwise the corresponding assets might breach the
-            // AssetsAvailable
-            {
-                auto const maybeShares = assetsToSharesWithdraw(
-                    vault,
-                    sleShareIssuance,
-                    assetsRecovered,
-                    TruncateShares::yes);
-                if (!maybeShares)
-                    return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-                sharesDestroyed = *maybeShares;
-            }
-
-            auto const maybeAssets = sharesToAssetsWithdraw(
-                vault, sleShareIssuance, sharesDestroyed);
-            if (!maybeAssets)
-                return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-            assetsRecovered = *maybeAssets;
-            if (assetsRecovered > *assetsAvailable)
-            {
-                // LCOV_EXCL_START
-                JLOG(j_.error())
-                    << "VaultClawback: invalid rounding of shares.";
-                return Unexpected(tecINTERNAL);
-                // LCOV_EXCL_STOP
-            }
-        }
-    }
-    catch (std::overflow_error const&)
-    {
-        // It's easy to hit this exception from Number with large enough
-        // Scale so we avoid spamming the log and only use debug here.
-        JLOG(j_.debug())  //
-            << "VaultClawback: overflow error with"
-            << " scale=" << (int)vault->at(sfScale).value()  //
-            << ", assetsTotal=" << vault->at(sfAssetsTotal).value()
-            << ", sharesTotal=" << sleShareIssuance->at(sfOutstandingAmount)
-            << ", amount=" << clawbackAmount.value();
-        return Unexpected(tecPATH_DRY);
-    }
-
-    return std::make_pair(assetsRecovered, sharesDestroyed);
+    return tesSUCCESS;
 }
 
 TER
@@ -328,7 +125,7 @@ VaultClawback::doApply()
     if (!vault)
         return tefINTERNAL;  // LCOV_EXCL_LINE
 
-    auto const mptIssuanceID = *vault->at(sfShareMPTID);
+    auto const mptIssuanceID = *((*vault)[sfShareMPTID]);
     auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID));
     if (!sleIssuance)
     {
@@ -337,47 +134,105 @@ VaultClawback::doApply()
         return tefINTERNAL;
         // LCOV_EXCL_STOP
     }
-    MPTIssue const share{mptIssuanceID};
 
     Asset const vaultAsset = vault->at(sfAsset);
-    STAmount const amount = clawbackAmount(vault, tx[~sfAmount], account_);
+    STAmount const amount = [&]() -> STAmount {
+        auto const maybeAmount = tx[~sfAmount];
+        if (maybeAmount)
+            return *maybeAmount;
+        return {sfAmount, vaultAsset, 0};
+    }();
+    XRPL_ASSERT(
+        amount.asset() == vaultAsset,
+        "xrpl::VaultClawback::doApply : matching asset");
 
     auto assetsAvailable = vault->at(sfAssetsAvailable);
     auto assetsTotal = vault->at(sfAssetsTotal);
-
     [[maybe_unused]] auto const lossUnrealized = vault->at(sfLossUnrealized);
     XRPL_ASSERT(
         lossUnrealized <= (assetsTotal - assetsAvailable),
         "xrpl::VaultClawback::doApply : loss and assets do balance");
 
     AccountID holder = tx[sfHolder];
+    MPTIssue const share{mptIssuanceID};
     STAmount sharesDestroyed = {share};
-    STAmount assetsRecovered = {vault->at(sfAsset)};
-
-    // The Owner is burning shares
-    if (account_ == vault->at(sfOwner) && amount.asset() == share)
+    STAmount assetsRecovered;
+    try
     {
-        sharesDestroyed = accountHolds(
-            view(),
-            holder,
-            share,
-            FreezeHandling::fhIGNORE_FREEZE,
-            AuthHandling::ahIGNORE_AUTH,
-            j_);
+        if (amount == beast::zero)
+        {
+            sharesDestroyed = accountHolds(
+                view(),
+                holder,
+                share,
+                FreezeHandling::fhIGNORE_FREEZE,
+                AuthHandling::ahIGNORE_AUTH,
+                j_);
+
+            auto const maybeAssets =
+                sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed);
+            if (!maybeAssets)
+                return tecINTERNAL;  // LCOV_EXCL_LINE
+            assetsRecovered = *maybeAssets;
+        }
+        else
+        {
+            assetsRecovered = amount;
+            {
+                auto const maybeShares =
+                    assetsToSharesWithdraw(vault, sleIssuance, assetsRecovered);
+                if (!maybeShares)
+                    return tecINTERNAL;  // LCOV_EXCL_LINE
+                sharesDestroyed = *maybeShares;
+            }
+
+            auto const maybeAssets =
+                sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed);
+            if (!maybeAssets)
+                return tecINTERNAL;  // LCOV_EXCL_LINE
+            assetsRecovered = *maybeAssets;
+        }
+
+        // Clamp to maximum.
+        if (assetsRecovered > *assetsAvailable)
+        {
+            assetsRecovered = *assetsAvailable;
+            // Note, it is important to truncate the number of shares, otherwise
+            // the corresponding assets might breach the AssetsAvailable
+            {
+                auto const maybeShares = assetsToSharesWithdraw(
+                    vault, sleIssuance, assetsRecovered, TruncateShares::yes);
+                if (!maybeShares)
+                    return tecINTERNAL;  // LCOV_EXCL_LINE
+                sharesDestroyed = *maybeShares;
+            }
+
+            auto const maybeAssets =
+                sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed);
+            if (!maybeAssets)
+                return tecINTERNAL;  // LCOV_EXCL_LINE
+            assetsRecovered = *maybeAssets;
+            if (assetsRecovered > *assetsAvailable)
+            {
+                // LCOV_EXCL_START
+                JLOG(j_.error())
+                    << "VaultClawback: invalid rounding of shares.";
+                return tecINTERNAL;
+                // LCOV_EXCL_STOP
+            }
+        }
     }
-    else  // The Issuer is clawbacking vault assets
+    catch (std::overflow_error const&)
     {
-        XRPL_ASSERT(
-            amount.asset() == vaultAsset,
-            "xrpl::VaultClawback::doApply : matching asset");
-
-        auto const clawbackParts =
-            assetsToClawback(vault, sleIssuance, holder, amount);
-        if (!clawbackParts)
-            return clawbackParts.error();
-
-        assetsRecovered = clawbackParts->first;
-        sharesDestroyed = clawbackParts->second;
+        // It's easy to hit this exception from Number with large enough Scale
+        // so we avoid spamming the log and only use debug here.
+        JLOG(j_.debug())  //
+            << "VaultClawback: overflow error with"
+            << " scale=" << (int)vault->at(sfScale).value()  //
+            << ", assetsTotal=" << vault->at(sfAssetsTotal).value()
+            << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount)
+            << ", amount=" << amount.value();
+        return tecPATH_DRY;
     }
 
     if (sharesDestroyed == beast::zero)
@@ -427,34 +282,30 @@ VaultClawback::doApply()
         // else quietly ignore, holder balance is not zero
     }
 
-    if (assetsRecovered > beast::zero)
-    {
-        // Transfer assets from vault to issuer.
-        if (auto const ter = accountSend(
-                view(),
-                vaultAccount,
-                account_,
-                assetsRecovered,
-                j_,
-                WaiveTransferFee::Yes);
-            !isTesSuccess(ter))
-            return ter;
+    // Transfer assets from vault to issuer.
+    if (auto const ter = accountSend(
+            view(),
+            vaultAccount,
+            account_,
+            assetsRecovered,
+            j_,
+            WaiveTransferFee::Yes);
+        !isTesSuccess(ter))
+        return ter;
 
-        // Sanity check
-        if (accountHolds(
-                view(),
-                vaultAccount,
-                assetsRecovered.asset(),
-                FreezeHandling::fhIGNORE_FREEZE,
-                AuthHandling::ahIGNORE_AUTH,
-                j_) < beast::zero)
-        {
-            // LCOV_EXCL_START
-            JLOG(j_.error())
-                << "VaultClawback: negative balance of vault assets.";
-            return tefINTERNAL;
-            // LCOV_EXCL_STOP
-        }
+    // Sanity check
+    if (accountHolds(
+            view(),
+            vaultAccount,
+            assetsRecovered.asset(),
+            FreezeHandling::fhIGNORE_FREEZE,
+            AuthHandling::ahIGNORE_AUTH,
+            j_) < beast::zero)
+    {
+        // LCOV_EXCL_START
+        JLOG(j_.error()) << "VaultClawback: negative balance of vault assets.";
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     return tesSUCCESS;

src/xrpld/app/tx/detail/VaultClawback.h

@@ -22,14 +22,6 @@ public:
 
     TER
     doApply() override;
-
-private:
-    Expected<std::pair<STAmount, STAmount>, TER>
-    assetsToClawback(
-        std::shared_ptr<SLE> const& vault,
-        std::shared_ptr<SLE const> const& sleShareIssuance,
-        AccountID const& holder,
-        STAmount const& clawbackAmount);
 };
 
 }  // namespace xrpl

src/xrpld/app/tx/detail/apply.cpp

@@ -41,22 +41,15 @@ checkValidity(
                 Validity::SigBad,
                 "Malformed: Invalid inner batch transaction."};
 
-        // This block should probably have never been included in the
-        // original `Batch` implementation. An inner transaction never
-        // has a valid signature.
-        bool const neverValid = rules.enabled(fixBatchInnerSigs);
-        if (!neverValid)
+        std::string reason;
+        if (!passesLocalChecks(tx, reason))
         {
-            std::string reason;
-            if (!passesLocalChecks(tx, reason))
-            {
-                router.setFlags(id, SF_LOCALBAD);
-                return {Validity::SigGoodOnly, reason};
-            }
-
-            router.setFlags(id, SF_SIGGOOD);
-            return {Validity::Valid, ""};
+            router.setFlags(id, SF_LOCALBAD);
+            return {Validity::SigGoodOnly, reason};
         }
+
+        router.setFlags(id, SF_SIGGOOD);
+        return {Validity::Valid, ""};
     }
 
     if (any(flags & SF_SIGBAD))