Fix the bug

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

.github/workflows/build-test.yml

@@ -102,21 +102,16 @@ jobs:
           echo 'CMake target: ${{ matrix.cmake_target }}'
           echo 'Config name: ${{ matrix.config_name }}'
 
-      - name: Clean workspace (MacOS)
+      - name: Cleanup workspace
-        if: ${{ inputs.os == 'macos' }}
+        if: ${{ runner.os == 'macOS' }}
-        run: |
+        uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e
-          WORKSPACE=${{ github.workspace }}
-          echo "Cleaning workspace '${WORKSPACE}'."
-          if [ -z "${WORKSPACE}" ] || [ "${WORKSPACE}" = "/" ]; then
-            echo "Invalid working directory '${WORKSPACE}'."
-            exit 1
-          fi
-          find "${WORKSPACE}" -depth 1 | xargs rm -rfv
 
       - name: Checkout repository
         uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
       - name: Prepare runner
         uses: XRPLF/actions/.github/actions/prepare-runner@638e0dc11ea230f91bd26622fb542116bb5254d5
+        with:
+          disable_ccache: false
 
       - name: Check configuration (Windows)
         if: ${{ inputs.os == 'windows' }}

.github/workflows/check-format.yml

@@ -17,41 +17,10 @@ jobs:
     runs-on: ubuntu-latest
     container: ghcr.io/xrplf/ci/tools-rippled-pre-commit
     steps:
-      # The $GITHUB_WORKSPACE and ${{ github.workspace }} might not point to the
-      # same directory for jobs running in containers. The actions/checkout step
-      # is *supposed* to checkout into $GITHUB_WORKSPACE and then add it to
-      # safe.directory (see instructions at https://github.com/actions/checkout)
-      # but that is apparently not happening for some container images. We
-      # therefore preemptively add both directories to safe.directory. See also
-      # https://github.com/actions/runner/issues/2058 for more details.
-      - name: Configure git safe.directory
-        run: |
-          git config --global --add safe.directory $GITHUB_WORKSPACE
-          git config --global --add safe.directory ${{ github.workspace }}
       - name: Checkout repository
         uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
-      - name: Check configuration
+      - name: Prepare runner
-        run: |
+        uses: XRPLF/actions/.github/actions/prepare-runner@638e0dc11ea230f91bd26622fb542116bb5254d5
-          echo 'Checking path.'
-          echo ${PATH} | tr ':' '\n'
-
-          echo 'Checking environment variables.'
-          env | sort
-
-          echo 'Checking pre-commit version.'
-          pre-commit --version
-
-          echo 'Checking clang-format version.'
-          clang-format --version
-
-          echo 'Checking NPM version.'
-          npm --version
-
-          echo 'Checking Node.js version.'
-          node --version
-
-          echo 'Checking prettier version.'
-          prettier --version
       - name: Format code
         run: pre-commit run --show-diff-on-failure --color=always --all-files
       - name: Check for differences

.github/workflows/notify-clio.yml

@@ -50,6 +50,10 @@ jobs:
           echo "channel=pr_${{ github.event.pull_request.number }}" >> "${GITHUB_OUTPUT}"
           echo 'Extracting version.'
           echo "version=$(cat src/libxrpl/protocol/BuildInfo.cpp | grep "versionString =" | awk -F '"' '{print $2}')" >> "${GITHUB_OUTPUT}"
+      - name: Calculate conan reference
+        id: conan_ref
+        run: |
+          echo "conan_ref=${{ steps.generate.outputs.version }}@${{ steps.generate.outputs.user }}/${{ steps.generate.outputs.channel }}" >> "${GITHUB_OUTPUT}"
       - name: Add Conan remote
         run: |
           echo "Adding Conan remote '${{ inputs.conan_remote_name }}' at ${{ inputs.conan_remote_url }}."
@@ -61,10 +65,9 @@ jobs:
       - name: Upload package
         run: |
           conan export --user=${{ steps.generate.outputs.user }} --channel=${{ steps.generate.outputs.channel }} .
-          conan upload --confirm --check --remote=${{ inputs.conan_remote_name }} xrpl/${{ steps.generate.outputs.version }}@${{ steps.generate.outputs.user }}/${{ steps.generate.outputs.channel }}
+          conan upload --confirm --check --remote=${{ inputs.conan_remote_name }} xrpl/${{ steps.conan_ref.outputs.conan_ref }}
     outputs:
-      channel: ${{ steps.generate.outputs.channel }}
+      conan_ref: ${{ steps.conan_ref.outputs.conan_ref }}
-      version: ${{ steps.generate.outputs.version }}
 
   notify:
     needs: upload
@@ -76,5 +79,5 @@ jobs:
         run: |
           gh api --method POST -H "Accept: application/vnd.github+json" -H "X-GitHub-Api-Version: 2022-11-28" \
           /repos/xrplf/clio/dispatches -f "event_type=check_libxrpl" \
-          -F "client_payload[version]=${{ needs.upload.outputs.version }}@${{ needs.upload.outputs.user }}/${{ needs.upload.outputs.channel }}" \
+          -F "client_payload[conan_ref]=${{ needs.upload.outputs.conan_ref }}" \
-          -F "client_payload[pr]=${{ github.event.pull_request.number }}"
+          -F "client_payload[pr_url]=${{ github.event.pull_request.html_url }}"

.github/workflows/on-pr.yml

@@ -75,6 +75,7 @@ jobs:
             tests/**
             CMakeLists.txt
             conanfile.py
+            conan.lock
       - name: Check whether to run
         # This step determines whether the rest of the workflow should
         # run. The rest of the workflow will run if this job runs AND at

.github/workflows/on-trigger.yml

@@ -32,6 +32,7 @@ on:
       - "tests/**"
       - "CMakeLists.txt"
       - "conanfile.py"
+      - "conan.lock"
 
   # Run at 06:32 UTC on every day of the week from Monday through Friday. This
   # will force all dependencies to be rebuilt, which is useful to verify that

.pre-commit-config.yaml

@@ -1,18 +1,5 @@
 # To run pre-commit hooks, first install pre-commit:
 # - `pip install pre-commit==${PRE_COMMIT_VERSION}`
-# - `pip install pre-commit-hooks==${PRE_COMMIT_HOOKS_VERSION}`
-#
-# Depending on your system, you can use `brew install` or `apt install` as well
-# for installing the pre-commit package, but `pip` is needed to install the
-# hooks; you can also use `pipx` if you prefer.
-# Next, install the required formatters:
-# - `pip install clang-format==${CLANG_VERSION}`
-# - `npm install prettier@${PRETTIER_VERSION}`
-#
-# See https://github.com/XRPLF/ci/blob/main/.github/workflows/tools-rippled.yml
-# for the versions used in the CI pipeline. You will need to have the exact same
-# versions of the tools installed on your system to produce the same results as
-# the pipeline.
 #
 # Then, run the following command to install the git hook scripts:
 # - `pre-commit install`
@@ -20,45 +7,33 @@
 # - `pre-commit run --all-files`
 # To manually run a specific hook, use:
 # - `pre-commit run <hook_id> --all-files`
-# To run the hooks against only the files changed in the current commit, use:
+# To run the hooks against only the staged files, use:
 # - `pre-commit run`
 repos:
-  - repo: local
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: 3e8a8703264a2f4a69428a0aa4dcb512790b2c8c # frozen: v6.0.0
+    hooks:
+      - id: trailing-whitespace
+      - id: end-of-file-fixer
+      - id: mixed-line-ending
+      - id: check-merge-conflict
+        args: [--assume-in-merge]
+
+  - repo: https://github.com/pre-commit/mirrors-clang-format
+    rev: 7d85583be209cb547946c82fbe51f4bc5dd1d017 # frozen: v18.1.8
     hooks:
       - id: clang-format
-        name: clang-format
+        args: [--style=file]
-        language: system
+        "types_or": [c++, c, proto]
-        entry: clang-format -i
+
-        files: '\.(cpp|hpp|h|ipp|proto)$'
+  - repo: https://github.com/rbubley/mirrors-prettier
-      - id: trailing-whitespace
+    rev: 5ba47274f9b181bce26a5150a725577f3c336011 # frozen: v3.6.2
-        name: trailing-whitespace
-        entry: trailing-whitespace-fixer
-        language: system
-        types: [text]
-      - id: end-of-file
-        name: end-of-file
-        entry: end-of-file-fixer
-        language: system
-        types: [text]
-      - id: mixed-line-ending
-        name: mixed-line-ending
-        entry: mixed-line-ending
-        language: system
-        types: [text]
-      - id: check-merge-conflict
-        name: check-merge-conflict
-        entry: check-merge-conflict --assume-in-merge
-        language: system
-        types: [text]
-  - repo: local
     hooks:
       - id: prettier
-        name: prettier
-        language: system
-        entry: prettier --ignore-unknown --write
 
 exclude: |
   (?x)^(
       external/.*|
-      .github/scripts/levelization/results/.*\.txt
+      .github/scripts/levelization/results/.*\.txt|
+      conan\.lock
   )$

BUILD.md

@@ -158,6 +158,10 @@ updated dependencies with the newer version. However, if we switch to a newer
 version that no longer requires a patch, no action is required on your part, as
 the new recipe will be automatically pulled from the official Conan Center.
 
+> [!NOTE]
+> You might need to add `--lockfile=""` to your `conan install` command
+> to avoid automatic use of the existing `conan.lock` file when you run `conan export` manually on your machine
+
 ### Conan profile tweaks
 
 #### Missing compiler version
@@ -466,6 +470,21 @@ tools.build:cxxflags=['-DBOOST_ASIO_DISABLE_CONCEPTS']
    The location of `rippled` binary in your build directory depends on your
    CMake generator. Pass `--help` to see the rest of the command line options.
 
+#### Conan lockfile
+
+To achieve reproducible dependencies, we use [Conan lockfile](https://docs.conan.io/2/tutorial/versioning/lockfiles.html).
+
+The `conan.lock` file in the repository contains a "snapshot" of the current dependencies.
+It is implicitly used when running `conan` commands, you don't need to specify it.
+
+You have to update this file every time you add a new dependency or change a revision or version of an existing dependency.
+
+To do that, run the following command in the repository root:
+
+```bash
+conan lock create . -o '&:jemalloc=True' -o '&:rocksdb=True'
+```
+
 ## Coverage report
 
 The coverage report is intended for developers using compilers GCC
@@ -564,7 +583,8 @@ After any updates or changes to dependencies, you may need to do the following:
    ```
 
 3. Re-run [conan export](#patched-recipes) if needed.
-4. Re-run [conan install](#build-and-test).
+4. [Regenerate lockfile](#conan-lockfile).
+5. Re-run [conan install](#build-and-test).
 
 ### `protobuf/port_def.inc` file not found
 

cmake/CodeCoverage.cmake

@@ -218,12 +218,12 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Clang)")
         set(COVERAGE_C_COMPILER_FLAGS "${COVERAGE_COMPILER_FLAGS} -fprofile-abs-path")
     endif()
 
-    check_cxx_compiler_flag(-fprofile-update HAVE_cxx_fprofile_update)
+    check_cxx_compiler_flag(-fprofile-update=atomic HAVE_cxx_fprofile_update)
     if(HAVE_cxx_fprofile_update)
         set(COVERAGE_CXX_COMPILER_FLAGS "${COVERAGE_COMPILER_FLAGS}  -fprofile-update=atomic")
     endif()
 
-    check_c_compiler_flag(-fprofile-update HAVE_c_fprofile_update)
+    check_c_compiler_flag(-fprofile-update=atomic HAVE_c_fprofile_update)
     if(HAVE_c_fprofile_update)
         set(COVERAGE_C_COMPILER_FLAGS "${COVERAGE_COMPILER_FLAGS} -fprofile-update=atomic")
     endif()

conan.lock

@@ -0,0 +1,56 @@
+{
+    "version": "0.5",
+    "requires": [
+        "zlib/1.3.1#b8bc2603263cf7eccbd6e17e66b0ed76%1756234269.497",
+        "xxhash/0.8.3#681d36a0a6111fc56e5e45ea182c19cc%1756234289.683",
+        "sqlite3/3.49.1#8631739a4c9b93bd3d6b753bac548a63%1756234266.869",
+        "soci/4.0.3#a9f8d773cd33e356b5879a4b0564f287%1756234262.318",
+        "snappy/1.1.10#968fef506ff261592ec30c574d4a7809%1756234314.246",
+        "rocksdb/10.0.1#85537f46e538974d67da0c3977de48ac%1756234304.347",
+        "re2/20230301#dfd6e2bf050eb90ddd8729cfb4c844a4%1756234257.976",
+        "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614",
+        "openssl/3.5.2#0c5a5e15ae569f45dff57adcf1770cf7%1756234259.61",
+        "nudb/2.0.9#c62cfd501e57055a7e0d8ee3d5e5427d%1756234237.107",
+        "lz4/1.10.0#59fc63cac7f10fbe8e05c7e62c2f3504%1756234228.999",
+        "libiconv/1.17#1e65319e945f2d31941a9d28cc13c058%1756223727.64",
+        "libbacktrace/cci.20210118#a7691bfccd8caaf66309df196790a5a1%1756230911.03",
+        "libarchive/3.8.1#5cf685686322e906cb42706ab7e099a8%1756234256.696",
+        "jemalloc/5.3.0#e951da9cf599e956cebc117880d2d9f8%1729241615.244",
+        "grpc/1.50.1#02291451d1e17200293a409410d1c4e1%1756234248.958",
+        "doctest/2.4.11#a4211dfc329a16ba9f280f9574025659%1756234220.819",
+        "date/3.0.4#f74bbba5a08fa388256688743136cb6f%1756234217.493",
+        "c-ares/1.34.5#b78b91e7cfb1f11ce777a285bbf169c6%1756234217.915",
+        "bzip2/1.0.8#00b4a4658791c1f06914e087f0e792f5%1756234261.716",
+        "boost/1.88.0#8852c0b72ce8271fb8ff7c53456d4983%1756223752.326",
+        "abseil/20230802.1#f0f91485b111dc9837a68972cb19ca7b%1756234220.907"
+    ],
+    "build_requires": [
+        "zlib/1.3.1#b8bc2603263cf7eccbd6e17e66b0ed76%1756234269.497",
+        "strawberryperl/5.32.1.1#707032463aa0620fa17ec0d887f5fe41%1756234281.733",
+        "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614",
+        "nasm/2.16.01#31e26f2ee3c4346ecd347911bd126904%1756234232.901",
+        "msys2/cci.latest#5b73b10144f73cc5bfe0572ed9be39e1%1751977009.857",
+        "m4/1.4.19#b38ced39a01e31fef5435bc634461fd2%1700758725.451",
+        "cmake/3.31.8#dde3bde00bb843687e55aea5afa0e220%1756234232.89",
+        "b2/5.3.3#107c15377719889654eb9a162a673975%1756234226.28",
+        "automake/1.16.5#b91b7c384c3deaa9d535be02da14d04f%1755524470.56",
+        "autoconf/2.71#51077f068e61700d65bb05541ea1e4b0%1731054366.86"
+    ],
+    "python_requires": [],
+    "overrides": {
+        "protobuf/3.21.12": [
+            null,
+            "protobuf/3.21.12"
+        ],
+        "lz4/1.9.4": [
+            "lz4/1.10.0"
+        ],
+        "boost/1.83.0": [
+            "boost/1.88.0"
+        ],
+        "sqlite3/3.44.2": [
+            "sqlite3/3.49.1"
+        ]
+    },
+    "config_requires": []
+}

include/xrpl/basics/Number.h

@@ -150,6 +150,24 @@ public:
         return (mantissa_ < 0) ? -1 : (mantissa_ ? 1 : 0);
     }
 
+    Number
+    truncate() const noexcept
+    {
+        if (exponent_ >= 0 || mantissa_ == 0)
+            return *this;
+
+        Number ret = *this;
+        while (ret.exponent_ < 0 && ret.mantissa_ != 0)
+        {
+            ret.exponent_ += 1;
+            ret.mantissa_ /= rep(10);
+        }
+        // We are guaranteed that normalize() will never throw an exception
+        // because exponent is either negative or zero at this point.
+        ret.normalize();
+        return ret;
+    }
+
     friend constexpr bool
     operator>(Number const& x, Number const& y) noexcept
     {

include/xrpl/protocol/Protocol.h

@@ -122,6 +122,13 @@ std::size_t constexpr maxDataPayloadLength = 256;
 /** Vault withdrawal policies */
 std::uint8_t constexpr vaultStrategyFirstComeFirstServe = 1;
 
+/** Default IOU scale factor for a Vault */
+std::uint8_t constexpr vaultDefaultIOUScale = 6;
+/** Maximum scale factor for a Vault. The number is chosen to ensure that
+1 IOU can be always converted to shares.
+10^19 > maxMPTokenAmount (2^64-1) > 10^18 */
+std::uint8_t constexpr vaultMaximumIOUScale = 18;
+
 /** Maximum recursion depth for vault shares being put as an asset inside
  * another vault; counted from 0 */
 std::uint8_t constexpr maxAssetCheckDepth = 5;

include/xrpl/protocol/detail/ledger_entries.macro

@@ -499,6 +499,7 @@ LEDGER_ENTRY(ltVAULT, 0x0084, Vault, vault, ({
     {sfLossUnrealized,       soeREQUIRED},
     {sfShareMPTID,           soeREQUIRED},
     {sfWithdrawalPolicy,     soeREQUIRED},
+    {sfScale,                soeDEFAULT},
     // no SharesTotal ever (use MPTIssuance.sfOutstandingAmount)
     // no PermissionedDomainID ever (use MPTIssuance.sfDomainID)
 }))

include/xrpl/protocol/detail/transactions.macro

@@ -483,6 +483,7 @@ TRANSACTION(ttVAULT_CREATE, 65, VaultCreate, Delegation::delegatable, ({
     {sfDomainID, soeOPTIONAL},
     {sfWithdrawalPolicy, soeOPTIONAL},
     {sfData, soeOPTIONAL},
+    {sfScale, soeOPTIONAL},
 }))
 
 /** This transaction updates a single asset vault. */

src/test/basics/Number_test.cpp

@@ -720,6 +720,30 @@ public:
         BEAST_EXPECT(res2 == STAmount{7518784});
     }
 
+    void
+    test_truncate()
+    {
+        BEAST_EXPECT(Number(25, +1).truncate() == Number(250, 0));
+        BEAST_EXPECT(Number(25, 0).truncate() == Number(25, 0));
+        BEAST_EXPECT(Number(25, -1).truncate() == Number(2, 0));
+        BEAST_EXPECT(Number(25, -2).truncate() == Number(0, 0));
+        BEAST_EXPECT(Number(99, -2).truncate() == Number(0, 0));
+
+        BEAST_EXPECT(Number(-25, +1).truncate() == Number(-250, 0));
+        BEAST_EXPECT(Number(-25, 0).truncate() == Number(-25, 0));
+        BEAST_EXPECT(Number(-25, -1).truncate() == Number(-2, 0));
+        BEAST_EXPECT(Number(-25, -2).truncate() == Number(0, 0));
+        BEAST_EXPECT(Number(-99, -2).truncate() == Number(0, 0));
+
+        BEAST_EXPECT(Number(0, 0).truncate() == Number(0, 0));
+        BEAST_EXPECT(Number(0, 30000).truncate() == Number(0, 0));
+        BEAST_EXPECT(Number(0, -30000).truncate() == Number(0, 0));
+        BEAST_EXPECT(Number(100, -30000).truncate() == Number(0, 0));
+        BEAST_EXPECT(Number(100, -30000).truncate() == Number(0, 0));
+        BEAST_EXPECT(Number(-100, -30000).truncate() == Number(0, 0));
+        BEAST_EXPECT(Number(-100, -30000).truncate() == Number(0, 0));
+    }
+
     void
     run() override
     {
@@ -740,6 +764,7 @@ public:
         test_stream();
         test_inc_dec();
         test_toSTAmount();
+        test_truncate();
     }
 };
 

src/test/core/Coroutine_test.cpp

@@ -175,12 +175,78 @@ public:
         BEAST_EXPECT(*lv == -1);
     }
 
+    void
+    stopJobQueueWhenCoroutineSuspended()
+    {
+        using namespace std::chrono_literals;
+        using namespace jtx;
+
+        testcase("Stop JobQueue when a coroutine is suspended");
+
+        Env env(*this, envconfig([](std::unique_ptr<Config> cfg) {
+            cfg->FORCE_MULTI_THREAD = true;
+            return cfg;
+        }));
+
+        bool started = false;
+        bool finished = false;
+        std::optional<bool> shouldStop;
+        std::condition_variable cv;
+        std::mutex m;
+        std::unique_lock<std::mutex> lk(m);
+        auto coro = env.app().getJobQueue().postCoro(
+            jtCLIENT, "Coroutine-Test", [&](auto const& c) {
+                started = true;
+                cv.notify_all();
+                c->yield();
+                finished = true;
+                shouldStop = c->shouldStop();
+                cv.notify_all();
+            });
+
+        cv.wait_for(lk, 5s, [&]() { return started; });
+        env.app().getJobQueue().stop();
+
+        cv.wait_for(lk, 5s, [&]() { return finished; });
+        BEAST_EXPECT(finished);
+        BEAST_EXPECT(shouldStop.has_value() && *shouldStop == true);
+    }
+
+    void
+    coroutineGetsDestroyedBeforeExecuting()
+    {
+        using namespace std::chrono_literals;
+        using namespace jtx;
+
+        testcase("Coroutine gets destroyed before executing");
+
+        Env env(*this, envconfig([](std::unique_ptr<Config> cfg) {
+            cfg->FORCE_MULTI_THREAD = true;
+            return cfg;
+        }));
+
+        {
+            auto coro = std::make_shared<JobQueue::Coro>(
+                Coro_create_t{},
+                env.app().getJobQueue(),
+                JobType::jtCLIENT,
+                "test",
+                [](auto coro) {
+
+                });
+        }
+
+        pass();
+    }
+
     void
     run() override
     {
         correct_order();
         incorrect_order();
         thread_specific_storage();
+        stopJobQueueWhenCoroutineSuspended();
+        coroutineGetsDestroyedBeforeExecuting();
     }
 };
 

src/test/jtx/Account.h

@@ -74,6 +74,10 @@ public:
 
     /** @} */
 
+    /** Create an Account from an account ID. Should only be used when the
+     * secret key is unavailable, such as for pseudo-accounts. */
+    explicit Account(std::string name, AccountID const& id);
+
     enum AcctStringType { base58Seed, other };
     /** Create an account from a base58 seed string.  Throws on invalid seed. */
     Account(AcctStringType stringType, std::string base58SeedStr);

src/test/jtx/impl/Account.cpp

@@ -86,6 +86,14 @@ Account::Account(AcctStringType stringType, std::string base58SeedStr)
 {
 }
 
+Account::Account(std::string name, AccountID const& id)
+    : Account(name, randomKeyPair(KeyType::secp256k1), privateCtorTag{})
+{
+    // override the randomly generated values
+    id_ = id;
+    human_ = toBase58(id_);
+}
+
 IOU
 Account::operator[](std::string const& s) const
 {

src/xrpld/app/paths/Pathfinder.h

@@ -166,7 +166,7 @@ private:
         int addFlags,
         std::function<bool(void)> const& continueCallback);
 
-    // Compute the liquidity for a path.  Return tesSUCCESS if it has has enough
+    // Compute the liquidity for a path.  Return tesSUCCESS if it has enough
     // liquidity to be worth keeping, otherwise an error.
     TER
     getPathLiquidity(

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

@@ -1510,6 +1510,12 @@ ValidMPTIssuance::finalize(
 
         if (tx.getTxnType() == ttESCROW_FINISH)
             return true;
+
+        if ((tx.getTxnType() == ttVAULT_CLAWBACK ||
+             tx.getTxnType() == ttVAULT_WITHDRAW) &&
+            mptokensDeleted_ == 1 && mptokensCreated_ == 0 &&
+            mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 0)
+            return true;
     }
 
     if (mptIssuancesCreated_ != 0)

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

@@ -21,8 +21,10 @@
 #include <xrpld/ledger/View.h>
 
 #include <xrpl/beast/utility/instrumentation.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>
@@ -151,7 +153,7 @@ VaultClawback::doApply()
     if (!vault)
         return tefINTERNAL;  // LCOV_EXCL_LINE
 
-    auto const mptIssuanceID = (*vault)[sfShareMPTID];
+    auto const mptIssuanceID = *((*vault)[sfShareMPTID]);
     auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID));
     if (!sleIssuance)
     {
@@ -161,68 +163,169 @@ VaultClawback::doApply()
         // LCOV_EXCL_STOP
     }
 
-    Asset const asset = vault->at(sfAsset);
+    Asset const vaultAsset = vault->at(sfAsset);
     STAmount const amount = [&]() -> STAmount {
         auto const maybeAmount = tx[~sfAmount];
         if (maybeAmount)
             return *maybeAmount;
-        return {sfAmount, asset, 0};
+        return {sfAmount, vaultAsset, 0};
     }();
     XRPL_ASSERT(
-        amount.asset() == asset,
+        amount.asset() == vaultAsset,
         "ripple::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),
+        "ripple::VaultClawback::doApply : loss and assets do balance");
+
     AccountID holder = tx[sfHolder];
-    STAmount assets, shares;
+    MPTIssue const share{mptIssuanceID};
+    STAmount sharesDestroyed = {share};
+    STAmount assetsRecovered;
+    try
+    {
         if (amount == beast::zero)
         {
-        Asset share = *(*vault)[sfShareMPTID];
+            sharesDestroyed = accountHolds(
-        shares = accountHolds(
                 view(),
                 holder,
                 share,
                 FreezeHandling::fhIGNORE_FREEZE,
                 AuthHandling::ahIGNORE_AUTH,
                 j_);
-        assets = sharesToAssetsWithdraw(vault, sleIssuance, shares);
+
+            auto const maybeAssets =
+                sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed);
+            if (!maybeAssets)
+                return tecINTERNAL;  // LCOV_EXCL_LINE
+            assetsRecovered = *maybeAssets;
         }
         else
         {
-        assets = amount;
+            assetsRecovered = amount;
-        shares = assetsToSharesWithdraw(vault, sleIssuance, assets);
+            {
+                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.
-    Number maxAssets = *vault->at(sfAssetsAvailable);
+        if (assetsRecovered > *assetsAvailable)
-    if (assets > maxAssets)
         {
-        assets = maxAssets;
+            assetsRecovered = *assetsAvailable;
-        shares = assetsToSharesWithdraw(vault, sleIssuance, assets);
+            // 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;
             }
 
-    if (shares == beast::zero)
+            auto const maybeAssets =
-        return tecINSUFFICIENT_FUNDS;
+                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
+            }
+        }
+    }
+    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=" << sleIssuance->at(sfOutstandingAmount)
+            << ", amount=" << amount.value();
+        return tecPATH_DRY;
+    }
 
-    vault->at(sfAssetsTotal) -= assets;
+    if (sharesDestroyed == beast::zero)
-    vault->at(sfAssetsAvailable) -= assets;
+        return tecPRECISION_LOSS;
+
+    assetsTotal -= assetsRecovered;
+    assetsAvailable -= assetsRecovered;
     view().update(vault);
 
     auto const& vaultAccount = vault->at(sfAccount);
     // Transfer shares from holder to vault.
-    if (auto ter = accountSend(
+    if (auto const ter = accountSend(
-            view(), holder, vaultAccount, shares, j_, WaiveTransferFee::Yes))
+            view(),
+            holder,
+            vaultAccount,
+            sharesDestroyed,
+            j_,
+            WaiveTransferFee::Yes);
+        !isTesSuccess(ter))
         return ter;
 
+    // Try to remove MPToken for shares, if the holder balance is zero. Vault
+    // pseudo-account will never set lsfMPTAuthorized, so we ignore flags.
+    // Keep MPToken if holder is the vault owner.
+    if (holder != vault->at(sfOwner))
+    {
+        if (auto const ter =
+                removeEmptyHolding(view(), holder, sharesDestroyed.asset(), j_);
+            isTesSuccess(ter))
+        {
+            JLOG(j_.debug())  //
+                << "VaultClawback: removed empty MPToken for vault shares"
+                << " MPTID=" << to_string(mptIssuanceID)  //
+                << " account=" << toBase58(holder);
+        }
+        else if (ter != tecHAS_OBLIGATIONS)
+        {
+            // LCOV_EXCL_START
+            JLOG(j_.error())  //
+                << "VaultClawback: failed to remove MPToken for vault shares"
+                << " MPTID=" << to_string(mptIssuanceID)  //
+                << " account=" << toBase58(holder)        //
+                << " with result: " << transToken(ter);
+            return ter;
+            // LCOV_EXCL_STOP
+        }
+        // else quietly ignore, holder balance is not zero
+    }
+
     // Transfer assets from vault to issuer.
-    if (auto ter = accountSend(
+    if (auto const ter = accountSend(
-            view(), vaultAccount, account_, assets, j_, WaiveTransferFee::Yes))
+            view(),
+            vaultAccount,
+            account_,
+            assetsRecovered,
+            j_,
+            WaiveTransferFee::Yes);
+        !isTesSuccess(ter))
         return ter;
 
     // Sanity check
     if (accountHolds(
             view(),
             vaultAccount,
-            assets.asset(),
+            assetsRecovered.asset(),
             FreezeHandling::fhIGNORE_FREEZE,
             AuthHandling::ahIGNORE_AUTH,
             j_) < beast::zero)

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

@@ -25,8 +25,10 @@
 #include <xrpl/protocol/Asset.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/Issue.h>
 #include <xrpl/protocol/MPTIssue.h>
 #include <xrpl/protocol/Protocol.h>
+#include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STNumber.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
@@ -84,6 +86,16 @@ VaultCreate::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
+    if (auto const scale = ctx.tx[~sfScale])
+    {
+        auto const vaultAsset = ctx.tx[sfAsset];
+        if (vaultAsset.holds<MPTIssue>() || vaultAsset.native())
+            return temMALFORMED;
+
+        if (scale > vaultMaximumIOUScale)
+            return temMALFORMED;
+    }
+
     return preflight2(ctx);
 }
 
@@ -97,8 +109,8 @@ VaultCreate::calculateBaseFee(ReadView const& view, STTx const& tx)
 TER
 VaultCreate::preclaim(PreclaimContext const& ctx)
 {
-    auto vaultAsset = ctx.tx[sfAsset];
+    auto const vaultAsset = ctx.tx[sfAsset];
-    auto account = ctx.tx[sfAccount];
+    auto const account = ctx.tx[sfAccount];
 
     if (vaultAsset.native())
         ;  // No special checks for XRP
@@ -148,7 +160,7 @@ VaultCreate::preclaim(PreclaimContext const& ctx)
             return tecOBJECT_NOT_FOUND;
     }
 
-    auto sequence = ctx.tx.getSeqValue();
+    auto const sequence = ctx.tx.getSeqValue();
     if (auto const accountId = pseudoAccountAddress(
             ctx.view, keylet::vault(account, sequence).key);
         accountId == beast::zero)
@@ -165,8 +177,8 @@ VaultCreate::doApply()
     // we can consider downgrading them to `tef` or `tem`.
 
     auto const& tx = ctx_.tx;
-    auto sequence = tx.getSeqValue();
+    auto const sequence = tx.getSeqValue();
-    auto owner = view().peek(keylet::account(account_));
+    auto const owner = view().peek(keylet::account(account_));
     if (owner == nullptr)
         return tefINTERNAL;  // LCOV_EXCL_LINE
 
@@ -190,6 +202,10 @@ VaultCreate::doApply()
         !isTesSuccess(ter))
         return ter;
 
+    std::uint8_t const scale = (asset.holds<MPTIssue>() || asset.native())
+        ? 0
+        : ctx_.tx[~sfScale].value_or(vaultDefaultIOUScale);
+
     auto txFlags = tx.getFlags();
     std::uint32_t mptFlags = 0;
     if ((txFlags & tfVaultShareNonTransferable) == 0)
@@ -209,12 +225,13 @@ VaultCreate::doApply()
             .account = pseudoId->value(),
             .sequence = 1,
             .flags = mptFlags,
+            .assetScale = scale,
             .metadata = tx[~sfMPTokenMetadata],
             .domainId = tx[~sfDomainID],
         });
     if (!maybeShare)
         return maybeShare.error();  // LCOV_EXCL_LINE
-    auto& share = *maybeShare;
+    auto const& mptIssuanceID = *maybeShare;
 
     vault->setFieldIssue(sfAsset, STIssue{sfAsset, asset});
     vault->at(sfFlags) = txFlags & tfVaultPrivate;
@@ -227,7 +244,7 @@ VaultCreate::doApply()
     // Leave default values for AssetTotal and AssetAvailable, both zero.
     if (auto value = tx[~sfAssetsMaximum])
         vault->at(sfAssetsMaximum) = *value;
-    vault->at(sfShareMPTID) = share;
+    vault->at(sfShareMPTID) = mptIssuanceID;
     if (auto value = tx[~sfData])
         vault->at(sfData) = *value;
     // Required field, default to vaultStrategyFirstComeFirstServe
@@ -235,9 +252,31 @@ VaultCreate::doApply()
         vault->at(sfWithdrawalPolicy) = *value;
     else
         vault->at(sfWithdrawalPolicy) = vaultStrategyFirstComeFirstServe;
-    // No `LossUnrealized`.
+    if (scale)
+        vault->at(sfScale) = scale;
     view().insert(vault);
 
+    // Explicitly create MPToken for the vault owner
+    if (auto const err = authorizeMPToken(
+            view(), mPriorBalance, mptIssuanceID, account_, ctx_.journal);
+        !isTesSuccess(err))
+        return err;
+
+    // If the vault is private, set the authorized flag for the vault owner
+    if (txFlags & tfVaultPrivate)
+    {
+        if (auto const err = authorizeMPToken(
+                view(),
+                mPriorBalance,
+                mptIssuanceID,
+                pseudoId,
+                ctx_.journal,
+                {},
+                account_);
+            !isTesSuccess(err))
+            return err;
+    }
+
     return tesSUCCESS;
 }
 

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

@@ -21,6 +21,7 @@
 #include <xrpld/ledger/View.h>
 
 #include <xrpl/protocol/Feature.h>
+#include <xrpl/protocol/MPTIssue.h>
 #include <xrpl/protocol/STNumber.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
@@ -128,7 +129,8 @@ VaultDelete::doApply()
 
     // Destroy the share issuance. Do not use MPTokenIssuanceDestroy for this,
     // no special logic needed. First run few checks, duplicated from preclaim.
-    auto const mpt = view().peek(keylet::mptIssuance(vault->at(sfShareMPTID)));
+    auto const shareMPTID = *vault->at(sfShareMPTID);
+    auto const mpt = view().peek(keylet::mptIssuance(shareMPTID));
     if (!mpt)
     {
         // LCOV_EXCL_START
@@ -137,6 +139,24 @@ VaultDelete::doApply()
         // LCOV_EXCL_STOP
     }
 
+    // Try to remove MPToken for vault shares for the vault owner if it exists.
+    if (auto const mptoken = view().peek(keylet::mptoken(shareMPTID, account_)))
+    {
+        if (auto const ter =
+                removeEmptyHolding(view(), account_, MPTIssue(shareMPTID), j_);
+            !isTesSuccess(ter))
+        {
+            // LCOV_EXCL_START
+            JLOG(j_.error())  //
+                << "VaultDelete: failed to remove vault owner's MPToken"
+                << " MPTID=" << to_string(shareMPTID)  //
+                << " account=" << toBase58(account_)   //
+                << " with result: " << transToken(ter);
+            return ter;
+            // LCOV_EXCL_STOP
+        }
+    }
+
     if (!view().dirRemove(
             keylet::ownerDir(pseudoID), (*mpt)[sfOwnerNode], mpt->key(), false))
     {

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

@@ -26,6 +26,7 @@
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/MPTIssue.h>
+#include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STNumber.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
@@ -138,7 +139,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
     if (isFrozen(ctx.view, account, vaultShare))
         return tecLOCKED;
 
-    if (vault->isFlag(tfVaultPrivate) && account != vault->at(sfOwner))
+    if (vault->isFlag(lsfVaultPrivate) && account != vault->at(sfOwner))
     {
         auto const maybeDomainID = sleIssuance->at(~sfDomainID);
         // Since this is a private vault and the account is not its owner, we
@@ -183,7 +184,7 @@ VaultDeposit::doApply()
     if (!vault)
         return tefINTERNAL;  // LCOV_EXCL_LINE
 
-    auto const assets = ctx_.tx[sfAmount];
+    auto const amount = ctx_.tx[sfAmount];
     // Make sure the depositor can hold shares.
     auto const mptIssuanceID = (*vault)[sfShareMPTID];
     auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID));
@@ -197,14 +198,14 @@ VaultDeposit::doApply()
 
     auto const& vaultAccount = vault->at(sfAccount);
     // Note, vault owner is always authorized
-    if ((vault->getFlags() & tfVaultPrivate) && account_ != vault->at(sfOwner))
+    if (vault->isFlag(lsfVaultPrivate) && account_ != vault->at(sfOwner))
     {
         if (auto const err = enforceMPTokenAuthorization(
                 ctx_.view(), mptIssuanceID, account_, mPriorBalance, j_);
             !isTesSuccess(err))
             return err;
     }
-    else
+    else  // !vault->isFlag(lsfVaultPrivate) || account_ == vault->at(sfOwner)
     {
         // No authorization needed, but must ensure there is MPToken
         auto sleMpt = view().read(keylet::mptoken(mptIssuanceID, account_));
@@ -221,8 +222,12 @@ VaultDeposit::doApply()
         }
 
         // If the vault is private, set the authorized flag for the vault owner
-        if (vault->isFlag(tfVaultPrivate))
+        if (vault->isFlag(lsfVaultPrivate))
         {
+            // This follows from the reverse of the outer enclosing if condition
+            XRPL_ASSERT(
+                account_ == vault->at(sfOwner),
+                "ripple::VaultDeposit::doApply : account is owner");
             if (auto const err = authorizeMPToken(
                     view(),
                     mPriorBalance,              // priorBalance
@@ -237,14 +242,52 @@ VaultDeposit::doApply()
         }
     }
 
+    STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited;
+    try
+    {
         // Compute exchange before transferring any amounts.
-    auto const shares = assetsToSharesDeposit(vault, sleIssuance, assets);
+        {
+            auto const maybeShares =
+                assetsToSharesDeposit(vault, sleIssuance, amount);
+            if (!maybeShares)
+                return tecINTERNAL;  // LCOV_EXCL_LINE
+            sharesCreated = *maybeShares;
+        }
+        if (sharesCreated == beast::zero)
+            return tecPRECISION_LOSS;
+
+        auto const maybeAssets =
+            sharesToAssetsDeposit(vault, sleIssuance, sharesCreated);
+        if (!maybeAssets)
+            return tecINTERNAL;  // LCOV_EXCL_LINE
+        else if (*maybeAssets > amount)
+        {
+            // LCOV_EXCL_START
+            JLOG(j_.error()) << "VaultDeposit: would take more than offered.";
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
+        assetsDeposited = *maybeAssets;
+    }
+    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())  //
+            << "VaultDeposit: overflow error with"
+            << " scale=" << (int)vault->at(sfScale).value()  //
+            << ", assetsTotal=" << vault->at(sfAssetsTotal).value()
+            << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount)
+            << ", amount=" << amount;
+        return tecPATH_DRY;
+    }
+
     XRPL_ASSERT(
-        shares.asset() != assets.asset(),
+        sharesCreated.asset() != assetsDeposited.asset(),
         "ripple::VaultDeposit::doApply : assets are not shares");
 
-    vault->at(sfAssetsTotal) += assets;
+    vault->at(sfAssetsTotal) += assetsDeposited;
-    vault->at(sfAssetsAvailable) += assets;
+    vault->at(sfAssetsAvailable) += assetsDeposited;
     view().update(vault);
 
     // A deposit must not push the vault over its limit.
@@ -253,15 +296,21 @@ VaultDeposit::doApply()
         return tecLIMIT_EXCEEDED;
 
     // Transfer assets from depositor to vault.
-    if (auto ter = accountSend(
+    if (auto const ter = accountSend(
-            view(), account_, vaultAccount, assets, j_, WaiveTransferFee::Yes))
+            view(),
+            account_,
+            vaultAccount,
+            assetsDeposited,
+            j_,
+            WaiveTransferFee::Yes);
+        !isTesSuccess(ter))
         return ter;
 
     // Sanity check
     if (accountHolds(
             view(),
             account_,
-            assets.asset(),
+            assetsDeposited.asset(),
             FreezeHandling::fhIGNORE_FREEZE,
             AuthHandling::ahIGNORE_AUTH,
             j_) < beast::zero)
@@ -273,8 +322,14 @@ VaultDeposit::doApply()
     }
 
     // Transfer shares from vault to depositor.
-    if (auto ter = accountSend(
+    if (auto const ter = accountSend(
-            view(), vaultAccount, account_, shares, j_, WaiveTransferFee::Yes))
+            view(),
+            vaultAccount,
+            account_,
+            sharesCreated,
+            j_,
+            WaiveTransferFee::Yes);
+        !isTesSuccess(ter))
         return ter;
 
     return tesSUCCESS;

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

@@ -108,7 +108,7 @@ VaultSet::preclaim(PreclaimContext const& ctx)
     if (auto const domain = ctx.tx[~sfDomainID])
     {
         // We can only set domain if private flag was originally set
-        if ((vault->getFlags() & tfVaultPrivate) == 0)
+        if (!vault->isFlag(lsfVaultPrivate))
         {
             JLOG(ctx.j.debug()) << "VaultSet: vault is not private";
             return tecNO_PERMISSION;
@@ -175,9 +175,9 @@ VaultSet::doApply()
     {
         if (*domainId != beast::zero)
         {
-            // In VaultSet::preclaim we enforce that tfVaultPrivate must have
+            // In VaultSet::preclaim we enforce that lsfVaultPrivate must have
             // been set in the vault. We currently do not support making such a
-            // vault public (i.e. removal of tfVaultPrivate flag). The
+            // vault public (i.e. removal of lsfVaultPrivate flag). The
             // sfDomainID flag must be set in the MPTokenIssuance object and can
             // be freely updated.
             sleIssuance->setFieldH256(sfDomainID, *domainId);

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

@@ -177,7 +177,7 @@ VaultWithdraw::doApply()
     if (!vault)
         return tefINTERNAL;  // LCOV_EXCL_LINE
 
-    auto const mptIssuanceID = (*vault)[sfShareMPTID];
+    auto const mptIssuanceID = *((*vault)[sfShareMPTID]);
     auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID));
     if (!sleIssuance)
     {
@@ -192,24 +192,57 @@ VaultWithdraw::doApply()
     // to deposit into it, and this means you are also indefinitely authorized
     // to withdraw from it.
 
-    auto amount = ctx_.tx[sfAmount];
+    auto const amount = ctx_.tx[sfAmount];
-    auto const asset = vault->at(sfAsset);
+    Asset const vaultAsset = vault->at(sfAsset);
-    auto const share = MPTIssue(mptIssuanceID);
+    MPTIssue const share{mptIssuanceID};
-    STAmount shares, assets;
+    STAmount sharesRedeemed = {share};
-    if (amount.asset() == asset)
+    STAmount assetsWithdrawn;
+    try
+    {
+        if (amount.asset() == vaultAsset)
         {
             // Fixed assets, variable shares.
-        assets = amount;
+            {
-        shares = assetsToSharesWithdraw(vault, sleIssuance, assets);
+                auto const maybeShares =
+                    assetsToSharesWithdraw(vault, sleIssuance, amount);
+                if (!maybeShares)
+                    return tecINTERNAL;  // LCOV_EXCL_LINE
+                sharesRedeemed = *maybeShares;
+            }
+
+            if (sharesRedeemed == beast::zero)
+                return tecPRECISION_LOSS;
+            auto const maybeAssets =
+                sharesToAssetsWithdraw(vault, sleIssuance, sharesRedeemed);
+            if (!maybeAssets)
+                return tecINTERNAL;  // LCOV_EXCL_LINE
+            assetsWithdrawn = *maybeAssets;
         }
         else if (amount.asset() == share)
         {
             // Fixed shares, variable assets.
-        shares = amount;
+            sharesRedeemed = amount;
-        assets = sharesToAssetsWithdraw(vault, sleIssuance, shares);
+            auto const maybeAssets =
+                sharesToAssetsWithdraw(vault, sleIssuance, sharesRedeemed);
+            if (!maybeAssets)
+                return tecINTERNAL;  // LCOV_EXCL_LINE
+            assetsWithdrawn = *maybeAssets;
         }
         else
             return tefINTERNAL;  // LCOV_EXCL_LINE
+    }
+    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())  //
+            << "VaultWithdraw: 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 (accountHolds(
             view(),
@@ -217,31 +250,72 @@ VaultWithdraw::doApply()
             share,
             FreezeHandling::fhZERO_IF_FROZEN,
             AuthHandling::ahIGNORE_AUTH,
-            j_) < shares)
+            j_) < sharesRedeemed)
     {
         JLOG(j_.debug()) << "VaultWithdraw: account doesn't hold enough shares";
         return tecINSUFFICIENT_FUNDS;
     }
 
-    // The vault must have enough assets on hand. The vault may hold assets that
+    auto assetsAvailable = vault->at(sfAssetsAvailable);
-    // it has already pledged. That is why we look at AssetAvailable instead of
+    auto assetsTotal = vault->at(sfAssetsTotal);
-    // the pseudo-account balance.
+    [[maybe_unused]] auto const lossUnrealized = vault->at(sfLossUnrealized);
-    if (*vault->at(sfAssetsAvailable) < assets)
+    XRPL_ASSERT(
+        lossUnrealized <= (assetsTotal - assetsAvailable),
+        "ripple::VaultWithdraw::doApply : loss and assets do balance");
+
+    // The vault must have enough assets on hand. The vault may hold assets
+    // that it has already pledged. That is why we look at AssetAvailable
+    // instead of the pseudo-account balance.
+    if (*assetsAvailable < assetsWithdrawn)
     {
         JLOG(j_.debug()) << "VaultWithdraw: vault doesn't hold enough assets";
         return tecINSUFFICIENT_FUNDS;
     }
 
-    vault->at(sfAssetsTotal) -= assets;
+    assetsTotal -= assetsWithdrawn;
-    vault->at(sfAssetsAvailable) -= assets;
+    assetsAvailable -= assetsWithdrawn;
     view().update(vault);
 
     auto const& vaultAccount = vault->at(sfAccount);
     // Transfer shares from depositor to vault.
-    if (auto ter = accountSend(
+    if (auto const ter = accountSend(
-            view(), account_, vaultAccount, shares, j_, WaiveTransferFee::Yes))
+            view(),
+            account_,
+            vaultAccount,
+            sharesRedeemed,
+            j_,
+            WaiveTransferFee::Yes);
+        !isTesSuccess(ter))
         return ter;
 
+    // Try to remove MPToken for shares, if the account balance is zero. Vault
+    // pseudo-account will never set lsfMPTAuthorized, so we ignore flags.
+    // Keep MPToken if holder is the vault owner.
+    if (account_ != vault->at(sfOwner))
+    {
+        if (auto const ter = removeEmptyHolding(
+                view(), account_, sharesRedeemed.asset(), j_);
+            isTesSuccess(ter))
+        {
+            JLOG(j_.debug())  //
+                << "VaultWithdraw: removed empty MPToken for vault shares"
+                << " MPTID=" << to_string(mptIssuanceID)  //
+                << " account=" << toBase58(account_);
+        }
+        else if (ter != tecHAS_OBLIGATIONS)
+        {
+            // LCOV_EXCL_START
+            JLOG(j_.error())  //
+                << "VaultWithdraw: failed to remove MPToken for vault shares"
+                << " MPTID=" << to_string(mptIssuanceID)  //
+                << " account=" << toBase58(account_)      //
+                << " with result: " << transToken(ter);
+            return ter;
+            // LCOV_EXCL_STOP
+        }
+        // else quietly ignore, account balance is not zero
+    }
+
     auto const dstAcct = [&]() -> AccountID {
         if (ctx_.tx.isFieldPresent(sfDestination))
             return ctx_.tx.getAccountID(sfDestination);
@@ -249,15 +323,21 @@ VaultWithdraw::doApply()
     }();
 
     // Transfer assets from vault to depositor or destination account.
-    if (auto ter = accountSend(
+    if (auto const ter = accountSend(
-            view(), vaultAccount, dstAcct, assets, j_, WaiveTransferFee::Yes))
+            view(),
+            vaultAccount,
+            dstAcct,
+            assetsWithdrawn,
+            j_,
+            WaiveTransferFee::Yes);
+        !isTesSuccess(ter))
         return ter;
 
     // Sanity check
     if (accountHolds(
             view(),
             vaultAccount,
-            assets.asset(),
+            assetsWithdrawn.asset(),
             FreezeHandling::fhIGNORE_FREEZE,
             AuthHandling::ahIGNORE_AUTH,
             j_) < beast::zero)

src/xrpld/core/Coro.ipp

@@ -34,17 +34,21 @@ JobQueue::Coro::Coro(
     : jq_(jq)
     , type_(type)
     , name_(name)
-    , running_(false)
     , coro_(
           [this, fn = std::forward<F>(f)](
               boost::coroutines::asymmetric_coroutine<void>::push_type&
                   do_yield) {
               yield_ = &do_yield;
               yield();
-              fn(shared_from_this());
+              // self makes Coro alive until this function returns
-#ifndef NDEBUG
+              std::shared_ptr<Coro> self;
-              finished_ = true;
+              if (!shouldStop())
-#endif
+              {
+                  self = shared_from_this();
+                  fn(self);
+              }
+              state_ = CoroState::Finished;
+              cv_.notify_all();
           },
           boost::coroutines::attributes(megabytes(1)))
 {
@@ -52,17 +56,36 @@ JobQueue::Coro::Coro(
 
 inline JobQueue::Coro::~Coro()
 {
+    XRPL_ASSERT(
+        state_ != CoroState::Running,
+        "ripple::JobQueue::Coro::~Coro : is not running");
+    exiting_ = true;
+    // Resume the coroutine so that it has a chance to clean things up
+    if (state_ == CoroState::Suspended)
+    {
+        resume();
+    }
+
 #ifndef NDEBUG
-    XRPL_ASSERT(finished_, "ripple::JobQueue::Coro::~Coro : is finished");
+    XRPL_ASSERT(
+        state_ == CoroState::Finished,
+        "ripple::JobQueue::Coro::~Coro : is finished");
 #endif
 }
 
 inline void
-JobQueue::Coro::yield() const
+JobQueue::Coro::yield()
 {
     {
         std::lock_guard lock(jq_.m_mutex);
+        if (shouldStop())
+        {
+            return;
+        }
+        state_ = CoroState::Suspended;
         ++jq_.nSuspend_;
+        jq_.m_suspendedCoros[this] = weak_from_this();
+        jq_.cv_.notify_all();
     }
     (*yield_)();
 }
@@ -70,11 +93,6 @@ JobQueue::Coro::yield() const
 inline bool
 JobQueue::Coro::post()
 {
-    {
-        std::lock_guard lk(mutex_run_);
-        running_ = true;
-    }
-
     // sp keeps 'this' alive
     if (jq_.addJob(
             type_, name_, [this, sp = shared_from_this()]() { resume(); }))
@@ -82,9 +100,6 @@ JobQueue::Coro::post()
         return true;
     }
 
-    // The coroutine will not run.  Clean up running_.
-    std::lock_guard lk(mutex_run_);
-    running_ = false;
     cv_.notify_all();
     return false;
 }
@@ -94,11 +109,17 @@ JobQueue::Coro::resume()
 {
     {
         std::lock_guard lk(mutex_run_);
-        running_ = true;
+        if (state_ != CoroState::Suspended)
+        {
+            return;
+        }
+        state_ = CoroState::Running;
     }
     {
         std::lock_guard lock(jq_.m_mutex);
+        jq_.m_suspendedCoros.erase(this);
         --jq_.nSuspend_;
+        jq_.cv_.notify_all();
     }
     auto saved = detail::getLocalValues().release();
     detail::getLocalValues().reset(&lvs_);
@@ -109,9 +130,6 @@ JobQueue::Coro::resume()
     coro_();
     detail::getLocalValues().release();
     detail::getLocalValues().reset(saved);
-    std::lock_guard lk(mutex_run_);
-    running_ = false;
-    cv_.notify_all();
 }
 
 inline bool
@@ -120,32 +138,11 @@ JobQueue::Coro::runnable() const
     return static_cast<bool>(coro_);
 }
 
-inline void
-JobQueue::Coro::expectEarlyExit()
-{
-#ifndef NDEBUG
-    if (!finished_)
-#endif
-    {
-        // expectEarlyExit() must only ever be called from outside the
-        // Coro's stack.  It you're inside the stack you can simply return
-        // and be done.
-        //
-        // That said, since we're outside the Coro's stack, we need to
-        // decrement the nSuspend that the Coro's call to yield caused.
-        std::lock_guard lock(jq_.m_mutex);
-        --jq_.nSuspend_;
-#ifndef NDEBUG
-        finished_ = true;
-#endif
-    }
-}
-
 inline void
 JobQueue::Coro::join()
 {
     std::unique_lock<std::mutex> lk(mutex_run_);
-    cv_.wait(lk, [this]() { return running_ == false; });
+    cv_.wait(lk, [this]() { return state_ != CoroState::Running; });
 }
 
 }  // namespace ripple

src/xrpld/core/JobQueue.h

@@ -60,20 +60,22 @@ public:
     /** Coroutines must run to completion. */
     class Coro : public std::enable_shared_from_this<Coro>
     {
+        friend class JobQueue;
+
     private:
+        enum class CoroState { None, Suspended, Running, Finished };
+
+        std::atomic_bool exiting_ = false;
         detail::LocalValues lvs_;
         JobQueue& jq_;
         JobType type_;
         std::string name_;
-        bool running_;
+        std::atomic<CoroState> state_ = CoroState::None;
         std::mutex mutex_;
         std::mutex mutex_run_;
         std::condition_variable cv_;
         boost::coroutines::asymmetric_coroutine<void>::pull_type coro_;
         boost::coroutines::asymmetric_coroutine<void>::push_type* yield_;
-#ifndef NDEBUG
-        bool finished_ = false;
-#endif
 
     public:
         // Private: Used in the implementation
@@ -97,7 +99,7 @@ public:
            post.
         */
         void
-        yield() const;
+        yield();
 
         /** Schedule coroutine execution.
             Effects:
@@ -131,13 +133,13 @@ public:
         bool
         runnable() const;
 
-        /** Once called, the Coro allows early exit without an assert. */
-        void
-        expectEarlyExit();
-
         /** Waits until coroutine returns from the user function. */
         void
         join();
+
+        /** Returns true if the coroutine should stop executing */
+        bool
+        shouldStop() const;
     };
 
     using JobFunction = std::function<void()>;
@@ -167,6 +169,10 @@ public:
     bool
     addJob(JobType type, std::string const& name, JobHandler&& jobHandler)
     {
+        if (!accepting_)
+        {
+            return false;
+        }
         if (auto optionalCountedJob =
                 jobCounter_.wrap(std::forward<JobHandler>(jobHandler)))
         {
@@ -249,6 +255,7 @@ private:
     std::uint64_t m_lastJob;
     std::set<Job> m_jobSet;
     JobCounter jobCounter_;
+    std::atomic_bool accepting_ = true;
     std::atomic_bool stopping_{false};
     std::atomic_bool stopped_{false};
     JobDataMap m_jobData;
@@ -260,6 +267,8 @@ private:
     // The number of suspended coroutines
     int nSuspend_ = 0;
 
+    std::map<void*, std::weak_ptr<Coro>> m_suspendedCoros;
+
     Workers m_workers;
 
     // Statistics tracking
@@ -270,6 +279,25 @@ private:
 
     std::condition_variable cv_;
 
+    void
+    onStopResumeCoros(std::map<void*, std::weak_ptr<Coro>>& coros)
+    {
+        for (auto& [_, coro] : coros)
+        {
+            if (auto coroPtr = coro.lock())
+            {
+                if (auto optionalCountedJob =
+                        jobCounter_.wrap([=]() { coroPtr->resume(); }))
+                {
+                    addRefCountedJob(
+                        coroPtr->type_,
+                        coroPtr->name_,
+                        std::move(*optionalCountedJob));
+                }
+            }
+        }
+    }
+
     void
     collect();
     JobTypeData&
@@ -412,6 +440,10 @@ template <class F>
 std::shared_ptr<JobQueue::Coro>
 JobQueue::postCoro(JobType t, std::string const& name, F&& f)
 {
+    if (!accepting_)
+    {
+        return nullptr;
+    }
     /*  First param is a detail type to make construction private.
         Last param is the function the coroutine runs. Signature of
         void(std::shared_ptr<Coro>).
@@ -422,7 +454,6 @@ JobQueue::postCoro(JobType t, std::string const& name, F&& f)
     {
         // The Coro was not successfully posted.  Disable it so it's destructor
         // can run with no negative side effects.  Then destroy it.
-        coro->expectEarlyExit();
         coro.reset();
     }
     return coro;

src/xrpld/core/detail/JobQueue.cpp

@@ -26,6 +26,12 @@
 
 namespace ripple {
 
+bool
+JobQueue::Coro::shouldStop() const
+{
+    return jq_.stopping_ || jq_.stopped_ || !jq_.accepting_ || exiting_;
+}
+
 JobQueue::JobQueue(
     int threadCount,
     beast::insight::Collector::ptr const& collector,
@@ -295,6 +301,22 @@ JobQueue::getJobTypeData(JobType type)
 void
 JobQueue::stop()
 {
+    // Once we stop accepting new jobs, all running coroutines won't be able to
+    // get suspended and yield() will return immediately, so we can safely
+    // move m_suspendedCoros, and we can assume that no coroutine will be
+    // suspended in the future.
+    std::map<void*, std::weak_ptr<Coro>> suspendedCoros;
+    {
+        std::unique_lock lock(m_mutex);
+        accepting_ = false;
+        suspendedCoros = std::move(m_suspendedCoros);
+    }
+    if (!suspendedCoros.empty())
+    {
+        // We should resume the suspended coroutines so that the coroutines
+        // get a chance to exit cleanly.
+        onStopResumeCoros(suspendedCoros);
+    }
     stopping_ = true;
     using namespace std::chrono_literals;
     jobCounter_.join("JobQueue", 1s, m_journal);
@@ -305,8 +327,9 @@ JobQueue::stop()
         // `Job::doJob` and the return of `JobQueue::processTask`. That is why
         // we must wait on the condition variable to make these assertions.
         std::unique_lock<std::mutex> lock(m_mutex);
-        cv_.wait(
+        cv_.wait(lock, [this] {
-            lock, [this] { return m_processCount == 0 && m_jobSet.empty(); });
+            return m_processCount == 0 && nSuspend_ == 0 && m_jobSet.empty();
+        });
         XRPL_ASSERT(
             m_processCount == 0,
             "ripple::JobQueue::stop : all processes completed");

src/xrpld/ledger/View.h

@@ -912,28 +912,41 @@ deleteAMMTrustLine(
     std::optional<AccountID> const& ammAccountID,
     beast::Journal j);
 
-// From the perspective of a vault,
+// From the perspective of a vault, return the number of shares to give the
-// return the number of shares to give the depositor
+// depositor when they deposit a fixed amount of assets. Since shares are MPT
-// when they deposit a fixed amount of assets.
+// this number is integral and always truncated in this calculation.
-[[nodiscard]] STAmount
+[[nodiscard]] std::optional<STAmount>
 assetsToSharesDeposit(
     std::shared_ptr<SLE const> const& vault,
     std::shared_ptr<SLE const> const& issuance,
     STAmount const& assets);
 
-// From the perspective of a vault,
+// From the perspective of a vault, return the number of assets to take from
-// return the number of shares to demand from the depositor
+// depositor when they receive a fixed amount of shares. Note, since shares are
-// when they ask to withdraw a fixed amount of assets.
+// MPT, they are always an integral number.
-[[nodiscard]] STAmount
+[[nodiscard]] std::optional<STAmount>
+sharesToAssetsDeposit(
+    std::shared_ptr<SLE const> const& vault,
+    std::shared_ptr<SLE const> const& issuance,
+    STAmount const& shares);
+
+enum class TruncateShares : bool { no = false, yes = true };
+
+// From the perspective of a vault, return the number of shares to demand from
+// the depositor when they ask to withdraw a fixed amount of assets. Since
+// shares are MPT this number is integral, and it will be rounded to nearest
+// unless explicitly requested to be truncated instead.
+[[nodiscard]] std::optional<STAmount>
 assetsToSharesWithdraw(
     std::shared_ptr<SLE const> const& vault,
     std::shared_ptr<SLE const> const& issuance,
-    STAmount const& assets);
+    STAmount const& assets,
+    TruncateShares truncate = TruncateShares::no);
 
-// From the perspective of a vault,
+// From the perspective of a vault, return the number of assets to give the
-// return the number of assets to give the depositor
+// depositor when they redeem a fixed amount of shares. Note, since shares are
-// when they redeem a fixed amount of shares.
+// MPT, they are always an integral number.
-[[nodiscard]] STAmount
+[[nodiscard]] std::optional<STAmount>
 sharesToAssetsWithdraw(
     std::shared_ptr<SLE const> const& vault,
     std::shared_ptr<SLE const> const& issuance,

src/xrpld/ledger/detail/View.cpp

@@ -2793,58 +2793,113 @@ rippleCredit(
         saAmount.asset().value());
 }
 
-[[nodiscard]] STAmount
+[[nodiscard]] std::optional<STAmount>
 assetsToSharesDeposit(
     std::shared_ptr<SLE const> const& vault,
     std::shared_ptr<SLE const> const& issuance,
     STAmount const& assets)
 {
+    XRPL_ASSERT(
+        !assets.negative(),
+        "ripple::assetsToSharesDeposit : non-negative assets");
     XRPL_ASSERT(
         assets.asset() == vault->at(sfAsset),
         "ripple::assetsToSharesDeposit : assets and vault match");
-    Number assetTotal = vault->at(sfAssetsTotal);
+    if (assets.negative() || assets.asset() != vault->at(sfAsset))
-    STAmount shares{vault->at(sfShareMPTID), static_cast<Number>(assets)};
+        return std::nullopt;  // LCOV_EXCL_LINE
+
+    Number const assetTotal = vault->at(sfAssetsTotal);
+    STAmount shares{vault->at(sfShareMPTID)};
     if (assetTotal == 0)
-        return shares;
+        return STAmount{
-    Number shareTotal = issuance->at(sfOutstandingAmount);
+            shares.asset(),
-    shares = shareTotal * (assets / assetTotal);
+            Number(assets.mantissa(), assets.exponent() + vault->at(sfScale))
+                .truncate()};
+
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
+    shares = (shareTotal * (assets / assetTotal)).truncate();
     return shares;
 }
 
-[[nodiscard]] STAmount
+[[nodiscard]] std::optional<STAmount>
+sharesToAssetsDeposit(
+    std::shared_ptr<SLE const> const& vault,
+    std::shared_ptr<SLE const> const& issuance,
+    STAmount const& shares)
+{
+    XRPL_ASSERT(
+        !shares.negative(),
+        "ripple::sharesToAssetsDeposit : non-negative shares");
+    XRPL_ASSERT(
+        shares.asset() == vault->at(sfShareMPTID),
+        "ripple::sharesToAssetsDeposit : shares and vault match");
+    if (shares.negative() || shares.asset() != vault->at(sfShareMPTID))
+        return std::nullopt;  // LCOV_EXCL_LINE
+
+    Number const assetTotal = vault->at(sfAssetsTotal);
+    STAmount assets{vault->at(sfAsset)};
+    if (assetTotal == 0)
+        return STAmount{
+            assets.asset(),
+            shares.mantissa(),
+            shares.exponent() - vault->at(sfScale),
+            false};
+
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
+    assets = assetTotal * (shares / shareTotal);
+    return assets;
+}
+
+[[nodiscard]] std::optional<STAmount>
 assetsToSharesWithdraw(
     std::shared_ptr<SLE const> const& vault,
     std::shared_ptr<SLE const> const& issuance,
-    STAmount const& assets)
+    STAmount const& assets,
+    TruncateShares truncate)
 {
+    XRPL_ASSERT(
+        !assets.negative(),
+        "ripple::assetsToSharesDeposit : non-negative assets");
     XRPL_ASSERT(
         assets.asset() == vault->at(sfAsset),
         "ripple::assetsToSharesWithdraw : assets and vault match");
+    if (assets.negative() || assets.asset() != vault->at(sfAsset))
+        return std::nullopt;  // LCOV_EXCL_LINE
+
     Number assetTotal = vault->at(sfAssetsTotal);
     assetTotal -= vault->at(sfLossUnrealized);
     STAmount shares{vault->at(sfShareMPTID)};
     if (assetTotal == 0)
         return shares;
-    Number shareTotal = issuance->at(sfOutstandingAmount);
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
-    shares = shareTotal * (assets / assetTotal);
+    Number result = shareTotal * (assets / assetTotal);
+    if (truncate == TruncateShares::yes)
+        result = result.truncate();
+    shares = result;
     return shares;
 }
 
-[[nodiscard]] STAmount
+[[nodiscard]] std::optional<STAmount>
 sharesToAssetsWithdraw(
     std::shared_ptr<SLE const> const& vault,
     std::shared_ptr<SLE const> const& issuance,
     STAmount const& shares)
 {
+    XRPL_ASSERT(
+        !shares.negative(),
+        "ripple::sharesToAssetsDeposit : non-negative shares");
     XRPL_ASSERT(
         shares.asset() == vault->at(sfShareMPTID),
         "ripple::sharesToAssetsWithdraw : shares and vault match");
+    if (shares.negative() || shares.asset() != vault->at(sfShareMPTID))
+        return std::nullopt;  // LCOV_EXCL_LINE
+
     Number assetTotal = vault->at(sfAssetsTotal);
     assetTotal -= vault->at(sfLossUnrealized);
     STAmount assets{vault->at(sfAsset)};
     if (assetTotal == 0)
         return assets;
-    Number shareTotal = issuance->at(sfOutstandingAmount);
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
     assets = assetTotal * (shares / shareTotal);
     return assets;
 }

src/xrpld/rpc/handlers/RipplePathFind.cpp

@@ -128,21 +128,17 @@ doRipplePathFind(RPC::JsonContext& context)
         // May 2017
         jvResult = context.app.getPathRequests().makeLegacyPathRequest(
             request,
-            [&context]() {
+            [coro = context.coro]() {
-                // Copying the shared_ptr keeps the coroutine alive up
+                // Capturing the shared_ptr keeps the coroutine alive up
                 // through the return.  Otherwise the storage under the
                 // captured reference could evaporate when we return from
-                // coroCopy->resume().  This is not strictly necessary, but
+                // coro->post().
-                // will make maintenance easier.
+                // When post() failed, we won't get a thread to let
-                std::shared_ptr<JobQueue::Coro> coroCopy{context.coro};
+                // the Coro finish. We should ignore the coroutine and
-                if (!coroCopy->post())
+                // let it destruct, as the JobQueu has been signaled to
-                {
+                // close, and resuming it manually messes up the internal
-                    // The post() failed, so we won't get a thread to let
+                // state in JobQueue.
-                    // the Coro finish.  We'll call Coro::resume() so the
+                coro->post();
-                    // Coro can finish on our thread.  Otherwise the
-                    // application will hang on shutdown.
-                    coroCopy->resume();
-                }
             },
             context.consumer,
             lpLedger,
@@ -150,6 +146,14 @@ doRipplePathFind(RPC::JsonContext& context)
         if (request)
         {
             context.coro->yield();
+            // Each time after we resume from yield(), we should
+            // check if cancellation has been requested. It would
+            // be a lot more elegant if we replace boost coroutine
+            // with c++ standard coroutine.
+            if (context.coro->shouldStop())
+            {
+                return jvResult;
+            }
             jvResult = request->doStatus(context.params);
         }