Revert "chore: use improved levelization script with threading and argparse"

This reverts commit 5c1d7d9ae9.

.github/workflows/levelization.yml

@@ -10,7 +10,7 @@ jobs:
     steps:
     - uses: actions/checkout@v3
     - name: Check levelization
-      run: Builds/levelization/levelization.sh
+      run: python Builds/levelization/levelization.py
     - name: Check for differences
       id: assert
       run: |
@@ -40,7 +40,7 @@ jobs:
           To fix it, you can do one of two things:
           1. Download and apply the patch generated as an artifact of this
              job to your repo, commit, and push.
-          2. Run './Builds/levelization/levelization.sh' in your repo,
+          2. Run 'python Builds/levelization/levelization.py' in your repo,
              commit, and push.
 
           See Builds/levelization/README.md for more info.

.gitignore

@@ -53,6 +53,9 @@ Builds/levelization/results/paths.txt
 Builds/levelization/results/includes/
 Builds/levelization/results/includedby/
 
+# Python
+__pycache__
+
 # Ignore tmp directory.
 tmp
 

Builds/levelization/README.md

@@ -50,7 +50,7 @@ that `test` code should *never* be included in `ripple` code.)
 
 ## Validation
 
-The [levelization.sh](levelization.sh) script takes no parameters,
+The [levelization.py](levelization.py) script takes no parameters,
 reads no environment variables, and can be run from any directory,
 as long as it is in the expected location in the rippled repo.
 It can be run at any time from within a checked out repo, and will
@@ -84,7 +84,7 @@ It generates many files of [results](results):
   Github Actions workflow to test that levelization loops haven't
   changed.  Unfortunately, if changes are detected, it can't tell if
   they are improvements or not, so if you have resolved any issues or
-  done anything else to improve levelization, run `levelization.sh`,
+  done anything else to improve levelization, run `levelization.py`,
   and commit the updated results.
 
 The  `loops.txt` and `ordering.txt` files relate the modules
@@ -108,7 +108,7 @@ The committed files hide the detailed values intentionally, to
 prevent false alarms and merging issues, and because it's easy to
 get those details locally.
 
-1. Run `levelization.sh`
+1. Run `levelization.py`
 2. Grep the modules in `paths.txt`.
    * For example, if a cycle is found `A ~= B`, simply `grep -w
      A Builds/levelization/results/paths.txt | grep -w B`

Builds/levelization/levelization.py

@@ -0,0 +1,283 @@
+#!/usr/bin/env python3
+
+"""
+Usage: levelization.py
+This script takes no parameters, and can be called from any directory in the file system.
+"""
+
+import os
+import re
+import sys
+from collections import defaultdict
+from pathlib import Path
+
+# Compile regex patterns once at module level
+INCLUDE_PATTERN = re.compile(r"^\s*#include.*/.*\.h")
+INCLUDE_PATH_PATTERN = re.compile(r'[<"]([^>"]+)[>"]')
+
+
+def dictionary_sort_key(s):
+    """
+    Create a sort key that mimics 'sort -d' (dictionary order).
+    Dictionary order only considers blanks and alphanumeric characters.
+    """
+    return "".join(c for c in s if c.isalnum() or c.isspace())
+
+
+def get_level(file_path):
+    """
+    Extract the level from a file path (second and third directory components).
+    Equivalent to bash: cut -d/ -f 2,3
+
+    Examples:
+        src/ripple/app/main.cpp -> ripple.app
+        src/test/app/Import_test.cpp -> test.app
+    """
+    parts = file_path.split("/")
+
+    if len(parts) >= 3:
+        level = f"{parts[1]}/{parts[2]}"
+    elif len(parts) >= 2:
+        level = f"{parts[1]}/toplevel"
+    else:
+        level = file_path
+
+    # If the "level" indicates a file, cut off the filename
+    if "." in level.split("/")[-1]:
+        # Use the "toplevel" label as a workaround for `sort`
+        # inconsistencies between different utility versions
+        level = level.rsplit("/", 1)[0] + "/toplevel"
+
+    return level.replace("/", ".")
+
+
+def extract_include_level(include_line):
+    """
+    Extract the include path from an #include directive.
+    Gets the first two directory components from the include path.
+    Equivalent to bash: cut -d/ -f 1,2
+
+    Examples:
+        #include <ripple/basics/base_uint.h> -> ripple.basics
+        #include "ripple/app/main/Application.h" -> ripple.app
+    """
+    match = INCLUDE_PATH_PATTERN.search(include_line)
+    if not match:
+        return None
+
+    include_path = match.group(1)
+    parts = include_path.split("/")
+
+    if len(parts) >= 2:
+        include_level = f"{parts[0]}/{parts[1]}"
+    else:
+        include_level = include_path
+
+    # If the "includelevel" indicates a file, cut off the filename
+    if "." in include_level.split("/")[-1]:
+        include_level = include_level.rsplit("/", 1)[0] + "/toplevel"
+
+    return include_level.replace("/", ".")
+
+
+def find_repository_directories(start_path, depth_limit=10):
+    """
+    Find the repository root by looking for src or include folders.
+    Walks up the directory tree from the start path.
+    """
+    current = start_path.resolve()
+
+    for _ in range(depth_limit):
+        src_path = current / "src"
+        include_path = current / "include"
+        has_src = src_path.exists()
+        has_include = include_path.exists()
+
+        if has_src or has_include:
+            dirs = []
+            if has_src:
+                dirs.append(src_path)
+            if has_include:
+                dirs.append(include_path)
+            return current, dirs
+
+        parent = current.parent
+        if parent == current:
+            break
+        current = parent
+
+    raise RuntimeError(
+        "Could not find repository root. "
+        "Expected to find a directory containing 'src' and/or 'include' folders."
+    )
+
+
+def main():
+    script_dir = Path(__file__).parent.resolve()
+    os.chdir(script_dir)
+
+    # Clean up and create results directory.
+    results_dir = script_dir / "results"
+    if results_dir.exists():
+        import shutil
+
+        shutil.rmtree(results_dir)
+    results_dir.mkdir()
+
+    # Find the repository root.
+    try:
+        repo_root, scan_dirs = find_repository_directories(script_dir)
+        print(f"Found repository root: {repo_root}")
+        for scan_dir in scan_dirs:
+            print(f"  Scanning: {scan_dir.relative_to(repo_root)}")
+    except RuntimeError as e:
+        print(f"Error: {e}", file=sys.stderr)
+        sys.exit(1)
+
+    # Find all #include directives.
+    print("\nScanning for raw includes...")
+    raw_includes = []
+    rawincludes_file = results_dir / "rawincludes.txt"
+
+    with open(rawincludes_file, "w", buffering=8192) as raw_f:
+        for dir_path in scan_dirs:
+            for file_path in dir_path.rglob("*"):
+                if not file_path.is_file():
+                    continue
+                try:
+                    rel_path_str = str(file_path.relative_to(repo_root))
+                    with open(
+                        file_path, "r", encoding="utf-8", errors="ignore", buffering=8192
+                    ) as f:
+                        for line in f:
+                            if "#include" not in line or "boost" in line:
+                                continue
+                            if INCLUDE_PATTERN.match(line):
+                                line_stripped = line.strip()
+                                entry = f"{rel_path_str}:{line_stripped}\n"
+                                print(entry, end="")
+                                raw_f.write(entry)
+                                raw_includes.append((rel_path_str, line_stripped))
+                except Exception as e:
+                    print(f"Error reading {file_path}: {e}", file=sys.stderr)
+
+    # Build levelization paths and count directly.
+    print("Build levelization paths")
+    path_counts = defaultdict(int)
+
+    for file_path, include_line in raw_includes:
+        include_level = extract_include_level(include_line)
+        if not include_level:
+            continue
+        level = get_level(file_path)
+        if level != include_level:
+            path_counts[(level, include_level)] += 1
+
+    # Sort and deduplicate paths.
+    print("Sort and deduplicate paths")
+    sorted_items = sorted(
+        path_counts.items(),
+        key=lambda x: (dictionary_sort_key(x[0][0]), dictionary_sort_key(x[0][1])),
+    )
+
+    paths_file = results_dir / "paths.txt"
+    with open(paths_file, "w") as f:
+        for (level, include_level), count in sorted_items:
+            line = f"{count:7} {level} {include_level}\n"
+            print(line.rstrip())
+            f.write(line)
+
+    # Split into flat-file database.
+    print("Split into flat-file database")
+    includes_dir = results_dir / "includes"
+    includedby_dir = results_dir / "includedby"
+    includes_dir.mkdir()
+    includedby_dir.mkdir()
+
+    includes_data = defaultdict(list)
+    includedby_data = defaultdict(list)
+
+    for (level, include_level), count in sorted_items:
+        includes_data[level].append((include_level, count))
+        includedby_data[include_level].append((level, count))
+
+    for level in sorted(includes_data.keys(), key=dictionary_sort_key):
+        with open(includes_dir / level, "w") as f:
+            for include_level, count in includes_data[level]:
+                line = f"{include_level} {count}\n"
+                print(line.rstrip())
+                f.write(line)
+
+    for include_level in sorted(includedby_data.keys(), key=dictionary_sort_key):
+        with open(includedby_dir / include_level, "w") as f:
+            for level, count in includedby_data[include_level]:
+                line = f"{level} {count}\n"
+                print(line.rstrip())
+                f.write(line)
+
+    # Search for loops.
+    print("Search for loops")
+    loops_file = results_dir / "loops.txt"
+    ordering_file = results_dir / "ordering.txt"
+
+    # Pre-load all include files into memory for fast lookup.
+    includes_cache = {}
+    includes_lookup = {}
+
+    for include_file in sorted(includes_dir.iterdir(), key=lambda p: p.name):
+        if not include_file.is_file():
+            continue
+        includes_cache[include_file.name] = []
+        includes_lookup[include_file.name] = {}
+        with open(include_file, "r") as f:
+            for line in f:
+                parts = line.strip().split()
+                if len(parts) >= 2:
+                    name, count = parts[0], int(parts[1])
+                    includes_cache[include_file.name].append((name, count))
+                    includes_lookup[include_file.name][name] = count
+
+    loops_found = set()
+
+    with open(loops_file, "w", buffering=8192) as loops_f, open(
+        ordering_file, "w", buffering=8192
+    ) as ordering_f:
+        for source in sorted(includes_cache.keys()):
+            for include, include_freq in includes_cache[source]:
+                if include not in includes_lookup:
+                    continue
+
+                source_freq = includes_lookup[include].get(source)
+
+                if source_freq is not None:
+                    loop_key = tuple(sorted([source, include]))
+                    if loop_key in loops_found:
+                        continue
+                    loops_found.add(loop_key)
+
+                    loops_f.write(f"Loop: {source} {include}\n")
+
+                    diff = include_freq - source_freq
+                    if diff > 3:
+                        loops_f.write(f"  {source} > {include}\n\n")
+                    elif diff < -3:
+                        loops_f.write(f"  {include} > {source}\n\n")
+                    elif source_freq == include_freq:
+                        loops_f.write(f"  {include} == {source}\n\n")
+                    else:
+                        loops_f.write(f"  {include} ~= {source}\n\n")
+                else:
+                    ordering_f.write(f"{source} > {include}\n")
+
+    # Print results.
+    print("\nOrdering:")
+    with open(ordering_file, "r") as f:
+        print(f.read(), end="")
+
+    print("\nLoops:")
+    with open(loops_file, "r") as f:
+        print(f.read(), end="")
+
+
+if __name__ == "__main__":
+    main()

Builds/levelization/levelization.sh

@@ -1,130 +0,0 @@
-#!/bin/bash
-
-# Usage: levelization.sh
-# This script takes no parameters, reads no environment variables,
-# and can be run from any directory, as long as it is in the expected
-# location in the repo.
-
-pushd $( dirname $0 )
-
-if [ -v PS1 ]
-then
-  # if the shell is interactive, clean up any flotsam before analyzing
-  git clean -ix
-fi
-
-# Ensure all sorting is ASCII-order consistently across platforms.
-export LANG=C
-
-rm -rfv results
-mkdir results
-includes="$( pwd )/results/rawincludes.txt"
-pushd ../..
-echo Raw includes:
-grep -r '^[ ]*#include.*/.*\.h' include src | \
-    grep -v boost | tee ${includes}
-popd
-pushd results
-
-oldifs=${IFS}
-IFS=:
-mkdir includes
-mkdir includedby
-echo Build levelization paths
-exec 3< ${includes} # open rawincludes.txt for input
-while read -r -u 3 file include
-do
-    level=$( echo ${file} | cut -d/ -f 2,3 )
-    # If the "level" indicates a file, cut off the filename
-    if [[ "${level##*.}" != "${level}" ]]
-    then
-        # Use the "toplevel" label as a workaround for `sort`
-        # inconsistencies between different utility versions
-        level="$( dirname ${level} )/toplevel"
-    fi
-    level=$( echo ${level} | tr '/' '.' )
-
-    includelevel=$( echo ${include} | sed 's/.*["<]//; s/[">].*//' | \
-        cut -d/ -f 1,2 )
-    if [[ "${includelevel##*.}" != "${includelevel}" ]]
-    then
-        # Use the "toplevel" label as a workaround for `sort`
-        # inconsistencies between different utility versions
-        includelevel="$( dirname ${includelevel} )/toplevel"
-    fi
-    includelevel=$( echo ${includelevel} | tr '/' '.' )
-
-    if [[ "$level" != "$includelevel" ]]
-    then
-        echo $level $includelevel | tee -a paths.txt
-    fi
-done
-echo Sort and dedup paths
-sort -ds paths.txt | uniq -c | tee sortedpaths.txt
-mv sortedpaths.txt paths.txt
-exec 3>&- #close fd 3
-IFS=${oldifs}
-unset oldifs
-
-echo Split into flat-file database
-exec 4<paths.txt # open paths.txt for input
-while read -r -u 4 count level include
-do
-    echo ${include} ${count} | tee -a includes/${level}
-    echo ${level} ${count} | tee -a includedby/${include}
-done
-exec 4>&- #close fd 4
-
-loops="$( pwd )/loops.txt"
-ordering="$( pwd )/ordering.txt"
-pushd includes
-echo Search for loops
-# Redirect stdout to a file
-exec 4>&1
-exec 1>"${loops}"
-for source in *
-do
-  if [[ -f "$source" ]]
-  then
-    exec 5<"${source}" # open for input
-    while read -r -u 5 include includefreq
-    do
-      if [[ -f $include ]]
-      then
-        if grep -q -w $source $include
-        then
-          if grep -q -w "Loop: $include $source" "${loops}"
-          then
-            continue
-          fi
-          sourcefreq=$( grep -w $source $include | cut -d\  -f2 )
-          echo "Loop: $source $include"
-          # If the counts are close, indicate that the two modules are
-          # on the same level, though they shouldn't be
-          if [[ $(( $includefreq - $sourcefreq )) -gt 3 ]]
-          then
-              echo -e "  $source > $include\n"
-          elif [[ $(( $sourcefreq - $includefreq )) -gt 3 ]]
-          then
-              echo -e "  $include > $source\n"
-          elif [[ $sourcefreq -eq $includefreq ]]
-          then
-              echo -e "  $include == $source\n"
-          else
-              echo -e "  $include ~= $source\n"
-          fi
-        else
-          echo "$source > $include" >> "${ordering}"
-        fi
-      fi
-    done
-    exec 5>&- #close fd 5
-  fi
-done
-exec 1>&4 #close fd 1
-exec 4>&- #close fd 4
-cat "${ordering}"
-cat "${loops}"
-popd
-popd
-popd

include/xrpl/protocol/IOUAmount.h

@@ -97,6 +97,12 @@ public:
 
     static IOUAmount
     minPositiveAmount();
+
+    friend std::ostream&
+    operator<<(std::ostream& os, IOUAmount const& x)
+    {
+        return os << to_string(x);
+    }
 };
 
 inline IOUAmount::IOUAmount(beast::Zero)

include/xrpl/protocol/Rules.h

@@ -28,6 +28,9 @@
 
 namespace ripple {
 
+bool
+isFeatureEnabled(uint256 const& feature);
+
 class DigestAwareReadView;
 
 /** Rules controlling protocol behavior. */

src/libxrpl/protocol/Rules.cpp

@@ -153,4 +153,12 @@ Rules::operator!=(Rules const& other) const
 {
     return !(*this == other);
 }
+
+bool
+isFeatureEnabled(uint256 const& feature)
+{
+    auto const& rules = getCurrentTransactionRules();
+    return rules && rules->enabled(feature);
+}
+
 }  // namespace ripple

src/test/jtx/AMM.h

@@ -125,7 +125,6 @@ class AMM
     STAmount const asset1_;
     STAmount const asset2_;
     uint256 const ammID_;
-    IOUAmount const initialLPTokens_;
     bool log_;
     bool doClose_;
     // Predict next purchase price
@@ -138,6 +137,7 @@ class AMM
     std::uint32_t const fee_;
     AccountID const ammAccount_;
     Issue const lptIssue_;
+    IOUAmount const initialLPTokens_;
 
 public:
     AMM(Env& env,
@@ -194,6 +194,12 @@ public:
         Issue const& issue2,
         std::optional<AccountID> const& account = std::nullopt) const;
 
+    std::tuple<STAmount, STAmount, STAmount>
+    balances(std::optional<AccountID> const& account = std::nullopt) const
+    {
+        return balances(asset1_.get<Issue>(), asset2_.get<Issue>(), account);
+    }
+
     [[nodiscard]] bool
     expectLPTokens(AccountID const& account, IOUAmount const& tokens) const;
 
@@ -428,6 +434,9 @@ private:
 
     [[nodiscard]] bool
     expectAuctionSlot(auto&& cb) const;
+
+    IOUAmount
+    initialTokens();
 };
 
 namespace amm {

src/test/jtx/AMMTest.h

@@ -33,6 +33,15 @@ class AMM;
 
 enum class Fund { All, Acct, Gw, IOUOnly };
 
+struct TestAMMArg
+{
+    std::optional<std::pair<STAmount, STAmount>> pool = std::nullopt;
+    std::uint16_t tfee = 0;
+    std::optional<jtx::ter> ter = std::nullopt;
+    std::vector<FeatureBitset> features = {supported_amendments()};
+    bool noLog = false;
+};
+
 void
 fund(
     jtx::Env& env,
@@ -85,6 +94,11 @@ protected:
         std::uint16_t tfee = 0,
         std::optional<jtx::ter> const& ter = std::nullopt,
         std::vector<FeatureBitset> const& features = {supported_amendments()});
+
+    void
+    testAMM(
+        std::function<void(jtx::AMM&, jtx::Env&)>&& cb,
+        TestAMMArg const& arg);
 };
 
 class AMMTest : public jtx::AMMTestBase

src/test/jtx/Env.h

@@ -646,6 +646,12 @@ public:
     void
     disableFeature(uint256 const feature);
 
+    bool
+    enabled(uint256 feature) const
+    {
+        return current()->rules().enabled(feature);
+    }
+
 private:
     void
     fund(bool setDefaultRipple, STAmount const& amount, Account const& account);

src/test/jtx/impl/AMM.cpp

@@ -18,8 +18,9 @@
 //==============================================================================
 
 #include <test/jtx/AMM.h>
-
 #include <test/jtx/Env.h>
+
+#include <xrpld/app/misc/AMMHelpers.h>
 #include <xrpld/app/misc/AMMUtils.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
 #include <xrpl/protocol/AMMCore.h>
@@ -38,12 +39,10 @@ number(STAmount const& a)
     return a;
 }
 
-static IOUAmount
-initialTokens(STAmount const& asset1, STAmount const& asset2)
+IOUAmount
+AMM::initialTokens()
 {
-    auto const product = number(asset1) * number(asset2);
-    return (IOUAmount)(product.mantissa() >= 0 ? root2(product)
-                                               : root2(-product));
+    return getLPTokensBalance();
 }
 
 AMM::AMM(
@@ -64,7 +63,6 @@ AMM::AMM(
     , asset1_(asset1)
     , asset2_(asset2)
     , ammID_(keylet::amm(asset1_.issue(), asset2_.issue()).key)
-    , initialLPTokens_(initialTokens(asset1, asset2))
     , log_(log)
     , doClose_(close)
     , lastPurchasePrice_(0)
@@ -77,6 +75,7 @@ AMM::AMM(
           asset1_.issue().currency,
           asset2_.issue().currency,
           ammAccount_))
+    , initialLPTokens_(initialTokens())
 {
 }
 

src/test/jtx/impl/AMMTest.cpp

@@ -17,9 +17,9 @@
 */
 //==============================================================================
 
-#include <test/jtx/AMMTest.h>
-
 #include <test/jtx/AMM.h>
+#include <test/jtx/AMMTest.h>
+#include <test/jtx/CaptureLogs.h>
 #include <test/jtx/Env.h>
 #include <test/jtx/pay.h>
 #include <xrpld/rpc/RPCHandler.h>
@@ -104,15 +104,37 @@ AMMTestBase::testAMM(
     std::uint16_t tfee,
     std::optional<jtx::ter> const& ter,
     std::vector<FeatureBitset> const& vfeatures)
+{
+    testAMM(
+        std::move(cb),
+        TestAMMArg{
+            .pool = pool, .tfee = tfee, .ter = ter, .features = vfeatures});
+}
+
+void
+AMMTestBase::testAMM(
+    std::function<void(jtx::AMM&, jtx::Env&)>&& cb,
+    TestAMMArg const& arg)
 {
     using namespace jtx;
 
-    for (auto const& features : vfeatures)
+    std::string logs;
+
+    for (auto const& features : arg.features)
     {
-        Env env{*this, features};
+        // Env env{
+        //     *this,
+        //     features,
+        //     arg.noLog ? std::make_unique<CaptureLogs>(&logs) : nullptr};
+        Env env(
+            *this,
+            envconfig(),
+            features,
+            nullptr,
+            beast::severities::kDisabled);
 
         auto const [asset1, asset2] =
-            pool ? *pool : std::make_pair(XRP(10000), USD(10000));
+            arg.pool ? *arg.pool : std::make_pair(XRP(10000), USD(10000));
         auto tofund = [&](STAmount const& a) -> STAmount {
             if (a.native())
             {
@@ -142,7 +164,7 @@ AMMTestBase::testAMM(
             alice,
             asset1,
             asset2,
-            CreateArg{.log = false, .tfee = tfee, .err = ter});
+            CreateArg{.log = false, .tfee = arg.tfee, .err = arg.ter});
         if (BEAST_EXPECT(
                 ammAlice.expectBalances(asset1, asset2, ammAlice.tokens())))
             cb(ammAlice, env);

src/test/rpc/AMMInfo_test.cpp

@@ -203,98 +203,107 @@ public:
     }
 
     void
-    testVoteAndBid()
+    testVoteAndBid(FeatureBitset features)
     {
         testcase("Vote and Bid");
 
         using namespace jtx;
-        testAMM([&](AMM& ammAlice, Env& env) {
-            BEAST_EXPECT(ammAlice.expectAmmRpcInfo(
-                XRP(10000), USD(10000), IOUAmount{10000000, 0}));
-            std::unordered_map<std::string, std::uint16_t> votes;
-            votes.insert({alice.human(), 0});
-            for (int i = 0; i < 7; ++i)
-            {
-                Account a(std::to_string(i));
-                votes.insert({a.human(), 50 * (i + 1)});
-                fund(env, gw, {a}, {USD(10000)}, Fund::Acct);
-                ammAlice.deposit(a, 10000000);
-                ammAlice.vote(a, 50 * (i + 1));
-            }
-            BEAST_EXPECT(ammAlice.expectTradingFee(175));
-            Account ed("ed");
-            Account bill("bill");
-            env.fund(XRP(1000), bob, ed, bill);
-            env(ammAlice.bid(
-                {.bidMin = 100, .authAccounts = {carol, bob, ed, bill}}));
-            BEAST_EXPECT(ammAlice.expectAmmRpcInfo(
-                XRP(80000),
-                USD(80000),
-                IOUAmount{79994400},
-                std::nullopt,
-                std::nullopt,
-                ammAlice.ammAccount()));
-            for (auto i = 0; i < 2; ++i)
-            {
-                std::unordered_set<std::string> authAccounts = {
-                    carol.human(), bob.human(), ed.human(), bill.human()};
-                auto const ammInfo = i ? ammAlice.ammRpcInfo()
-                                       : ammAlice.ammRpcInfo(
-                                             std::nullopt,
-                                             std::nullopt,
-                                             std::nullopt,
-                                             std::nullopt,
-                                             ammAlice.ammAccount());
-                auto const& amm = ammInfo[jss::amm];
-                try
+        testAMM(
+            [&](AMM& ammAlice, Env& env) {
+                BEAST_EXPECT(ammAlice.expectAmmRpcInfo(
+                    XRP(10000), USD(10000), IOUAmount{10000000, 0}));
+                std::unordered_map<std::string, std::uint16_t> votes;
+                votes.insert({alice.human(), 0});
+                for (int i = 0; i < 7; ++i)
                 {
-                    // votes
-                    auto const voteSlots = amm[jss::vote_slots];
-                    auto votesCopy = votes;
-                    for (std::uint8_t i = 0; i < 8; ++i)
+                    Account a(std::to_string(i));
+                    votes.insert({a.human(), 50 * (i + 1)});
+                    fund(env, gw, {a}, {USD(10001)}, Fund::Acct);
+                    ammAlice.deposit(a, 10000000);
+                    ammAlice.vote(a, 50 * (i + 1));
+                }
+                BEAST_EXPECT(ammAlice.expectTradingFee(175));
+                Account ed("ed");
+                Account bill("bill");
+                env.fund(XRP(1000), bob, ed, bill);
+                env(ammAlice.bid(
+                    {.bidMin = 100, .authAccounts = {carol, bob, ed, bill}}));
+                BEAST_EXPECT(ammAlice.expectAmmRpcInfo(
+                    XRPAmount(80000000005),
+                    STAmount{USD, UINT64_C(80'000'00000000005), -11},
+                    IOUAmount{79994400},
+                    std::nullopt,
+                    std::nullopt,
+                    ammAlice.ammAccount()));
+                for (auto i = 0; i < 2; ++i)
+                {
+                    std::unordered_set<std::string> authAccounts = {
+                        carol.human(), bob.human(), ed.human(), bill.human()};
+                    auto const ammInfo = i ? ammAlice.ammRpcInfo()
+                                           : ammAlice.ammRpcInfo(
+                                                 std::nullopt,
+                                                 std::nullopt,
+                                                 std::nullopt,
+                                                 std::nullopt,
+                                                 ammAlice.ammAccount());
+                    auto const& amm = ammInfo[jss::amm];
+                    try
                     {
-                        if (!BEAST_EXPECT(
-                                votes[voteSlots[i][jss::account].asString()] ==
-                                    voteSlots[i][jss::trading_fee].asUInt() &&
-                                voteSlots[i][jss::vote_weight].asUInt() ==
-                                    12500))
+                        // votes
+                        auto const voteSlots = amm[jss::vote_slots];
+                        auto votesCopy = votes;
+                        for (std::uint8_t i = 0; i < 8; ++i)
+                        {
+                            if (!BEAST_EXPECT(
+                                    votes[voteSlots[i][jss::account]
+                                              .asString()] ==
+                                        voteSlots[i][jss::trading_fee]
+                                            .asUInt() &&
+                                    voteSlots[i][jss::vote_weight].asUInt() ==
+                                        12500))
+                                return;
+                            votes.erase(voteSlots[i][jss::account].asString());
+                        }
+                        if (!BEAST_EXPECT(votes.empty()))
                             return;
-                        votes.erase(voteSlots[i][jss::account].asString());
-                    }
-                    if (!BEAST_EXPECT(votes.empty()))
-                        return;
-                    votes = votesCopy;
+                        votes = votesCopy;
 
-                    // bid
-                    auto const auctionSlot = amm[jss::auction_slot];
-                    for (std::uint8_t i = 0; i < 4; ++i)
-                    {
-                        if (!BEAST_EXPECT(authAccounts.contains(
+                        // bid
+                        auto const auctionSlot = amm[jss::auction_slot];
+                        for (std::uint8_t i = 0; i < 4; ++i)
+                        {
+                            if (!BEAST_EXPECT(authAccounts.contains(
+                                    auctionSlot[jss::auth_accounts][i]
+                                               [jss::account]
+                                                   .asString())))
+                                return;
+                            authAccounts.erase(
                                 auctionSlot[jss::auth_accounts][i][jss::account]
-                                    .asString())))
+                                    .asString());
+                        }
+                        if (!BEAST_EXPECT(authAccounts.empty()))
                             return;
-                        authAccounts.erase(
-                            auctionSlot[jss::auth_accounts][i][jss::account]
-                                .asString());
+                        BEAST_EXPECT(
+                            auctionSlot[jss::account].asString() ==
+                                alice.human() &&
+                            auctionSlot[jss::discounted_fee].asUInt() == 17 &&
+                            auctionSlot[jss::price][jss::value].asString() ==
+                                "5600" &&
+                            auctionSlot[jss::price][jss::currency].asString() ==
+                                to_string(ammAlice.lptIssue().currency) &&
+                            auctionSlot[jss::price][jss::issuer].asString() ==
+                                to_string(ammAlice.lptIssue().account));
+                    }
+                    catch (std::exception const& e)
+                    {
+                        fail(e.what(), __FILE__, __LINE__);
                     }
-                    if (!BEAST_EXPECT(authAccounts.empty()))
-                        return;
-                    BEAST_EXPECT(
-                        auctionSlot[jss::account].asString() == alice.human() &&
-                        auctionSlot[jss::discounted_fee].asUInt() == 17 &&
-                        auctionSlot[jss::price][jss::value].asString() ==
-                            "5600" &&
-                        auctionSlot[jss::price][jss::currency].asString() ==
-                            to_string(ammAlice.lptIssue().currency) &&
-                        auctionSlot[jss::price][jss::issuer].asString() ==
-                            to_string(ammAlice.lptIssue().account));
                 }
-                catch (std::exception const& e)
-                {
-                    fail(e.what(), __FILE__, __LINE__);
-                }
-            }
-        });
+            },
+            std::nullopt,
+            0,
+            std::nullopt,
+            {features});
     }
 
     void
@@ -337,9 +346,11 @@ public:
     void
     run() override
     {
+        using namespace jtx;
+        auto const all = supported_amendments();
         testErrors();
         testSimpleRpc();
-        testVoteAndBid();
+        testVoteAndBid(all);
         testFreeze();
         testInvalidAmmField();
     }

src/xrpld/app/misc/AMMHelpers.h

@@ -51,6 +51,8 @@ reduceOffer(auto const& amount)
 
 }  // namespace detail
 
+enum class IsDeposit : bool { No = false, Yes = true };
+
 /** Calculate LP Tokens given AMM pool reserves.
  * @param asset1 AMM one side of the pool reserve
  * @param asset2 AMM another side of the pool reserve
@@ -70,7 +72,7 @@ ammLPTokens(
  * @return tokens
  */
 STAmount
-lpTokensIn(
+lpTokensOut(
     STAmount const& asset1Balance,
     STAmount const& asset1Deposit,
     STAmount const& lptAMMBalance,
@@ -99,7 +101,7 @@ ammAssetIn(
  * @return tokens out amount
  */
 STAmount
-lpTokensOut(
+lpTokensIn(
     STAmount const& asset1Balance,
     STAmount const& asset1Withdraw,
     STAmount const& lptAMMBalance,
@@ -113,7 +115,7 @@ lpTokensOut(
  * @return calculated asset amount
  */
 STAmount
-withdrawByTokens(
+ammAssetOut(
     STAmount const& assetBalance,
     STAmount const& lptAMMBalance,
     STAmount const& lpTokens,
@@ -517,13 +519,13 @@ square(Number const& n);
  * withdraw to cancel out the precision loss.
  * @param lptAMMBalance LPT AMM Balance
  * @param lpTokens LP tokens to deposit or withdraw
- * @param isDeposit true if deposit, false if withdraw
+ * @param isDeposit Yes if deposit, No if withdraw
  */
 STAmount
 adjustLPTokens(
     STAmount const& lptAMMBalance,
     STAmount const& lpTokens,
-    bool isDeposit);
+    IsDeposit isDeposit);
 
 /** Calls adjustLPTokens() and adjusts deposit or withdraw amounts if
  * the adjusted LP tokens are less than the provided LP tokens.
@@ -533,7 +535,7 @@ adjustLPTokens(
  * @param lptAMMBalance LPT AMM Balance
  * @param lpTokens LP tokens to deposit or withdraw
  * @param tfee trading fee in basis points
- * @param isDeposit true if deposit, false if withdraw
+ * @param isDeposit Yes if deposit, No if withdraw
  * @return
  */
 std::tuple<STAmount, std::optional<STAmount>, STAmount>
@@ -544,7 +546,7 @@ adjustAmountsByLPTokens(
     STAmount const& lptAMMBalance,
     STAmount const& lpTokens,
     std::uint16_t tfee,
-    bool isDeposit);
+    IsDeposit isDeposit);
 
 /** Positive solution for quadratic equation:
  * x = (-b + sqrt(b**2 + 4*a*c))/(2*a)
@@ -552,6 +554,134 @@ adjustAmountsByLPTokens(
 Number
 solveQuadraticEq(Number const& a, Number const& b, Number const& c);
 
+STAmount
+multiply(STAmount const& amount, Number const& frac, Number::rounding_mode rm);
+
+namespace detail {
+
+inline Number::rounding_mode
+getLPTokenRounding(IsDeposit isDeposit)
+{
+    // Minimize on deposit, maximize on withdraw to ensure
+    // AMM invariant sqrt(poolAsset1 * poolAsset2) >= LPTokensBalance
+    return isDeposit == IsDeposit::Yes ? Number::downward : Number::upward;
+}
+
+inline Number::rounding_mode
+getAssetRounding(IsDeposit isDeposit)
+{
+    // Maximize on deposit, minimize on withdraw to ensure
+    // AMM invariant sqrt(poolAsset1 * poolAsset2) >= LPTokensBalance
+    return isDeposit == IsDeposit::Yes ? Number::upward : Number::downward;
+}
+
+}  // namespace detail
+
+/** Round AMM equal deposit/withdrawal amount. Deposit/withdrawal formulas
+ * calculate the amount as a fractional value of the pool balance. The rounding
+ * takes place on the last step of multiplying the balance by the fraction if
+ * AMMv1_3 is enabled.
+ */
+template <typename A>
+STAmount
+getRoundedAsset(
+    Rules const& rules,
+    STAmount const& balance,
+    A const& frac,
+    IsDeposit isDeposit)
+{
+    auto const rm = detail::getAssetRounding(isDeposit);
+    return multiply(balance, frac, rm);
+}
+
+/** Round AMM single deposit/withdrawal amount.
+ * The lambda's are used to delay evaluation until the function
+ * is executed so that the calculation is not done twice. noRoundCb() is
+ * called if AMMv1_3 is disabled. Otherwise, the rounding is set and
+ * the amount is:
+ *   isDeposit is Yes - the balance multiplied by productCb()
+ *   isDeposit is No - the result of productCb(). The rounding is
+ *     the same for all calculations in productCb()
+ */
+STAmount
+getRoundedAsset(
+    Rules const& rules,
+    std::function<Number()>&& noRoundCb,
+    STAmount const& balance,
+    std::function<Number()>&& productCb,
+    IsDeposit isDeposit);
+
+/** Round AMM deposit/withdrawal LPToken amount. Deposit/withdrawal formulas
+ * calculate the lptokens as a fractional value of the AMM total lptokens.
+ * The rounding takes place on the last step of multiplying the balance by
+ * the fraction if AMMv1_3 is enabled. The tokens are then
+ * adjusted to factor in the loss in precision (we only keep 16 significant
+ * digits) when adding the lptokens to the balance.
+ */
+STAmount
+getRoundedLPTokens(
+    Rules const& rules,
+    STAmount const& balance,
+    Number const& frac,
+    IsDeposit isDeposit);
+
+/** Round AMM single deposit/withdrawal LPToken amount.
+ * The lambda's are used to delay evaluation until the function is executed
+ * so that the calculations are not done twice.
+ * noRoundCb() is called if AMMv1_3 is disabled. Otherwise, the rounding is set
+ * and the lptokens are:
+ *   if isDeposit is Yes - the result of productCb(). The rounding is
+ *     the same for all calculations in productCb()
+ *   if isDeposit is No - the balance multiplied by productCb()
+ * The lptokens are then adjusted to factor in the loss in precision
+ * (we only keep 16 significant digits) when adding the lptokens to the balance.
+ */
+STAmount
+getRoundedLPTokens(
+    Rules const& rules,
+    std::function<Number()>&& noRoundCb,
+    STAmount const& lptAMMBalance,
+    std::function<Number()>&& productCb,
+    IsDeposit isDeposit);
+
+/* Next two functions adjust asset in/out amount to factor in the adjusted
+ * lptokens. The lptokens are calculated from the asset in/out. The lptokens are
+ * then adjusted to factor in the loss in precision. The adjusted lptokens might
+ * be less than the initially calculated tokens. Therefore, the asset in/out
+ * must be adjusted. The rounding might result in the adjusted amount being
+ * greater than the original asset in/out amount. If this happens,
+ * then the original amount is reduced by the difference in the adjusted amount
+ * and the original amount. The actual tokens and the actual adjusted amount
+ * are then recalculated. The minimum of the original and the actual
+ * adjusted amount is returned.
+ */
+std::pair<STAmount, STAmount>
+adjustAssetInByTokens(
+    Rules const& rules,
+    STAmount const& balance,
+    STAmount const& amount,
+    STAmount const& lptAMMBalance,
+    STAmount const& tokens,
+    std::uint16_t tfee);
+std::pair<STAmount, STAmount>
+adjustAssetOutByTokens(
+    Rules const& rules,
+    STAmount const& balance,
+    STAmount const& amount,
+    STAmount const& lptAMMBalance,
+    STAmount const& tokens,
+    std::uint16_t tfee);
+
+/** Find a fraction of tokens after the tokens are adjusted. The fraction
+ * is used to adjust equal deposit/withdraw amount.
+ */
+Number
+adjustFracByTokens(
+    Rules const& rules,
+    STAmount const& lptAMMBalance,
+    STAmount const& tokens,
+    Number const& frac);
+
 }  // namespace ripple
 
 #endif  // RIPPLE_APP_MISC_AMMHELPERS_H_INCLUDED

src/xrpld/app/misc/AMMUtils.h

@@ -123,6 +123,17 @@ isOnlyLiquidityProvider(
     Issue const& ammIssue,
     AccountID const& lpAccount);
 
+/** Due to rounding, the LPTokenBalance of the last LP might
+ * not match the LP's trustline balance. If it's within the tolerance,
+ * update LPTokenBalance to match the LP's trustline balance.
+ */
+Expected<bool, TER>
+verifyAndAdjustLPTokenBalance(
+    Sandbox& sb,
+    STAmount const& lpTokens,
+    std::shared_ptr<SLE>& ammSle,
+    AccountID const& account);
+
 }  // namespace ripple
 
 #endif  // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED

src/xrpld/app/misc/detail/AMMHelpers.cpp

@@ -27,6 +27,9 @@ ammLPTokens(
     STAmount const& asset2,
     Issue const& lptIssue)
 {
+    // AMM invariant: sqrt(asset1 * asset2) >= LPTokensBalance
+    auto const rounding = Number::downward;
+    NumberRoundModeGuard g(rounding);
     auto const tokens = root2(asset1 * asset2);
     return toSTAmount(lptIssue, tokens);
 }
@@ -38,7 +41,7 @@ ammLPTokens(
  * where f1 = 1 - tfee, f2 = (1 - tfee/2)/f1
  */
 STAmount
-lpTokensIn(
+lpTokensOut(
     STAmount const& asset1Balance,
     STAmount const& asset1Deposit,
     STAmount const& lptAMMBalance,
@@ -48,8 +51,10 @@ lpTokensIn(
     auto const f2 = feeMultHalf(tfee) / f1;
     Number const r = asset1Deposit / asset1Balance;
     auto const c = root2(f2 * f2 + r / f1) - f2;
-    auto const t = lptAMMBalance * (r - c) / (1 + c);
-    return toSTAmount(lptAMMBalance.issue(), t);
+
+    // minimize tokens out
+    auto const frac = (r - c) / (1 + c);
+    return multiply(lptAMMBalance, frac, Number::downward);
 }
 
 /* Equation 4 solves equation 3 for b:
@@ -78,8 +83,10 @@ ammAssetIn(
     auto const a = 1 / (t2 * t2);
     auto const b = 2 * d / t2 - 1 / f1;
     auto const c = d * d - f2 * f2;
-    return toSTAmount(
-        asset1Balance.issue(), asset1Balance * solveQuadraticEq(a, b, c));
+
+    // maximize deposit
+    auto const frac = solveQuadraticEq(a, b, c);
+    return multiply(asset1Balance, frac, Number::upward);
 }
 
 /* Equation 7:
@@ -87,7 +94,7 @@ ammAssetIn(
  * where R = b/B, c = R*fee + 2 - fee
  */
 STAmount
-lpTokensOut(
+lpTokensIn(
     STAmount const& asset1Balance,
     STAmount const& asset1Withdraw,
     STAmount const& lptAMMBalance,
@@ -96,8 +103,10 @@ lpTokensOut(
     Number const fr = asset1Withdraw / asset1Balance;
     auto const f1 = getFee(tfee);
     auto const c = fr * f1 + 2 - f1;
-    auto const t = lptAMMBalance * (c - root2(c * c - 4 * fr)) / 2;
-    return toSTAmount(lptAMMBalance.issue(), t);
+
+    // maximize tokens in
+    auto const frac = (c - root2(c * c - 4 * fr)) / 2;
+    return multiply(lptAMMBalance, frac, Number::upward);
 }
 
 /* Equation 8 solves equation 7 for b:
@@ -111,7 +120,7 @@ lpTokensOut(
  * R = (t1**2 + t1*(f - 2)) / (t1*f - 1)
  */
 STAmount
-withdrawByTokens(
+ammAssetOut(
     STAmount const& assetBalance,
     STAmount const& lptAMMBalance,
     STAmount const& lpTokens,
@@ -119,8 +128,10 @@ withdrawByTokens(
 {
     auto const f = getFee(tfee);
     Number const t1 = lpTokens / lptAMMBalance;
-    auto const b = assetBalance * (t1 * t1 - t1 * (2 - f)) / (t1 * f - 1);
-    return toSTAmount(assetBalance.issue(), b);
+
+    // minimize withdraw
+    auto const frac = (t1 * t1 - t1 * (2 - f)) / (t1 * f - 1);
+    return multiply(assetBalance, frac, Number::downward);
 }
 
 Number
@@ -133,12 +144,12 @@ STAmount
 adjustLPTokens(
     STAmount const& lptAMMBalance,
     STAmount const& lpTokens,
-    bool isDeposit)
+    IsDeposit isDeposit)
 {
     // Force rounding downward to ensure adjusted tokens are less or equal
     // to requested tokens.
     saveNumberRoundMode rm(Number::setround(Number::rounding_mode::downward));
-    if (isDeposit)
+    if (isDeposit == IsDeposit::Yes)
         return (lptAMMBalance + lpTokens) - lptAMMBalance;
     return (lpTokens - lptAMMBalance) + lptAMMBalance;
 }
@@ -151,47 +162,10 @@ adjustAmountsByLPTokens(
     STAmount const& lptAMMBalance,
     STAmount const& lpTokens,
     std::uint16_t tfee,
-    bool isDeposit)
+    IsDeposit isDeposit)
 {
-    auto const lpTokensActual =
-        adjustLPTokens(lptAMMBalance, lpTokens, isDeposit);
-
-    if (lpTokensActual == beast::zero)
-    {
-        auto const amount2Opt =
-            amount2 ? std::make_optional(STAmount{}) : std::nullopt;
-        return std::make_tuple(STAmount{}, amount2Opt, lpTokensActual);
-    }
-
-    if (lpTokensActual < lpTokens)
-    {
-        // Equal trade
-        if (amount2)
-        {
-            Number const fr = lpTokensActual / lpTokens;
-            auto const amountActual = toSTAmount(amount.issue(), fr * amount);
-            auto const amount2Actual =
-                toSTAmount(amount2->issue(), fr * *amount2);
-            return std::make_tuple(amountActual, amount2Actual, lpTokensActual);
-        }
-
-        // Single trade
-        auto const amountActual = [&]() {
-            if (isDeposit)
-                return ammAssetIn(
-                    amountBalance, lptAMMBalance, lpTokensActual, tfee);
-            return withdrawByTokens(
-                amountBalance, lptAMMBalance, lpTokensActual, tfee);
-        }();
-
-        return std::make_tuple(amountActual, std::nullopt, lpTokensActual);
-    }
-
-    XRPL_ASSERT(
-        lpTokensActual == lpTokens,
-        "ripple::adjustAmountsByLPTokens : LP tokens match actual");
-
-    return {amount, amount2, lpTokensActual};
+    // AMMv1_3 amendment adjusts tokens and amounts in deposit/withdraw
+    return std::make_tuple(amount, amount2, lpTokens);
 }
 
 Number
@@ -215,4 +189,117 @@ solveQuadraticEqSmallest(Number const& a, Number const& b, Number const& c)
         return (2 * c) / (-b + root2(d));
 }
 
+STAmount
+multiply(STAmount const& amount, Number const& frac, Number::rounding_mode rm)
+{
+    NumberRoundModeGuard g(rm);
+    auto const t = amount * frac;
+    return toSTAmount(amount.issue(), t, rm);
+}
+
+STAmount
+getRoundedAsset(
+    Rules const& rules,
+    std::function<Number()>&& noRoundCb,
+    STAmount const& balance,
+    std::function<Number()>&& productCb,
+    IsDeposit isDeposit)
+{
+    auto const rm = detail::getAssetRounding(isDeposit);
+    if (isDeposit == IsDeposit::Yes)
+        return multiply(balance, productCb(), rm);
+    NumberRoundModeGuard g(rm);
+    return toSTAmount(balance.issue(), productCb(), rm);
+}
+
+STAmount
+getRoundedLPTokens(
+    Rules const& rules,
+    STAmount const& balance,
+    Number const& frac,
+    IsDeposit isDeposit)
+{
+    auto const rm = detail::getLPTokenRounding(isDeposit);
+    auto const tokens = multiply(balance, frac, rm);
+    return adjustLPTokens(balance, tokens, isDeposit);
+}
+
+STAmount
+getRoundedLPTokens(
+    Rules const& rules,
+    std::function<Number()>&& noRoundCb,
+    STAmount const& lptAMMBalance,
+    std::function<Number()>&& productCb,
+    IsDeposit isDeposit)
+{
+    auto const tokens = [&] {
+        auto const rm = detail::getLPTokenRounding(isDeposit);
+        if (isDeposit == IsDeposit::Yes)
+        {
+            NumberRoundModeGuard g(rm);
+            return toSTAmount(lptAMMBalance.issue(), productCb(), rm);
+        }
+        return multiply(lptAMMBalance, productCb(), rm);
+    }();
+    return adjustLPTokens(lptAMMBalance, tokens, isDeposit);
+}
+
+std::pair<STAmount, STAmount>
+adjustAssetInByTokens(
+    Rules const& rules,
+    STAmount const& balance,
+    STAmount const& amount,
+    STAmount const& lptAMMBalance,
+    STAmount const& tokens,
+    std::uint16_t tfee)
+{
+    auto assetAdj = ammAssetIn(balance, lptAMMBalance, tokens, tfee);
+    auto tokensAdj = tokens;
+    // Rounding didn't work the right way.
+    // Try to adjust the original deposit amount by difference
+    // in adjust and original amount. Then adjust tokens and deposit amount.
+    if (assetAdj > amount)
+    {
+        auto const adjAmount = amount - (assetAdj - amount);
+        auto const t = lpTokensOut(balance, adjAmount, lptAMMBalance, tfee);
+        tokensAdj = adjustLPTokens(lptAMMBalance, t, IsDeposit::Yes);
+        assetAdj = ammAssetIn(balance, lptAMMBalance, tokensAdj, tfee);
+    }
+    return {tokensAdj, std::min(amount, assetAdj)};
+}
+
+std::pair<STAmount, STAmount>
+adjustAssetOutByTokens(
+    Rules const& rules,
+    STAmount const& balance,
+    STAmount const& amount,
+    STAmount const& lptAMMBalance,
+    STAmount const& tokens,
+    std::uint16_t tfee)
+{
+    auto assetAdj = ammAssetOut(balance, lptAMMBalance, tokens, tfee);
+    auto tokensAdj = tokens;
+    // Rounding didn't work the right way.
+    // Try to adjust the original deposit amount by difference
+    // in adjust and original amount. Then adjust tokens and deposit amount.
+    if (assetAdj > amount)
+    {
+        auto const adjAmount = amount - (assetAdj - amount);
+        auto const t = lpTokensIn(balance, adjAmount, lptAMMBalance, tfee);
+        tokensAdj = adjustLPTokens(lptAMMBalance, t, IsDeposit::No);
+        assetAdj = ammAssetOut(balance, lptAMMBalance, tokensAdj, tfee);
+    }
+    return {tokensAdj, std::min(amount, assetAdj)};
+}
+
+Number
+adjustFracByTokens(
+    Rules const& rules,
+    STAmount const& lptAMMBalance,
+    STAmount const& tokens,
+    Number const& frac)
+{
+    return tokens / lptAMMBalance;
+}
+
 }  // namespace ripple

src/xrpld/app/misc/detail/AMMUtils.cpp

@@ -16,6 +16,8 @@
     OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */
 //==============================================================================
+
+#include <xrpld/app/misc/AMMHelpers.h>
 #include <xrpld/app/misc/AMMUtils.h>
 #include <xrpld/ledger/Sandbox.h>
 #include <xrpl/basics/Log.h>
@@ -462,4 +464,32 @@ isOnlyLiquidityProvider(
     return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
 }
 
+Expected<bool, TER>
+verifyAndAdjustLPTokenBalance(
+    Sandbox& sb,
+    STAmount const& lpTokens,
+    std::shared_ptr<SLE>& ammSle,
+    AccountID const& account)
+{
+    if (auto const res = isOnlyLiquidityProvider(sb, lpTokens.issue(), account);
+        !res)
+        return Unexpected<TER>(res.error());
+    else if (res.value())
+    {
+        if (withinRelativeDistance(
+                lpTokens,
+                ammSle->getFieldAmount(sfLPTokenBalance),
+                Number{1, -3}))
+        {
+            ammSle->setFieldAmount(sfLPTokenBalance, lpTokens);
+            sb.update(ammSle);
+        }
+        else
+        {
+            return Unexpected<TER>(tecAMM_INVALID_TOKENS);
+        }
+    }
+    return true;
+}
+
 }  // namespace ripple

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

@@ -79,6 +79,21 @@ AMMBid::preflight(PreflightContext const& ctx)
             JLOG(ctx.j.debug()) << "AMM Bid: Invalid number of AuthAccounts.";
             return temMALFORMED;
         }
+        else
+        {
+            AccountID account = ctx.tx[sfAccount];
+            std::set<AccountID> unique;
+            for (auto const& obj : authAccounts)
+            {
+                auto authAccount = obj[sfAccount];
+                if (authAccount == account || unique.contains(authAccount))
+                {
+                    JLOG(ctx.j.debug()) << "AMM Bid: Invalid auth.account.";
+                    return temMALFORMED;
+                }
+                unique.insert(authAccount);
+            }
+        }
     }
 
     return preflight2(ctx);
@@ -233,7 +248,9 @@ applyBid(
             auctionSlot.makeFieldAbsent(sfAuthAccounts);
         // Burn the remaining bid amount
         auto const saBurn = adjustLPTokens(
-            lptAMMBalance, toSTAmount(lptAMMBalance.issue(), burn), false);
+            lptAMMBalance,
+            toSTAmount(lptAMMBalance.issue(), burn),
+            IsDeposit::No);
         if (saBurn >= lptAMMBalance)
         {
             // This error case should never occur.

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

@@ -151,6 +151,17 @@ AMMClawback::applyGuts(Sandbox& sb)
     if (!accountSle)
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
+    // retrieve LP token balance inside the amendment gate to avoid
+    // inconsistent error behavior
+    auto const lpTokenBalance = ammLPHolds(sb, *ammSle, holder, j_);
+    if (lpTokenBalance == beast::zero)
+        return tecAMM_BALANCE;
+
+    if (auto const res =
+            verifyAndAdjustLPTokenBalance(sb, lpTokenBalance, ammSle, holder);
+        !res)
+        return res.error();  // LCOV_EXCL_LINE
+
     auto const expected = ammHolds(
         sb,
         *ammSle,
@@ -248,10 +259,11 @@ AMMClawback::equalWithdrawMatchingOneAmount(
     STAmount const& amount)
 {
     auto frac = Number{amount} / amountBalance;
-    auto const amount2Withdraw = amount2Balance * frac;
+    auto amount2Withdraw = amount2Balance * frac;
 
     auto const lpTokensWithdraw =
         toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac);
+
     if (lpTokensWithdraw > holdLPtokens)
         // if lptoken balance less than what the issuer intended to clawback,
         // clawback all the tokens. Because we are doing a two-asset withdrawal,
@@ -272,18 +284,33 @@ AMMClawback::equalWithdrawMatchingOneAmount(
             mPriorBalance,
             ctx_.journal);
 
-    // Because we are doing a two-asset withdrawal,
-    // tfee is actually not used, so pass tfee as 0.
+    auto const& rules = sb.rules();
+
+    auto tokensAdj =
+        getRoundedLPTokens(rules, lptAMMBalance, frac, IsDeposit::No);
+
+    // LCOV_EXCL_START
+    if (tokensAdj == beast::zero)
+        return {tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, std::nullopt};
+    // LCOV_EXCL_STOP
+
+    frac = adjustFracByTokens(rules, lptAMMBalance, tokensAdj, frac);
+    auto amount2Rounded =
+        getRoundedAsset(rules, amount2Balance, frac, IsDeposit::No);
+
+    auto amountRounded =
+        getRoundedAsset(rules, amountBalance, frac, IsDeposit::No);
+
     return AMMWithdraw::withdraw(
         sb,
         ammSle,
         ammAccount,
         holder,
         amountBalance,
-        amount,
-        toSTAmount(amount2Balance.issue(), amount2Withdraw),
+        amountRounded,
+        amount2Rounded,
         lptAMMBalance,
-        toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac),
+        tokensAdj,
         0,
         FreezeHandling::fhIGNORE_FREEZE,
         WithdrawAll::No,

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

@@ -545,7 +545,7 @@ AMMDeposit::deposit(
                 lptAMMBalance,
                 lpTokensDeposit,
                 tfee,
-                true);
+                IsDeposit::Yes);
 
     if (lpTokensDepositActual <= beast::zero)
     {
@@ -628,6 +628,15 @@ AMMDeposit::deposit(
     return {tesSUCCESS, lptAMMBalance + lpTokensDepositActual};
 }
 
+static STAmount
+adjustLPTokensOut(
+    Rules const& rules,
+    STAmount const& lptAMMBalance,
+    STAmount const& lpTokensDeposit)
+{
+    return adjustLPTokens(lptAMMBalance, lpTokensDeposit, IsDeposit::Yes);
+}
+
 /** Proportional deposit of pools assets in exchange for the specified
  * amount of LPTokens.
  */
@@ -645,16 +654,25 @@ AMMDeposit::equalDepositTokens(
 {
     try
     {
+        auto const tokensAdj =
+            adjustLPTokensOut(view.rules(), lptAMMBalance, lpTokensDeposit);
+        if (tokensAdj == beast::zero)
+            return {tecAMM_INVALID_TOKENS, STAmount{}};
         auto const frac =
-            divide(lpTokensDeposit, lptAMMBalance, lptAMMBalance.issue());
+            divide(tokensAdj, lptAMMBalance, lptAMMBalance.issue());
+        // amounts factor in the adjusted tokens
+        auto const amountDeposit =
+            getRoundedAsset(view.rules(), amountBalance, frac, IsDeposit::Yes);
+        auto const amount2Deposit =
+            getRoundedAsset(view.rules(), amount2Balance, frac, IsDeposit::Yes);
         return deposit(
             view,
             ammAccount,
             amountBalance,
-            multiply(amountBalance, frac, amountBalance.issue()),
-            multiply(amount2Balance, frac, amount2Balance.issue()),
+            amountDeposit,
+            amount2Deposit,
             lptAMMBalance,
-            lpTokensDeposit,
+            tokensAdj,
             depositMin,
             deposit2Min,
             std::nullopt,
@@ -711,37 +729,49 @@ AMMDeposit::equalDepositLimit(
     std::uint16_t tfee)
 {
     auto frac = Number{amount} / amountBalance;
-    auto tokens = toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac);
-    if (tokens == beast::zero)
-        return {tecAMM_FAILED, STAmount{}};
-    auto const amount2Deposit = amount2Balance * frac;
+    auto tokensAdj =
+        getRoundedLPTokens(view.rules(), lptAMMBalance, frac, IsDeposit::Yes);
+    if (tokensAdj == beast::zero)
+    {
+        return {tecAMM_INVALID_TOKENS, STAmount{}};
+    }
+    // factor in the adjusted tokens
+    frac = adjustFracByTokens(view.rules(), lptAMMBalance, tokensAdj, frac);
+    auto const amount2Deposit =
+        getRoundedAsset(view.rules(), amount2Balance, frac, IsDeposit::Yes);
     if (amount2Deposit <= amount2)
         return deposit(
             view,
             ammAccount,
             amountBalance,
             amount,
-            toSTAmount(amount2Balance.issue(), amount2Deposit),
+            amount2Deposit,
             lptAMMBalance,
-            tokens,
+            tokensAdj,
             std::nullopt,
             std::nullopt,
             lpTokensDepositMin,
             tfee);
     frac = Number{amount2} / amount2Balance;
-    tokens = toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac);
-    if (tokens == beast::zero)
-        return {tecAMM_FAILED, STAmount{}};
-    auto const amountDeposit = amountBalance * frac;
+    tokensAdj =
+        getRoundedLPTokens(view.rules(), lptAMMBalance, frac, IsDeposit::Yes);
+    if (tokensAdj == beast::zero)
+    {
+        return {tecAMM_INVALID_TOKENS, STAmount{}};  // LCOV_EXCL_LINE
+    }
+    // factor in the adjusted tokens
+    frac = adjustFracByTokens(view.rules(), lptAMMBalance, tokensAdj, frac);
+    auto const amountDeposit =
+        getRoundedAsset(view.rules(), amountBalance, frac, IsDeposit::Yes);
     if (amountDeposit <= amount)
         return deposit(
             view,
             ammAccount,
             amountBalance,
-            toSTAmount(amountBalance.issue(), amountDeposit),
+            amountDeposit,
             amount2,
             lptAMMBalance,
-            tokens,
+            tokensAdj,
             std::nullopt,
             std::nullopt,
             lpTokensDepositMin,
@@ -767,17 +797,27 @@ AMMDeposit::singleDeposit(
     std::optional<STAmount> const& lpTokensDepositMin,
     std::uint16_t tfee)
 {
-    auto const tokens = lpTokensIn(amountBalance, amount, lptAMMBalance, tfee);
+    auto const tokens = adjustLPTokensOut(
+        view.rules(),
+        lptAMMBalance,
+        lpTokensOut(amountBalance, amount, lptAMMBalance, tfee));
     if (tokens == beast::zero)
-        return {tecAMM_FAILED, STAmount{}};
+    {
+        return {tecAMM_INVALID_TOKENS, STAmount{}};
+    }
+    // factor in the adjusted tokens
+    auto const [tokensAdj, amountDepositAdj] = adjustAssetInByTokens(
+        view.rules(), amountBalance, amount, lptAMMBalance, tokens, tfee);
+    if (tokensAdj == beast::zero)
+        return {tecAMM_INVALID_TOKENS, STAmount{}};  // LCOV_EXCL_LINE
     return deposit(
         view,
         ammAccount,
         amountBalance,
-        amount,
+        amountDepositAdj,
         std::nullopt,
         lptAMMBalance,
-        tokens,
+        tokensAdj,
         std::nullopt,
         std::nullopt,
         lpTokensDepositMin,
@@ -801,8 +841,13 @@ AMMDeposit::singleDepositTokens(
     STAmount const& lpTokensDeposit,
     std::uint16_t tfee)
 {
+    auto const tokensAdj =
+        adjustLPTokensOut(view.rules(), lptAMMBalance, lpTokensDeposit);
+    if (tokensAdj == beast::zero)
+        return {tecAMM_INVALID_TOKENS, STAmount{}};
+    // the adjusted tokens are factored in
     auto const amountDeposit =
-        ammAssetIn(amountBalance, lptAMMBalance, lpTokensDeposit, tfee);
+        ammAssetIn(amountBalance, lptAMMBalance, tokensAdj, tfee);
     if (amountDeposit > amount)
         return {tecAMM_FAILED, STAmount{}};
     return deposit(
@@ -812,7 +857,7 @@ AMMDeposit::singleDepositTokens(
         amountDeposit,
         std::nullopt,
         lptAMMBalance,
-        lpTokensDeposit,
+        tokensAdj,
         std::nullopt,
         std::nullopt,
         std::nullopt,
@@ -856,20 +901,29 @@ AMMDeposit::singleDepositEPrice(
 {
     if (amount != beast::zero)
     {
-        auto const tokens =
-            lpTokensIn(amountBalance, amount, lptAMMBalance, tfee);
+        auto const tokens = adjustLPTokensOut(
+            view.rules(),
+            lptAMMBalance,
+            lpTokensOut(amountBalance, amount, lptAMMBalance, tfee));
         if (tokens <= beast::zero)
-            return {tecAMM_FAILED, STAmount{}};
-        auto const ep = Number{amount} / tokens;
+        {
+            return {tecAMM_INVALID_TOKENS, STAmount{}};
+        }
+        // factor in the adjusted tokens
+        auto const [tokensAdj, amountDepositAdj] = adjustAssetInByTokens(
+            view.rules(), amountBalance, amount, lptAMMBalance, tokens, tfee);
+        if (tokensAdj == beast::zero)
+            return {tecAMM_INVALID_TOKENS, STAmount{}};  // LCOV_EXCL_LINE
+        auto const ep = Number{amountDepositAdj} / tokensAdj;
         if (ep <= ePrice)
             return deposit(
                 view,
                 ammAccount,
                 amountBalance,
-                amount,
+                amountDepositAdj,
                 std::nullopt,
                 lptAMMBalance,
-                tokens,
+                tokensAdj,
                 std::nullopt,
                 std::nullopt,
                 std::nullopt,
@@ -900,21 +954,37 @@ AMMDeposit::singleDepositEPrice(
     auto const a1 = c * c;
     auto const b1 = c * c * f2 * f2 + 2 * c - d * d;
     auto const c1 = 2 * c * f2 * f2 + 1 - 2 * d * f2;
-    auto const amountDeposit = toSTAmount(
-        amountBalance.issue(),
-        f1 * amountBalance * solveQuadraticEq(a1, b1, c1));
+    auto amtNoRoundCb = [&] {
+        return f1 * amountBalance * solveQuadraticEq(a1, b1, c1);
+    };
+    auto amtProdCb = [&] { return f1 * solveQuadraticEq(a1, b1, c1); };
+    auto const amountDeposit = getRoundedAsset(
+        view.rules(), amtNoRoundCb, amountBalance, amtProdCb, IsDeposit::Yes);
     if (amountDeposit <= beast::zero)
         return {tecAMM_FAILED, STAmount{}};
-    auto const tokens =
-        toSTAmount(lptAMMBalance.issue(), amountDeposit / ePrice);
+    auto tokNoRoundCb = [&] { return amountDeposit / ePrice; };
+    auto tokProdCb = [&] { return amountDeposit / ePrice; };
+    auto const tokens = getRoundedLPTokens(
+        view.rules(), tokNoRoundCb, lptAMMBalance, tokProdCb, IsDeposit::Yes);
+    // factor in the adjusted tokens
+    auto const [tokensAdj, amountDepositAdj] = adjustAssetInByTokens(
+        view.rules(),
+        amountBalance,
+        amountDeposit,
+        lptAMMBalance,
+        tokens,
+        tfee);
+    if (tokensAdj == beast::zero)
+        return {tecAMM_INVALID_TOKENS, STAmount{}};  // LCOV_EXCL_LINE
+
     return deposit(
         view,
         ammAccount,
         amountBalance,
-        amountDeposit,
+        amountDepositAdj,
         std::nullopt,
         lptAMMBalance,
-        tokens,
+        tokensAdj,
         std::nullopt,
         std::nullopt,
         std::nullopt,

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

@@ -313,24 +313,9 @@ AMMWithdraw::applyGuts(Sandbox& sb)
     // might not match the LP's trustline balance
 
     if (auto const res =
-            isOnlyLiquidityProvider(sb, lpTokens.issue(), account_);
+            verifyAndAdjustLPTokenBalance(sb, lpTokens, ammSle, account_);
         !res)
         return {res.error(), false};
-    else if (res.value())
-    {
-        if (withinRelativeDistance(
-                lpTokens,
-                ammSle->getFieldAmount(sfLPTokenBalance),
-                Number{1, -3}))
-        {
-            ammSle->setFieldAmount(sfLPTokenBalance, lpTokens);
-            sb.update(ammSle);
-        }
-        else
-        {
-            return {tecAMM_INVALID_TOKENS, false};
-        }
-    }
 
     auto const tfee = getTradingFee(ctx_.view(), *ammSle, account_);
 
@@ -523,7 +508,7 @@ AMMWithdraw::withdraw(
                 lpTokensAMMBalance,
                 lpTokensWithdraw,
                 tfee,
-                false);
+                IsDeposit::No);
         return std::make_tuple(
             amountWithdraw, amount2Withdraw, lpTokensWithdraw);
     }();
@@ -682,6 +667,20 @@ AMMWithdraw::withdraw(
         amount2WithdrawActual);
 }
 
+static STAmount
+adjustLPTokensIn(
+    Rules const& rules,
+    STAmount const& lptAMMBalance,
+    STAmount const& lpTokensWithdraw,
+    WithdrawAll withdrawAll)
+{
+    if (withdrawAll == WithdrawAll::Yes)
+        return lpTokensWithdraw;
+    return adjustLPTokens(lptAMMBalance, lpTokensWithdraw, IsDeposit::No);
+}
+
+/** Proportional withdrawal of pool assets for the amount of LPTokens.
+ */
 std::pair<TER, STAmount>
 AMMWithdraw::equalWithdrawTokens(
     Sandbox& view,
@@ -785,16 +784,22 @@ AMMWithdraw::equalWithdrawTokens(
                 journal);
         }
 
-        auto const frac = divide(lpTokensWithdraw, lptAMMBalance, noIssue());
-        auto const withdrawAmount =
-            multiply(amountBalance, frac, amountBalance.issue());
-        auto const withdraw2Amount =
-            multiply(amount2Balance, frac, amount2Balance.issue());
+        auto const tokensAdj = adjustLPTokensIn(
+            view.rules(), lptAMMBalance, lpTokensWithdraw, withdrawAll);
+        if (tokensAdj == beast::zero)
+            return {
+                tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, std::nullopt};
+        // the adjusted tokens are factored in
+        auto const frac = divide(tokensAdj, lptAMMBalance, noIssue());
+        auto const amountWithdraw =
+            getRoundedAsset(view.rules(), amountBalance, frac, IsDeposit::No);
+        auto const amount2Withdraw =
+            getRoundedAsset(view.rules(), amount2Balance, frac, IsDeposit::No);
         // LP is making equal withdrawal by tokens but the requested amount
         // of LP tokens is likely too small and results in one-sided pool
         // withdrawal due to round off. Fail so the user withdraws
         // more tokens.
-        if (withdrawAmount == beast::zero || withdraw2Amount == beast::zero)
+        if (amountWithdraw == beast::zero || amount2Withdraw == beast::zero)
             return {tecAMM_FAILED, STAmount{}, STAmount{}, STAmount{}};
 
         return withdraw(
@@ -803,10 +808,10 @@ AMMWithdraw::equalWithdrawTokens(
             ammAccount,
             account,
             amountBalance,
-            withdrawAmount,
-            withdraw2Amount,
+            amountWithdraw,
+            amount2Withdraw,
             lptAMMBalance,
-            lpTokensWithdraw,
+            tokensAdj,
             tfee,
             freezeHanding,
             withdrawAll,
@@ -861,7 +866,16 @@ AMMWithdraw::equalWithdrawLimit(
     std::uint16_t tfee)
 {
     auto frac = Number{amount} / amountBalance;
-    auto const amount2Withdraw = amount2Balance * frac;
+    auto amount2Withdraw =
+        getRoundedAsset(view.rules(), amount2Balance, frac, IsDeposit::No);
+    auto tokensAdj =
+        getRoundedLPTokens(view.rules(), lptAMMBalance, frac, IsDeposit::No);
+    if (tokensAdj == beast::zero)
+        return {tecAMM_INVALID_TOKENS, STAmount{}};
+    // factor in the adjusted tokens
+    frac = adjustFracByTokens(view.rules(), lptAMMBalance, tokensAdj, frac);
+    amount2Withdraw =
+        getRoundedAsset(view.rules(), amount2Balance, frac, IsDeposit::No);
     if (amount2Withdraw <= amount2)
     {
         return withdraw(
@@ -870,26 +884,34 @@ AMMWithdraw::equalWithdrawLimit(
             ammAccount,
             amountBalance,
             amount,
-            toSTAmount(amount2.issue(), amount2Withdraw),
+            amount2Withdraw,
             lptAMMBalance,
-            toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac),
+            tokensAdj,
             tfee);
     }
 
     frac = Number{amount2} / amount2Balance;
-    auto const amountWithdraw = amountBalance * frac;
-    XRPL_ASSERT(
-        amountWithdraw <= amount,
-        "ripple::AMMWithdraw::equalWithdrawLimit : maximum amountWithdraw");
+    auto amountWithdraw =
+        getRoundedAsset(view.rules(), amountBalance, frac, IsDeposit::No);
+    tokensAdj =
+        getRoundedLPTokens(view.rules(), lptAMMBalance, frac, IsDeposit::No);
+    if (tokensAdj == beast::zero)
+        return {tecAMM_INVALID_TOKENS, STAmount{}};  // LCOV_EXCL_LINE
+    // factor in the adjusted tokens
+    frac = adjustFracByTokens(view.rules(), lptAMMBalance, tokensAdj, frac);
+    amountWithdraw =
+        getRoundedAsset(view.rules(), amountBalance, frac, IsDeposit::No);
+    if (amountWithdraw > amount)
+        return {tecAMM_FAILED, STAmount{}};  // LCOV_EXCL_LINE
     return withdraw(
         view,
         ammSle,
         ammAccount,
         amountBalance,
-        toSTAmount(amount.issue(), amountWithdraw),
+        amountWithdraw,
         amount2,
         lptAMMBalance,
-        toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac),
+        tokensAdj,
         tfee);
 }
 
@@ -908,19 +930,29 @@ AMMWithdraw::singleWithdraw(
     STAmount const& amount,
     std::uint16_t tfee)
 {
-    auto const tokens = lpTokensOut(amountBalance, amount, lptAMMBalance, tfee);
+    auto const tokens = adjustLPTokensIn(
+        view.rules(),
+        lptAMMBalance,
+        lpTokensIn(amountBalance, amount, lptAMMBalance, tfee),
+        isWithdrawAll(ctx_.tx));
     if (tokens == beast::zero)
-        return {tecAMM_FAILED, STAmount{}};
-
+    {
+        return {tecAMM_INVALID_TOKENS, STAmount{}};
+    }
+    // factor in the adjusted tokens
+    auto const [tokensAdj, amountWithdrawAdj] = adjustAssetOutByTokens(
+        view.rules(), amountBalance, amount, lptAMMBalance, tokens, tfee);
+    if (tokensAdj == beast::zero)
+        return {tecAMM_INVALID_TOKENS, STAmount{}};  // LCOV_EXCL_LINE
     return withdraw(
         view,
         ammSle,
         ammAccount,
         amountBalance,
-        amount,
+        amountWithdrawAdj,
         std::nullopt,
         lptAMMBalance,
-        tokens,
+        tokensAdj,
         tfee);
 }
 
@@ -945,8 +977,13 @@ AMMWithdraw::singleWithdrawTokens(
     STAmount const& lpTokensWithdraw,
     std::uint16_t tfee)
 {
+    auto const tokensAdj = adjustLPTokensIn(
+        view.rules(), lptAMMBalance, lpTokensWithdraw, isWithdrawAll(ctx_.tx));
+    if (tokensAdj == beast::zero)
+        return {tecAMM_INVALID_TOKENS, STAmount{}};
+    // the adjusted tokens are factored in
     auto const amountWithdraw =
-        withdrawByTokens(amountBalance, lptAMMBalance, lpTokensWithdraw, tfee);
+        ammAssetOut(amountBalance, lptAMMBalance, tokensAdj, tfee);
     if (amount == beast::zero || amountWithdraw >= amount)
     {
         return withdraw(
@@ -957,7 +994,7 @@ AMMWithdraw::singleWithdrawTokens(
             amountWithdraw,
             std::nullopt,
             lptAMMBalance,
-            lpTokensWithdraw,
+            tokensAdj,
             tfee);
     }
 
@@ -1006,11 +1043,24 @@ AMMWithdraw::singleWithdrawEPrice(
     // t = T*(T + A*E*(f - 2))/(T*f - A*E)
     Number const ae = amountBalance * ePrice;
     auto const f = getFee(tfee);
-    auto const tokens = lptAMMBalance * (lptAMMBalance + ae * (f - 2)) /
-        (lptAMMBalance * f - ae);
-    if (tokens <= 0)
-        return {tecAMM_FAILED, STAmount{}};
-    auto const amountWithdraw = toSTAmount(amount.issue(), tokens / ePrice);
+    auto tokNoRoundCb = [&] {
+        return lptAMMBalance * (lptAMMBalance + ae * (f - 2)) /
+            (lptAMMBalance * f - ae);
+    };
+    auto tokProdCb = [&] {
+        return (lptAMMBalance + ae * (f - 2)) / (lptAMMBalance * f - ae);
+    };
+    auto const tokensAdj = getRoundedLPTokens(
+        view.rules(), tokNoRoundCb, lptAMMBalance, tokProdCb, IsDeposit::No);
+    if (tokensAdj <= beast::zero)
+    {
+        return {tecAMM_INVALID_TOKENS, STAmount{}};
+    }
+    auto amtNoRoundCb = [&] { return tokensAdj / ePrice; };
+    auto amtProdCb = [&] { return tokensAdj / ePrice; };
+    // the adjusted tokens are factored in
+    auto const amountWithdraw = getRoundedAsset(
+        view.rules(), amtNoRoundCb, amount, amtProdCb, IsDeposit::No);
     if (amount == beast::zero || amountWithdraw >= amount)
     {
         return withdraw(
@@ -1021,7 +1071,7 @@ AMMWithdraw::singleWithdrawEPrice(
             amountWithdraw,
             std::nullopt,
             lptAMMBalance,
-            toSTAmount(lptAMMBalance.issue(), tokens),
+            tokensAdj,
             tfee);
     }
 

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

@@ -301,7 +301,7 @@ private:
         std::uint16_t tfee);
 
     /** Check from the flags if it's withdraw all */
-    WithdrawAll
+    static WithdrawAll
     isWithdrawAll(STTx const& tx);
 };
 

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

@@ -17,6 +17,9 @@
 */
 //==============================================================================
 
+#include <xrpld/app/misc/AMMHelpers.h>
+#include <xrpld/app/misc/AMMUtils.h>
+#include <xrpld/app/misc/CredentialHelpers.h>
 #include <xrpld/app/tx/detail/InvariantCheck.h>
 
 #include <xrpld/app/misc/CredentialHelpers.h>
@@ -1674,4 +1677,309 @@ ValidPermissionedDomain::finalize(
         (sleStatus_[1] ? check(*sleStatus_[1], j) : true);
 }
 
+void
+ValidAMM::visitEntry(
+    bool isDelete,
+    std::shared_ptr<SLE const> const& before,
+    std::shared_ptr<SLE const> const& after)
+{
+    if (isDelete)
+        return;
+
+    if (after)
+    {
+        auto const type = after->getType();
+        // AMM object changed
+        if (type == ltAMM)
+        {
+            ammAccount_ = after->getAccountID(sfAccount);
+            lptAMMBalanceAfter_ = after->getFieldAmount(sfLPTokenBalance);
+        }
+        // AMM pool changed
+        else if (
+            (type == ltRIPPLE_STATE && after->getFlags() & lsfAMMNode) ||
+            (type == ltACCOUNT_ROOT && after->isFieldPresent(sfAMMID)))
+        {
+            ammPoolChanged_ = true;
+        }
+    }
+
+    if (before)
+    {
+        // AMM object changed
+        if (before->getType() == ltAMM)
+        {
+            lptAMMBalanceBefore_ = before->getFieldAmount(sfLPTokenBalance);
+        }
+    }
+}
+
+static bool
+validBalances(
+    STAmount const& amount,
+    STAmount const& amount2,
+    STAmount const& lptAMMBalance,
+    ValidAMM::ZeroAllowed zeroAllowed)
+{
+    bool const positive = amount > beast::zero && amount2 > beast::zero &&
+        lptAMMBalance > beast::zero;
+    if (zeroAllowed == ValidAMM::ZeroAllowed::Yes)
+        return positive ||
+            (amount == beast::zero && amount2 == beast::zero &&
+             lptAMMBalance == beast::zero);
+    return positive;
+}
+
+bool
+ValidAMM::finalizeVote(bool enforce, beast::Journal const& j) const
+{
+    if (lptAMMBalanceAfter_ != lptAMMBalanceBefore_ || ammPoolChanged_)
+    {
+        // LPTokens and the pool can not change on vote
+        // LCOV_EXCL_START
+        JLOG(j.error()) << "AMMVote invariant failed: "
+                        << lptAMMBalanceBefore_.value_or(STAmount{}) << " "
+                        << lptAMMBalanceAfter_.value_or(STAmount{}) << " "
+                        << ammPoolChanged_;
+        if (enforce)
+            return false;
+        // LCOV_EXCL_STOP
+    }
+
+    return true;
+}
+
+bool
+ValidAMM::finalizeBid(bool enforce, beast::Journal const& j) const
+{
+    if (ammPoolChanged_)
+    {
+        // The pool can not change on bid
+        // LCOV_EXCL_START
+        JLOG(j.error()) << "AMMBid invariant failed: pool changed";
+        if (enforce)
+            return false;
+        // LCOV_EXCL_STOP
+    }
+    // LPTokens are burnt, therefore there should be fewer LPTokens
+    else if (
+        lptAMMBalanceBefore_ && lptAMMBalanceAfter_ &&
+        (*lptAMMBalanceAfter_ > *lptAMMBalanceBefore_ ||
+         *lptAMMBalanceAfter_ <= beast::zero))
+    {
+        // LCOV_EXCL_START
+        JLOG(j.error()) << "AMMBid invariant failed: " << *lptAMMBalanceBefore_
+                        << " " << *lptAMMBalanceAfter_;
+        if (enforce)
+            return false;
+        // LCOV_EXCL_STOP
+    }
+
+    return true;
+}
+
+bool
+ValidAMM::finalizeCreate(
+    STTx const& tx,
+    ReadView const& view,
+    bool enforce,
+    beast::Journal const& j) const
+{
+    if (!ammAccount_)
+    {
+        // LCOV_EXCL_START
+        JLOG(j.error())
+            << "AMMCreate invariant failed: AMM object is not created";
+        if (enforce)
+            return false;
+        // LCOV_EXCL_STOP
+    }
+    else
+    {
+        auto const [amount, amount2] = ammPoolHolds(
+            view,
+            *ammAccount_,
+            tx[sfAmount].get<Issue>(),
+            tx[sfAmount2].get<Issue>(),
+            fhIGNORE_FREEZE,
+            j);
+        // Create invariant:
+        // sqrt(amount * amount2) == LPTokens
+        // all balances are greater than zero
+        if (!validBalances(
+                amount, amount2, *lptAMMBalanceAfter_, ZeroAllowed::No) ||
+            ammLPTokens(amount, amount2, lptAMMBalanceAfter_->issue()) !=
+                *lptAMMBalanceAfter_)
+        {
+            JLOG(j.error()) << "AMMCreate invariant failed: " << amount << " "
+                            << amount2 << " " << *lptAMMBalanceAfter_;
+            if (enforce)
+                return false;
+        }
+    }
+
+    return true;
+}
+
+bool
+ValidAMM::finalizeDelete(bool enforce, TER res, beast::Journal const& j) const
+{
+    if (ammAccount_)
+    {
+        // LCOV_EXCL_START
+        std::string const msg = (res == tesSUCCESS)
+            ? "AMM object is not deleted on tesSUCCESS"
+            : "AMM object is changed on tecINCOMPLETE";
+        JLOG(j.error()) << "AMMDelete invariant failed: " << msg;
+        if (enforce)
+            return false;
+        // LCOV_EXCL_STOP
+    }
+
+    return true;
+}
+
+bool
+ValidAMM::finalizeDEX(bool enforce, beast::Journal const& j) const
+{
+    if (ammAccount_)
+    {
+        // LCOV_EXCL_START
+        JLOG(j.error()) << "AMM swap invariant failed: AMM object changed";
+        if (enforce)
+            return false;
+        // LCOV_EXCL_STOP
+    }
+
+    return true;
+}
+
+bool
+ValidAMM::generalInvariant(
+    ripple::STTx const& tx,
+    ripple::ReadView const& view,
+    ZeroAllowed zeroAllowed,
+    beast::Journal const& j) const
+{
+    auto const [amount, amount2] = ammPoolHolds(
+        view,
+        *ammAccount_,
+        tx[sfAsset].get<Issue>(),
+        tx[sfAsset2].get<Issue>(),
+        fhIGNORE_FREEZE,
+        j);
+    // Deposit and Withdrawal invariant:
+    // sqrt(amount * amount2) >= LPTokens
+    // all balances are greater than zero
+    // unless on last withdrawal
+    auto const poolProductMean = root2(amount * amount2);
+    bool const nonNegativeBalances =
+        validBalances(amount, amount2, *lptAMMBalanceAfter_, zeroAllowed);
+    bool const strongInvariantCheck = poolProductMean >= *lptAMMBalanceAfter_;
+    // Allow for a small relative error if strongInvariantCheck fails
+    auto weakInvariantCheck = [&]() {
+        return *lptAMMBalanceAfter_ != beast::zero &&
+            withinRelativeDistance(
+                poolProductMean, Number{*lptAMMBalanceAfter_}, Number{1, -11});
+    };
+    if (!nonNegativeBalances ||
+        (!strongInvariantCheck && !weakInvariantCheck()))
+    {
+        JLOG(j.error()) << "AMM " << tx.getTxnType() << " invariant failed: "
+                        << tx.getHash(HashPrefix::transactionID) << " "
+                        << ammPoolChanged_ << " " << amount << " " << amount2
+                        << " " << poolProductMean << " "
+                        << lptAMMBalanceAfter_->getText() << " "
+                        << ((*lptAMMBalanceAfter_ == beast::zero)
+                                ? Number{1}
+                                : ((*lptAMMBalanceAfter_ - poolProductMean) /
+                                   poolProductMean));
+        return false;
+    }
+
+    return true;
+}
+
+bool
+ValidAMM::finalizeDeposit(
+    ripple::STTx const& tx,
+    ripple::ReadView const& view,
+    bool enforce,
+    beast::Journal const& j) const
+{
+    if (!ammAccount_)
+    {
+        // LCOV_EXCL_START
+        JLOG(j.error()) << "AMMDeposit invariant failed: AMM object is deleted";
+        if (enforce)
+            return false;
+        // LCOV_EXCL_STOP
+    }
+    else if (!generalInvariant(tx, view, ZeroAllowed::No, j) && enforce)
+        return false;
+
+    return true;
+}
+
+bool
+ValidAMM::finalizeWithdraw(
+    ripple::STTx const& tx,
+    ripple::ReadView const& view,
+    bool enforce,
+    beast::Journal const& j) const
+{
+    if (!ammAccount_)
+    {
+        // Last Withdraw or Clawback deleted AMM
+    }
+    else if (!generalInvariant(tx, view, ZeroAllowed::Yes, j))
+    {
+        if (enforce)
+            return false;
+    }
+
+    return true;
+}
+
+bool
+ValidAMM::finalize(
+    STTx const& tx,
+    TER const result,
+    XRPAmount const,
+    ReadView const& view,
+    beast::Journal const& j)
+{
+    // Delete may return tecINCOMPLETE if there are too many
+    // trustlines to delete.
+    if (result != tesSUCCESS && result != tecINCOMPLETE)
+        return true;
+
+    bool const enforce = true;  // view.rules().enabled(fixAMMv1_3);
+
+    switch (tx.getTxnType())
+    {
+        case ttAMM_CREATE:
+            return finalizeCreate(tx, view, enforce, j);
+        case ttAMM_DEPOSIT:
+            return finalizeDeposit(tx, view, enforce, j);
+        case ttAMM_CLAWBACK:
+        case ttAMM_WITHDRAW:
+            return finalizeWithdraw(tx, view, enforce, j);
+        case ttAMM_BID:
+            return finalizeBid(enforce, j);
+        case ttAMM_VOTE:
+            return finalizeVote(enforce, j);
+        case ttAMM_DELETE:
+            return finalizeDelete(enforce, result, j);
+        case ttCHECK_CASH:
+        case ttOFFER_CREATE:
+        case ttPAYMENT:
+            return finalizeDEX(enforce, j);
+        default:
+            break;
+    }
+
+    return true;
+}
+
 }  // namespace ripple

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

@@ -617,6 +617,69 @@ public:
         beast::Journal const&);
 };
 
+class ValidAMM
+{
+    std::optional<AccountID> ammAccount_;
+    std::optional<STAmount> lptAMMBalanceAfter_;
+    std::optional<STAmount> lptAMMBalanceBefore_;
+    bool ammPoolChanged_;
+
+public:
+    enum class ZeroAllowed : bool { No = false, Yes = true };
+
+    ValidAMM() : ammPoolChanged_{false}
+    {
+    }
+    void
+    visitEntry(
+        bool,
+        std::shared_ptr<SLE const> const&,
+        std::shared_ptr<SLE const> const&);
+
+    bool
+    finalize(
+        STTx const&,
+        TER const,
+        XRPAmount const,
+        ReadView const&,
+        beast::Journal const&);
+
+private:
+    bool
+    finalizeBid(bool enforce, beast::Journal const&) const;
+    bool
+    finalizeVote(bool enforce, beast::Journal const&) const;
+    bool
+    finalizeCreate(
+        STTx const&,
+        ReadView const&,
+        bool enforce,
+        beast::Journal const&) const;
+    bool
+    finalizeDelete(bool enforce, TER res, beast::Journal const&) const;
+    bool
+    finalizeDeposit(
+        STTx const&,
+        ReadView const&,
+        bool enforce,
+        beast::Journal const&) const;
+    // Includes clawback
+    bool
+    finalizeWithdraw(
+        STTx const&,
+        ReadView const&,
+        bool enforce,
+        beast::Journal const&) const;
+    bool
+    finalizeDEX(bool enforce, beast::Journal const&) const;
+    bool
+    generalInvariant(
+        STTx const&,
+        ReadView const&,
+        ZeroAllowed zeroAllowed,
+        beast::Journal const&) const;
+};
+
 // additional invariant checks can be declared above and then added to this
 // tuple
 using InvariantChecks = std::tuple<
@@ -636,7 +699,8 @@ using InvariantChecks = std::tuple<
     NFTokenCountTracking,
     ValidClawback,
     ValidMPTIssuance,
-    ValidPermissionedDomain>;
+    ValidPermissionedDomain,
+    ValidAMM>;
 
 /**
  * @brief get a tuple of all invariant checks

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

@@ -21,6 +21,8 @@
 #define RIPPLE_APP_BOOK_OFFER_H_INCLUDED
 
 #include <xrpld/ledger/View.h>
+
+#include <xrpl/basics/Log.h>
 #include <xrpl/basics/contract.h>
 #include <xrpl/protocol/Quality.h>
 #include <xrpl/protocol/Rules.h>
@@ -169,8 +171,21 @@ public:
      * always returns true.
      */
     bool
-    checkInvariant(TAmounts<TIn, TOut> const&, beast::Journal j) const
+    checkInvariant(TAmounts<TIn, TOut> const& consumed, beast::Journal j) const
     {
+        if (consumed.in > m_amounts.in || consumed.out > m_amounts.out)
+        {
+            // LCOV_EXCL_START
+            JLOG(j.error())
+                << "AMMOffer::checkInvariant failed: consumed "
+                << to_string(consumed.in) << " " << to_string(consumed.out)
+                << " amounts " << to_string(m_amounts.in) << " "
+                << to_string(m_amounts.out);
+
+            return false;
+            // LCOV_EXCL_STOP
+        }
+
         return true;
     }
 };