Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions include/xrpl/ledger/CachedView.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class CachedViewImpl : public DigestAwareReadView
return base_.succ(key, last);
}

std::optional<key_type>
pred(key_type const& key, std::optional<key_type> const& first = std::nullopt) const override
{
return base_.pred(key, first);
}

std::unique_ptr<SlesType::iter_base>
slesBegin() const override
{
Expand Down
3 changes: 3 additions & 0 deletions include/xrpl/ledger/Ledger.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ class Ledger final : public std::enable_shared_from_this<Ledger>,
std::optional<uint256>
succ(uint256 const& key, std::optional<uint256> const& last = std::nullopt) const override;

std::optional<uint256>
pred(uint256 const& key, std::optional<uint256> const& first = std::nullopt) const override;

SLE::const_pointer
read(Keylet const& k) const override;

Expand Down
3 changes: 3 additions & 0 deletions include/xrpl/ledger/OpenView.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ class OpenView final : public ReadView, public TxsRawView
std::optional<key_type>
succ(key_type const& key, std::optional<key_type> const& last = std::nullopt) const override;

std::optional<key_type>
pred(key_type const& key, std::optional<key_type> const& first = std::nullopt) const override;

SLE::const_pointer
read(Keylet const& k) const override;

Expand Down
13 changes: 13 additions & 0 deletions include/xrpl/ledger/ReadView.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,19 @@ class ReadView
[[nodiscard]] virtual std::optional<key_type>
succ(key_type const& key, std::optional<key_type> const& last = std::nullopt) const = 0;

/** Return the key of the previous state item.

This returns the key of the first state item
whose key is less than the specified key. If
no such key is present, std::nullopt is returned.

If `first` is engaged, returns std::nullopt when
the key returned would be outside the open
interval (first, key).
*/
[[nodiscard]] virtual std::optional<key_type>
pred(key_type const& key, std::optional<key_type> const& first = std::nullopt) const = 0;

/** Return the state item associated with a key.

Effects:
Expand Down
3 changes: 3 additions & 0 deletions include/xrpl/ledger/detail/ApplyStateTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class ApplyStateTable
[[nodiscard]] std::optional<key_type>
succ(ReadView const& base, key_type const& key, std::optional<key_type> const& last) const;

[[nodiscard]] std::optional<key_type>
pred(ReadView const& base, key_type const& key, std::optional<key_type> const& first) const;

[[nodiscard]] SLE::const_pointer
read(ReadView const& base, Keylet const& k) const;

Expand Down
3 changes: 3 additions & 0 deletions include/xrpl/ledger/detail/ApplyViewBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ class ApplyViewBase : public ApplyView, public RawView
[[nodiscard]] std::optional<key_type>
succ(key_type const& key, std::optional<key_type> const& last = std::nullopt) const override;

[[nodiscard]] std::optional<key_type>
pred(key_type const& key, std::optional<key_type> const& first = std::nullopt) const override;

[[nodiscard]] SLE::const_pointer
read(Keylet const& k) const override;

Expand Down
3 changes: 3 additions & 0 deletions include/xrpl/ledger/detail/RawStateTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ class RawStateTable
[[nodiscard]] std::optional<key_type>
succ(ReadView const& base, key_type const& key, std::optional<key_type> const& last) const;

[[nodiscard]] std::optional<key_type>
pred(ReadView const& base, key_type const& key, std::optional<key_type> const& first) const;

void
erase(SLE::ref sle);

Expand Down
249 changes: 249 additions & 0 deletions include/xrpl/ledger/helpers/AMMCurve.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,249 @@
// Pluggable AMM curve architecture.
// Concentrated liquidity (CurveType 1) based on XRPL-Standards Discussion #427
// by Roman Thpt (@RomThpt), which adapted Uniswap v3 tick math, fee tier
// structure, and fee accounting to the XRPL. This implementation extends that
// work with a pluggable curve interface, StableSwap, and Smart AMM.
// See: https://github.com/XRPLF/XRPL-Standards/discussions/427

#pragma once

#include <xrpl/basics/Expected.h>
#include <xrpl/basics/Number.h>
#include <xrpl/ledger/helpers/AMMHelpers.h>
#include <xrpl/protocol/AMMCore.h>
#include <xrpl/protocol/AmountConversions.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/Quality.h>
#include <xrpl/protocol/Rules.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STObject.h>
#include <xrpl/protocol/TER.h>

namespace xrpl {

class ReadView;
class ApplyView;

struct CurveContext
{
ReadView const* view = nullptr;
uint256 const* ammID = nullptr;
// Optional out: set to true if a CL swap-walk terminated because it hit
// maxTickCrossings. Callers pass a pointer when they want to detect
// the cap (e.g. to emit tecAMM_TICK_CAP_HIT); pass nullptr to ignore.
bool* tickCapHit = nullptr;
};

class CurveInterface
{
public:
virtual ~CurveInterface() = default;

virtual Expected<STAmount, TER>
swapIn(
STAmount const& poolIn,
STAmount const& poolOut,
STAmount const& assetIn,
std::uint16_t tfee,
STObject const* ammSle,
CurveContext const& ctx = {}) const = 0;

virtual Expected<STAmount, TER>
swapOut(
STAmount const& poolIn,
STAmount const& poolOut,
STAmount const& assetOut,
std::uint16_t tfee,
STObject const* ammSle,
CurveContext const& ctx = {}) const = 0;

virtual Expected<Number, TER>
spotPrice(
STAmount const& poolIn,
STAmount const& poolOut,
std::uint16_t tfee,
STObject const* ammSle,
CurveContext const& ctx = {}) const = 0;

[[nodiscard]] virtual TER
validateParams(STObject const& tx) const = 0;

virtual Expected<STAmount, TER>
initialLPTokens(
STAmount const& asset1,
STAmount const& asset2,
Issue const& lptIssue,
STObject const* txParams) const = 0;

virtual bool
checkInvariant(
STAmount const& oldIn,
STAmount const& oldOut,
STAmount const& newIn,
STAmount const& newOut,
STObject const* ammSle) const = 0;

// Apply a realized swap to the AMM SLE. Trustline balances are updated
// by the caller (BookStep). For CP and StableSwap the pool state is fully
// implicit in trustline balances, so the default is a no-op. CL must
// mutate currentTick/activeLiquidity/feeGrowthGlobal and flip
// feeGrowthOutside on crossed ticks, because none of that state is
// derivable from the trustlines alone.
virtual TER
applySwap(
ApplyView& /*view*/,
uint256 const& /*ammID*/,
STAmount const& /*assetIn*/,
STAmount const& /*assetOut*/,
std::uint16_t /*tfee*/,
STObject const* /*curveParams*/) const
{
return tesSUCCESS;
}
};

CurveInterface const*
getCurve(std::uint8_t curveType, Rules const& rules);

// Max output the pool can deliver before crossing the next initialised
// tick boundary in the swap direction. Audit #19: caller (AMMLiquidity's
// offer generation) uses this to cap an advertised AMM offer at the
// current tick range, so the offer's quality reflects only the marginal
// range — not a blended average across multiple tick crossings.
//
// Returns std::nullopt if there is no further tick in the swap direction
// (no cap; offer is bounded only by reserves) or if the pool has no
// active liquidity. Returns 0 if the swap is already at the boundary.
// Caller should treat nullopt as "no cap".
std::optional<Number>
maxClOutputWithinCurrentRange(
ReadView const& view,
uint256 const& ammID,
STObject const& ammSle,
bool zeroForOne);

// Equivalent for CtBinned: cap the advertised AMMOffer output at the
// active bin's reserve. Without this cap, AMMLiquidity quotes against
// the pool's aggregate balance — which spans multiple bins at different
// prices — and BookStep mispricesthe offer's quality vs CLOB. Capping
// per active bin lets BookStep iterate naturally, getting each bin's
// marginal price one offer at a time.
//
// Returns std::nullopt if no active bin exists (empty pool) or the
// active bin lacks the output asset.
std::optional<Number>
maxBinnedOutputAtActiveBin(
ReadView const& view,
uint256 const& ammID,
STObject const& ammSle,
bool inIsAsset0);

// ─── CL tick bitmap ─────────────────────────────────────────────────────
//
// Sparse 256-tick-per-word presence bitmap. See spec §3.6.

// Convert a tick to its (wordIndex, bitInWord) position. Offset-binary so
// arithmetic stays in unsigned domain. Uses kTickBitmapOffset (= -minTick)
// from AMMCore.h — single source of truth, do NOT duplicate inline.
inline std::pair<std::uint16_t, std::uint8_t>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function accepts any std::int32_t tick but performs no bounds check against [minTick, maxTick] before computing the bitmap position. For a tick one below minTick (-887273), the signed addition produces -1, which wraps to UINT32_MAX on the unsigned cast, and the subsequent static_cast<std::uint16_t> then truncates to 65535 — a wordIndex at the far end of the address space rather than an error. Neither setTickBitmap nor clearTickBitmap add this guard either, so they silently address the wrong bitmap SLE and corrupt tick-presence state. Current callers (AMMDeposit preflight) do validate tick bounds upstream, so there is no active exploit path, but the function is marked noexcept with no internal defense and is a fragile trap for any future caller. Add XRPL_ASSERT(tick >= minTick && tick <= maxTick, "tickToBitmapPos: tick out of range") inside setTickBitmap/clearTickBitmap before calling this function.

Suggested fix

Add XRPL_ASSERT(tick >= minTick && tick <= maxTick, "setTickBitmap: tick out of range") at the top of both setTickBitmap and clearTickBitmap before the call to tickToBitmapPos. This catches misuse in debug builds and documents the precondition at the enforcement site rather than relying solely on distant preflight checks.

tickToBitmapPos(std::int32_t tick) noexcept
{
auto const offsetT = static_cast<std::uint32_t>(
tick + static_cast<std::int32_t>(kTickBitmapOffset));
return {static_cast<std::uint16_t>(offsetT >> 8),
static_cast<std::uint8_t>(offsetT & 0xFFu)};
}

inline std::int32_t
bitmapPosToTick(std::uint16_t wordIndex, std::uint8_t bitInWord) noexcept
{
auto const offsetT =
(static_cast<std::uint32_t>(wordIndex) << 8) | bitInWord;
return static_cast<std::int32_t>(offsetT) -
static_cast<std::int32_t>(kTickBitmapOffset);
}

// Bit-test for the bitmap word storage. `bits` is the raw `sfBitmapBits`
// value read off the SLE. Convention: bit i = LSB of byte (i/8), little-
// endian within bytes. The convention is internal — callers that write
// bits must use the same scheme (and do, via the maintenance helpers).
inline bool
bitmapBitIsSet(uint256 const& bits, std::uint8_t pos) noexcept
{
return ((bits.data()[pos / 8]) >> (pos % 8)) & 1u;
}

// AMMDeposit and AMMWithdraw call these when a tick crosses the
// "initialised / uninitialised" boundary (sfLiquidityGross transitioning
// 0↔>0). Each pool has a sparse set of `ltAMM_TICK_BITMAP` SLEs covering
// 256 ticks each; setting / clearing creates and deletes those SLEs on
// demand. Idempotent — calling set on an already-set bit is a no-op.
//
// Returns tesSUCCESS on the happy path. The current implementation has
// no failure path beyond the AMM SLE being missing; reserved as TER for
// forward compatibility.
TER
setTickBitmap(ApplyView& view, uint256 const& ammID, std::int32_t tick, beast::Journal j);

TER
clearTickBitmap(ApplyView& view, uint256 const& ammID, std::int32_t tick, beast::Journal j);

inline std::uint8_t
getCurveType(SLE const& ammSle)
{
if (ammSle.isFieldPresent(sfCurveType))
return ammSle.getFieldU8(sfCurveType);
return CtConstantProduct;
}

template <typename TIn, typename TOut>
TOut
curveSwapIn(
TAmounts<TIn, TOut> const& pool,
TIn const& assetIn,
std::uint16_t tfee,
std::uint8_t curveType,
STObject const* ammSle,
CurveContext const& cctx = {})
{
if (curveType == CtConstantProduct)
return swapAssetIn(pool, assetIn, tfee);

if (auto const* curve = getCurve(curveType, *getCurrentTransactionRules()))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both template functions unconditionally dereference getCurrentTransactionRules() — which returns std::optional<Rules> const& — without checking whether the optional is engaged. Every other AMM helper in the codebase (swapAssetIn, swapAssetOut, AMMHelpers.h, Offer.h, STAmount.cpp) guards this call with !rules || before accessing the value, because the optional IS empty outside a Transactor context. The code path path_find / ripple_path_find RPC → PathRequest → Pathfinder → RippleCalc::rippleCalculate → BookStep → AMMLiquidity::getOffer → curveSwapIn/curveSwapOut installs no CurrentTransactionRulesGuard anywhere along the chain, so the optional is always empty when called from RPC. Dereferencing an empty optional is undefined behavior; in practice it is a null-pointer read that crashes the node, giving any remote caller with RPC access a trivial denial-of-service against any validator running a non-constant-product AMM pool.

Suggested fix

Replace getCurve(curveType, *getCurrentTransactionRules()) with (getCurrentTransactionRules() ? getCurve(curveType, *getCurrentTransactionRules()) : nullptr), or restructure the block as if (auto const& rules = getCurrentTransactionRules()) { if (auto const* curve = getCurve(curveType, *rules)) { ... } }. The same fix is needed at line 238 in curveSwapOut.

{
auto const stPoolIn = toSTAmount(pool.in);
auto const stPoolOut = toSTAmount(pool.out);
auto const stAssetIn = toSTAmount(assetIn);
if (auto const result = curve->swapIn(stPoolIn, stPoolOut, stAssetIn, tfee, ammSle, cctx))
return get<TOut>(*result);
}
return toAmount<TOut>(getAsset(pool.out), 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When getCurve() returns nullptr or when curve->swapIn()/swapOut() returns an error TER, both template functions silently substitute sentinel amounts instead of propagating the failure: curveSwapIn returns a zero-output amount and curveSwapOut returns the maximum representable input amount. None of the six call sites in AMMLiquidity.cpp and AMMOffer.cpp treat these sentinels as errors — they assign them directly into offer amounts and forward them to BookStep. The curveSwapOut max-input sentinel is especially dangerous: BookStep's subsequent mulRatio() call multiplies max-native or max-IOU amounts by the transfer rate, producing an STAmount overflow; even without overflow, the settlement input disagrees with the quality BookStep computed from pool balances, causing mispriced payment fills. Change both templates to return Expected<TOut,TER> / Expected<TIn,TER> so errors propagate cleanly to BookStep.

Suggested fix

Change the return type of curveSwapIn to Expected<TOut, TER> and curveSwapOut to Expected<TIn, TER>. Return the TER from the failed curve->swapIn/swapOut call instead of the sentinel. Update all callers in AMMLiquidity.cpp (generateFibSeqOffer lines 85/109, maxOffer lines 159/212) and AMMOffer.cpp (limitIn line 161, limitOut line 135) to propagate or handle the error.

}

template <typename TIn, typename TOut>
TIn
curveSwapOut(
TAmounts<TIn, TOut> const& pool,
TOut const& assetOut,
std::uint16_t tfee,
std::uint8_t curveType,
STObject const* ammSle,
CurveContext const& cctx = {})
{
if (curveType == CtConstantProduct)
return swapAssetOut(pool, assetOut, tfee);

if (auto const* curve = getCurve(curveType, *getCurrentTransactionRules()))
{
auto const stPoolIn = toSTAmount(pool.in);
auto const stPoolOut = toSTAmount(pool.out);
auto const stAssetOut = toSTAmount(assetOut);
if (auto const result = curve->swapOut(stPoolIn, stPoolOut, stAssetOut, tfee, ammSle, cctx))
return get<TIn>(*result);
}
return toMaxAmount<TIn>(getAsset(pool.in));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When getCurve returns nullptr or curve->swapOut returns an error (e.g., activeLiquidity == 0, tick cap hit, amendment disabled), curveSwapOut silently falls back to toMaxAmount<TIn> — the maximum representable value — with no indication of failure. This is asymmetric with curveSwapIn at line 222, which returns zero on failure; the zero is safely filtered by the > beast::kZero guard in AMMLiquidity::getOffer, but toMaxAmount is a large positive value that passes that guard. The resulting AMMOffer has a legitimate-looking out and a sky-high in, which flows from generateFibSeqOffer and maxOffer into AMMOffer::limitOut and from there directly into BookStep strand accounting without any saturation check. Callers have no way to distinguish a legitimately expensive swap from a failed one. The return type should be Expected<TIn, TER> so errors propagate explicitly, or the fallback should be unified to zero (with callers updated) so both failure modes are caught by the existing > beast::kZero filter.

Suggested fix

Change the return type of curveSwapOut to Expected<TIn, TER> and return Unexpected(tecAMM_FAILED) on the failure path, matching the CurveInterface::swapOut contract. Update call sites in AMMOffer::limitOut and AMMLiquidity::maxOffer/generateFibSeqOffer to handle the Expected and propagate the error upward rather than silently consuming a sentinel value.

}

} // namespace xrpl
10 changes: 8 additions & 2 deletions include/xrpl/ledger/helpers/AMMHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,8 @@ ammLPHolds(
Asset const& asset2,
AccountID const& ammAccount,
AccountID const& lpAccount,
beast::Journal const j);
beast::Journal const j,
std::uint8_t curveType = CtConstantProduct);

STAmount
ammLPHolds(
Expand All @@ -826,7 +827,12 @@ ammAccountHolds(ReadView const& view, AccountID const& ammAccountID, Asset const
* AMM object and account are deleted. Otherwise tecINCOMPLETE is returned.
*/
TER
deleteAMMAccount(Sandbox& view, Asset const& asset, Asset const& asset2, beast::Journal j);
deleteAMMAccount(
Sandbox& view,
Asset const& asset,
Asset const& asset2,
beast::Journal j,
std::uint8_t curveType = 0);

/** Initialize Auction and Voting slots and set the trading/discounted fee.
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value for curveType uses the bare literal 0 rather than the named constant CtConstantProduct, which is inconsistent with ammLPHolds in the same diff. While numerically identical today, any future caller that omits curveType when deleting a non-CP pool (CL/StableSwap/Binned) will silently pass 0 to keylet::amm(asset, asset2, curveType), producing the CP-variant keylet — either finding the wrong pool to delete if a CP pool shares the same asset pair, or returning tecINTERNAL and leaving the non-CP pool in the ledger permanently. Remove the default entirely and require callers to pass curveType explicitly, mirroring the pattern already used in AMMDelete::doApply and AMMWithdraw::deleteAMMAccountIfEmpty.

Suggested fix

Remove the = 0 default from the declaration: deleteAMMAccount(Sandbox& view, Asset const& asset, Asset const& asset2, beast::Journal j, std::uint8_t curveType);. All existing call sites already pass curveType explicitly, so no callers need updating. If a default is truly required, use = CtConstantProduct for semantic clarity, matching the convention in ammLPHolds.

Expand Down
19 changes: 19 additions & 0 deletions include/xrpl/ledger/helpers/AMMTickMath.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#pragma once

#include <xrpl/basics/Number.h>
#include <xrpl/protocol/AMMCore.h>

#include <cstdint>

namespace xrpl {

Number
tickToSqrtPrice(std::int32_t tick);

std::int32_t
sqrtPriceToTick(Number const& sqrtPrice);

bool
isValidTick(std::int32_t tick, std::int32_t tickSpacing);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function passes tick directly to std::abs() then casts to unsigned with no prior bounds check against [minTick, maxTick]. For tick == INT32_MIN, std::abs(INT32_MIN) is signed integer overflow — undefined behavior in C++. Nine call sites in AMMCurve.cpp (swapIn, swapOut, spotPrice, applySwap, maxClOutputWithinCurrentRange) pass sfCurrentTick or findNextTick() results read directly from SLEs without re-checking the legal range at the call site. Under normal operation these values stay within bounds because the write paths are constrained, but the absence of a defensive check means any SLE with an out-of-range tick field causes UB inside a swap computation. The function should validate its input — either clamp to [minTick, maxTick] and document the contract, or return an error for out-of-range inputs so callers can propagate a tec* code instead of hitting UB.

Suggested fix

Add a bounds check at the top of tickToSqrtPrice: if (tick < minTick || tick > maxTick) return Number{0}; (or return an Expected<Number,TER>). Callers in AMMCurve.cpp that read sfCurrentTick should assert the value is within [minTick, maxTick] immediately after the read.


} // namespace xrpl
Loading