From 351cd456b917e3e6b17e039df4c399cf727ff9e4 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Fri, 22 May 2026 19:04:24 -0400 Subject: [PATCH 01/27] margin: add place_reduce_only_market_order_and_repay_loan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundles a reduce-only market order with a loan repayment so a leveraged position can be wound down atomically. The standalone reduce-only entry checks risk_ratio on the swap-only state, before any repay — a market close pays the spread, which strictly lowers the oracle-valued ratio and aborts. Repaying first deleverages the manager, lifting the ratio and absorbing the slippage; the monotonic check then passes on the net state. Slippage stays bounded by the price-tolerance band, so deferring the solvency check past the repay does not weaken security. This is also the only wind-down path for a manager whose ratio has drifted into the liquidation..min_borrow danger zone: it cannot reach the borrow floor in a single swap, but it can climb out by deleveraging. Exposes margin_manager::risk_ratio_int and ::repay as public(package) so pool_proxy can compute the single-debt-pool risk ratio and repay; both were already the internal impls (no new wrappers). Co-Authored-By: Claude Opus 4.7 --- .claude/rules/move.md | 10 + .../sources/margin_manager.move | 92 ++-- .../deepbook_margin/sources/pool_proxy.move | 115 ++++ .../tests/helper/test_helpers.move | 107 ++++ .../tests/pool_proxy_tests.move | 493 ++++++++++++++++++ 5 files changed, 774 insertions(+), 43 deletions(-) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index 46e7f2969..73994262d 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -98,6 +98,16 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - Avoid deprecated Sui framework functions. Use the current recommended API (e.g., `coin_registry::new_currency_with_otw` instead of `coin::create_currency`). If a deprecated function must be used, add a comment explaining why the replacement doesn't work for this case. +## DeepBook Margin: risk ratio, solvency, and slippage + +- `risk_ratio` (`margin_manager.move:risk_ratio_int`) is `assets / debt` with **assets valued at the Pyth oracle price, not the orderbook execution price**. Debt only changes on `borrow`/`repay`/`liquidate`, never on a trade. Consequence: any close/trade that *executes worse than oracle* (real slippage) hands oracle-measured value to the counterparty while debt is fixed, so it **strictly lowers the measured risk ratio**. + +- Two distinct post-trade invariants in `pool_proxy.move`: regular trades use `assert_post_trade_solvent` (floor = `min_borrow_risk_ratio`, ~1.25); reduce-only trades use `assert_reduce_only_monotonic` (`ratio_after >= ratio_before`). The monotonic check fires on the **swap-only intermediate state, before any `repay`**, where slippage is pure value leak — so a reduce-only *market* close that pays the spread aborts on `EReduceOnlyMustImproveRiskRatio`. This is intentional and is asserted by `pool_proxy_tests.move::test_place_reduce_only_market_order_ok`, which is an `expected_failure`. Deleveraging only improves the ratio when the `repay` is included; check the invariant on the net (swap+repay) state, not the swap alone. + +- Slippage tolerance and solvency are orthogonal knobs. Allowable slippage belongs in the per-pool `price_tolerance` band enforced by `margin_registry.move::assert_price` (bids capped above oracle, asks capped below; 1%–50%, default 5%) — not in the risk-ratio check. Do not relax the risk-ratio floor to make room for slippage. + +- Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. + ## Tool Calling Instructions - `sui move build` to build the package, must be run in a directory with Move.toml in it diff --git a/packages/deepbook_margin/sources/margin_manager.move b/packages/deepbook_margin/sources/margin_manager.move index ed23e4abb..3bf386c77 100644 --- a/packages/deepbook_margin/sources/margin_manager.move +++ b/packages/deepbook_margin/sources/margin_manager.move @@ -1357,9 +1357,12 @@ public(package) fun deposit_int( balance_manager.deposit_with_cap(deposit_cap, coin, ctx); } -// === Private Functions === -// Get the risk ratio of the margin manager. -fun risk_ratio_int( +/// Risk ratio of the manager against the single debt-side `margin_pool`. +/// Exposed at package scope for the bundled close-and-repay proxy flow, which +/// holds only the `&mut MarginPool` it is about to repay into and so +/// cannot use the two-pool `risk_ratio` helper (that would need an immutable +/// borrow of the same pool). Returns `max_risk_ratio` when there is no debt. +public(package) fun risk_ratio_int( self: &MarginManager, registry: &MarginRegistry, base_oracle: &PriceInfoObject, @@ -1389,6 +1392,49 @@ fun risk_ratio_int( } } +/// Repays the loan using the margin manager. Owner is validated via +/// `repay_withdraw`. `amount` of `none` repays as much as the manager's +/// `RepayAsset` balance allows, capped at the outstanding debt. +/// Returns the total amount repaid. +public(package) fun repay( + self: &mut MarginManager, + margin_pool: &mut MarginPool, + amount: Option, + clock: &Clock, + ctx: &mut TxContext, +): u64 { + let borrowed_shares = self.borrowed_base_shares.max(self.borrowed_quote_shares); + let borrowed_amount = margin_pool.borrow_shares_to_amount(borrowed_shares, clock); + let available_balance = self.balance_manager().balance(); + let repay_amount = amount.destroy_with_default(available_balance); + let repay_amount = repay_amount.min(borrowed_amount); + let repay_shares = math::mul(borrowed_shares, math::div(repay_amount, borrowed_amount)); + + let coin: Coin = self.repay_withdraw(repay_amount, ctx); + margin_pool.repay(repay_shares, coin, clock); + + if (type_name::with_defining_ids() == type_name::with_defining_ids()) { + self.borrowed_base_shares = self.borrowed_base_shares - repay_shares; + } else { + self.borrowed_quote_shares = self.borrowed_quote_shares - repay_shares; + }; + + if (self.borrowed_base_shares == 0 && self.borrowed_quote_shares == 0) { + self.margin_pool_id = option::none(); + }; + + event::emit(LoanRepaidEvent { + margin_manager_id: self.id(), + margin_pool_id: margin_pool.id(), + repay_amount, + repay_shares, + timestamp: clock.timestamp_ms(), + }); + + repay_amount +} + +// === Private Functions === fun risk_ratio_int_unsafe( self: &MarginManager, registry: &MarginRegistry, @@ -1476,46 +1522,6 @@ fun validate_owner( assert!(ctx.sender() == self.owner, EInvalidMarginManagerOwner); } -/// Repays the loan using the margin manager. -/// Returns the total amount repaid -fun repay( - self: &mut MarginManager, - margin_pool: &mut MarginPool, - amount: Option, - clock: &Clock, - ctx: &mut TxContext, -): u64 { - let borrowed_shares = self.borrowed_base_shares.max(self.borrowed_quote_shares); - let borrowed_amount = margin_pool.borrow_shares_to_amount(borrowed_shares, clock); - let available_balance = self.balance_manager().balance(); - let repay_amount = amount.destroy_with_default(available_balance); - let repay_amount = repay_amount.min(borrowed_amount); - let repay_shares = math::mul(borrowed_shares, math::div(repay_amount, borrowed_amount)); - - let coin: Coin = self.repay_withdraw(repay_amount, ctx); - margin_pool.repay(repay_shares, coin, clock); - - if (type_name::with_defining_ids() == type_name::with_defining_ids()) { - self.borrowed_base_shares = self.borrowed_base_shares - repay_shares; - } else { - self.borrowed_quote_shares = self.borrowed_quote_shares - repay_shares; - }; - - if (self.borrowed_base_shares == 0 && self.borrowed_quote_shares == 0) { - self.margin_pool_id = option::none(); - }; - - event::emit(LoanRepaidEvent { - margin_manager_id: self.id(), - margin_pool_id: margin_pool.id(), - repay_amount, - repay_shares, - timestamp: clock.timestamp_ms(), - }); - - repay_amount -} - fun liquidation_withdraw( self: &mut MarginManager, withdraw_amount: u64, diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index b771277d3..922c00b45 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -445,6 +445,121 @@ public fun place_reduce_only_market_order_v2( order_info } +/// Closes (winds down) a leveraged position atomically: places a reduce-only +/// market order, settles the taker proceeds, repays as much of the loan as the +/// settled balance allows, and only then enforces the monotonic solvency +/// invariant on the *net* (post-repay) state. +/// +/// Why this needs to be a single entry rather than composing +/// `place_reduce_only_market_order_v2` + `repay_*` in a PTB: the standalone +/// reduce-only entry checks `risk_ratio_after >= risk_ratio_before` on the +/// swap-only state, before any repay. A market close pays the spread, so the +/// swap alone strictly lowers the oracle-valued risk ratio (debt is unchanged +/// until repay) and that standalone check aborts. Repaying first deleverages +/// the manager, which lifts the ratio and absorbs the slippage; the monotonic +/// check then passes on the net result. Slippage stays bounded by the +/// price-tolerance band (`assert_price`), so deferring the solvency check past +/// the repay does not weaken security. This is also the only close path +/// available to a manager whose ratio has drifted into the +/// `liquidation..min_borrow` band — it cannot reach the borrow floor in a +/// single swap, but it can climb out by deleveraging. +/// +/// Takes a single `&mut MarginPool` (the borrowed side) rather than +/// both margin pools, mirroring `liquidate`: the pool must be borrowed mutably +/// to repay, so the risk ratio is computed against it via +/// `risk_ratio_int`. `calculate_debts` validates both that +/// `margin_pool` is the manager's actual debt pool and that `DebtAsset` is the +/// borrowed side. +public fun place_reduce_only_market_order_and_repay_loan( + registry: &MarginRegistry, + margin_manager: &mut MarginManager, + pool: &mut Pool, + margin_pool: &mut MarginPool, + base_oracle: &PriceInfoObject, + quote_oracle: &PriceInfoObject, + client_order_id: u64, + self_matching_option: u8, + quantity: u64, + is_bid: bool, + pay_with_deep: bool, + clock: &Clock, + ctx: &mut TxContext, +): OrderInfo { + registry.load_inner(); + assert!(margin_manager.deepbook_pool() == pool.id(), EIncorrectDeepBookPool); + + let (base_debt, quote_debt) = margin_manager.calculate_debts(margin_pool, clock); + let (base_asset, quote_asset) = margin_manager.calculate_assets(pool); + + let (effective_price, quote_quantity) = calculate_effective_price( + pool, + quantity, + is_bid, + pay_with_deep, + clock, + ); + + // Reduce-only: a bid may only buy back as much base as the net base debt; + // an ask may only sell for as much quote as the net quote debt. + assert!( + (is_bid && base_debt > base_asset && quantity <= base_debt - base_asset) || + (!is_bid && quote_debt > quote_asset && quote_quantity <= quote_debt - quote_asset), + ENotReduceOnlyOrder, + ); + + registry.assert_price(pool.id(), effective_price, is_bid, clock); + + let risk_ratio_before = margin_manager.risk_ratio_int( + registry, + base_oracle, + quote_oracle, + pool, + margin_pool, + clock, + ); + + let trade_proof = margin_manager.trade_proof(ctx); + let balance_manager = margin_manager.balance_manager_trading_mut(ctx); + let order_info = pool.place_market_order( + balance_manager, + &trade_proof, + client_order_id, + self_matching_option, + quantity, + is_bid, + pay_with_deep, + clock, + ctx, + ); + // Move the taker proceeds out of pool-settled state into the manager's + // balance so `repay` can draw on them. + pool.withdraw_settled_amounts(balance_manager, &trade_proof); + + // Repay as much of the debt as the (now-settled) debt-asset balance allows. + margin_manager.repay(margin_pool, option::none(), clock, ctx); + + // Net-state solvency: if any debt remains the close must not have worsened + // the ratio. A full repay clears the debt (trivially solvent), so the check + // is skipped — `risk_ratio_int` would return `max_risk_ratio` for + // zero debt anyway. + if ( + margin_manager.borrowed_base_shares() > 0 + || margin_manager.borrowed_quote_shares() > 0 + ) { + let risk_ratio_after = margin_manager.risk_ratio_int( + registry, + base_oracle, + quote_oracle, + pool, + margin_pool, + clock, + ); + assert!(risk_ratio_after >= risk_ratio_before, EReduceOnlyMustImproveRiskRatio); + }; + + order_info +} + /// Modifies an order public fun modify_order( registry: &MarginRegistry, diff --git a/packages/deepbook_margin/tests/helper/test_helpers.move b/packages/deepbook_margin/tests/helper/test_helpers.move index 629e1f82b..76093c374 100644 --- a/packages/deepbook_margin/tests/helper/test_helpers.move +++ b/packages/deepbook_margin/tests/helper/test_helpers.move @@ -1045,6 +1045,71 @@ public fun setup_orderbook_liquidity_stablecoin( return_shared(pool); } +/// Places a single bid + ask at the given 1e9-scaled prices. For tests that +/// drift the oracle and need liquidity around the new price (the fixed +/// `setup_orderbook_liquidity_stablecoin` bids/asks sit at $0.99/$1.01). +public fun setup_orderbook_liquidity_at_prices( + scenario: &mut Scenario, + pool_id: ID, + bid_price: u64, + ask_price: u64, + clock: &Clock, +) { + use deepbook::balance_manager; + + scenario.next_tx(test_constants::user2()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut balance_manager = balance_manager::new(scenario.ctx()); + + balance_manager.deposit( + mint_coin(1_000_000 * test_constants::usdc_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + balance_manager.deposit( + mint_coin(1_000_000 * test_constants::usdt_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + balance_manager.deposit( + mint_coin(10000 * test_constants::deep_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + + let trade_proof = balance_manager.generate_proof_as_owner(scenario.ctx()); + + pool.place_limit_order( + &mut balance_manager, + &trade_proof, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + ask_price, + 10000 * test_constants::usdc_multiplier(), + false, + false, + constants::max_u64(), + clock, + scenario.ctx(), + ); + + pool.place_limit_order( + &mut balance_manager, + &trade_proof, + 2, + constants::no_restriction(), + constants::self_matching_allowed(), + bid_price, + 10000 * test_constants::usdc_multiplier(), + true, + false, + constants::max_u64(), + clock, + scenario.ctx(), + ); + + transfer::public_transfer(balance_manager, test_constants::user2()); + return_shared(pool); +} + /// Sets up orderbook liquidity with prices OUTSIDE the 5% tolerance for testing price bound failures. /// Asks at $1.10 (10% above oracle) - will fail upper bound check for market buys /// Bids at $0.90 (10% below oracle) - will fail lower bound check for market sells @@ -1379,6 +1444,48 @@ public fun place_reduce_only_market_order_v2_for_test( order_info } +/// Wraps `place_reduce_only_market_order_and_repay_loan` with demo ($1.00) +/// oracle prices built/destroyed inline. For tests that drift the oracle, call +/// the proxy entry directly with custom price objects instead. +public fun place_reduce_only_market_order_and_repay_loan_for_test( + scenario: &mut Scenario, + registry: &MarginRegistry, + mm: &mut deepbook_margin::margin_manager::MarginManager, + pool: &mut Pool, + margin_pool: &mut MarginPool, + client_order_id: u64, + self_matching_option: u8, + quantity: u64, + is_bid: bool, + pay_with_deep: bool, + clock: &Clock, +): deepbook::order_info::OrderInfo { + let base_oracle = build_price_info_for_type(scenario, clock); + let quote_oracle = build_price_info_for_type(scenario, clock); + let order_info = pool_proxy::place_reduce_only_market_order_and_repay_loan< + BaseAsset, + QuoteAsset, + DebtAsset, + >( + registry, + mm, + pool, + margin_pool, + &base_oracle, + "e_oracle, + client_order_id, + self_matching_option, + quantity, + is_bid, + pay_with_deep, + clock, + scenario.ctx(), + ); + destroy(base_oracle); + destroy(quote_oracle); + order_info +} + public fun execute_conditional_orders_v2_for_test( scenario: &mut Scenario, base_margin_pool: &MarginPool, diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index 9986bcd14..07ea84ca4 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -29,7 +29,9 @@ use deepbook_margin::{ return_shared_3, build_demo_usdc_price_info_object, build_demo_usdt_price_info_object, + build_demo_usdc_price_info_object_with_price, setup_orderbook_liquidity_stablecoin, + setup_orderbook_liquidity_at_prices, setup_orderbook_liquidity_out_of_bounds_stablecoin } }; @@ -1308,6 +1310,497 @@ fun test_place_reduce_only_market_order_ok_ask() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } +// === Place Reduce Only Market Order And Repay Loan Tests === + +// Identical position to the `test_place_reduce_only_market_order_ok` +// expected_failure above: a reduce-only market BID buying USDC fills at $1.01 +// (1% above oracle). On the swap-only path that strictly lowers risk_ratio and +// aborts on the monotonic invariant. Bundling the repay turns the slippage into +// a net deleverage, so risk_ratio improves instead and the close succeeds. +#[test] +fun reduce_only_and_repay_bid_succeeds_where_swap_only_fails() { + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(10000 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 500 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 300 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + + let shares_before = mm.borrowed_base_shares(); + let rr_before = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + + let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< + USDC, + USDT, + USDC, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + 2, + constants::self_matching_allowed(), + 100 * test_constants::usdc_multiplier(), + true, + false, + &clock, + ); + destroy(order_info); + + let rr_after = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + + // Some base debt was repaid (but not all — net debt position remains), and + // the deleverage strictly improved solvency despite the 1% buy slippage. + assert!(mm.borrowed_base_shares() < shares_before); + assert!(mm.borrowed_base_shares() > 0); + assert!(rr_after > rr_before); + + return_shared_3!(mm, pool, base_pool); + return_shared(quote_pool); + destroy(usdc_price); + destroy(usdt_price); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +// Sell-side counterpart: reduce-only market ASK selling USDC fills at $0.99 (1% +// below oracle), which aborts the swap-only path. Repaying the proceeds against +// the quote debt deleverages and lifts risk_ratio, so the close succeeds. +#[test] +fun reduce_only_and_repay_ask_succeeds_where_swap_only_fails() { + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(10000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 500 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 300 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + + let shares_before = mm.borrowed_quote_shares(); + let rr_before = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + + let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< + USDC, + USDT, + USDT, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut quote_pool, + 2, + constants::self_matching_allowed(), + 100 * test_constants::usdc_multiplier(), + false, + false, + &clock, + ); + destroy(order_info); + + let rr_after = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + + assert!(mm.borrowed_quote_shares() < shares_before); + assert!(mm.borrowed_quote_shares() > 0); + assert!(rr_after > rr_before); + + return_shared_3!(mm, pool, quote_pool); + return_shared(base_pool); + destroy(usdc_price); + destroy(usdt_price); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +// The motivating scenario: a leveraged long whose risk_ratio has drifted to +// 1.20 — below `min_borrow` (1.25) but above `liquidation` (1.10). It is stuck +// in the danger zone: it cannot borrow, and a normal market order would abort +// (`place_market_order_v2` requires post-trade risk_ratio >= min_borrow). The +// bundled close-and-repay is its only wind-down path: it deleverages the +// position back above the borrow floor with a market order. +// +// Position: deposit 2000 USDC, borrow 1000 USDT, withdraw the 1000 USDT +// (risk_ratio at $1.00 is exactly 2.0 = min_withdraw). USDC then drifts to +// $0.60, giving risk_ratio = 2000 * 0.60 / 1000 = 1.20. +#[test] +fun reduce_only_and_repay_closes_from_danger_zone() { + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + // Liquidity around the drifted $0.60 oracle: a $0.59 bid (within the 5% + // band, ~1.7% adverse) for the manager's sell to fill against. + setup_orderbook_liquidity_at_prices( + &mut scenario, + pool_id, + 590_000_000, + 610_000_000, + &clock, + ); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(2000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy(usdc_price); + + // Drift USDC to $0.60 and refresh the stored price the band keys off of. + let usdc_drifted = build_demo_usdc_price_info_object_with_price( + &mut scenario, + 60_000_000, + &clock, + ); + pool_proxy::update_current_price( + &mut registry, + &pool, + &usdc_drifted, + &usdt_price, + &clock, + ); + + let pool_id_inner = pool.id(); + let shares_before = mm.borrowed_quote_shares(); + let rr_before = mm.risk_ratio( + ®istry, + &usdc_drifted, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + // Exactly 1.20 — in the danger zone: below min_borrow, above liquidation. + assert!(rr_before == 1_200_000_000); + assert!(rr_before < registry.min_borrow_risk_ratio(pool_id_inner)); + assert!(rr_before >= registry.liquidation_risk_ratio(pool_id_inner)); + + let order_info = pool_proxy::place_reduce_only_market_order_and_repay_loan( + ®istry, + &mut mm, + &mut pool, + &mut quote_pool, + &usdc_drifted, + &usdt_price, + 2, + constants::self_matching_allowed(), + 1000 * test_constants::usdc_multiplier(), + false, + false, + &clock, + scenario.ctx(), + ); + destroy(order_info); + + let rr_after = mm.risk_ratio( + ®istry, + &usdc_drifted, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + + // Debt repaid and the position climbed back above the borrow floor. + assert!(mm.borrowed_quote_shares() < shares_before); + assert!(rr_after > rr_before); + assert!(rr_after >= registry.min_borrow_risk_ratio(pool_id_inner)); + + return_shared_3!(mm, pool, quote_pool); + return_shared(base_pool); + destroy(usdc_drifted); + destroy(usdt_price); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +// A reduce-only-and-repay order whose quantity exceeds the net debt is rejected +// just like the plain reduce-only entries: net base debt is 500 - 200 = 300, so +// a 400 USDC bid is not reduce-only. +#[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] +fun reduce_only_and_repay_quantity_exceeds_debt_aborts() { + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(10000 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 500 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 300 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + + let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< + USDC, + USDT, + USDC, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + 2, + constants::self_matching_allowed(), + 400 * test_constants::usdc_multiplier(), + true, + false, + &clock, + ); + destroy(order_info); + + abort +} + #[test, expected_failure(abort_code = pool_proxy::EIncorrectDeepBookPool)] fun test_place_reduce_only_market_order_incorrect_pool() { let ( From 60e96732a1d3d8ac7bdeb3705c9ccfa4e912f425 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Fri, 22 May 2026 19:09:29 -0400 Subject: [PATCH 02/27] margin: address review - keep repay in place, drop redundant settle - Keep `repay` at its original location, only widening it to public(package) (minimal in-place visibility change rather than a relocation). - Drop the post-trade `withdraw_settled_amounts` call: `place_market_order` already settles the taker fill into the manager's balance via `vault.settle_balance_manager`, so the proceeds are drawable by `repay`. Co-Authored-By: Claude Opus 4.7 --- .../sources/margin_manager.move | 84 +++++++++---------- .../deepbook_margin/sources/pool_proxy.move | 13 ++- 2 files changed, 48 insertions(+), 49 deletions(-) diff --git a/packages/deepbook_margin/sources/margin_manager.move b/packages/deepbook_margin/sources/margin_manager.move index 3bf386c77..dafd05969 100644 --- a/packages/deepbook_margin/sources/margin_manager.move +++ b/packages/deepbook_margin/sources/margin_manager.move @@ -1392,48 +1392,6 @@ public(package) fun risk_ratio_int( } } -/// Repays the loan using the margin manager. Owner is validated via -/// `repay_withdraw`. `amount` of `none` repays as much as the manager's -/// `RepayAsset` balance allows, capped at the outstanding debt. -/// Returns the total amount repaid. -public(package) fun repay( - self: &mut MarginManager, - margin_pool: &mut MarginPool, - amount: Option, - clock: &Clock, - ctx: &mut TxContext, -): u64 { - let borrowed_shares = self.borrowed_base_shares.max(self.borrowed_quote_shares); - let borrowed_amount = margin_pool.borrow_shares_to_amount(borrowed_shares, clock); - let available_balance = self.balance_manager().balance(); - let repay_amount = amount.destroy_with_default(available_balance); - let repay_amount = repay_amount.min(borrowed_amount); - let repay_shares = math::mul(borrowed_shares, math::div(repay_amount, borrowed_amount)); - - let coin: Coin = self.repay_withdraw(repay_amount, ctx); - margin_pool.repay(repay_shares, coin, clock); - - if (type_name::with_defining_ids() == type_name::with_defining_ids()) { - self.borrowed_base_shares = self.borrowed_base_shares - repay_shares; - } else { - self.borrowed_quote_shares = self.borrowed_quote_shares - repay_shares; - }; - - if (self.borrowed_base_shares == 0 && self.borrowed_quote_shares == 0) { - self.margin_pool_id = option::none(); - }; - - event::emit(LoanRepaidEvent { - margin_manager_id: self.id(), - margin_pool_id: margin_pool.id(), - repay_amount, - repay_shares, - timestamp: clock.timestamp_ms(), - }); - - repay_amount -} - // === Private Functions === fun risk_ratio_int_unsafe( self: &MarginManager, @@ -1522,6 +1480,48 @@ fun validate_owner( assert!(ctx.sender() == self.owner, EInvalidMarginManagerOwner); } +/// Repays the loan using the margin manager. Owner is validated via +/// `repay_withdraw`. `amount` of `none` repays as much as the manager's +/// `RepayAsset` balance allows, capped at the outstanding debt. +/// Returns the total amount repaid. +public(package) fun repay( + self: &mut MarginManager, + margin_pool: &mut MarginPool, + amount: Option, + clock: &Clock, + ctx: &mut TxContext, +): u64 { + let borrowed_shares = self.borrowed_base_shares.max(self.borrowed_quote_shares); + let borrowed_amount = margin_pool.borrow_shares_to_amount(borrowed_shares, clock); + let available_balance = self.balance_manager().balance(); + let repay_amount = amount.destroy_with_default(available_balance); + let repay_amount = repay_amount.min(borrowed_amount); + let repay_shares = math::mul(borrowed_shares, math::div(repay_amount, borrowed_amount)); + + let coin: Coin = self.repay_withdraw(repay_amount, ctx); + margin_pool.repay(repay_shares, coin, clock); + + if (type_name::with_defining_ids() == type_name::with_defining_ids()) { + self.borrowed_base_shares = self.borrowed_base_shares - repay_shares; + } else { + self.borrowed_quote_shares = self.borrowed_quote_shares - repay_shares; + }; + + if (self.borrowed_base_shares == 0 && self.borrowed_quote_shares == 0) { + self.margin_pool_id = option::none(); + }; + + event::emit(LoanRepaidEvent { + margin_manager_id: self.id(), + margin_pool_id: margin_pool.id(), + repay_amount, + repay_shares, + timestamp: clock.timestamp_ms(), + }); + + repay_amount +} + fun liquidation_withdraw( self: &mut MarginManager, withdraw_amount: u64, diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index 922c00b45..10fc0bf3f 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -446,9 +446,9 @@ public fun place_reduce_only_market_order_v2( } /// Closes (winds down) a leveraged position atomically: places a reduce-only -/// market order, settles the taker proceeds, repays as much of the loan as the -/// settled balance allows, and only then enforces the monotonic solvency -/// invariant on the *net* (post-repay) state. +/// market order (which settles the taker fill into the manager's balance), +/// repays as much of the loan as that balance allows, and only then enforces +/// the monotonic solvency invariant on the *net* (post-repay) state. /// /// Why this needs to be a single entry rather than composing /// `place_reduce_only_market_order_v2` + `repay_*` in a PTB: the standalone @@ -531,11 +531,10 @@ public fun place_reduce_only_market_order_and_repay_loan Date: Fri, 22 May 2026 19:16:56 -0400 Subject: [PATCH 03/27] margin: take both pools in close-and-repay; no margin_manager changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rework the entry to take both margin pools (distinct objects, so no &mut aliasing) and use the existing public `risk_ratio` (two-pool) and `repay_base`/`repay_quote`, mirroring `place_reduce_only_market_order_v2`. This reverts margin_manager.move entirely: `risk_ratio_int` and `repay` stay private — no public(package) widening was needed. Also tightens the entry's doc comment. Co-Authored-By: Claude Opus 4.7 --- .../sources/margin_manager.move | 18 ++--- .../deepbook_margin/sources/pool_proxy.move | 75 +++++++++---------- .../tests/helper/test_helpers.move | 9 ++- .../tests/pool_proxy_tests.move | 17 +++-- 4 files changed, 54 insertions(+), 65 deletions(-) diff --git a/packages/deepbook_margin/sources/margin_manager.move b/packages/deepbook_margin/sources/margin_manager.move index dafd05969..ed23e4abb 100644 --- a/packages/deepbook_margin/sources/margin_manager.move +++ b/packages/deepbook_margin/sources/margin_manager.move @@ -1357,12 +1357,9 @@ public(package) fun deposit_int( balance_manager.deposit_with_cap(deposit_cap, coin, ctx); } -/// Risk ratio of the manager against the single debt-side `margin_pool`. -/// Exposed at package scope for the bundled close-and-repay proxy flow, which -/// holds only the `&mut MarginPool` it is about to repay into and so -/// cannot use the two-pool `risk_ratio` helper (that would need an immutable -/// borrow of the same pool). Returns `max_risk_ratio` when there is no debt. -public(package) fun risk_ratio_int( +// === Private Functions === +// Get the risk ratio of the margin manager. +fun risk_ratio_int( self: &MarginManager, registry: &MarginRegistry, base_oracle: &PriceInfoObject, @@ -1392,7 +1389,6 @@ public(package) fun risk_ratio_int( } } -// === Private Functions === fun risk_ratio_int_unsafe( self: &MarginManager, registry: &MarginRegistry, @@ -1480,11 +1476,9 @@ fun validate_owner( assert!(ctx.sender() == self.owner, EInvalidMarginManagerOwner); } -/// Repays the loan using the margin manager. Owner is validated via -/// `repay_withdraw`. `amount` of `none` repays as much as the manager's -/// `RepayAsset` balance allows, capped at the outstanding debt. -/// Returns the total amount repaid. -public(package) fun repay( +/// Repays the loan using the margin manager. +/// Returns the total amount repaid +fun repay( self: &mut MarginManager, margin_pool: &mut MarginPool, amount: Option, diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index 10fc0bf3f..8ce904395 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -445,36 +445,22 @@ public fun place_reduce_only_market_order_v2( order_info } -/// Closes (winds down) a leveraged position atomically: places a reduce-only -/// market order (which settles the taker fill into the manager's balance), -/// repays as much of the loan as that balance allows, and only then enforces -/// the monotonic solvency invariant on the *net* (post-repay) state. +/// Atomically winds down a leveraged position: places a reduce-only market +/// order, repays the loan with the proceeds, then requires the net (post-repay) +/// risk ratio to be at least the pre-trade ratio. /// -/// Why this needs to be a single entry rather than composing -/// `place_reduce_only_market_order_v2` + `repay_*` in a PTB: the standalone -/// reduce-only entry checks `risk_ratio_after >= risk_ratio_before` on the -/// swap-only state, before any repay. A market close pays the spread, so the -/// swap alone strictly lowers the oracle-valued risk ratio (debt is unchanged -/// until repay) and that standalone check aborts. Repaying first deleverages -/// the manager, which lifts the ratio and absorbs the slippage; the monotonic -/// check then passes on the net result. Slippage stays bounded by the -/// price-tolerance band (`assert_price`), so deferring the solvency check past -/// the repay does not weaken security. This is also the only close path -/// available to a manager whose ratio has drifted into the -/// `liquidation..min_borrow` band — it cannot reach the borrow floor in a -/// single swap, but it can climb out by deleveraging. -/// -/// Takes a single `&mut MarginPool` (the borrowed side) rather than -/// both margin pools, mirroring `liquidate`: the pool must be borrowed mutably -/// to repay, so the risk ratio is computed against it via -/// `risk_ratio_int`. `calculate_debts` validates both that -/// `margin_pool` is the manager's actual debt pool and that `DebtAsset` is the -/// borrowed side. -public fun place_reduce_only_market_order_and_repay_loan( +/// The post-repay check is the point. A market close pays the spread, which +/// alone lowers the oracle-valued ratio (debt is unchanged until repay) and +/// would abort the plain reduce-only path. Repaying first deleverages and +/// absorbs the slippage (still bounded by the `assert_price` band), and lets a +/// manager in the `liquidation..min_borrow` band climb out — it cannot reach +/// the borrow floor in a single swap. +public fun place_reduce_only_market_order_and_repay_loan( registry: &MarginRegistry, margin_manager: &mut MarginManager, pool: &mut Pool, - margin_pool: &mut MarginPool, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, base_oracle: &PriceInfoObject, quote_oracle: &PriceInfoObject, client_order_id: u64, @@ -488,7 +474,11 @@ public fun place_reduce_only_market_order_and_repay_loan(pool); let (effective_price, quote_quantity) = calculate_effective_price( @@ -499,8 +489,8 @@ public fun place_reduce_only_market_order_and_repay_loan base_asset && quantity <= base_debt - base_asset) || (!is_bid && quote_debt > quote_asset && quote_quantity <= quote_debt - quote_asset), @@ -509,12 +499,13 @@ public fun place_reduce_only_market_order_and_repay_loan 0 || margin_manager.borrowed_quote_shares() > 0 ) { - let risk_ratio_after = margin_manager.risk_ratio_int( + let risk_ratio_after = margin_manager.risk_ratio( registry, base_oracle, quote_oracle, pool, - margin_pool, + base_margin_pool, + quote_margin_pool, clock, ); assert!(risk_ratio_after >= risk_ratio_before, EReduceOnlyMustImproveRiskRatio); diff --git a/packages/deepbook_margin/tests/helper/test_helpers.move b/packages/deepbook_margin/tests/helper/test_helpers.move index 76093c374..371aef07f 100644 --- a/packages/deepbook_margin/tests/helper/test_helpers.move +++ b/packages/deepbook_margin/tests/helper/test_helpers.move @@ -1447,12 +1447,13 @@ public fun place_reduce_only_market_order_v2_for_test( /// Wraps `place_reduce_only_market_order_and_repay_loan` with demo ($1.00) /// oracle prices built/destroyed inline. For tests that drift the oracle, call /// the proxy entry directly with custom price objects instead. -public fun place_reduce_only_market_order_and_repay_loan_for_test( +public fun place_reduce_only_market_order_and_repay_loan_for_test( scenario: &mut Scenario, registry: &MarginRegistry, mm: &mut deepbook_margin::margin_manager::MarginManager, pool: &mut Pool, - margin_pool: &mut MarginPool, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, client_order_id: u64, self_matching_option: u8, quantity: u64, @@ -1465,12 +1466,12 @@ public fun place_reduce_only_market_order_and_repay_loan_for_test( registry, mm, pool, - margin_pool, + base_margin_pool, + quote_margin_pool, &base_oracle, "e_oracle, client_order_id, diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index 07ea84ca4..9c56c9787 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -1336,7 +1336,7 @@ fun reduce_only_and_repay_bid_succeeds_where_swap_only_fails() { let mut pool = scenario.take_shared_by_id>(pool_id); let mut registry = scenario.take_shared(); let mut base_pool = scenario.take_shared_by_id>(base_pool_id); - let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); let deepbook_registry = scenario.take_shared_by_id(registry_id); margin_manager::new( &pool, @@ -1397,13 +1397,13 @@ fun reduce_only_and_repay_bid_succeeds_where_swap_only_fails() { let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< USDC, USDT, - USDC, >( &mut scenario, ®istry, &mut mm, &mut pool, &mut base_pool, + &mut quote_pool, 2, constants::self_matching_allowed(), 100 * test_constants::usdc_multiplier(), @@ -1457,7 +1457,7 @@ fun reduce_only_and_repay_ask_succeeds_where_swap_only_fails() { scenario.next_tx(test_constants::user1()); let mut pool = scenario.take_shared_by_id>(pool_id); let mut registry = scenario.take_shared(); - let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); let deepbook_registry = scenario.take_shared_by_id(registry_id); margin_manager::new( @@ -1519,12 +1519,12 @@ fun reduce_only_and_repay_ask_succeeds_where_swap_only_fails() { let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< USDC, USDT, - USDT, >( &mut scenario, ®istry, &mut mm, &mut pool, + &mut base_pool, &mut quote_pool, 2, constants::self_matching_allowed(), @@ -1592,7 +1592,7 @@ fun reduce_only_and_repay_closes_from_danger_zone() { scenario.next_tx(test_constants::user1()); let mut pool = scenario.take_shared_by_id>(pool_id); let mut registry = scenario.take_shared(); - let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); let deepbook_registry = scenario.take_shared_by_id(registry_id); margin_manager::new( @@ -1671,10 +1671,11 @@ fun reduce_only_and_repay_closes_from_danger_zone() { assert!(rr_before < registry.min_borrow_risk_ratio(pool_id_inner)); assert!(rr_before >= registry.liquidation_risk_ratio(pool_id_inner)); - let order_info = pool_proxy::place_reduce_only_market_order_and_repay_loan( + let order_info = pool_proxy::place_reduce_only_market_order_and_repay_loan( ®istry, &mut mm, &mut pool, + &mut base_pool, &mut quote_pool, &usdc_drifted, &usdt_price, @@ -1732,7 +1733,7 @@ fun reduce_only_and_repay_quantity_exceeds_debt_aborts() { let mut pool = scenario.take_shared_by_id>(pool_id); let mut registry = scenario.take_shared(); let mut base_pool = scenario.take_shared_by_id>(base_pool_id); - let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); let deepbook_registry = scenario.take_shared_by_id(registry_id); margin_manager::new( &pool, @@ -1782,13 +1783,13 @@ fun reduce_only_and_repay_quantity_exceeds_debt_aborts() { let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< USDC, USDT, - USDC, >( &mut scenario, ®istry, &mut mm, &mut pool, &mut base_pool, + &mut quote_pool, 2, constants::self_matching_allowed(), 400 * test_constants::usdc_multiplier(), From 0213a674034cdaddb458957ebbedb96232cc6c53 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Mon, 22 Jun 2026 13:01:27 -0400 Subject: [PATCH 04/27] margin: gross-holdings cap for reduce-only exit (asymmetric ask/bid) Move the reduce-only quantity cap from net debt to gross holdings so a position can be fully unwound: the ask (closing a long) may sell up to the full base the manager holds, while the bid (covering a short) stays capped at the net short (base_debt - base_asset) so it cannot flip into a fresh long. Applied to place_reduce_only_limit_order_v2 and the close-and-repay entry. Document place_reduce_only_market_order_v2 as superseded. Co-Authored-By: Claude Opus 4.8 --- .../deepbook_margin/sources/pool_proxy.move | 39 ++- .../tests/pool_proxy_tests.move | 228 +++++++++++++++++- 2 files changed, 247 insertions(+), 20 deletions(-) diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index 8ce904395..fd964b41f 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -299,13 +299,17 @@ public fun place_reduce_only_limit_order_v2( } else { margin_manager.calculate_debts(quote_margin_pool, clock) }; - let (base_asset, quote_asset) = margin_manager.calculate_assets( - pool, - ); - + let (base_asset, _) = margin_manager.calculate_assets(pool); + + // Reduce-only. The ask (closing a long) may sell up to the full base the + // manager holds, so a long can be fully unwound — selling base can only + // shrink base exposure, never flip it short. The bid (covering a short) + // stays capped at the net short (`base_debt - base_asset`): buying past it + // would flip the short into a fresh long, i.e. open exposure rather than + // reduce it. The monotonic risk-ratio check below guards value leak. assert!( (is_bid && base_debt > base_asset && quantity <= base_debt - base_asset) || - (!is_bid && quote_debt > quote_asset && math::mul(quantity, price) <= quote_debt - quote_asset), + (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -355,7 +359,16 @@ public fun place_reduce_only_limit_order_v2( order_info } -/// Places a reduce-only market order in the pool. Used when margin trading is disabled. +/// Places a reduce-only market order in the pool. Used when margin trading is +/// disabled. +/// +/// Superseded by `place_reduce_only_market_order_and_repay_loan`. A market +/// (taker) fill always pays the spread, which lowers the oracle-valued +/// `risk_ratio` while the debt is unchanged, so the swap-only monotonic check +/// here rejects essentially every taker fill. The `_and_repay` variant +/// deleverages with the proceeds so the net-state ratio actually improves. Kept +/// callable for existing integrators; its symmetric net-debt cap is retained as +/// legacy (the live entries cap the ask on gross base held instead). public fun place_reduce_only_market_order_v2( registry: &MarginRegistry, margin_manager: &mut MarginManager, @@ -479,9 +492,9 @@ public fun place_reduce_only_market_order_and_repay_loan( } else { margin_manager.calculate_debts(quote_margin_pool, clock) }; - let (base_asset, quote_asset) = margin_manager.calculate_assets(pool); + let (base_asset, _) = margin_manager.calculate_assets(pool); - let (effective_price, quote_quantity) = calculate_effective_price( + let (effective_price, _) = calculate_effective_price( pool, quantity, is_bid, @@ -489,11 +502,15 @@ public fun place_reduce_only_market_order_and_repay_loan( clock, ); - // Reduce-only: a bid buys back at most the net base debt; an ask sells for - // at most the net quote debt. + // Reduce-only. The ask (closing a long) may sell up to the full base the + // manager holds — selling can only shrink base exposure, never flip it + // short — so a long fully closes and the repay absorbs any proceeds past the + // debt. The bid (covering a short) stays capped at the net short + // (`base_debt - base_asset`) so buying back can't overshoot into a fresh + // long. The net-state monotonic check below guards value leak. assert!( (is_bid && base_debt > base_asset && quantity <= base_debt - base_asset) || - (!is_bid && quote_debt > quote_asset && quote_quantity <= quote_debt - quote_asset), + (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index 9c56c9787..e2b413c1e 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -35,7 +35,7 @@ use deepbook_margin::{ setup_orderbook_liquidity_out_of_bounds_stablecoin } }; -use std::unit_test::destroy; +use std::unit_test::{assert_eq, destroy}; use sui::test_scenario::return_shared; use token::deep::DEEP; @@ -692,6 +692,106 @@ fun test_place_reduce_only_limit_order_ok_ask() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } +// Pure reduce-only limit ask selling *more* than the net quote debt — the #5 +// case the old net-debt cap forbade. Long: 10000 USDC collateral, 500 USDT +// debt, ~200 USDT free (net quote debt ~300). A reduce-only limit ask for 500 +// USDC now rests (it is below the 10000 gross base the manager holds); the old +// cap rejected anything above ~300. +#[test] +fun reduce_only_limit_ask_above_net_debt_ok() { + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(10000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 500 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn_coin = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 300 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn_coin); + + // 500 USDC > the ~300 net quote debt, < the 10000 gross base held. + let order_info = test_helpers::place_reduce_only_limit_order_v2_for_test( + &mut scenario, + ®istry, + &base_pool, + "e_pool, + &mut mm, + &mut pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 1_000_000_000, + 500 * test_constants::usdc_multiplier(), + false, + false, + 2000000, + &clock, + ); + + // The order rested (no book liquidity) at the full requested size. + assert_eq!(order_info.original_quantity(), 500 * test_constants::usdc_multiplier()); + assert_eq!(order_info.executed_quantity(), 0); + + destroy(order_info); + return_shared_2!(mm, pool); + return_shared_2!(base_pool, quote_pool); + destroy(usdc_price); + destroy(usdt_price); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + #[test, expected_failure(abort_code = pool_proxy::EIncorrectDeepBookPool)] fun test_place_reduce_only_limit_order_incorrect_pool() { let ( @@ -1068,8 +1168,9 @@ fun test_place_reduce_only_limit_order_not_reduce_only_quantity_ask() { ); destroy(coin); - // User has USDC debt, tries to buy more USDC than debt - // This should fail because user is trying to buy more USDC than debt + // Long position (USDC collateral, USDT debt). The ask tries to sell 10001 + // USDC but the manager holds only 10000 — beyond the gross-holdings cap, so + // it aborts. Selling between net debt and gross holdings is now allowed. test_helpers::place_reduce_only_limit_order_v2_for_test( &mut scenario, ®istry, @@ -1083,10 +1184,10 @@ fun test_place_reduce_only_limit_order_not_reduce_only_quantity_ask() { constants::self_matching_allowed(), constants::float_scaling(), // price - 101 * test_constants::usdc_multiplier(), - // quantity + 10001 * test_constants::usdc_multiplier(), + // quantity (exceeds the 10000 USDC the manager holds) false, - // is_bid = false (buying USDT) + // is_bid = false (selling USDC) false, 2000000, // expire_timestamp @@ -1711,9 +1812,116 @@ fun reduce_only_and_repay_closes_from_danger_zone() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } -// A reduce-only-and-repay order whose quantity exceeds the net debt is rejected -// just like the plain reduce-only entries: net base debt is 500 - 200 = 300, so -// a 400 USDC bid is not reduce-only. +// Full close where the ask sells *more* base than the net quote debt — the case +// the old net-debt cap forbade. Long position: 10000 USDC collateral, 500 USDT +// debt, ~200 USDT free, so net quote debt is ~300. Selling 600 USDC produces +// ~594 USDT, which repays the whole 500 debt and clears the loan. Demonstrates +// the gross-holdings cap (issues #4 and #5). +#[test] +fun reduce_only_and_repay_fully_closes_selling_above_net_debt() { + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(10000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 500 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 300 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + + let shares_before = mm.borrowed_quote_shares(); + let base_before = mm.base_balance(); + + // Sell 600 USDC: above the ~300 net quote debt, below the 10000 gross base. + let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< + USDC, + USDT, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 2, + constants::self_matching_allowed(), + 600 * test_constants::usdc_multiplier(), + false, + false, + &clock, + ); + destroy(order_info); + + // The proceeds cleared the entire loan and base was sold down. + assert!(shares_before > 0); + assert_eq!(mm.borrowed_quote_shares(), 0); + assert!(mm.base_balance() < base_before); + + return_shared_3!(mm, pool, quote_pool); + return_shared(base_pool); + destroy(usdc_price); + destroy(usdt_price); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +// A reduce-only-and-repay bid whose quantity exceeds the net base debt is +// rejected: the bid covers a short, so it stays capped at the net short +// (500 - 200 = 300) to avoid flipping into a long. A 400 USDC bid is not +// reduce-only. #[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] fun reduce_only_and_repay_quantity_exceeds_debt_aborts() { let ( @@ -1780,6 +1988,8 @@ fun reduce_only_and_repay_quantity_exceeds_debt_aborts() { ); destroy(withdrawn); + // Short position: net base debt is 500 - 200 = 300, so a 400 USDC bid + // overshoots the net short and is not reduce-only. let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< USDC, USDT, From 23ee9b98c13f37dfe59022df1280660651c72a96 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Mon, 22 Jun 2026 13:05:23 -0400 Subject: [PATCH 05/27] docs(move rules): asymmetric reduce-only cap (gross sell / net buy) Capture the flip-into-fresh-long bug and why the unwind cap must be gross on the selling side but net on the buying side. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index 73994262d..009d055d4 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -106,6 +106,8 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - Slippage tolerance and solvency are orthogonal knobs. Allowable slippage belongs in the per-pool `price_tolerance` band enforced by `margin_registry.move::assert_price` (bids capped above oracle, asks capped below; 1%–50%, default 5%) — not in the risk-ratio check. Do not relax the risk-ratio floor to make room for slippage. +- The reduce-only **quantity** cap must be **asymmetric**, not symmetric. Cap the *selling* side (closing a long: `quantity <= base_asset`) on **gross holdings**; cap the *buying* side (covering a short: `quantity <= base_debt - base_asset`) on the **net** short. Reason: selling base can only shrink base exposure — you can't sell base you don't hold, so it can never flip you short — so gross is safe and lets a long fully unwind (fixes "can't fully sell the asset side" / "full close overshoots"). Buying base *can* overshoot the net short and leave a long after repay, i.e. **open fresh opposite-side exposure through a "reduce-only" order**, which bypasses the trading-disabled / below-`min_borrow` restriction. A symmetric gross cap on both sides is the bug. Guarded in `pool_proxy.move` (`place_reduce_only_limit_order_v2`, `place_reduce_only_market_order_and_repay_loan`); the superseded `place_reduce_only_market_order_v2` keeps the legacy symmetric net cap. This is a concrete instance of the `code-review.md` rule "a branch that treats the same parameter differently must be mathematically necessary, not accidental policy." + - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. ## Tool Calling Instructions From bb83ae25422670c203d3edf44743544a28a6cce0 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Mon, 22 Jun 2026 13:29:26 -0400 Subject: [PATCH 06/27] =?UTF-8?q?margin:=20min=5Fopen=5Frisk=5Fratio=20?= =?UTF-8?q?=E2=80=94=20separate=20opening=20floor=20from=20borrow=20floor?= =?UTF-8?q?=20(#2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening near-max leverage aborted because the post-trade solvency check reused min_borrow (1.25), leaving no room for the opening trade's spread. Add a per-pool min_open_risk_ratio floor between liquidation and min_borrow: assert_post_trade_solvent now checks against it instead. Default is the midpoint of liquidation and min_borrow, stored as an optional dynamic-field override (mirrors MaxOrderTtlKey) that self-heals if a later risk-param change strands it. Admin-set via set_min_open_risk_ratio. Co-Authored-By: Claude Opus 4.8 --- .../sources/margin_registry.move | 70 +++++ .../deepbook_margin/sources/pool_proxy.move | 26 +- .../tests/pool_proxy_tests.move | 290 +++++++++++++++++- 3 files changed, 370 insertions(+), 16 deletions(-) diff --git a/packages/deepbook_margin/sources/margin_registry.move b/packages/deepbook_margin/sources/margin_registry.move index ecdebec29..e21a65708 100644 --- a/packages/deepbook_margin/sources/margin_registry.move +++ b/packages/deepbook_margin/sources/margin_registry.move @@ -102,6 +102,11 @@ public struct CurrentPriceData has store { /// Stored value is `u64`; absent means use `margin_constants::default_max_order_ttl_ms()`. public struct MaxOrderTtlKey(ID) has copy, drop, store; +/// Dynamic field key for the per-pool `min_open_risk_ratio` override (9 +/// decimals). Stored value is `u64`; absent means use the midpoint of +/// `liquidation_risk_ratio` and `min_borrow_risk_ratio`. +public struct MinOpenRiskRatioKey(ID) has copy, drop, store; + // === Caps === public struct MarginAdminCap has key, store { id: UID, @@ -175,6 +180,12 @@ public struct MaxOrderTtlUpdated has copy, drop { timestamp: u64, } +public struct MinOpenRiskRatioUpdated has copy, drop { + pool_id: ID, + min_open_risk_ratio: u64, + timestamp: u64, +} + fun init(_: MARGIN_REGISTRY, ctx: &mut TxContext) { let id = object::new(ctx); let margin_registry_inner = MarginRegistryInner { @@ -543,6 +554,43 @@ public fun set_max_order_ttl( }); } +/// Set the per-pool `min_open_risk_ratio` override — the post-trade solvency +/// floor enforced on opening (risk-increasing) orders. Must sit in +/// `(liquidation_risk_ratio, min_borrow_risk_ratio]`: above liquidation so an +/// open can't land in the liquidatable zone, at or below the borrow floor. +/// Absent an override the floor defaults to the midpoint of liquidation and +/// min_borrow. Only Admin can set it. +public fun set_min_open_risk_ratio( + self: &mut MarginRegistry, + _admin_cap: &MarginAdminCap, + pool: &Pool, + min_open_risk_ratio: u64, + clock: &Clock, +) { + self.load_inner(); + let pool_id = pool.id(); + let liquidation = self.liquidation_risk_ratio(pool_id); + let min_borrow = self.min_borrow_risk_ratio(pool_id); + assert!( + min_open_risk_ratio > liquidation && min_open_risk_ratio <= min_borrow, + EInvalidRiskParam, + ); + + let key = MinOpenRiskRatioKey(pool_id); + if (self.id.exists_with_type(key)) { + let stored: &mut u64 = self.id.borrow_mut(key); + *stored = min_open_risk_ratio; + } else { + self.id.add(key, min_open_risk_ratio); + }; + + event::emit(MinOpenRiskRatioUpdated { + pool_id, + min_open_risk_ratio, + timestamp: clock.timestamp_ms(), + }); +} + // === Public Helper Functions === /// Create a PoolConfig with margin pool IDs and risk parameters /// Enable is false by default, must be enabled after registration @@ -690,6 +738,28 @@ public fun target_liquidation_risk_ratio(self: &MarginRegistry, deepbook_pool_id config.risk_ratios.target_liquidation_risk_ratio } +/// Post-trade solvency floor for *opening* (risk-increasing) orders, sitting in +/// `(liquidation_risk_ratio, min_borrow_risk_ratio]`. Lets a max-leverage open +/// absorb the opening trade's spread — which lands the post-trade ratio just +/// under the borrow floor — without aborting, while staying above the +/// liquidatable zone. Defaults to the midpoint of liquidation and min_borrow; an +/// admin override (`set_min_open_risk_ratio`) is honored only while it stays in +/// the valid band, so a later risk-param change can't strand it below +/// liquidation. +public fun min_open_risk_ratio(self: &MarginRegistry, deepbook_pool_id: ID): u64 { + let liquidation = self.liquidation_risk_ratio(deepbook_pool_id); + let min_borrow = self.min_borrow_risk_ratio(deepbook_pool_id); + let default = (liquidation + min_borrow) / 2; + + let key = MinOpenRiskRatioKey(deepbook_pool_id); + if (self.id.exists_with_type(key)) { + let stored = *self.id.borrow(key); + if (stored > liquidation && stored <= min_borrow) stored else default + } else { + default + } +} + public fun user_liquidation_reward(self: &MarginRegistry, deepbook_pool_id: ID): u64 { let config = self.get_pool_config(deepbook_pool_id); config.user_liquidation_reward diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index fd964b41f..fbf9f075b 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -21,9 +21,10 @@ const EPoolNotEnabledForMarginTrading: u64 = 2; const ENotReduceOnlyOrder: u64 = 3; const EIncorrectDeepBookPool: u64 = 4; const ENoLiquidityInOrderbook: u64 = 5; -/// Post-trade risk ratio dropped below `min_borrow_risk_ratio`. -/// Raised by the v2 order placement entries when the manager would be left -/// in a state borrowing would be forbidden from. +/// Post-trade risk ratio dropped below `min_open_risk_ratio` (the opening +/// solvency floor between liquidation and the borrow floor). Raised by the v2 +/// order placement entries when an opening trade would leave the manager in the +/// liquidatable zone. const EInsufficientRiskRatioAfterTrade: u64 = 6; /// Reduce-only fill leaked value to the counterparty: the manager's /// risk_ratio after the trade is lower than before. Reduce-only orders must @@ -140,10 +141,11 @@ public fun place_reduce_only_market_order( // Each v2 entry mirrors its v1 counterpart and additionally recomputes // `risk_ratio` after the order settles (using Pyth via the public // `MarginManager::risk_ratio` helper). For non-reduce-only entries the -// post-trade ratio must be at least `min_borrow_risk_ratio` — same threshold -// the borrow path enforces, so trading cannot push a manager below where -// borrowing was already forbidden. Skipped when the manager has no debt -// (nothing to be insolvent against). +// post-trade ratio must be at least `min_open_risk_ratio` — a floor between +// liquidation and the borrow floor, so a max-leverage open can absorb the +// opening trade's spread (which lands the ratio just under `min_borrow`) +// without aborting, while staying above the liquidatable zone. Skipped when +// the manager has no debt (nothing to be insolvent against). // // For reduce-only entries the post-trade ratio must be `>= risk_ratio_before` // (monotonic improvement). The borrow-floor check would trap users in the @@ -821,10 +823,12 @@ fun calculate_effective_price( } } -/// Asserts the manager remains above the borrow-floor risk ratio after a +/// Asserts the manager remains solvent after an opening (risk-increasing) /// trade. Skipped when the manager has no debt (nothing to be insolvent -/// against). Threshold reuses `min_borrow_risk_ratio` so trading cannot push -/// a manager below the level borrowing is already gated at. +/// against). Threshold is `min_open_risk_ratio` — between liquidation and the +/// borrow floor — so a max-leverage open can absorb the opening trade's spread +/// (which lands the ratio just under `min_borrow`) without aborting, while +/// staying above the liquidatable zone. fun assert_post_trade_solvent( margin_manager: &MarginManager, registry: &MarginRegistry, @@ -852,7 +856,7 @@ fun assert_post_trade_solvent( clock, ); assert!( - risk_ratio_after >= registry.min_borrow_risk_ratio(pool.id()), + risk_ratio_after >= registry.min_open_risk_ratio(pool.id()), EInsufficientRiskRatioAfterTrade, ); } diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index e2b413c1e..e72144982 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -5662,14 +5662,16 @@ fun place_reduce_only_market_order_v1_aborts() { abort 999 } -// === Post-trade solvency regression test === +// === Post-trade open-floor (min_open_risk_ratio) tests === // // Borrow USDC against USDT collateral right at `min_borrow_risk_ratio` (1.25, // the borrow floor in test config), then sell the borrowed USDC against the -// standard orderbook bid (0.99). The 1% adverse fill drops `risk_ratio` from -// 1.25 to ~1.24, breaching the floor; the v2 invariant aborts. -#[test, expected_failure(abort_code = pool_proxy::EInsufficientRiskRatioAfterTrade)] -fun place_limit_order_v2_borrow_at_floor_then_adverse_fill_aborts() { +// standard orderbook bid (0.99). The 1% adverse fill drops `risk_ratio` to +// 1.2475 — below the borrow floor but above the default `min_open_risk_ratio` +// (midpoint of liquidation 1.10 and min_borrow 1.25 = 1.175, in test config). +// The opening trade is allowed, so a max-leverage open can absorb its own spread. +#[test] +fun place_limit_order_v2_borrow_at_floor_then_adverse_fill_ok() { let ( mut scenario, clock, @@ -5757,6 +5759,121 @@ fun place_limit_order_v2_borrow_at_floor_then_adverse_fill_aborts() { &clock, ); + // Post-trade risk_ratio = 499/400 = 1.2475: below the 1.25 borrow floor, + // above the 1.175 open floor, so the opening order is accepted. + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + let rr = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + assert_eq!(rr, 1_247_500_000); + assert!(rr < registry.min_borrow_risk_ratio(pool_id)); + assert!(rr >= registry.min_open_risk_ratio(pool_id)); + + return_shared_3!(mm, pool, quote_pool); + return_shared(base_pool); + destroy_2!(usdc_price, usdt_price); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +// Same scenario, but the admin sets `min_open_risk_ratio` to the borrow floor +// (the strict opt-out), so the 1.2475 fill is now below the open floor and the +// opening order aborts. Exercises the override setter and the abort path. +#[test, expected_failure(abort_code = pool_proxy::EInsufficientRiskRatioAfterTrade)] +fun place_limit_order_v2_adverse_fill_aborts_with_strict_open_floor() { + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + // Raise the open floor to the borrow floor: no slippage headroom. + let borrow_floor = registry.min_borrow_risk_ratio(pool_id); + registry.set_min_open_risk_ratio( + &_admin_cap, + &pool, + borrow_floor, + &clock, + ); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(100 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(100 * test_constants::deep_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 400 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy_2!(usdc_price, usdt_price); + + let _ = test_helpers::place_limit_order_v2_for_test( + &mut scenario, + ®istry, + &base_pool, + "e_pool, + &mut mm, + &mut pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 990_000_000, + 100 * test_constants::usdc_multiplier(), + false, // is_bid = false (sell) + true, // pay_with_deep + 2_000_000, + &clock, + ); + abort 999 } @@ -5906,6 +6023,169 @@ fun set_max_order_ttl_within_bounds_ok() { scenario.end(); } +#[test] +fun min_open_risk_ratio_defaults_to_midpoint() { + let (mut scenario, clock, admin_cap, maintainer_cap) = setup_margin_registry(); + let _base_pool_id = create_margin_pool( + &mut scenario, + &maintainer_cap, + default_protocol_config(), + &clock, + ); + let _quote_pool_id = create_margin_pool( + &mut scenario, + &maintainer_cap, + default_protocol_config(), + &clock, + ); + let (pool_id, _registry_id) = create_pool_for_testing(&mut scenario); + + scenario.next_tx(test_constants::admin()); + let mut registry = scenario.take_shared(); + enable_deepbook_margin_on_pool( + pool_id, + &mut registry, + &admin_cap, + &clock, + &mut scenario, + ); + return_shared(registry); + + scenario.next_tx(test_constants::admin()); + let registry = scenario.take_shared(); + // Default = midpoint of liquidation (1.10) and min_borrow (1.25) = 1.175. + assert_eq!(registry.min_open_risk_ratio(pool_id), 1_175_000_000); + + return_shared(registry); + destroy(admin_cap); + destroy(maintainer_cap); + destroy(clock); + scenario.end(); +} + +#[test] +fun set_min_open_risk_ratio_within_band_ok() { + let (mut scenario, clock, admin_cap, maintainer_cap) = setup_margin_registry(); + let _base_pool_id = create_margin_pool( + &mut scenario, + &maintainer_cap, + default_protocol_config(), + &clock, + ); + let _quote_pool_id = create_margin_pool( + &mut scenario, + &maintainer_cap, + default_protocol_config(), + &clock, + ); + let (pool_id, _registry_id) = create_pool_for_testing(&mut scenario); + + scenario.next_tx(test_constants::admin()); + let mut registry = scenario.take_shared(); + enable_deepbook_margin_on_pool( + pool_id, + &mut registry, + &admin_cap, + &clock, + &mut scenario, + ); + return_shared(registry); + + scenario.next_tx(test_constants::admin()); + let pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + + // 1.20 is within (liquidation 1.10, min_borrow 1.25]. + registry.set_min_open_risk_ratio(&admin_cap, &pool, 1_200_000_000, &clock); + assert_eq!(registry.min_open_risk_ratio(pool_id), 1_200_000_000); + + // Update again to exercise the mutable path, not just first-insert. + registry.set_min_open_risk_ratio(&admin_cap, &pool, 1_150_000_000, &clock); + assert_eq!(registry.min_open_risk_ratio(pool_id), 1_150_000_000); + + return_shared_2!(registry, pool); + destroy(admin_cap); + destroy(maintainer_cap); + destroy(clock); + scenario.end(); +} + +#[test, expected_failure(abort_code = margin_registry::EInvalidRiskParam)] +fun set_min_open_risk_ratio_too_low_aborts() { + let (mut scenario, clock, admin_cap, maintainer_cap) = setup_margin_registry(); + let _base_pool_id = create_margin_pool( + &mut scenario, + &maintainer_cap, + default_protocol_config(), + &clock, + ); + let _quote_pool_id = create_margin_pool( + &mut scenario, + &maintainer_cap, + default_protocol_config(), + &clock, + ); + let (pool_id, _registry_id) = create_pool_for_testing(&mut scenario); + + scenario.next_tx(test_constants::admin()); + let mut registry = scenario.take_shared(); + enable_deepbook_margin_on_pool( + pool_id, + &mut registry, + &admin_cap, + &clock, + &mut scenario, + ); + return_shared(registry); + + scenario.next_tx(test_constants::admin()); + let pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + + // Equal to liquidation (1.10) is not strictly above it: out of band. + registry.set_min_open_risk_ratio(&admin_cap, &pool, 1_100_000_000, &clock); + + abort 999 +} + +#[test, expected_failure(abort_code = margin_registry::EInvalidRiskParam)] +fun set_min_open_risk_ratio_too_high_aborts() { + let (mut scenario, clock, admin_cap, maintainer_cap) = setup_margin_registry(); + let _base_pool_id = create_margin_pool( + &mut scenario, + &maintainer_cap, + default_protocol_config(), + &clock, + ); + let _quote_pool_id = create_margin_pool( + &mut scenario, + &maintainer_cap, + default_protocol_config(), + &clock, + ); + let (pool_id, _registry_id) = create_pool_for_testing(&mut scenario); + + scenario.next_tx(test_constants::admin()); + let mut registry = scenario.take_shared(); + enable_deepbook_margin_on_pool( + pool_id, + &mut registry, + &admin_cap, + &clock, + &mut scenario, + ); + return_shared(registry); + + scenario.next_tx(test_constants::admin()); + let pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + + // Above the borrow floor (1.25): out of band. + registry.set_min_open_risk_ratio(&admin_cap, &pool, 1_250_000_001, &clock); + + abort 999 +} + #[test] fun max_order_ttl_lazy_default() { // Pools that have never had `set_max_order_ttl` called still get the From 413a13a3eb6f5d8f418d4550de52e63687257966 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Mon, 22 Jun 2026 13:30:32 -0400 Subject: [PATCH 07/27] docs(move rules): open floor (min_open_risk_ratio) distinct from borrow floor Capture why the post-trade open floor must differ from the borrow gate and the self-healing derived-default override pattern. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index 009d055d4..e20d277bb 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -110,6 +110,8 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. +- The post-trade solvency floor for *opening* (risk-increasing) orders is a **separate** threshold from the borrow gate: `min_open_risk_ratio`, strictly between `liquidation` and `min_borrow`. Why they must differ: the borrow gate passes at *exactly* `min_borrow` (pre-trade), but any opening trade pays the spread, so the post-trade ratio lands just under `min_borrow` — if `assert_post_trade_solvent` reused `min_borrow`, a max-leverage open would *always* abort. `min_open` (default = midpoint of liquidation and min_borrow) gives the open room to absorb its own spread while staying above the liquidatable zone. It's a per-pool admin override stored as an optional dynamic field (the `MaxOrderTtlKey` pattern); the getter derives the midpoint default from the pool's current `liquidation`/`min_borrow` (so it tracks risk-param edits) and **ignores a stranded override** outside `(liquidation, min_borrow]` rather than returning an unsafe value (self-heal). Only the opening paths (`place_limit_order_v2`/`place_market_order_v2` → `assert_post_trade_solvent`) use `min_open`; the borrow gate and the TP/SL post-fill check still use `min_borrow`. + ## Tool Calling Instructions - `sui move build` to build the package, must be run in a directory with Move.toml in it From 57dac08cd6e4aa6476048555c2479eb9097dc802 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Mon, 22 Jun 2026 14:59:35 -0400 Subject: [PATCH 08/27] =?UTF-8?q?margin:=20v3=20conditional=20execution=20?= =?UTF-8?q?=E2=80=94=20deleverage=20so=20stops=20fire=20in=20danger=20band?= =?UTF-8?q?=20(#8,=20#10)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add execute_conditional_orders_v3: takes the margin pools as &mut and, after each market-type conditional fill, repays the loan with the proceeds, then gates on net (post-repay) risk_ratio >= the pre-fill ratio. This lets a stop-loss fire in the liquidation..min_borrow band — a swap alone only lowers the oracle-valued ratio, which the v2 borrow-floor gate rejects — and the order executes and is removed instead of looping the bot (#10). v2 is left unchanged for upgrade compatibility; integrators migrate to v3. Move the repay owner check from the internal repay into the public repay_base/ repay_quote wrappers so the permissionless executor can deleverage (safe: repay only moves the manager's own funds against its own debt). Share the collect/place/finalize helpers between the v2 and v3 executors. Co-Authored-By: Claude Opus 4.8 --- .../sources/margin_manager.move | 407 +++++++++++++----- .../tests/helper/test_helpers.move | 25 ++ .../deepbook_margin/tests/tpsl_tests.move | 253 +++++++++++ 3 files changed, 586 insertions(+), 99 deletions(-) diff --git a/packages/deepbook_margin/sources/margin_manager.move b/packages/deepbook_margin/sources/margin_manager.move index ed23e4abb..9860321c8 100644 --- a/packages/deepbook_margin/sources/margin_manager.move +++ b/packages/deepbook_margin/sources/margin_manager.move @@ -272,45 +272,85 @@ public fun execute_conditional_orders_v2( clock, ); + let orders_to_process = self.collect_triggered_orders(current_price); + let mut order_infos = vector[]; let mut executed_ids = vector[]; let mut expired_ids = vector[]; let mut insufficient_funds_ids = vector[]; let mut out_of_bounds_ids = vector[]; - // Collect orders to process (to avoid borrow conflicts) - let mut orders_to_process = vector[]; - - // Collect trigger_below orders (sorted high to low) - let mut i = 0; - while (i < self.take_profit_stop_loss.trigger_below().length()) { - let conditional_order = &self.take_profit_stop_loss.trigger_below()[i]; + self.process_collected_orders_v2( + pool, + registry, + base_margin_pool, + quote_margin_pool, + base_price_info_object, + quote_price_info_object, + orders_to_process, + &mut order_infos, + &mut executed_ids, + &mut expired_ids, + &mut insufficient_funds_ids, + &mut out_of_bounds_ids, + max_orders_to_execute, + clock, + ctx, + ); - // Break early if price doesn't trigger - if (current_price >= conditional_order.condition().trigger_price()) { - break - }; + self.finalize_conditional_execution( + pool.id(), + expired_ids, + insufficient_funds_ids, + out_of_bounds_ids, + executed_ids, + clock, + ); - orders_to_process.push_back(*conditional_order); - i = i + 1; - }; + order_infos +} - // Collect trigger_above orders (sorted low to high) - i = 0; - while (i < self.take_profit_stop_loss.trigger_above().length()) { - let conditional_order = &self.take_profit_stop_loss.trigger_above()[i]; +/// Execute conditional orders, deleveraging on each market-type fill. +/// Permissionless, like `execute_conditional_orders_v2`, with the same trigger +/// and cancellation handling — but takes the margin pools as `&mut` and repays +/// the loan with the market proceeds before gating on the net (post-repay) +/// `risk_ratio` being at least the pre-fill ratio. +/// +/// This is what lets a stop-loss fire in the `liquidation..min_borrow` danger +/// band: a swap alone only lowers the oracle-valued ratio (so the v2 borrow-floor +/// gate rejects it), while repaying actually improves it. If a single triggered +/// fill would worsen net solvency the whole txn aborts — no partial-state +/// landing. +public fun execute_conditional_orders_v3( + self: &mut MarginManager, + pool: &mut Pool, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, + base_price_info_object: &PriceInfoObject, + quote_price_info_object: &PriceInfoObject, + registry: &MarginRegistry, + max_orders_to_execute: u64, + clock: &Clock, + ctx: &mut TxContext, +): vector { + registry.load_inner(); + assert!(pool.id() == self.deepbook_pool(), EIncorrectDeepBookPool); + let current_price = calculate_price( + registry, + base_price_info_object, + quote_price_info_object, + clock, + ); - // Break early if price doesn't trigger - if (current_price <= conditional_order.condition().trigger_price()) { - break - }; + let orders_to_process = self.collect_triggered_orders(current_price); - orders_to_process.push_back(*conditional_order); - i = i + 1; - }; + let mut order_infos = vector[]; + let mut executed_ids = vector[]; + let mut expired_ids = vector[]; + let mut insufficient_funds_ids = vector[]; + let mut out_of_bounds_ids = vector[]; - // Process collected orders - self.process_collected_orders_v2( + self.process_collected_orders_v3( pool, registry, base_margin_pool, @@ -328,32 +368,14 @@ public fun execute_conditional_orders_v2( ctx, ); - let manager_id = self.id(); - let pool_id = pool.id(); - - insufficient_funds_ids.do!(|id| { - self.take_profit_stop_loss.emit_insufficient_funds_event(manager_id, id, clock); - }); - - out_of_bounds_ids.do!(|id| { - self.take_profit_stop_loss.emit_price_out_of_bounds_event(manager_id, id, clock); - }); - - let mut cancelled_ids = expired_ids; - cancelled_ids.append(insufficient_funds_ids); - cancelled_ids.append(out_of_bounds_ids); - cancelled_ids.do!(|id| { - self.take_profit_stop_loss.cancel_conditional_order(manager_id, id, clock); - }); - - self - .take_profit_stop_loss - .remove_executed_conditional_orders( - manager_id, - pool_id, - executed_ids, - clock, - ); + self.finalize_conditional_execution( + pool.id(), + expired_ids, + insufficient_funds_ids, + out_of_bounds_ids, + executed_ids, + clock, + ); order_infos } @@ -842,11 +864,11 @@ public fun liquidate( clock, ); let quote_out = max_quote_out.min(quote_asset); - let base_coin = self.liquidation_withdraw( + let base_coin = self.withdraw_without_owner_check( base_out, ctx, ); - let quote_coin = self.liquidation_withdraw( + let quote_coin = self.withdraw_without_owner_check( quote_out, ctx, ); @@ -862,11 +884,11 @@ public fun liquidate( clock, ); let base_out = max_base_out.min(base_asset); - let base_coin = self.liquidation_withdraw( + let base_coin = self.withdraw_without_owner_check( base_out, ctx, ); - let quote_coin = self.liquidation_withdraw( + let quote_coin = self.withdraw_without_owner_check( quote_out, ctx, ); @@ -1476,8 +1498,14 @@ fun validate_owner( assert!(ctx.sender() == self.owner, EInvalidMarginManagerOwner); } -/// Repays the loan using the margin manager. -/// Returns the total amount repaid +/// Repays the loan using the margin manager. Returns the total amount repaid. +/// +/// Does not check ownership: owner-gating is the caller's responsibility. The +/// public `repay_base`/`repay_quote` wrappers validate the owner; the +/// permissionless conditional executor (`execute_conditional_orders_v3`) +/// intentionally repays without an owner check, which is safe because repay only +/// moves the manager's own funds to pay down its own debt — it deleverages and +/// cannot extract value. fun repay( self: &mut MarginManager, margin_pool: &mut MarginPool, @@ -1492,7 +1520,7 @@ fun repay( let repay_amount = repay_amount.min(borrowed_amount); let repay_shares = math::mul(borrowed_shares, math::div(repay_amount, borrowed_amount)); - let coin: Coin = self.repay_withdraw(repay_amount, ctx); + let coin: Coin = self.withdraw_without_owner_check(repay_amount, ctx); margin_pool.repay(repay_shares, coin, clock); if (type_name::with_defining_ids() == type_name::with_defining_ids()) { @@ -1516,7 +1544,11 @@ fun repay( repay_amount } -fun liquidation_withdraw( +/// Withdraws from the manager's balance manager without checking ownership. +/// Callers must either gate on the owner themselves or perform an owner-neutral +/// operation: liquidation (anyone may unwind an unhealthy manager) and repay +/// (only deleverages, paying the manager's own debt). +fun withdraw_without_owner_check( self: &mut MarginManager, withdraw_amount: u64, ctx: &mut TxContext, @@ -1530,24 +1562,6 @@ fun liquidation_withdraw( ) } -/// This can only be called by the manager owner -fun repay_withdraw( - self: &mut MarginManager, - withdraw_amount: u64, - ctx: &mut TxContext, -): Coin { - validate_owner(self, ctx); - let balance_manager = &mut self.balance_manager; - - let coin = balance_manager.withdraw_with_cap( - &self.withdraw_cap, - withdraw_amount, - ctx, - ); - - coin -} - /// Helper function to determine if margin manager can borrow from a margin pool fun can_borrow( self: &MarginManager, @@ -1716,22 +1730,49 @@ fun place_market_order_conditional_v2( ) } -/// Helper function to process collected conditional orders. -/// -/// After each successful `place_pending_order_v2` call, recompute `risk_ratio` -/// against Pyth via the public `risk_ratio` helper. If the manager has debt -/// and the post-fill ratio is below `min_borrow_risk_ratio`, abort the entire -/// txn with `EInsufficientRiskRatioAfterTrade`. We do not allow partial-fill -/// landing — one bad fill reverts everything so the manager is never left in -/// a state borrowing would have been forbidden from. -fun process_collected_orders_v2( +/// Collects the conditional orders whose trigger condition is met at +/// `current_price`. `trigger_below` orders fire when price falls to/below their +/// trigger (stored high→low, so stop at the first that doesn't fire); +/// `trigger_above` orders fire when price rises to/above their trigger (stored +/// low→high). +fun collect_triggered_orders( + self: &MarginManager, + current_price: u64, +): vector { + let mut orders_to_process = vector[]; + + let mut i = 0; + while (i < self.take_profit_stop_loss.trigger_below().length()) { + let conditional_order = &self.take_profit_stop_loss.trigger_below()[i]; + if (current_price >= conditional_order.condition().trigger_price()) { + break + }; + orders_to_process.push_back(*conditional_order); + i = i + 1; + }; + + i = 0; + while (i < self.take_profit_stop_loss.trigger_above().length()) { + let conditional_order = &self.take_profit_stop_loss.trigger_above()[i]; + if (current_price <= conditional_order.condition().trigger_price()) { + break + }; + orders_to_process.push_back(*conditional_order); + i = i + 1; + }; + + orders_to_process +} + +/// Places the triggered conditional orders, routing each to its outcome bucket +/// (executed / expired / insufficient-funds / out-of-bounds). Shared by the v2 +/// and v3 executors; the solvency gate and any deleveraging are applied by the +/// caller. Returns whether any *market* order executed (the v3 executor repays +/// only then). +fun place_triggered_orders( self: &mut MarginManager, pool: &mut Pool, registry: &MarginRegistry, - base_margin_pool: &MarginPool, - quote_margin_pool: &MarginPool, - base_oracle: &PriceInfoObject, - quote_oracle: &PriceInfoObject, orders: vector, order_infos: &mut vector, executed_ids: &mut vector, @@ -1741,7 +1782,8 @@ fun process_collected_orders_v2( max_orders_to_execute: u64, clock: &Clock, ctx: &TxContext, -) { +): bool { + let mut market_filled = false; let mut i = 0; while (i < orders.length() && order_infos.length() < max_orders_to_execute) { let conditional_order = &orders[i]; @@ -1829,6 +1871,9 @@ fun process_collected_orders_v2( ctx, ); + if (!pending_order.is_limit_order()) { + market_filled = true; + }; order_infos.push_back(order_info); executed_ids.push_back(conditional_order_id); } else { @@ -1847,12 +1892,85 @@ fun process_collected_orders_v2( i = i + 1; }; - // Post-loop solvency check. Move PTBs are atomic, so checking once after - // all fills is functionally equivalent to checking after each fill — a - // breach of the invariant aborts the whole txn either way. Skipped when - // no fills landed (executed_ids empty) and skipped when manager has no - // debt (nothing to be insolvent against). Saves up to N-1 Pyth reads on - // the happy path versus per-fill checking. + market_filled +} + +/// Emits the outcome events and reconciles the conditional-order queue: +/// insufficient-funds, out-of-bounds, and expired orders are cancelled (with +/// their event); executed orders are removed. Shared by the v2 and v3 executors. +fun finalize_conditional_execution( + self: &mut MarginManager, + pool_id: ID, + expired_ids: vector, + insufficient_funds_ids: vector, + out_of_bounds_ids: vector, + executed_ids: vector, + clock: &Clock, +) { + let manager_id = self.id(); + + insufficient_funds_ids.do!(|id| { + self.take_profit_stop_loss.emit_insufficient_funds_event(manager_id, id, clock); + }); + + out_of_bounds_ids.do!(|id| { + self.take_profit_stop_loss.emit_price_out_of_bounds_event(manager_id, id, clock); + }); + + let mut cancelled_ids = expired_ids; + cancelled_ids.append(insufficient_funds_ids); + cancelled_ids.append(out_of_bounds_ids); + cancelled_ids.do!(|id| { + self.take_profit_stop_loss.cancel_conditional_order(manager_id, id, clock); + }); + + self + .take_profit_stop_loss + .remove_executed_conditional_orders( + manager_id, + pool_id, + executed_ids, + clock, + ); +} + +/// v2 solvency gate (legacy): places the triggered orders, then requires a +/// post-fill `risk_ratio >= min_borrow_risk_ratio` whenever a fill landed and +/// the manager has debt. A swap alone only lowers the oracle-valued ratio, so a +/// stop-loss in the `liquidation..min_borrow` band aborts here — use +/// `execute_conditional_orders_v3`, which deleverages and uses a monotonic gate. +fun process_collected_orders_v2( + self: &mut MarginManager, + pool: &mut Pool, + registry: &MarginRegistry, + base_margin_pool: &MarginPool, + quote_margin_pool: &MarginPool, + base_oracle: &PriceInfoObject, + quote_oracle: &PriceInfoObject, + orders: vector, + order_infos: &mut vector, + executed_ids: &mut vector, + expired_ids: &mut vector, + insufficient_funds_ids: &mut vector, + out_of_bounds_ids: &mut vector, + max_orders_to_execute: u64, + clock: &Clock, + ctx: &TxContext, +) { + self.place_triggered_orders( + pool, + registry, + orders, + order_infos, + executed_ids, + expired_ids, + insufficient_funds_ids, + out_of_bounds_ids, + max_orders_to_execute, + clock, + ctx, + ); + if ( !executed_ids.is_empty() && (self.borrowed_base_shares > 0 || self.borrowed_quote_shares > 0) @@ -1873,6 +1991,97 @@ fun process_collected_orders_v2( }; } +/// v3 solvency gate with deleveraging. Captures `risk_ratio` before any fill, +/// places the triggered orders, repays the debt side with the market proceeds, +/// then requires the net (post-repay) `risk_ratio` to be at least the pre-fill +/// ratio. Deleveraging lets a stop-loss fire even in the `liquidation..min_borrow` +/// danger band — a swap alone only lowers the oracle-valued ratio, so the v2 +/// borrow-floor gate would reject it. Repay is skipped when no market order +/// filled (resting limit orders don't change the ratio) or the manager has no +/// debt; the monotonic gate is skipped when the manager had no debt to begin +/// with. +fun process_collected_orders_v3( + self: &mut MarginManager, + pool: &mut Pool, + registry: &MarginRegistry, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, + base_oracle: &PriceInfoObject, + quote_oracle: &PriceInfoObject, + orders: vector, + order_infos: &mut vector, + executed_ids: &mut vector, + expired_ids: &mut vector, + insufficient_funds_ids: &mut vector, + out_of_bounds_ids: &mut vector, + max_orders_to_execute: u64, + clock: &Clock, + ctx: &mut TxContext, +) { + let has_debt = self.borrowed_base_shares > 0 || self.borrowed_quote_shares > 0; + let risk_ratio_before = if (has_debt) { + self.risk_ratio( + registry, + base_oracle, + quote_oracle, + pool, + base_margin_pool, + quote_margin_pool, + clock, + ) + } else { + 0 + }; + + let market_filled = self.place_triggered_orders( + pool, + registry, + orders, + order_infos, + executed_ids, + expired_ids, + insufficient_funds_ids, + out_of_bounds_ids, + max_orders_to_execute, + clock, + ctx, + ); + + // Deleverage with the market proceeds so the fill improves (not just holds) + // solvency. The repay only moves the manager's own funds against its own + // debt, so it is safe in this permissionless path. + if (market_filled && has_debt) { + if (self.has_base_debt()) { + self.repay( + base_margin_pool, + option::none(), + clock, + ctx, + ); + } else { + self.repay( + quote_margin_pool, + option::none(), + clock, + ctx, + ); + }; + }; + + if (has_debt && !executed_ids.is_empty()) { + let risk_ratio_after = self.risk_ratio( + registry, + base_oracle, + quote_oracle, + pool, + base_margin_pool, + quote_margin_pool, + clock, + ); + assert!(risk_ratio_after >= risk_ratio_before, EInsufficientRiskRatioAfterTrade); + }; +} + fun balance_manager_unsafe_mut( self: &mut MarginManager, ): &mut BalanceManager { diff --git a/packages/deepbook_margin/tests/helper/test_helpers.move b/packages/deepbook_margin/tests/helper/test_helpers.move index 371aef07f..dac2158a2 100644 --- a/packages/deepbook_margin/tests/helper/test_helpers.move +++ b/packages/deepbook_margin/tests/helper/test_helpers.move @@ -1511,3 +1511,28 @@ public fun execute_conditional_orders_v2_for_test( scenario.ctx(), ) } + +public fun execute_conditional_orders_v3_for_test( + scenario: &mut Scenario, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, + mm: &mut deepbook_margin::margin_manager::MarginManager, + pool: &mut Pool, + base_price_info_object: &PriceInfoObject, + quote_price_info_object: &PriceInfoObject, + registry: &MarginRegistry, + max_orders_to_execute: u64, + clock: &Clock, +): vector { + mm.execute_conditional_orders_v3( + pool, + base_margin_pool, + quote_margin_pool, + base_price_info_object, + quote_price_info_object, + registry, + max_orders_to_execute, + clock, + scenario.ctx(), + ) +} diff --git a/packages/deepbook_margin/tests/tpsl_tests.move b/packages/deepbook_margin/tests/tpsl_tests.move index b1fd05cbd..b1cf7b1f3 100644 --- a/packages/deepbook_margin/tests/tpsl_tests.move +++ b/packages/deepbook_margin/tests/tpsl_tests.move @@ -4227,3 +4227,256 @@ fun test_tpsl_mixed_orders_some_cancelled_some_executed() { return_shared(deepbook_registry); cleanup_margin_test(margin_registry, admin_cap, maintainer_cap, clock, scenario); } + +// === v3 conditional execution: deleverage so stops fire in the danger band === + +#[test] +fun execute_conditional_orders_v3_stop_loss_deleverages_in_danger_band() { + // Alice opens a leveraged position at SUI=$2: 1000 SUI collateral + 8000 USDC + // borrowed, risk_ratio = 1.25 (the borrow floor). SUI falls to $0.95, + // dropping the ratio to ~1.119 — the danger band (above liquidation 1.10, + // below borrow 1.25). Her market stop-loss fires: v3 sells SUI and repays the + // loan, so the deleveraged ratio clears the gate and the order executes and is + // removed. The v2 path aborts on this exact setup (paired test below). + let ( + mut scenario, + clock, + admin_cap, + maintainer_cap, + usdc_pool_id, + sui_pool_id, + pool_id, + registry_id, + ) = setup_sui_usdc_deepbook_margin(); + + setup_orderbook_liquidity_low_bids(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut margin_registry = scenario.take_shared(); + let pool = scenario.take_shared>(); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut margin_registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + return_shared(pool); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let pool = scenario.take_shared>(); + let mut usdc_pool = scenario.take_shared_by_id>(usdc_pool_id); + let sui_price_high = build_sui_price_info_object_with_price(&mut scenario, 200, &clock); // $2.00 + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + + mm.deposit( + &margin_registry, + &sui_price_high, + &usdc_price, + mint_coin(1000 * test_constants::sui_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + // Borrow 8000 USDC: ratio = (1000*2 + 8000) / 8000 = 1.25 at SUI=$2. + mm.borrow_quote( + &margin_registry, + &mut usdc_pool, + &sui_price_high, + &usdc_price, + &pool, + 8000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + + let condition = tpsl::new_condition(true, 1_500_000); // trigger_below $1.50 + let pending_order = tpsl::new_pending_market_order( + 1, + constants::self_matching_allowed(), + 150 * test_constants::sui_multiplier(), // sell 150 SUI (< 200 bid liquidity) + false, // is_bid = false (sell) + false, // pay_with_deep + ); + mm.add_conditional_order( + &pool, + &sui_price_high, + &usdc_price, + &margin_registry, + 1, + condition, + pending_order, + &clock, + scenario.ctx(), + ); + + destroy_2!(sui_price_high, usdc_price); + return_shared_2!(mm, pool); + return_shared(usdc_pool); + return_shared(margin_registry); + + scenario.next_tx(test_constants::user2()); + let sui_price_low = build_sui_price_info_object_with_price(&mut scenario, 95, &clock); // $0.95 + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + let mut pool = scenario.take_shared>(); + let mut margin_registry = scenario.take_shared(); + let mut mm = scenario.take_shared>(); + pool_proxy::update_current_price( + &mut margin_registry, + &pool, + &sui_price_low, + &usdc_price, + &clock, + ); + + let mut sui_pool = scenario.take_shared_by_id>(sui_pool_id); + let mut usdc_pool = scenario.take_shared_by_id>(usdc_pool_id); + + let debt_before = mm.borrowed_quote_shares(); + assert!(debt_before > 0); + + let order_infos = test_helpers::execute_conditional_orders_v3_for_test( + &mut scenario, + &mut sui_pool, + &mut usdc_pool, + &mut mm, + &mut pool, + &sui_price_low, + &usdc_price, + &margin_registry, + 10, + &clock, + ); + + // The stop-loss fired, v3 repaid the loan with the proceeds (debt reduced), + // and the order was removed from the queue — no stuck retry (#8, #10). + assert!(order_infos.length() == 1); + assert!(mm.borrowed_quote_shares() < debt_before); + assert!(mm.conditional_order_ids().length() == 0); + + destroy(order_infos); + return_shared(sui_pool); + return_shared(usdc_pool); + destroy_2!(sui_price_low, usdc_price); + return_shared_2!(mm, pool); + cleanup_margin_test(margin_registry, admin_cap, maintainer_cap, clock, scenario); +} + +#[test, expected_failure(abort_code = margin_manager::EInsufficientRiskRatioAfterTrade)] +fun execute_conditional_orders_v2_stop_loss_aborts_in_danger_band() { + // Same danger-band setup as the v3 test above, but executed through v2: the + // stop-loss sells without repaying, so the post-fill ratio (~1.119) stays + // below the borrow floor and the v2 gate aborts the whole txn. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + usdc_pool_id, + sui_pool_id, + pool_id, + registry_id, + ) = setup_sui_usdc_deepbook_margin(); + + setup_orderbook_liquidity_low_bids(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut margin_registry = scenario.take_shared(); + let pool = scenario.take_shared>(); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut margin_registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + return_shared(pool); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let pool = scenario.take_shared>(); + let mut usdc_pool = scenario.take_shared_by_id>(usdc_pool_id); + let sui_price_high = build_sui_price_info_object_with_price(&mut scenario, 200, &clock); + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + + mm.deposit( + &margin_registry, + &sui_price_high, + &usdc_price, + mint_coin(1000 * test_constants::sui_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + &margin_registry, + &mut usdc_pool, + &sui_price_high, + &usdc_price, + &pool, + 8000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + + let condition = tpsl::new_condition(true, 1_500_000); + let pending_order = tpsl::new_pending_market_order( + 1, + constants::self_matching_allowed(), + 150 * test_constants::sui_multiplier(), + false, + false, + ); + mm.add_conditional_order( + &pool, + &sui_price_high, + &usdc_price, + &margin_registry, + 1, + condition, + pending_order, + &clock, + scenario.ctx(), + ); + + destroy_2!(sui_price_high, usdc_price); + return_shared_2!(mm, pool); + return_shared(usdc_pool); + return_shared(margin_registry); + + scenario.next_tx(test_constants::user2()); + let sui_price_low = build_sui_price_info_object_with_price(&mut scenario, 95, &clock); + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + let mut pool = scenario.take_shared>(); + let mut margin_registry = scenario.take_shared(); + let mut mm = scenario.take_shared>(); + pool_proxy::update_current_price( + &mut margin_registry, + &pool, + &sui_price_low, + &usdc_price, + &clock, + ); + + let sui_pool = scenario.take_shared_by_id>(sui_pool_id); + let usdc_pool = scenario.take_shared_by_id>(usdc_pool_id); + + let order_infos = test_helpers::execute_conditional_orders_v2_for_test( + &mut scenario, + &sui_pool, + &usdc_pool, + &mut mm, + &mut pool, + &sui_price_low, + &usdc_price, + &margin_registry, + 10, + &clock, + ); + destroy(order_infos); + + abort 999 +} From af8d0eb870290a9058a772bb952de2590988ae19 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Mon, 22 Jun 2026 15:01:08 -0400 Subject: [PATCH 09/27] docs(move rules): v3 conditional deleverage + upgrade-compat _vN rule Capture why danger-band stops must deleverage (not just swap), the permissionless-repay owner-gating move, and the new-_vN-entry rule for &mut/signature changes on the deployed margin package. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index e20d277bb..c82a5207b 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -110,7 +110,9 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. -- The post-trade solvency floor for *opening* (risk-increasing) orders is a **separate** threshold from the borrow gate: `min_open_risk_ratio`, strictly between `liquidation` and `min_borrow`. Why they must differ: the borrow gate passes at *exactly* `min_borrow` (pre-trade), but any opening trade pays the spread, so the post-trade ratio lands just under `min_borrow` — if `assert_post_trade_solvent` reused `min_borrow`, a max-leverage open would *always* abort. `min_open` (default = midpoint of liquidation and min_borrow) gives the open room to absorb its own spread while staying above the liquidatable zone. It's a per-pool admin override stored as an optional dynamic field (the `MaxOrderTtlKey` pattern); the getter derives the midpoint default from the pool's current `liquidation`/`min_borrow` (so it tracks risk-param edits) and **ignores a stranded override** outside `(liquidation, min_borrow]` rather than returning an unsafe value (self-heal). Only the opening paths (`place_limit_order_v2`/`place_market_order_v2` → `assert_post_trade_solvent`) use `min_open`; the borrow gate and the TP/SL post-fill check still use `min_borrow`. +- The post-trade solvency floor for *opening* (risk-increasing) orders is a **separate** threshold from the borrow gate: `min_open_risk_ratio`, strictly between `liquidation` and `min_borrow`. Why they must differ: the borrow gate passes at *exactly* `min_borrow` (pre-trade), but any opening trade pays the spread, so the post-trade ratio lands just under `min_borrow` — if `assert_post_trade_solvent` reused `min_borrow`, a max-leverage open would *always* abort. `min_open` (default = midpoint of liquidation and min_borrow) gives the open room to absorb its own spread while staying above the liquidatable zone. It's a per-pool admin override stored as an optional dynamic field (the `MaxOrderTtlKey` pattern); the getter derives the midpoint default from the pool's current `liquidation`/`min_borrow` (so it tracks risk-param edits) and **ignores a stranded override** outside `(liquidation, min_borrow]` rather than returning an unsafe value (self-heal). Only the opening paths (`place_limit_order_v2`/`place_market_order_v2` → `assert_post_trade_solvent`) use `min_open`; the borrow gate and the v2 TP/SL post-fill check still use `min_borrow`. + +- TP/SL conditional execution that must fire in the danger band has to **deleverage**, not just trade — `execute_conditional_orders_v3` repays the loan with the market proceeds after each fill, then gates on net (post-repay) monotonic `risk_ratio`. The v2 executor (swap-only, `min_borrow` gate) can't fire a danger-band stop and leaves the order looping the bot; v3 fixes both. Two design constraints worth remembering: (1) the executor is **permissionless**, so to call `repay` there the owner check was moved *out* of internal `repay` into the public `repay_base`/`repay_quote` wrappers — repay is owner-neutral (only moves the manager's own funds against its own debt, can't extract value), the same reason `withdraw_without_owner_check` backs both liquidation and repay. (2) Deleverage needs the margin pools as `&mut`, but `execute_conditional_orders_v2` takes them `&` — changing a **public** param from `&T` to `&mut T` breaks Sui upgrade compatibility, so this shipped as a **new** `_v3` entry, not a v2 signature change (same reason v1→v2 added entries). General rule for this deployed package: prefer a new `_vN` entry over changing any existing public signature; private-fn bodies, new functions, and new dynamic fields are the compatible levers. ## Tool Calling Instructions From 56f1414e36120c8d954e7304a3349d0a09b7fba4 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Mon, 22 Jun 2026 15:03:00 -0400 Subject: [PATCH 10/27] margin: document TP/SL lifetime semantics (#9) Clarify on add_conditional_order that the conditional itself never expires; a market pending order is the "until cancelled" stop (fires + deleverages via v3), while a limit pending order's placed order is clamped to max_order_ttl_ms as the stale-price guard. Co-Authored-By: Claude Opus 4.8 --- .../deepbook_margin/sources/margin_manager.move | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/deepbook_margin/sources/margin_manager.move b/packages/deepbook_margin/sources/margin_manager.move index 9860321c8..8b35bd4a4 100644 --- a/packages/deepbook_margin/sources/margin_manager.move +++ b/packages/deepbook_margin/sources/margin_manager.move @@ -168,8 +168,18 @@ public struct WithdrawCollateralEvent has copy, drop { } // === Functions - Take Profit Stop Loss === -/// Add a conditional order. -/// Specifies the conditions under which the order is triggered and the pending order to be placed. +/// Add a conditional order (take-profit / stop-loss). Specifies the condition +/// under which it triggers and the pending order to place when it does. +/// +/// Lifetime: the conditional order itself is never clamped — it rests in the +/// queue until it triggers or is cancelled. A *market* pending order +/// (`tpsl::new_pending_market_order`) has no expiry, so it is the "until +/// cancelled" stop: it waits indefinitely and, when triggered, fires and +/// deleverages via `execute_conditional_orders_v3` (so it can protect even in +/// the danger band). A *limit* pending order is intentionally transient — when +/// it triggers, the resting order it places is clamped to `max_order_ttl_ms` +/// (default 3 days) by `clamp_expire_timestamp`, the same stale-price guard as +/// any margin limit order. For a permanent stop, use a market pending order. public fun add_conditional_order( self: &mut MarginManager, pool: &Pool, From 55146a45fd6760a8d43c7e513460779b3ab74d77 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Mon, 22 Jun 2026 15:23:03 -0400 Subject: [PATCH 11/27] margin: tests for free-borrowed-funds order (#3) and limit-type SL via v3 (#8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the two coverage gaps from the audit: a normal order placed with free borrowed funds in the [min_open, min_borrow) band succeeds (#3), and a limit-type stop-loss rests (not aborts) and is removed via v3 in the danger band (#8). Test-only — no source changes. Co-Authored-By: Claude Opus 4.8 --- .../tests/pool_proxy_tests.move | 109 ++++++++++++++ .../deepbook_margin/tests/tpsl_tests.move | 136 ++++++++++++++++++ 2 files changed, 245 insertions(+) diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index e72144982..c81e30007 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -6463,3 +6463,112 @@ fun place_limit_order_v2_no_debt_at_oracle_price_ok() { return_shared_2!(mm, pool); cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } + +// #3: a manager with already-borrowed *free* funds, sitting in the +// [min_open, min_borrow) band, can still place a normal order — the post-trade +// floor is `min_open`, not `min_borrow`. Deposit 100 USDC, borrow 400 USDT +// (ratio 1.25 at $1), then use the free USDT to market-buy 100 USDC. The buy +// pays the $1.01 ask, landing risk_ratio at 1.2475 — below the borrow floor but +// above the open floor — so it is accepted. Under the old min_borrow gate this +// aborted (the #3 regression). +#[test] +fun place_market_order_v2_with_free_borrowed_funds_in_band_ok() { + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(100 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(100 * test_constants::deep_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + // Borrow 400 USDT: ratio = (100 + 400) / 400 = 1.25 at $1. + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 400 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy_2!(usdc_price, usdt_price); + + // Use the free borrowed USDT to market-buy 100 USDC — no new borrow. + let order_info = test_helpers::place_market_order_v2_for_test( + &mut scenario, + ®istry, + &base_pool, + "e_pool, + &mut mm, + &mut pool, + 1, + constants::self_matching_allowed(), + 100 * test_constants::usdc_multiplier(), + true, // is_bid = true (buy) + true, // pay_with_deep + &clock, + ); + destroy(order_info); + + // Post-trade risk_ratio = (200 + 299) / 400 = 1.2475: in the band, accepted. + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + let rr = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + assert_eq!(rr, 1_247_500_000); + assert!(rr < registry.min_borrow_risk_ratio(pool_id)); + assert!(rr >= registry.min_open_risk_ratio(pool_id)); + + destroy_2!(usdc_price, usdt_price); + return_shared_3!(mm, pool, quote_pool); + return_shared(base_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} diff --git a/packages/deepbook_margin/tests/tpsl_tests.move b/packages/deepbook_margin/tests/tpsl_tests.move index b1cf7b1f3..069bd3624 100644 --- a/packages/deepbook_margin/tests/tpsl_tests.move +++ b/packages/deepbook_margin/tests/tpsl_tests.move @@ -4480,3 +4480,139 @@ fun execute_conditional_orders_v2_stop_loss_aborts_in_danger_band() { abort 999 } + +#[test] +fun execute_conditional_orders_v3_limit_stop_loss_rests_in_danger_band() { + // A *limit*-type stop-loss in the danger band: v3 places it (it rests as a + // maker, leaving the ratio unchanged, so the monotonic gate passes) and + // removes it from the queue — it is not aborted like the v2 path. No fill, + // so no repay. Same 1.119 danger-band setup as the market v3 test. + let ( + mut scenario, + clock, + admin_cap, + maintainer_cap, + usdc_pool_id, + sui_pool_id, + pool_id, + registry_id, + ) = setup_sui_usdc_deepbook_margin(); + + setup_orderbook_liquidity_low_bids(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut margin_registry = scenario.take_shared(); + let pool = scenario.take_shared>(); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut margin_registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + return_shared(pool); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let pool = scenario.take_shared>(); + let mut usdc_pool = scenario.take_shared_by_id>(usdc_pool_id); + let sui_price_high = build_sui_price_info_object_with_price(&mut scenario, 200, &clock); + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + + mm.deposit( + &margin_registry, + &sui_price_high, + &usdc_price, + mint_coin(1000 * test_constants::sui_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + &margin_registry, + &mut usdc_pool, + &sui_price_high, + &usdc_price, + &pool, + 8000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + + // Limit SL: trigger_below $1.50, sell 150 SUI at $0.96 — above the $0.95 best + // bid, so it rests (maker) rather than crossing. + let condition = tpsl::new_condition(true, 1_500_000); + let pending_order = tpsl::new_pending_limit_order( + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 960_000, // $0.96 (rests above the $0.95 bid) + 150 * test_constants::sui_multiplier(), + false, // is_bid = false (sell) + false, // pay_with_deep + constants::max_u64(), + ); + mm.add_conditional_order( + &pool, + &sui_price_high, + &usdc_price, + &margin_registry, + 1, + condition, + pending_order, + &clock, + scenario.ctx(), + ); + + destroy_2!(sui_price_high, usdc_price); + return_shared_2!(mm, pool); + return_shared(usdc_pool); + return_shared(margin_registry); + + scenario.next_tx(test_constants::user2()); + let sui_price_low = build_sui_price_info_object_with_price(&mut scenario, 95, &clock); // $0.95 + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + let mut pool = scenario.take_shared>(); + let mut margin_registry = scenario.take_shared(); + let mut mm = scenario.take_shared>(); + pool_proxy::update_current_price( + &mut margin_registry, + &pool, + &sui_price_low, + &usdc_price, + &clock, + ); + + let mut sui_pool = scenario.take_shared_by_id>(sui_pool_id); + let mut usdc_pool = scenario.take_shared_by_id>(usdc_pool_id); + + let debt_before = mm.borrowed_quote_shares(); + + let order_infos = test_helpers::execute_conditional_orders_v3_for_test( + &mut scenario, + &mut sui_pool, + &mut usdc_pool, + &mut mm, + &mut pool, + &sui_price_low, + &usdc_price, + &margin_registry, + 10, + &clock, + ); + + // The limit SL was placed (rested, no fill → no repay) and removed from the + // queue; v2 would have aborted on this danger-band setup. + assert!(order_infos.length() == 1); + assert!(order_infos[0].executed_quantity() == 0); + assert!(mm.borrowed_quote_shares() == debt_before); + assert!(mm.conditional_order_ids().length() == 0); + + destroy(order_infos); + return_shared(sui_pool); + return_shared(usdc_pool); + destroy_2!(sui_price_low, usdc_price); + return_shared_2!(mm, pool); + cleanup_margin_test(margin_registry, admin_cap, maintainer_cap, clock, scenario); +} From 5188bb2e894efc60a9cee5f42c0a7a8b42c47a1c Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Wed, 24 Jun 2026 11:04:53 -0400 Subject: [PATCH 12/27] margin: add place_market_order_and_repay_loan + reduce-only min_size floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a non-reduce-only place_market_order_and_repay_loan: place a market order, repay the debt side, then gate on min_open post-repay. It's the clean close/deleverage tool — a full close drives debt to 0 (risk_ratio MAX) so it passes even in the danger band where place_market_order_v2 aborts, and it has no quantity cap so a non-lot-aligned (accrued-interest) debt can be cleared by overbuying past it. Safe: the repay caps the debt reduction, zero debt has no bad-debt risk, and assert_price bounds slippage. Floor the reduce-only bid cap at one min_size — (base_debt - base_asset).max( min_size) — so a sub-lot net debt isn't stuck in reduce-only (margin-disabled) mode. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 4 +- .../deepbook_margin/sources/pool_proxy.move | 113 ++++++- .../tests/helper/test_helpers.move | 37 +++ .../tests/pool_proxy_tests.move | 293 ++++++++++++++++++ 4 files changed, 433 insertions(+), 14 deletions(-) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index baae602d7..fa9507563 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -260,7 +260,9 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - Slippage tolerance and solvency are orthogonal knobs. Allowable slippage belongs in the per-pool `price_tolerance` band enforced by `margin_registry.move::assert_price` (bids capped above oracle, asks capped below; 1%–50%, default 5%) — not in the risk-ratio check. Do not relax the risk-ratio floor to make room for slippage. -- The reduce-only **quantity** cap must be **asymmetric**, not symmetric. Cap the *selling* side (closing a long: `quantity <= base_asset`) on **gross holdings**; cap the *buying* side (covering a short: `quantity <= base_debt - base_asset`) on the **net** short. Reason: selling base can only shrink base exposure — you can't sell base you don't hold, so it can never flip you short — so gross is safe and lets a long fully unwind (fixes "can't fully sell the asset side" / "full close overshoots"). Buying base *can* overshoot the net short and leave a long after repay, i.e. **open fresh opposite-side exposure through a "reduce-only" order**, which bypasses the trading-disabled / below-`min_borrow` restriction. A symmetric gross cap on both sides is the bug. Guarded in `pool_proxy.move` (`place_reduce_only_limit_order_v2`, `place_reduce_only_market_order_and_repay_loan`); the superseded `place_reduce_only_market_order_v2` keeps the legacy symmetric net cap. This is a concrete instance of the `code-review.md` rule "a branch that treats the same parameter differently must be mathematically necessary, not accidental policy." +- The reduce-only **quantity** cap is **reduce-only *semantics*, not a solvency guard**. Solvency is the monotonic check while debt remains, plus the fact that zero debt has no bad-debt risk (a full close drives `risk_ratio` to MAX — which is why the close path *skips* the monotonic check once debt hits 0). So the cap's only job is "don't let a reduce-only order open fresh opposite-side exposure." It is asymmetric: the *ask* (closing a long) caps on **gross holdings** (`quantity <= base_asset`) — selling base can only shrink base exposure, never flip short; the *bid* (covering a short) caps on the **net short, floored at one `min_size`** (`quantity <= (base_debt - base_asset).max(min_size)`) — net so a buy can't overshoot into a fresh long *while debt remains*, floored so a sub-lot net debt (accrued-interest dust) can still be covered instead of leaving the position stuck (`min_size` from `pool.pool_book_params()`). Guarded in `pool_proxy.move`'s reduce-only entries. + +- For **closing** (rather than strict reduce-only), prefer the non-reduce-only `pool_proxy::place_market_order_and_repay_loan`: it places a market order, repays the debt side, then gates on `min_open_risk_ratio` **post-repay**, so a danger-band close passes where `place_market_order_v2` aborts (the repay runs *before* the solvency check). It has **no quantity cap** on purpose — to clear a non-lot-aligned debt you must overbuy *past* the debt, and any tight cap (net *or* gross) re-hits the lot wall at the debt boundary. That's safe because the repay caps the actual debt reduction at the outstanding debt, zero debt has no bad-debt risk, any surplus is the user's own holding, and `assert_price` still bounds slippage. The cap was never carrying safety here; the repay is. Reduce-only entries stay restrictive for margin-trading-disabled mode, where this endpoint's `pool_enabled` gate blocks it. - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index fbf9f075b..16eb1fa79 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -302,15 +302,17 @@ public fun place_reduce_only_limit_order_v2( margin_manager.calculate_debts(quote_margin_pool, clock) }; let (base_asset, _) = margin_manager.calculate_assets(pool); + let (_, _, min_size) = pool.pool_book_params(); // Reduce-only. The ask (closing a long) may sell up to the full base the // manager holds, so a long can be fully unwound — selling base can only - // shrink base exposure, never flip it short. The bid (covering a short) - // stays capped at the net short (`base_debt - base_asset`): buying past it - // would flip the short into a fresh long, i.e. open exposure rather than - // reduce it. The monotonic risk-ratio check below guards value leak. + // shrink base exposure, never flip it short. The bid (covering a short) is + // capped at the net short (`base_debt - base_asset`), but never below one + // `min_size` order, so a sub-lot net debt (e.g. accrued-interest dust) can + // still be covered instead of leaving the position stuck. The monotonic + // risk-ratio check below guards value leak. assert!( - (is_bid && base_debt > base_asset && quantity <= base_debt - base_asset) || + (is_bid && base_debt > base_asset && quantity <= (base_debt - base_asset).max(min_size)) || (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -407,10 +409,13 @@ public fun place_reduce_only_market_order_v2( clock, ); - // The order is a bid, and quantity is less than the net base debt. - // The order is a ask, and quote quantity is less than the net quote debt. + // Reduce-only (legacy net-debt cap on both sides). The bid is never capped + // below one `min_size` order, so a sub-lot net debt can still be covered. + // This entry is superseded for closing (see the doc above); the floor only + // keeps it consistent with the live reduce-only entries. + let (_, _, min_size) = pool.pool_book_params(); assert!( - (is_bid && base_debt > base_asset && quantity <= base_debt - base_asset) || + (is_bid && base_debt > base_asset && quantity <= (base_debt - base_asset).max(min_size)) || (!is_bid && quote_debt > quote_asset && quote_quantity <= quote_debt - quote_asset), ENotReduceOnlyOrder, ); @@ -495,6 +500,7 @@ public fun place_reduce_only_market_order_and_repay_loan( margin_manager.calculate_debts(quote_margin_pool, clock) }; let (base_asset, _) = margin_manager.calculate_assets(pool); + let (_, _, min_size) = pool.pool_book_params(); let (effective_price, _) = calculate_effective_price( pool, @@ -506,12 +512,13 @@ public fun place_reduce_only_market_order_and_repay_loan( // Reduce-only. The ask (closing a long) may sell up to the full base the // manager holds — selling can only shrink base exposure, never flip it - // short — so a long fully closes and the repay absorbs any proceeds past the - // debt. The bid (covering a short) stays capped at the net short - // (`base_debt - base_asset`) so buying back can't overshoot into a fresh - // long. The net-state monotonic check below guards value leak. + // short. The bid (covering a short) is capped at the net short + // (`base_debt - base_asset`), but never below one `min_size` order, so a + // sub-lot net debt (e.g. accrued-interest dust) can still be covered instead + // of leaving the position stuck. The net-state monotonic check below guards + // value leak. assert!( - (is_bid && base_debt > base_asset && quantity <= base_debt - base_asset) || + (is_bid && base_debt > base_asset && quantity <= (base_debt - base_asset).max(min_size)) || (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -571,6 +578,86 @@ public fun place_reduce_only_market_order_and_repay_loan( order_info } +/// Atomically places a market order and repays the loan with the proceeds, +/// gating post-repay solvency on `min_open_risk_ratio`. This is the everyday +/// close / deleverage tool: unlike `place_market_order_v2`, the repay runs +/// *before* the solvency check, so the deleverage credit lets a position close +/// cleanly even in the `liquidation..min_borrow` band — a full close drives debt +/// to 0 (`risk_ratio` MAX), which always passes. +/// +/// Not reduce-only: there is no quantity cap, so a close may overshoot the debt +/// (e.g. round past accrued-interest dust that isn't lot-aligned). That is safe +/// — the repay only ever reduces debt, zero debt has no bad-debt risk to the +/// lending pool, any surplus base/quote is the manager's own holding, and +/// `assert_price` still bounds slippage. The repay is the safety mechanism here, +/// not a quantity cap. Requires margin trading enabled; in reduce-only mode use +/// `place_reduce_only_market_order_and_repay_loan`. +public fun place_market_order_and_repay_loan( + registry: &MarginRegistry, + margin_manager: &mut MarginManager, + pool: &mut Pool, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, + base_oracle: &PriceInfoObject, + quote_oracle: &PriceInfoObject, + client_order_id: u64, + self_matching_option: u8, + quantity: u64, + is_bid: bool, + pay_with_deep: bool, + clock: &Clock, + ctx: &mut TxContext, +): OrderInfo { + registry.load_inner(); + assert!(margin_manager.deepbook_pool() == pool.id(), EIncorrectDeepBookPool); + assert!(registry.pool_enabled(pool), EPoolNotEnabledForMarginTrading); + + let (effective_price, _) = calculate_effective_price( + pool, + quantity, + is_bid, + pay_with_deep, + clock, + ); + registry.assert_price(pool.id(), effective_price, is_bid, clock); + + let trade_proof = margin_manager.trade_proof(ctx); + let balance_manager = margin_manager.balance_manager_trading_mut(ctx); + let order_info = pool.place_market_order( + balance_manager, + &trade_proof, + client_order_id, + self_matching_option, + quantity, + is_bid, + pay_with_deep, + clock, + ctx, + ); + + // Repay the debt side with the settled proceeds *before* the solvency check, + // so a deleveraging close passes where the bare swap would not. Skipped when + // the manager has no debt. + if (margin_manager.has_base_debt()) { + margin_manager.repay_base(registry, base_margin_pool, option::none(), clock, ctx); + } else if (margin_manager.borrowed_quote_shares() > 0) { + margin_manager.repay_quote(registry, quote_margin_pool, option::none(), clock, ctx); + }; + + assert_post_trade_solvent( + margin_manager, + registry, + pool, + base_margin_pool, + quote_margin_pool, + base_oracle, + quote_oracle, + clock, + ); + + order_info +} + /// Modifies an order public fun modify_order( registry: &MarginRegistry, diff --git a/packages/deepbook_margin/tests/helper/test_helpers.move b/packages/deepbook_margin/tests/helper/test_helpers.move index dac2158a2..53a59d465 100644 --- a/packages/deepbook_margin/tests/helper/test_helpers.move +++ b/packages/deepbook_margin/tests/helper/test_helpers.move @@ -1487,6 +1487,43 @@ public fun place_reduce_only_market_order_and_repay_loan_for_test( + scenario: &mut Scenario, + registry: &MarginRegistry, + mm: &mut deepbook_margin::margin_manager::MarginManager, + pool: &mut Pool, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, + client_order_id: u64, + self_matching_option: u8, + quantity: u64, + is_bid: bool, + pay_with_deep: bool, + clock: &Clock, +): deepbook::order_info::OrderInfo { + let base_oracle = build_price_info_for_type(scenario, clock); + let quote_oracle = build_price_info_for_type(scenario, clock); + let order_info = pool_proxy::place_market_order_and_repay_loan( + registry, + mm, + pool, + base_margin_pool, + quote_margin_pool, + &base_oracle, + "e_oracle, + client_order_id, + self_matching_option, + quantity, + is_bid, + pay_with_deep, + clock, + scenario.ctx(), + ); + destroy(base_oracle); + destroy(quote_oracle); + order_info +} + public fun execute_conditional_orders_v2_for_test( scenario: &mut Scenario, base_margin_pool: &MarginPool, diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index c81e30007..2f8badcdd 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -6572,3 +6572,296 @@ fun place_market_order_v2_with_free_borrowed_funds_in_band_ok() { return_shared(base_pool); cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } + +// === place_market_order_and_repay_loan (non-reduce-only close) === + +#[test] +fun place_market_order_and_repay_loan_fully_closes_long() { + // A long fully closes via the non-reduce-only and-repay: sell base, repay the + // quote loan, debt -> 0 (risk_ratio MAX), which clears the min_open gate. + // Deposit 10000 USDC, borrow 500 USDT, withdraw 300 USDT; sell 600 USDC, + // whose proceeds repay the entire 500 USDT debt. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(10000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 500 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 300 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy_2!(usdc_price, usdt_price); + + let order_info = test_helpers::place_market_order_and_repay_loan_for_test( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::self_matching_allowed(), + 600 * test_constants::usdc_multiplier(), + false, // is_bid = false (sell base to close the long) + false, + &clock, + ); + destroy(order_info); + + // Loan fully repaid -> position closed. + assert_eq!(mm.borrowed_quote_shares(), 0); + + return_shared_3!(mm, pool, quote_pool); + return_shared(base_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +#[test] +fun place_market_order_and_repay_loan_overbuys_short_past_debt() { + // A short (USDC debt, holding USDT) closes with a bid that buys *more* than + // the debt — there is no reduce-only quantity cap, so it can overshoot to + // clear the loan (e.g. round past dust). Owe 100 USDC, buy 101, repay 100, + // debt -> 0 with ~1 USDC surplus. The reduce-only bid (capped at the net + // short) would reject 101. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(250 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 100 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + // Withdraw the borrowed USDC so the manager is a real short: owe 100 USDC, + // hold 250 USDT (ratio 2.5, above min_withdraw 2.0). + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 100 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy_2!(usdc_price, usdt_price); + + let order_info = test_helpers::place_market_order_and_repay_loan_for_test( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::self_matching_allowed(), + 101 * test_constants::usdc_multiplier(), // buy 101 to cover a 100 debt + true, // is_bid = true (buy base to cover the short) + false, + &clock, + ); + destroy(order_info); + + // Overbought past the debt and fully closed. + assert_eq!(mm.borrowed_base_shares(), 0); + + return_shared_3!(mm, pool, base_pool); + return_shared(quote_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +#[test] +fun reduce_only_bid_allows_min_size_when_net_debt_is_sub_lot() { + // Reduce-only "not stuck" floor: when the net short is below one min_size + // order, the bid is still allowed up to min_size so a dust debt can be + // covered. Borrow 100 USDC, withdraw only min_size/2 (5000) of it, leaving a + // net short of 5000 — below the 10000 min_size. A reduce-only limit bid for + // min_size is accepted; the unfloored net cap (5000) would have rejected it. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(200 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 100 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + // Withdraw only half a min_size of the borrowed USDC, leaving net short 5000. + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + constants::min_size() / 2, + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy_2!(usdc_price, usdt_price); + + // Reduce-only limit bid for exactly one min_size, resting at $0.99. + let order_info = test_helpers::place_reduce_only_limit_order_v2_for_test( + &mut scenario, + ®istry, + &base_pool, + "e_pool, + &mut mm, + &mut pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 990_000_000, // $0.99 (rests below the $1.01 ask) + constants::min_size(), + true, // is_bid = true + false, + 2_000_000, + &clock, + ); + + assert_eq!(order_info.original_quantity(), constants::min_size()); + assert_eq!(order_info.executed_quantity(), 0); + + destroy(order_info); + return_shared_3!(mm, pool, base_pool); + return_shared(quote_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} From 815d79dc16b2cbaee94857a861f52498ae0e6fd5 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Wed, 24 Jun 2026 11:17:15 -0400 Subject: [PATCH 13/27] margin: reduce-only bid cap rounds net short up to the next lot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the reduce-only bid cap from net.max(min_size) to reduce_only_bid_cap = (net.div_ceil(lot_size) * lot_size).max(min_size), so a non-lot-aligned net debt (e.g. owe 40.001 with lot 0.1) can be cleared by buying the next lot up — not just the sub-min_size case. The over-round is bounded by one lot, so the post-repay residual long is always under one lot (dust), never opening meaningful new exposure. Applied to all reduce-only entries via a shared helper. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 2 +- .../deepbook_margin/sources/pool_proxy.move | 49 +++++---- .../tests/pool_proxy_tests.move | 102 ++++++++++++++++++ 3 files changed, 133 insertions(+), 20 deletions(-) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index fa9507563..da5603cec 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -260,7 +260,7 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - Slippage tolerance and solvency are orthogonal knobs. Allowable slippage belongs in the per-pool `price_tolerance` band enforced by `margin_registry.move::assert_price` (bids capped above oracle, asks capped below; 1%–50%, default 5%) — not in the risk-ratio check. Do not relax the risk-ratio floor to make room for slippage. -- The reduce-only **quantity** cap is **reduce-only *semantics*, not a solvency guard**. Solvency is the monotonic check while debt remains, plus the fact that zero debt has no bad-debt risk (a full close drives `risk_ratio` to MAX — which is why the close path *skips* the monotonic check once debt hits 0). So the cap's only job is "don't let a reduce-only order open fresh opposite-side exposure." It is asymmetric: the *ask* (closing a long) caps on **gross holdings** (`quantity <= base_asset`) — selling base can only shrink base exposure, never flip short; the *bid* (covering a short) caps on the **net short, floored at one `min_size`** (`quantity <= (base_debt - base_asset).max(min_size)`) — net so a buy can't overshoot into a fresh long *while debt remains*, floored so a sub-lot net debt (accrued-interest dust) can still be covered instead of leaving the position stuck (`min_size` from `pool.pool_book_params()`). Guarded in `pool_proxy.move`'s reduce-only entries. +- The reduce-only **quantity** cap is **reduce-only *semantics*, not a solvency guard**. Solvency is the monotonic check while debt remains, plus the fact that zero debt has no bad-debt risk (a full close drives `risk_ratio` to MAX — which is why the close path *skips* the monotonic check once debt hits 0). So the cap's only job is "don't let a reduce-only order open fresh opposite-side exposure." It is asymmetric: the *ask* (closing a long) caps on **gross holdings** (`quantity <= base_asset`) — selling base can only shrink base exposure, never flip short; the *bid* (covering a short) caps on the **net short rounded up to the next lot, floored at one `min_size`** (`reduce_only_bid_cap` = `(net.div_ceil(lot_size) * lot_size).max(min_size)`, lot/min from `pool.pool_book_params()`) — net so a buy can't overshoot into a fresh long *while debt remains*, but rounded up so a *non-lot-aligned* debt (accrued interest, e.g. owe 40.001 with lot 0.1) can be cleared by buying the next lot, and floored so a *sub-`min_size`* net debt isn't stuck. Both relaxations are bounded by one lot/min_size, so the post-repay residual long is always under one lot (dust) — never meaningful new exposure. Guarded in `pool_proxy.move`'s reduce-only entries. - For **closing** (rather than strict reduce-only), prefer the non-reduce-only `pool_proxy::place_market_order_and_repay_loan`: it places a market order, repays the debt side, then gates on `min_open_risk_ratio` **post-repay**, so a danger-band close passes where `place_market_order_v2` aborts (the repay runs *before* the solvency check). It has **no quantity cap** on purpose — to clear a non-lot-aligned debt you must overbuy *past* the debt, and any tight cap (net *or* gross) re-hits the lot wall at the debt boundary. That's safe because the repay caps the actual debt reduction at the outstanding debt, zero debt has no bad-debt risk, any surplus is the user's own holding, and `assert_price` still bounds slippage. The cap was never carrying safety here; the repay is. Reduce-only entries stay restrictive for margin-trading-disabled mode, where this endpoint's `pool_enabled` gate blocks it. diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index 16eb1fa79..dc191ac8b 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -302,17 +302,16 @@ public fun place_reduce_only_limit_order_v2( margin_manager.calculate_debts(quote_margin_pool, clock) }; let (base_asset, _) = margin_manager.calculate_assets(pool); - let (_, _, min_size) = pool.pool_book_params(); // Reduce-only. The ask (closing a long) may sell up to the full base the // manager holds, so a long can be fully unwound — selling base can only // shrink base exposure, never flip it short. The bid (covering a short) is - // capped at the net short (`base_debt - base_asset`), but never below one - // `min_size` order, so a sub-lot net debt (e.g. accrued-interest dust) can - // still be covered instead of leaving the position stuck. The monotonic - // risk-ratio check below guards value leak. + // capped at the net short rounded up to the next lot and floored at one + // `min_size` order (`reduce_only_bid_cap`), so a non-lot-aligned or sub-lot + // net debt (e.g. accrued-interest dust) can still be fully covered. The + // monotonic risk-ratio check below guards value leak. assert!( - (is_bid && base_debt > base_asset && quantity <= (base_debt - base_asset).max(min_size)) || + (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -409,13 +408,13 @@ public fun place_reduce_only_market_order_v2( clock, ); - // Reduce-only (legacy net-debt cap on both sides). The bid is never capped - // below one `min_size` order, so a sub-lot net debt can still be covered. - // This entry is superseded for closing (see the doc above); the floor only - // keeps it consistent with the live reduce-only entries. - let (_, _, min_size) = pool.pool_book_params(); + // Reduce-only (legacy net-debt cap; ask still uses the old quote net cap). + // The bid cap rounds the net short up to the next lot and floors it at one + // `min_size` (`reduce_only_bid_cap`) so a non-lot-aligned/sub-lot debt can be + // covered. This entry is superseded for closing (see the doc above); the cap + // only keeps it consistent with the live reduce-only entries. assert!( - (is_bid && base_debt > base_asset && quantity <= (base_debt - base_asset).max(min_size)) || + (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || (!is_bid && quote_debt > quote_asset && quote_quantity <= quote_debt - quote_asset), ENotReduceOnlyOrder, ); @@ -500,7 +499,6 @@ public fun place_reduce_only_market_order_and_repay_loan( margin_manager.calculate_debts(quote_margin_pool, clock) }; let (base_asset, _) = margin_manager.calculate_assets(pool); - let (_, _, min_size) = pool.pool_book_params(); let (effective_price, _) = calculate_effective_price( pool, @@ -512,13 +510,13 @@ public fun place_reduce_only_market_order_and_repay_loan( // Reduce-only. The ask (closing a long) may sell up to the full base the // manager holds — selling can only shrink base exposure, never flip it - // short. The bid (covering a short) is capped at the net short - // (`base_debt - base_asset`), but never below one `min_size` order, so a - // sub-lot net debt (e.g. accrued-interest dust) can still be covered instead - // of leaving the position stuck. The net-state monotonic check below guards - // value leak. + // short. The bid (covering a short) is capped at the net short rounded up to + // the next lot and floored at one `min_size` order (`reduce_only_bid_cap`), + // so a non-lot-aligned or sub-lot net debt (e.g. accrued-interest dust) can + // still be fully covered. The net-state monotonic check below guards value + // leak. assert!( - (is_bid && base_debt > base_asset && quantity <= (base_debt - base_asset).max(min_size)) || + (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -889,6 +887,19 @@ public fun claim_rebates( // === Internal Functions === +/// Reduce-only bid quantity cap for covering a short. The net short is rounded +/// *up* to the next lot so a non-lot-aligned debt (e.g. accrued interest) can be +/// fully cleared, and floored at one `min_size` order so a sub-lot net debt +/// isn't stuck. The round-up is bounded by one lot, so the post-repay residual +/// long is always under one lot (dust) — it never opens meaningful new exposure. +fun reduce_only_bid_cap( + pool: &Pool, + net_debt: u64, +): u64 { + let (_, lot_size, min_size) = pool.pool_book_params(); + (net_debt.div_ceil(lot_size) * lot_size).max(min_size) +} + /// Calculates the effective price for a market order by querying the pool. /// Returns (effective_price, quote_amount) where quote_amount is quote_in for bids and quote_out for asks. fun calculate_effective_price( diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index 2f8badcdd..a1d964bee 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -6865,3 +6865,105 @@ fun reduce_only_bid_allows_min_size_when_net_debt_is_sub_lot() { return_shared(quote_pool); cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } + +#[test] +fun reduce_only_and_repay_rounds_net_debt_up_to_lot_to_fully_close() { + // Reduce-only round-up-to-lot floor: a non-lot-aligned net short can be fully + // closed by buying the next lot up. Borrow 30 USDC, withdraw all but 500 raw, + // leaving a net short of ~29_999_500 (not lot-aligned, lot = 1000). The + // reduce-only and-repay bid for 30_000_000 (the net rounded up to the next + // lot) is accepted and clears the loan; the un-rounded net cap would reject + // it, stranding the dust. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(100 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 30 * test_constants::usdc_multiplier(), // 30 USDC = 30_000_000 (lot-aligned) + &clock, + scenario.ctx(), + ); + // Withdraw all but 500 raw, leaving net short = 30_000_000 - 500 = 29_999_500 + // (not a multiple of lot_size 1000). + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 30 * test_constants::usdc_multiplier() - 500, + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy_2!(usdc_price, usdt_price); + + // Buy 30_000_000 — the net short (~29_999_500) rounded up to the next lot. + let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< + USDC, + USDT, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::self_matching_allowed(), + 30 * test_constants::usdc_multiplier(), + true, // is_bid = true (cover the short) + false, + &clock, + ); + destroy(order_info); + + // Rounding up to the lot let the dust debt be fully cleared. + assert_eq!(mm.borrowed_base_shares(), 0); + + return_shared_3!(mm, pool, base_pool); + return_shared(quote_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} From 7ead5f181e8e09986a46607d54baa432509eb06a Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Wed, 24 Jun 2026 11:55:46 -0400 Subject: [PATCH 14/27] margin: add place_reduce_only_limit_order_and_repay_loan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit sibling of place_reduce_only_market_order_and_repay_loan: places a limit order, repays the debt with the settled taker fills before the net-state monotonic check. Closes the gap where a crossing reduce-only limit's taker portion pays the spread and aborts place_reduce_only_limit_order_v2's swap-only monotonic check — the price-bounded danger-band reduce. Resting remainder just locks balance (counted in assets), so the ratio holds. Same reduce-only cap. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 2 + .../deepbook_margin/sources/pool_proxy.move | 112 ++++++++++ .../tests/helper/test_helpers.move | 46 ++++ .../tests/pool_proxy_tests.move | 197 ++++++++++++++++++ 4 files changed, 357 insertions(+) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index da5603cec..56b912f9f 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -264,6 +264,8 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - For **closing** (rather than strict reduce-only), prefer the non-reduce-only `pool_proxy::place_market_order_and_repay_loan`: it places a market order, repays the debt side, then gates on `min_open_risk_ratio` **post-repay**, so a danger-band close passes where `place_market_order_v2` aborts (the repay runs *before* the solvency check). It has **no quantity cap** on purpose — to clear a non-lot-aligned debt you must overbuy *past* the debt, and any tight cap (net *or* gross) re-hits the lot wall at the debt boundary. That's safe because the repay caps the actual debt reduction at the outstanding debt, zero debt has no bad-debt risk, any surplus is the user's own holding, and `assert_price` still bounds slippage. The cap was never carrying safety here; the repay is. Reduce-only entries stay restrictive for margin-trading-disabled mode, where this endpoint's `pool_enabled` gate blocks it. +- The `…_and_repay_loan` pattern (repay before the solvency check) has a **limit** sibling, `place_reduce_only_limit_order_and_repay_loan`, for *price-bounded* reduces in the danger band. A limit order has three outcomes that matter here: **fully rests** (maker — no fill, locks balance counted in assets, ratio unchanged → the plain `place_reduce_only_limit_order_v2` already passes, no repay needed); **partial taker + maker rest** (the taker fills pay the spread, so the swap-only monotonic check in `…_v2` aborts — *this* is the gap the and-repay variant closes by repaying the settled taker proceeds first); **fully taker** (≈ a market order, covered by the market and-repay). So a `…_and_repay_loan` limit variant only earns its keep for the middle case — a crossing reduce-only limit. Note the repay uses `amount: none`, so it also pays down from *idle* balance, not only taker proceeds (intended: the `_and_repay` family deleverages; a user who just wants a resting order without deleveraging should use `place_reduce_only_limit_order_v2`). + - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. - The post-trade solvency floor for *opening* (risk-increasing) orders is a **separate** threshold from the borrow gate: `min_open_risk_ratio`, strictly between `liquidation` and `min_borrow`. Why they must differ: the borrow gate passes at *exactly* `min_borrow` (pre-trade), but any opening trade pays the spread, so the post-trade ratio lands just under `min_borrow` — if `assert_post_trade_solvent` reused `min_borrow`, a max-leverage open would *always* abort. `min_open` (default = midpoint of liquidation and min_borrow) gives the open room to absorb its own spread while staying above the liquidatable zone. It's a per-pool admin override stored as an optional dynamic field (the `MaxOrderTtlKey` pattern); the getter derives the midpoint default from the pool's current `liquidation`/`min_borrow` (so it tracks risk-param edits) and **ignores a stranded override** outside `(liquidation, min_borrow]` rather than returning an unsafe value (self-heal). Only the opening paths (`place_limit_order_v2`/`place_market_order_v2` → `assert_post_trade_solvent`) use `min_open`; the borrow gate and the v2 TP/SL post-fill check still use `min_borrow`. diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index dc191ac8b..43677a9d0 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -576,6 +576,118 @@ public fun place_reduce_only_market_order_and_repay_loan( order_info } +/// Reduce-only **limit** order that atomically repays the loan with the taker +/// fills. It is the limit/maker behaviour of `place_reduce_only_limit_order_v2` +/// plus the repay-then-net-monotonic gate of +/// `place_reduce_only_market_order_and_repay_loan`: the portion that crosses the +/// book fills immediately and settles, the rest rests as a maker, then the +/// settled (taker) proceeds repay the debt before the monotonic check on the net +/// (post-repay) state. +/// +/// This is the danger-band tool for a *price-bounded* reduce: a crossing +/// reduce-only limit pays the spread on its taker fills, which alone would abort +/// `place_reduce_only_limit_order_v2`'s swap-only monotonic check; repaying first +/// deleverages so the net ratio holds. The resting remainder only locks balance +/// (counted in assets), so it doesn't move the ratio. Unfilled-and-resting +/// behaves exactly like `place_reduce_only_limit_order_v2` (nothing to repay). +public fun place_reduce_only_limit_order_and_repay_loan( + registry: &MarginRegistry, + margin_manager: &mut MarginManager, + pool: &mut Pool, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, + base_oracle: &PriceInfoObject, + quote_oracle: &PriceInfoObject, + client_order_id: u64, + order_type: u8, + self_matching_option: u8, + price: u64, + quantity: u64, + is_bid: bool, + pay_with_deep: bool, + expire_timestamp: u64, + clock: &Clock, + ctx: &mut TxContext, +): OrderInfo { + registry.load_inner(); + assert!(margin_manager.deepbook_pool() == pool.id(), EIncorrectDeepBookPool); + + registry.assert_price(pool.id(), price, is_bid, clock); + let expire_timestamp = registry.clamp_expire_timestamp(pool.id(), expire_timestamp, clock); + + let (base_debt, quote_debt) = if (margin_manager.has_base_debt()) { + margin_manager.calculate_debts(base_margin_pool, clock) + } else { + margin_manager.calculate_debts(quote_margin_pool, clock) + }; + let (base_asset, _) = margin_manager.calculate_assets(pool); + + // Same reduce-only cap as `place_reduce_only_limit_order_v2` (see there): the + // ask sells up to gross base held; the bid covers up to the net short rounded + // up to a lot and floored at one `min_size`. + assert!( + (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || + (!is_bid && quote_debt > 0 && quantity <= base_asset), + ENotReduceOnlyOrder, + ); + + let risk_ratio_before = margin_manager.risk_ratio( + registry, + base_oracle, + quote_oracle, + pool, + base_margin_pool, + quote_margin_pool, + clock, + ); + + let trade_proof = margin_manager.trade_proof(ctx); + let balance_manager = margin_manager.balance_manager_trading_mut(ctx); + let order_info = pool.place_limit_order( + balance_manager, + &trade_proof, + client_order_id, + order_type, + self_matching_option, + price, + quantity, + is_bid, + pay_with_deep, + expire_timestamp, + clock, + ctx, + ); + + // Repay the debt side with the settled taker fills before the monotonic + // check, so a crossing reduce-only limit deleverages instead of aborting. A + // fully-resting order has no taker proceeds, so this repays nothing. + if (margin_manager.has_base_debt()) { + margin_manager.repay_base(registry, base_margin_pool, option::none(), clock, ctx); + } else { + margin_manager.repay_quote(registry, quote_margin_pool, option::none(), clock, ctx); + }; + + // Net-state solvency: if debt remains, the order must not have worsened the + // ratio. A full repay clears the debt, so the check is skipped. + if ( + margin_manager.borrowed_base_shares() > 0 + || margin_manager.borrowed_quote_shares() > 0 + ) { + let risk_ratio_after = margin_manager.risk_ratio( + registry, + base_oracle, + quote_oracle, + pool, + base_margin_pool, + quote_margin_pool, + clock, + ); + assert!(risk_ratio_after >= risk_ratio_before, EReduceOnlyMustImproveRiskRatio); + }; + + order_info +} + /// Atomically places a market order and repays the loan with the proceeds, /// gating post-repay solvency on `min_open_risk_ratio`. This is the everyday /// close / deleverage tool: unlike `place_market_order_v2`, the repay runs diff --git a/packages/deepbook_margin/tests/helper/test_helpers.move b/packages/deepbook_margin/tests/helper/test_helpers.move index 53a59d465..d9e640593 100644 --- a/packages/deepbook_margin/tests/helper/test_helpers.move +++ b/packages/deepbook_margin/tests/helper/test_helpers.move @@ -1524,6 +1524,52 @@ public fun place_market_order_and_repay_loan_for_test( order_info } +public fun place_reduce_only_limit_order_and_repay_loan_for_test( + scenario: &mut Scenario, + registry: &MarginRegistry, + mm: &mut deepbook_margin::margin_manager::MarginManager, + pool: &mut Pool, + base_margin_pool: &mut MarginPool, + quote_margin_pool: &mut MarginPool, + client_order_id: u64, + order_type: u8, + self_matching_option: u8, + price: u64, + quantity: u64, + is_bid: bool, + pay_with_deep: bool, + expire_timestamp: u64, + clock: &Clock, +): deepbook::order_info::OrderInfo { + let base_oracle = build_price_info_for_type(scenario, clock); + let quote_oracle = build_price_info_for_type(scenario, clock); + let order_info = pool_proxy::place_reduce_only_limit_order_and_repay_loan< + BaseAsset, + QuoteAsset, + >( + registry, + mm, + pool, + base_margin_pool, + quote_margin_pool, + &base_oracle, + "e_oracle, + client_order_id, + order_type, + self_matching_option, + price, + quantity, + is_bid, + pay_with_deep, + expire_timestamp, + clock, + scenario.ctx(), + ); + destroy(base_oracle); + destroy(quote_oracle); + order_info +} + public fun execute_conditional_orders_v2_for_test( scenario: &mut Scenario, base_margin_pool: &MarginPool, diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index a1d964bee..c6551876f 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -6967,3 +6967,200 @@ fun reduce_only_and_repay_rounds_net_debt_up_to_lot_to_fully_close() { return_shared(quote_pool); cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } + +// === place_reduce_only_limit_order_and_repay_loan === + +#[test] +fun reduce_only_limit_and_repay_partial_taker_fill_repays() { + // A reduce-only limit order that crosses the book partially: the taker + // portion fills and settles, the remainder rests as a maker, and the taker + // proceeds repay the loan before the net-monotonic check. Long: hold 15000 + // USDC, 500 USDT debt. Ask 12000 USDC at $0.99 taker-fills the 10000-USDC bid + // depth (~9900 USDT, repays the whole 500 debt) and rests 2000. The non-repay + // place_reduce_only_limit_order_v2 aborts on this — see the paired test. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(15000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 500 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 300 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy_2!(usdc_price, usdt_price); + + let order_info = test_helpers::place_reduce_only_limit_order_and_repay_loan_for_test< + USDC, + USDT, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 990_000_000, // $0.99 — crosses the $0.99 bid + 12000 * test_constants::usdc_multiplier(), + false, // is_bid = false (sell to close the long) + false, + 2_000_000, + &clock, + ); + + // The taker portion filled the 10000-USDC bid depth; its proceeds cleared the + // loan; the remaining 2000 rests as a maker. + assert_eq!(order_info.executed_quantity(), 10000 * test_constants::usdc_multiplier()); + assert_eq!(mm.borrowed_quote_shares(), 0); + + destroy(order_info); + return_shared_3!(mm, pool, quote_pool); + return_shared(base_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +#[test, expected_failure(abort_code = pool_proxy::EReduceOnlyMustImproveRiskRatio)] +fun reduce_only_limit_v2_taker_fill_aborts() { + // The same crossing reduce-only ask, but via the non-repay + // place_reduce_only_limit_order_v2: the taker fill pays the spread, lowering + // the swap-only ratio, so the monotonic check aborts (the gap the and-repay + // variant closes). + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + setup_orderbook_liquidity_stablecoin(&mut scenario, pool_id, &clock); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(15000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 500 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 300 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy_2!(usdc_price, usdt_price); + + let _ = test_helpers::place_reduce_only_limit_order_v2_for_test( + &mut scenario, + ®istry, + &base_pool, + "e_pool, + &mut mm, + &mut pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 990_000_000, + 12000 * test_constants::usdc_multiplier(), + false, + false, + 2_000_000, + &clock, + ); + + abort 999 +} From 527f9deb31e4da79287f16578a4ad2c5ed2a5328 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Wed, 24 Jun 2026 12:24:01 -0400 Subject: [PATCH 15/27] margin: make reduce-only market v2 ask cap gross; fix div_ceil CI build - place_reduce_only_market_order_v2 ask now uses the gross base_asset cap (quote_debt > 0 && quantity <= base_asset), consistent with place_reduce_only_limit_order_v2; drops the stale net-quote ask limit and the now-unused quote_asset/quote_quantity locals. - reduce_only_bid_cap: compute the lot ceiling with modulo arithmetic instead of u64.div_ceil, which CI's older Sui std lacks (math::div_round_up is FLOAT_SCALING-scaled, not a plain integer ceiling, so it's not usable here). - Repurpose the market-v2 ask reduce-only test to assert the gross cap. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 2 +- .../deepbook_margin/sources/pool_proxy.move | 25 +++++++++++-------- .../tests/pool_proxy_tests.move | 7 +++--- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index 56b912f9f..3377d4b8c 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -260,7 +260,7 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - Slippage tolerance and solvency are orthogonal knobs. Allowable slippage belongs in the per-pool `price_tolerance` band enforced by `margin_registry.move::assert_price` (bids capped above oracle, asks capped below; 1%–50%, default 5%) — not in the risk-ratio check. Do not relax the risk-ratio floor to make room for slippage. -- The reduce-only **quantity** cap is **reduce-only *semantics*, not a solvency guard**. Solvency is the monotonic check while debt remains, plus the fact that zero debt has no bad-debt risk (a full close drives `risk_ratio` to MAX — which is why the close path *skips* the monotonic check once debt hits 0). So the cap's only job is "don't let a reduce-only order open fresh opposite-side exposure." It is asymmetric: the *ask* (closing a long) caps on **gross holdings** (`quantity <= base_asset`) — selling base can only shrink base exposure, never flip short; the *bid* (covering a short) caps on the **net short rounded up to the next lot, floored at one `min_size`** (`reduce_only_bid_cap` = `(net.div_ceil(lot_size) * lot_size).max(min_size)`, lot/min from `pool.pool_book_params()`) — net so a buy can't overshoot into a fresh long *while debt remains*, but rounded up so a *non-lot-aligned* debt (accrued interest, e.g. owe 40.001 with lot 0.1) can be cleared by buying the next lot, and floored so a *sub-`min_size`* net debt isn't stuck. Both relaxations are bounded by one lot/min_size, so the post-repay residual long is always under one lot (dust) — never meaningful new exposure. Guarded in `pool_proxy.move`'s reduce-only entries. +- The reduce-only **quantity** cap is **reduce-only *semantics*, not a solvency guard**. Solvency is the monotonic check while debt remains, plus the fact that zero debt has no bad-debt risk (a full close drives `risk_ratio` to MAX — which is why the close path *skips* the monotonic check once debt hits 0). So the cap's only job is "don't let a reduce-only order open fresh opposite-side exposure." It is asymmetric: the *ask* (closing a long) caps on **gross holdings** (`quantity <= base_asset`) — selling base can only shrink base exposure, never flip short; the *bid* (covering a short) caps on the **net short rounded up to the next lot, floored at one `min_size`** (`reduce_only_bid_cap` = ceil `net` up to the next `lot_size`, then `.max(min_size)`; lot/min from `pool.pool_book_params()`. **Compute the lot ceiling with modulo arithmetic** (`net - net % lot + lot`), *not* `u64.div_ceil` — that method is new in recent Sui std and CI's older toolchain fails to build with `No known method 'div_ceil' on type 'u64'`. Also do **not** reach for `deepbook::math::div_round_up`: it's `FLOAT_SCALING`-scaled (a fixed-point divide), not a plain integer ceiling) — net so a buy can't overshoot into a fresh long *while debt remains*, but rounded up so a *non-lot-aligned* debt (accrued interest, e.g. owe 40.001 with lot 0.1) can be cleared by buying the next lot, and floored so a *sub-`min_size`* net debt isn't stuck. Both relaxations are bounded by one lot/min_size, so the post-repay residual long is always under one lot (dust) — never meaningful new exposure. Guarded in `pool_proxy.move`'s reduce-only entries. - For **closing** (rather than strict reduce-only), prefer the non-reduce-only `pool_proxy::place_market_order_and_repay_loan`: it places a market order, repays the debt side, then gates on `min_open_risk_ratio` **post-repay**, so a danger-band close passes where `place_market_order_v2` aborts (the repay runs *before* the solvency check). It has **no quantity cap** on purpose — to clear a non-lot-aligned debt you must overbuy *past* the debt, and any tight cap (net *or* gross) re-hits the lot wall at the debt boundary. That's safe because the repay caps the actual debt reduction at the outstanding debt, zero debt has no bad-debt risk, any surplus is the user's own holding, and `assert_price` still bounds slippage. The cap was never carrying safety here; the repay is. Reduce-only entries stay restrictive for margin-trading-disabled mode, where this endpoint's `pool_enabled` gate blocks it. diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index 43677a9d0..b3941b821 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -396,11 +396,9 @@ public fun place_reduce_only_market_order_v2( } else { margin_manager.calculate_debts(quote_margin_pool, clock) }; - let (base_asset, quote_asset) = margin_manager.calculate_assets( - pool, - ); + let (base_asset, _) = margin_manager.calculate_assets(pool); - let (effective_price, quote_quantity) = calculate_effective_price( + let (effective_price, _) = calculate_effective_price( pool, quantity, is_bid, @@ -408,14 +406,14 @@ public fun place_reduce_only_market_order_v2( clock, ); - // Reduce-only (legacy net-debt cap; ask still uses the old quote net cap). - // The bid cap rounds the net short up to the next lot and floors it at one - // `min_size` (`reduce_only_bid_cap`) so a non-lot-aligned/sub-lot debt can be - // covered. This entry is superseded for closing (see the doc above); the cap - // only keeps it consistent with the live reduce-only entries. + // Reduce-only, same cap as the other entries: the ask sells up to the gross + // base held; the bid covers the net short rounded up to the next lot and + // floored at one `min_size` (`reduce_only_bid_cap`). Superseded for closing + // (see the doc above) — a market taker fill can't satisfy the monotonic check + // without a repay, so use `place_reduce_only_market_order_and_repay_loan`. assert!( (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || - (!is_bid && quote_debt > quote_asset && quote_quantity <= quote_debt - quote_asset), + (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -1009,7 +1007,12 @@ fun reduce_only_bid_cap( net_debt: u64, ): u64 { let (_, lot_size, min_size) = pool.pool_book_params(); - (net_debt.div_ceil(lot_size) * lot_size).max(min_size) + // Ceil `net_debt` up to a whole multiple of `lot_size`, then floor at one + // `min_size`. Written as modulo arithmetic (rather than `div_ceil`) to build + // on every Sui std version. + let remainder = net_debt % lot_size; + let rounded_up = if (remainder == 0) net_debt else net_debt - remainder + lot_size; + rounded_up.max(min_size) } /// Calculates the effective price for a market order by querying the pool. diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index c6551876f..f1ec0074b 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -2361,8 +2361,9 @@ fun test_place_reduce_only_market_order_not_reduce_only_quantity_ask() { ); destroy(coin); - // User has USDT debt of 100, tries to sell enough to get more than 100 USDT (quote_quantity > debt) - // Selling 150 USDC at ~1:1 should yield ~150 USDT, exceeding the 100 USDT debt + // User holds 10000 USDC base and has USDT (quote) debt. The reduce-only ask + // cap is the gross base held, so selling more base than held (11000 > 10000) + // is not a reduce-only order. test_helpers::place_reduce_only_market_order_v2_for_test( &mut scenario, ®istry, @@ -2372,7 +2373,7 @@ fun test_place_reduce_only_market_order_not_reduce_only_quantity_ask() { &mut pool, 5, constants::self_matching_allowed(), - 150 * test_constants::usdc_multiplier(), + 11000 * test_constants::usdc_multiplier(), false, // is_bid = false (selling USDC to get USDT) false, From 15199204780c1c4072a971b87236b078d9be89e9 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Wed, 24 Jun 2026 12:36:07 -0400 Subject: [PATCH 16/27] margin: test place_market_order_and_repay_loan pool-disabled guard; fix stale doc - Add place_market_order_and_repay_loan_aborts_when_pool_disabled: the pool_enabled gate is this endpoint's only guard not shared with the reduce-only and-repay siblings, and had no negative test. - Fix stale doc on place_reduce_only_market_order_v2 (it no longer keeps a symmetric net-debt cap; ask is now gross-base, bid is round-up-to-lot, matching the other reduce-only entries). Co-Authored-By: Claude Opus 4.8 --- .../deepbook_margin/sources/pool_proxy.move | 4 +- .../tests/pool_proxy_tests.move | 78 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index b3941b821..1b90300e6 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -370,8 +370,8 @@ public fun place_reduce_only_limit_order_v2( /// `risk_ratio` while the debt is unchanged, so the swap-only monotonic check /// here rejects essentially every taker fill. The `_and_repay` variant /// deleverages with the proceeds so the net-state ratio actually improves. Kept -/// callable for existing integrators; its symmetric net-debt cap is retained as -/// legacy (the live entries cap the ask on gross base held instead). +/// callable for existing integrators; its quantity caps match the other +/// reduce-only entries (gross-base ask, round-up-to-lot bid). public fun place_reduce_only_market_order_v2( registry: &MarginRegistry, margin_manager: &mut MarginManager, diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index f1ec0074b..bba70753e 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -6767,6 +6767,84 @@ fun place_market_order_and_repay_loan_overbuys_short_past_debt() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } +#[test, expected_failure(abort_code = pool_proxy::EPoolNotEnabledForMarginTrading)] +fun place_market_order_and_repay_loan_aborts_when_pool_disabled() { + // Unlike the reduce-only and-repay siblings, the non-reduce-only and-repay + // requires margin trading enabled — in reduce-only mode a user must close via + // place_reduce_only_market_order_and_repay_loan. Disabling the pool makes this + // endpoint abort on its pool_enabled gate (the guard that distinguishes it). + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(250 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 100 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy_2!(usdc_price, usdt_price); + + // Drop the pool out of margin trading; the and-repay close must now refuse. + registry.disable_deepbook_pool(&_admin_cap, &mut pool, &clock); + + let _ = test_helpers::place_market_order_and_repay_loan_for_test( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::self_matching_allowed(), + 101 * test_constants::usdc_multiplier(), + true, + false, + &clock, + ); + + abort 999 +} + #[test] fun reduce_only_bid_allows_min_size_when_net_debt_is_sub_lot() { // Reduce-only "not stuck" floor: when the net short is below one min_size From e8d42f996b00590883359f43744680d2b00b0dd4 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Wed, 24 Jun 2026 14:20:44 -0400 Subject: [PATCH 17/27] =?UTF-8?q?margin:=20address=20review=20=E2=80=94=20?= =?UTF-8?q?owner-gate=20test,=20surplus=20assert,=20comment=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add test_repay_base_fails_non_owner: the owner gate that backstops withdraw_without_owner_check had no negative test (review #1). - Strengthen overbuys-short test to a clear overshoot (150 vs 100 debt) and assert the 50 surplus lands as manager-owned base, not lost/aborted (#7). - Fix reduce_only_bid_cap doc: the residual over-cover is bounded by one lot OR one min_size (floor branch can exceed a lot when min_size > lot_size), not 'always under one lot'; note monotonic/solvency is the real guard (#3/#4). - Pin the single-debt-side invariant (single margin_pool_id) at the v3 repay dispatch so has_base_debt-else-quote reads as exhaustive (#6). Co-Authored-By: Claude Opus 4.8 --- .../sources/margin_manager.move | 5 +- .../deepbook_margin/sources/pool_proxy.move | 10 ++-- .../tests/margin_manager_tests.move | 49 +++++++++++++++++++ .../tests/pool_proxy_tests.move | 16 +++--- 4 files changed, 69 insertions(+), 11 deletions(-) diff --git a/packages/deepbook_margin/sources/margin_manager.move b/packages/deepbook_margin/sources/margin_manager.move index 8b35bd4a4..7b470807e 100644 --- a/packages/deepbook_margin/sources/margin_manager.move +++ b/packages/deepbook_margin/sources/margin_manager.move @@ -2059,7 +2059,10 @@ fun process_collected_orders_v3( // Deleverage with the market proceeds so the fill improves (not just holds) // solvency. The repay only moves the manager's own funds against its own - // debt, so it is safe in this permissionless path. + // debt, so it is safe in this permissionless path. A manager holds at most + // one debt side at a time (single `margin_pool_id: Option`, enforced by + // `can_borrow`), so `has_base_debt()`-else-quote is an exhaustive dispatch, + // not a both-sides-simultaneously assumption. if (market_filled && has_debt) { if (self.has_base_debt()) { self.repay( diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index 1b90300e6..cd8dcb75e 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -999,9 +999,13 @@ public fun claim_rebates( /// Reduce-only bid quantity cap for covering a short. The net short is rounded /// *up* to the next lot so a non-lot-aligned debt (e.g. accrued interest) can be -/// fully cleared, and floored at one `min_size` order so a sub-lot net debt -/// isn't stuck. The round-up is bounded by one lot, so the post-repay residual -/// long is always under one lot (dust) — it never opens meaningful new exposure. +/// fully cleared, and floored at one `min_size` so a sub-`min_size` net debt +/// isn't stuck below the orderbook minimum. The over-cover is therefore bounded +/// by one lot (round-up branch) or one `min_size` (floor branch — which can +/// exceed a lot when `min_size > lot_size`); either way it's a fixed bound, not +/// open-ended exposure. The reduce-only monotonic check — or, in the and-repay +/// entries, the post-repay solvency gate — is what actually guards value; this +/// cap only enforces reduce-only *semantics*. fun reduce_only_bid_cap( pool: &Pool, net_debt: u64, diff --git a/packages/deepbook_margin/tests/margin_manager_tests.move b/packages/deepbook_margin/tests/margin_manager_tests.move index 51ad7751d..0bede1670 100644 --- a/packages/deepbook_margin/tests/margin_manager_tests.move +++ b/packages/deepbook_margin/tests/margin_manager_tests.move @@ -890,6 +890,55 @@ fun test_repay_fails_wrong_pool() { abort } +#[test, expected_failure(abort_code = margin_manager::EInvalidMarginManagerOwner)] +fun test_repay_base_fails_non_owner() { + // The owner gate that backstops withdraw_without_owner_check: a non-owner + // cannot reach `repay` (and the underlying withdraw) through the public + // repay_base wrapper — validate_owner aborts first. The only owner-unchecked + // callers of withdraw_without_owner_check are the internal permissionless + // paths (liquidation, conditional execution), which route funds into the + // manager's own debt or to a liquidator under the reward formula, never to an + // arbitrary caller. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + _usdc_pool_id, + usdt_pool_id, + _pool_id, + registry_id, + ) = setup_usdc_usdt_deepbook_margin(); + + scenario.next_tx(test_constants::user1()); + let mut registry = scenario.take_shared(); + let pool = scenario.take_shared>(); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + // A different account (not the manager owner) attempts to repay. + scenario.next_tx(test_constants::user2()); + let mut mm = scenario.take_shared>(); + let mut usdt_pool = scenario.take_shared_by_id>(usdt_pool_id); + + mm.repay_base( + ®istry, + &mut usdt_pool, + option::none(), + &clock, + scenario.ctx(), + ); + + abort 999 +} + #[test] fun test_repay_full_with_none() { let ( diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index bba70753e..a47d1731e 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -6672,11 +6672,11 @@ fun place_market_order_and_repay_loan_fully_closes_long() { #[test] fun place_market_order_and_repay_loan_overbuys_short_past_debt() { - // A short (USDC debt, holding USDT) closes with a bid that buys *more* than - // the debt — there is no reduce-only quantity cap, so it can overshoot to - // clear the loan (e.g. round past dust). Owe 100 USDC, buy 101, repay 100, - // debt -> 0 with ~1 USDC surplus. The reduce-only bid (capped at the net - // short) would reject 101. + // A short (USDC debt, holding USDT) closes with a bid that buys well *past* + // the debt — there is no reduce-only quantity cap, so the overshoot is + // allowed. Owe 100 USDC, buy 150, repay 100, debt -> 0 with a 50 USDC surplus + // that lands as the manager's own holding (not an abort, not lost). The + // reduce-only bid (capped at the net short rounded to a lot) would reject 150. let ( mut scenario, clock, @@ -6752,15 +6752,17 @@ fun place_market_order_and_repay_loan_overbuys_short_past_debt() { &mut quote_pool, 1, constants::self_matching_allowed(), - 101 * test_constants::usdc_multiplier(), // buy 101 to cover a 100 debt + 150 * test_constants::usdc_multiplier(), // buy 150 to cover a 100 debt true, // is_bid = true (buy base to cover the short) false, &clock, ); destroy(order_info); - // Overbought past the debt and fully closed. + // Overbought well past the debt and fully closed; the 50 USDC overshoot is + // retained as the manager's own base balance, not lost or aborted. assert_eq!(mm.borrowed_base_shares(), 0); + assert_eq!(mm.base_balance(), 50 * test_constants::usdc_multiplier()); return_shared_3!(mm, pool, base_pool); return_shared(quote_pool); From f23f6e5c148c5082e64b5af9a268f9133b8a1c6e Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Wed, 24 Jun 2026 15:50:12 -0400 Subject: [PATCH 18/27] margin: gate place_market_order_and_repay_loan on monotonic, not min_open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per integration feedback: the min_open opening floor is mismatched for a close/deleverage tool — it rejects partial danger-band improvements (e.g. 1.12 -> 1.15 where 1.15 < min_open). Swap it for the net-state monotonic gate (post-repay risk_ratio >= pre-trade, skipped when a full repay clears debt). Safe because a market (taker) fill settles immediately, so the monotonic check reflects the true final state: any exposure-increasing trade lowers the ratio and aborts, any deleveraging trade is allowed at any size. This makes the quantity cap unnecessary for the market case (monotonic subsumes reduce-only intent for immediate fills). - Rename EReduceOnlyMustImproveRiskRatio -> ERiskRatioMustNotWorsen (value 7 unchanged) since the gate is now shared with this non-reduce-only entry; broaden its doc. Integrators get one consistent code for a failed close. - Add place_market_order_and_repay_loan_partial_close_below_min_open_ok: a partial danger-band close that improves but stays below min_open now passes (would abort under the old floor). The worsening-aborts direction is already covered by the shared ERiskRatioMustNotWorsen reduce-only taker-fill tests. Co-Authored-By: Claude Opus 4.8 --- .../deepbook_margin/sources/pool_proxy.move | 90 +++++++--- .../tests/pool_proxy_tests.move | 165 +++++++++++++++++- 2 files changed, 224 insertions(+), 31 deletions(-) diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index cd8dcb75e..d1014eb33 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -26,10 +26,11 @@ const ENoLiquidityInOrderbook: u64 = 5; /// order placement entries when an opening trade would leave the manager in the /// liquidatable zone. const EInsufficientRiskRatioAfterTrade: u64 = 6; -/// Reduce-only fill leaked value to the counterparty: the manager's -/// risk_ratio after the trade is lower than before. Reduce-only orders must -/// monotonically improve (or hold) solvency. -const EReduceOnlyMustImproveRiskRatio: u64 = 7; +/// A risk-reducing fill left the manager's net (post-repay) risk_ratio lower +/// than before the trade. The reduce-only entries and the market close+repay +/// tool (`place_market_order_and_repay_loan`) require the post-trade ratio to +/// monotonically improve (or hold). +const ERiskRatioMustNotWorsen: u64 = 7; /// Deprecated v1 entry was called. Use the `_v2` variant which enforces a /// post-trade risk_ratio invariant. const EDeprecatedUseV2: u64 = 8; @@ -568,7 +569,7 @@ public fun place_reduce_only_market_order_and_repay_loan( quote_margin_pool, clock, ); - assert!(risk_ratio_after >= risk_ratio_before, EReduceOnlyMustImproveRiskRatio); + assert!(risk_ratio_after >= risk_ratio_before, ERiskRatioMustNotWorsen); }; order_info @@ -680,25 +681,30 @@ public fun place_reduce_only_limit_order_and_repay_loan( quote_margin_pool, clock, ); - assert!(risk_ratio_after >= risk_ratio_before, EReduceOnlyMustImproveRiskRatio); + assert!(risk_ratio_after >= risk_ratio_before, ERiskRatioMustNotWorsen); }; order_info } /// Atomically places a market order and repays the loan with the proceeds, -/// gating post-repay solvency on `min_open_risk_ratio`. This is the everyday -/// close / deleverage tool: unlike `place_market_order_v2`, the repay runs -/// *before* the solvency check, so the deleverage credit lets a position close -/// cleanly even in the `liquidation..min_borrow` band — a full close drives debt -/// to 0 (`risk_ratio` MAX), which always passes. +/// gating on a **monotonic** net-state check: if any debt remains after the +/// repay, the post-repay `risk_ratio` must be at least the pre-trade ratio +/// (improve-or-hold). A full close drives debt to 0 (`risk_ratio` MAX), which +/// always passes. /// -/// Not reduce-only: there is no quantity cap, so a close may overshoot the debt -/// (e.g. round past accrued-interest dust that isn't lot-aligned). That is safe -/// — the repay only ever reduces debt, zero debt has no bad-debt risk to the -/// lending pool, any surplus base/quote is the manager's own holding, and -/// `assert_price` still bounds slippage. The repay is the safety mechanism here, -/// not a quantity cap. Requires margin trading enabled; in reduce-only mode use +/// This is the everyday close / deleverage tool. The monotonic gate — rather than +/// the `min_open` opening floor used by `place_market_order_v2` — lets a position +/// in the `liquidation..min_borrow` danger band wind down *partially*: a small +/// close that lifts the ratio from, say, 1.12 to 1.15 is allowed even though 1.15 +/// is still below `min_open`, which the opening floor would reject. +/// +/// Not reduce-only and uncapped, but the monotonic check makes a quantity cap +/// unnecessary: a market (taker) fill settles immediately, so any genuinely +/// exposure-*increasing* trade lowers the ratio and aborts here, while any +/// deleveraging trade is allowed at any size — an overshoot past the debt is fine +/// (surplus is the manager's own holding) and `assert_price` still bounds +/// slippage. Requires margin trading enabled; in reduce-only mode use /// `place_reduce_only_market_order_and_repay_loan`. public fun place_market_order_and_repay_loan( registry: &MarginRegistry, @@ -729,6 +735,25 @@ public fun place_market_order_and_repay_loan( ); registry.assert_price(pool.id(), effective_price, is_bid, clock); + // Capture the pre-trade ratio for the monotonic gate, but only while the + // manager has debt — `risk_ratio` is only meaningful against a debt. + let risk_ratio_before = if ( + margin_manager.borrowed_base_shares() > 0 + || margin_manager.borrowed_quote_shares() > 0 + ) { + margin_manager.risk_ratio( + registry, + base_oracle, + quote_oracle, + pool, + base_margin_pool, + quote_margin_pool, + clock, + ) + } else { + 0 + }; + let trade_proof = margin_manager.trade_proof(ctx); let balance_manager = margin_manager.balance_manager_trading_mut(ctx); let order_info = pool.place_market_order( @@ -752,16 +777,25 @@ public fun place_market_order_and_repay_loan( margin_manager.repay_quote(registry, quote_margin_pool, option::none(), clock, ctx); }; - assert_post_trade_solvent( - margin_manager, - registry, - pool, - base_margin_pool, - quote_margin_pool, - base_oracle, - quote_oracle, - clock, - ); + // Net-state monotonic gate: if debt remains, the trade+repay must not have + // worsened the ratio. A full repay clears the debt (`risk_ratio` -> MAX), so + // the check is skipped. Debt only decreases here, so surviving debt implies + // there was debt before and `risk_ratio_before` is a real ratio. + if ( + margin_manager.borrowed_base_shares() > 0 + || margin_manager.borrowed_quote_shares() > 0 + ) { + let risk_ratio_after = margin_manager.risk_ratio( + registry, + base_oracle, + quote_oracle, + pool, + base_margin_pool, + quote_margin_pool, + clock, + ); + assert!(risk_ratio_after >= risk_ratio_before, ERiskRatioMustNotWorsen); + }; order_info } @@ -1103,5 +1137,5 @@ fun assert_reduce_only_monotonic( quote_margin_pool, clock, ); - assert!(risk_ratio_after >= risk_ratio_before, EReduceOnlyMustImproveRiskRatio); + assert!(risk_ratio_after >= risk_ratio_before, ERiskRatioMustNotWorsen); } diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index a47d1731e..ac35a07fd 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -1209,7 +1209,7 @@ fun test_place_reduce_only_limit_order_not_reduce_only_quantity_ask() { // reduce-only fills must monotonically improve (or hold) solvency. The // matching limit-order path (placed at exact oracle price) still passes its // `_ok` test above. -#[test, expected_failure(abort_code = pool_proxy::EReduceOnlyMustImproveRiskRatio)] +#[test, expected_failure(abort_code = pool_proxy::ERiskRatioMustNotWorsen)] fun test_place_reduce_only_market_order_ok() { let ( mut scenario, @@ -1313,7 +1313,7 @@ fun test_place_reduce_only_market_order_ok() { // Symmetric to `test_place_reduce_only_market_order_ok` — sell-side reduce-only // market hits bids at $0.99 (1% below oracle), degrading risk_ratio. Aborts on // the monotonic invariant. -#[test, expected_failure(abort_code = pool_proxy::EReduceOnlyMustImproveRiskRatio)] +#[test, expected_failure(abort_code = pool_proxy::ERiskRatioMustNotWorsen)] fun test_place_reduce_only_market_order_ok_ask() { let ( mut scenario, @@ -1812,6 +1812,165 @@ fun reduce_only_and_repay_closes_from_danger_zone() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } +#[test] +fun place_market_order_and_repay_loan_partial_close_below_min_open_ok() { + // The monotonic gate (vs. the min_open floor) lets a danger-band position + // improve *partially* without reaching min_open. Long: 2000 USDC collateral, + // 1000 USDT debt; drift USDC to $0.60 -> ratio 1.20. min_open is raised to the + // borrow floor so the post-close ratio stays below it; a small ask (sell 200 + // USDC, repay ~118 USDT) lifts the ratio but leaves it under min_open. The old + // min_open gate would abort this; the monotonic gate passes it because the + // ratio improved. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + setup_orderbook_liquidity_at_prices( + &mut scenario, + pool_id, + 590_000_000, + 610_000_000, + &clock, + ); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(2000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy(usdc_price); + + let usdc_drifted = build_demo_usdc_price_info_object_with_price( + &mut scenario, + 60_000_000, + &clock, + ); + pool_proxy::update_current_price( + &mut registry, + &pool, + &usdc_drifted, + &usdt_price, + &clock, + ); + + let pool_id_inner = pool.id(); + // Raise min_open to the borrow floor so the post-close ratio is guaranteed to + // stay below it — isolating the monotonic gate as the reason the close passes. + let strict_floor = registry.min_borrow_risk_ratio(pool_id_inner); + registry.set_min_open_risk_ratio( + &_admin_cap, + &pool, + strict_floor, + &clock, + ); + + let shares_before = mm.borrowed_quote_shares(); + let rr_before = mm.risk_ratio( + ®istry, + &usdc_drifted, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + assert!(rr_before < registry.min_open_risk_ratio(pool_id_inner)); + assert!(rr_before >= registry.liquidation_risk_ratio(pool_id_inner)); + + let order_info = pool_proxy::place_market_order_and_repay_loan( + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + &usdc_drifted, + &usdt_price, + 2, + constants::self_matching_allowed(), + 200 * test_constants::usdc_multiplier(), + false, // ask: sell USDC base to repay the USDT debt + false, + &clock, + scenario.ctx(), + ); + destroy(order_info); + + let rr_after = mm.risk_ratio( + ®istry, + &usdc_drifted, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + + // Debt partially repaid (not cleared), the ratio improved, and it is still + // below min_open — exactly the state the old floor gate would have rejected. + assert!(mm.borrowed_quote_shares() > 0); + assert!(mm.borrowed_quote_shares() < shares_before); + assert!(rr_after > rr_before); + assert!(rr_after < registry.min_open_risk_ratio(pool_id_inner)); + + return_shared_3!(mm, pool, quote_pool); + return_shared(base_pool); + destroy(usdc_drifted); + destroy(usdt_price); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + // Full close where the ask sells *more* base than the net quote debt — the case // the old net-debt cap forbade. Long position: 10000 USDC collateral, 500 USDT // debt, ~200 USDT free, so net quote debt is ~300. Selling 600 USDC produces @@ -7155,7 +7314,7 @@ fun reduce_only_limit_and_repay_partial_taker_fill_repays() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } -#[test, expected_failure(abort_code = pool_proxy::EReduceOnlyMustImproveRiskRatio)] +#[test, expected_failure(abort_code = pool_proxy::ERiskRatioMustNotWorsen)] fun reduce_only_limit_v2_taker_fill_aborts() { // The same crossing reduce-only ask, but via the non-repay // place_reduce_only_limit_order_v2: the taker fill pays the spread, lowering From b70b281b23cf76b2cb45757d116c66b82abaacf0 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Wed, 24 Jun 2026 16:59:18 -0400 Subject: [PATCH 19/27] docs(move.md): sync DeepBook Margin notes with the monotonic gate change - place_market_order_and_repay_loan gates on the net-state monotonic check, not the min_open floor (it no longer calls assert_post_trade_solvent). - Error constant renamed EReduceOnlyMustImproveRiskRatio -> ERiskRatioMustNotWorsen (code 7 unchanged), now shared with that non-reduce-only entry. - Fix the reduce_only_bid_cap over-cover bound: one lot OR one min_size (the floor branch can exceed a lot when min_size > lot_size), not 'always under one lot'. - Fix the assert_post_trade_solvent floor mention (min_open, not min_borrow). - Add the design rationale for why the monotonic gate is market-only and a limit and-repay must keep the cap (placement-time check vs. ungated maker fill). Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index 3377d4b8c..958a84f28 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -256,16 +256,18 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - `risk_ratio` (`margin_manager.move:risk_ratio_int`) is `assets / debt` with **assets valued at the Pyth oracle price, not the orderbook execution price**. Debt only changes on `borrow`/`repay`/`liquidate`, never on a trade. Consequence: any close/trade that *executes worse than oracle* (real slippage) hands oracle-measured value to the counterparty while debt is fixed, so it **strictly lowers the measured risk ratio**. -- Two distinct post-trade invariants in `pool_proxy.move`: regular trades use `assert_post_trade_solvent` (floor = `min_borrow_risk_ratio`, ~1.25); reduce-only trades use `assert_reduce_only_monotonic` (`ratio_after >= ratio_before`). The monotonic check fires on the **swap-only intermediate state, before any `repay`**, where slippage is pure value leak — so a reduce-only *market* close that pays the spread aborts on `EReduceOnlyMustImproveRiskRatio`. This is intentional and is asserted by `pool_proxy_tests.move::test_place_reduce_only_market_order_ok`, which is an `expected_failure`. Deleveraging only improves the ratio when the `repay` is included; check the invariant on the net (swap+repay) state, not the swap alone. +- Two distinct post-trade invariants in `pool_proxy.move`: opening trades use `assert_post_trade_solvent` (floor = `min_open_risk_ratio`, between liquidation and the borrow floor); reduce-only / close trades use a monotonic check (`ratio_after >= ratio_before`). The monotonic check fires on the **swap-only intermediate state, before any `repay`**, where slippage is pure value leak — so a reduce-only *market* close that pays the spread aborts on `ERiskRatioMustNotWorsen`. (This constant was renamed from `EReduceOnlyMustImproveRiskRatio`, abort code `7` unchanged, once the gate became shared with the non-reduce-only `place_market_order_and_repay_loan` — see below.) This is intentional and is asserted by `pool_proxy_tests.move::test_place_reduce_only_market_order_ok`, which is an `expected_failure`. Deleveraging only improves the ratio when the `repay` is included; check the invariant on the net (swap+repay) state, not the swap alone. - Slippage tolerance and solvency are orthogonal knobs. Allowable slippage belongs in the per-pool `price_tolerance` band enforced by `margin_registry.move::assert_price` (bids capped above oracle, asks capped below; 1%–50%, default 5%) — not in the risk-ratio check. Do not relax the risk-ratio floor to make room for slippage. -- The reduce-only **quantity** cap is **reduce-only *semantics*, not a solvency guard**. Solvency is the monotonic check while debt remains, plus the fact that zero debt has no bad-debt risk (a full close drives `risk_ratio` to MAX — which is why the close path *skips* the monotonic check once debt hits 0). So the cap's only job is "don't let a reduce-only order open fresh opposite-side exposure." It is asymmetric: the *ask* (closing a long) caps on **gross holdings** (`quantity <= base_asset`) — selling base can only shrink base exposure, never flip short; the *bid* (covering a short) caps on the **net short rounded up to the next lot, floored at one `min_size`** (`reduce_only_bid_cap` = ceil `net` up to the next `lot_size`, then `.max(min_size)`; lot/min from `pool.pool_book_params()`. **Compute the lot ceiling with modulo arithmetic** (`net - net % lot + lot`), *not* `u64.div_ceil` — that method is new in recent Sui std and CI's older toolchain fails to build with `No known method 'div_ceil' on type 'u64'`. Also do **not** reach for `deepbook::math::div_round_up`: it's `FLOAT_SCALING`-scaled (a fixed-point divide), not a plain integer ceiling) — net so a buy can't overshoot into a fresh long *while debt remains*, but rounded up so a *non-lot-aligned* debt (accrued interest, e.g. owe 40.001 with lot 0.1) can be cleared by buying the next lot, and floored so a *sub-`min_size`* net debt isn't stuck. Both relaxations are bounded by one lot/min_size, so the post-repay residual long is always under one lot (dust) — never meaningful new exposure. Guarded in `pool_proxy.move`'s reduce-only entries. +- The reduce-only **quantity** cap is **reduce-only *semantics*, not a solvency guard**. Solvency is the monotonic check while debt remains, plus the fact that zero debt has no bad-debt risk (a full close drives `risk_ratio` to MAX — which is why the close path *skips* the monotonic check once debt hits 0). So the cap's only job is "don't let a reduce-only order open fresh opposite-side exposure." It is asymmetric: the *ask* (closing a long) caps on **gross holdings** (`quantity <= base_asset`) — selling base can only shrink base exposure, never flip short; the *bid* (covering a short) caps on the **net short rounded up to the next lot, floored at one `min_size`** (`reduce_only_bid_cap` = ceil `net` up to the next `lot_size`, then `.max(min_size)`; lot/min from `pool.pool_book_params()`. **Compute the lot ceiling with modulo arithmetic** (`net - net % lot + lot`), *not* `u64.div_ceil` — that method is new in recent Sui std and CI's older toolchain fails to build with `No known method 'div_ceil' on type 'u64'`. Also do **not** reach for `deepbook::math::div_round_up`: it's `FLOAT_SCALING`-scaled (a fixed-point divide), not a plain integer ceiling) — net so a buy can't overshoot into a fresh long *while debt remains*, but rounded up so a *non-lot-aligned* debt (accrued interest, e.g. owe 40.001 with lot 0.1) can be cleared by buying the next lot, and floored so a *sub-`min_size`* net debt isn't stuck. The over-cover is a fixed bound — one lot (round-up branch) or one `min_size` (floor branch, **which can exceed a lot when `min_size > lot_size`**, so don't claim "always under one lot") — not open-ended exposure. The monotonic check (or, in the and-repay entries, the post-repay solvency gate) is what actually guards value; this cap only enforces reduce-only *semantics*. Guarded in `pool_proxy.move`'s reduce-only entries. -- For **closing** (rather than strict reduce-only), prefer the non-reduce-only `pool_proxy::place_market_order_and_repay_loan`: it places a market order, repays the debt side, then gates on `min_open_risk_ratio` **post-repay**, so a danger-band close passes where `place_market_order_v2` aborts (the repay runs *before* the solvency check). It has **no quantity cap** on purpose — to clear a non-lot-aligned debt you must overbuy *past* the debt, and any tight cap (net *or* gross) re-hits the lot wall at the debt boundary. That's safe because the repay caps the actual debt reduction at the outstanding debt, zero debt has no bad-debt risk, any surplus is the user's own holding, and `assert_price` still bounds slippage. The cap was never carrying safety here; the repay is. Reduce-only entries stay restrictive for margin-trading-disabled mode, where this endpoint's `pool_enabled` gate blocks it. +- For **closing** (rather than strict reduce-only), prefer the non-reduce-only `pool_proxy::place_market_order_and_repay_loan`: it places a market order, repays the debt side *before* the gate, then applies the **net-state monotonic check** (post-repay `risk_ratio >=` the pre-trade ratio; skipped once a full repay clears the debt → `risk_ratio` MAX). **Gate history (this branch):** it first used `assert_post_trade_solvent` (the `min_open` floor), then switched to the monotonic gate so a danger-band position can improve *partially* without having to reach `min_open` (e.g. lift 1.12 → 1.15 even though 1.15 < `min_open`, which the floor rejected). Safe because a market (taker) fill settles immediately, so the monotonic check reflects the true final state: any exposure-*increasing* trade lowers the ratio and aborts, any deleveraging trade is allowed at any size. So there is **no quantity cap** — the monotonic gate *subsumes* the reduce-only cap for immediate fills — and an overshoot past the debt is fine (the repay caps debt reduction at the outstanding debt, zero debt has no bad-debt risk, surplus is the user's own holding, `assert_price` bounds slippage). Tested by `place_market_order_and_repay_loan_partial_close_below_min_open_ok`. Requires `pool_enabled`; in reduce-only mode use `place_reduce_only_market_order_and_repay_loan`. - The `…_and_repay_loan` pattern (repay before the solvency check) has a **limit** sibling, `place_reduce_only_limit_order_and_repay_loan`, for *price-bounded* reduces in the danger band. A limit order has three outcomes that matter here: **fully rests** (maker — no fill, locks balance counted in assets, ratio unchanged → the plain `place_reduce_only_limit_order_v2` already passes, no repay needed); **partial taker + maker rest** (the taker fills pay the spread, so the swap-only monotonic check in `…_v2` aborts — *this* is the gap the and-repay variant closes by repaying the settled taker proceeds first); **fully taker** (≈ a market order, covered by the market and-repay). So a `…_and_repay_loan` limit variant only earns its keep for the middle case — a crossing reduce-only limit. Note the repay uses `amount: none`, so it also pays down from *idle* balance, not only taker proceeds (intended: the `_and_repay` family deleverages; a user who just wants a resting order without deleveraging should use `place_reduce_only_limit_order_v2`). +- **Why the monotonic gate is safe for a *market* and-repay but a *limit* one must keep the cap.** The gate runs **once, at placement**. A market order is 100% taker, so check-time == final state — airtight, which is why `place_market_order_and_repay_loan` can drop the cap and rely on monotonic alone. A crossing limit is taker (checked, improved) **+** a resting maker remainder that is **ratio-neutral at placement** (locked balance counts as assets), so it passes the check *invisibly* and then fills **later, ungated and un-repaid**. That deferred maker fill can leave the position worse than the checked state in three ways: it accumulates exposure past the debt (no second repay — flips a close into a directional position), it converts a stabilizing quote cushion into volatile base exposure (same ratio, but drops `Q·x/D` faster on the next `x%` move), or in a fast move it fills at its stale limit price off-oracle (direct value leak — `assert_price` only bounded the price vs the *placement* oracle). So for limits the reduce-only **quantity cap is the substitute for the missing maker-fill gate**: it bounds the resting remainder so an ungated fill can only ever *reduce*, never establish new exposure. Net rule: monotonic alone is sufficient only when the whole order fills under the check (market / fully-taker); keep the size cap on any **resting (maker) remainder**. + - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. - The post-trade solvency floor for *opening* (risk-increasing) orders is a **separate** threshold from the borrow gate: `min_open_risk_ratio`, strictly between `liquidation` and `min_borrow`. Why they must differ: the borrow gate passes at *exactly* `min_borrow` (pre-trade), but any opening trade pays the spread, so the post-trade ratio lands just under `min_borrow` — if `assert_post_trade_solvent` reused `min_borrow`, a max-leverage open would *always* abort. `min_open` (default = midpoint of liquidation and min_borrow) gives the open room to absorb its own spread while staying above the liquidatable zone. It's a per-pool admin override stored as an optional dynamic field (the `MaxOrderTtlKey` pattern); the getter derives the midpoint default from the pool's current `liquidation`/`min_borrow` (so it tracks risk-param edits) and **ignores a stranded override** outside `(liquidation, min_borrow]` rather than returning an unsafe value (self-heal). Only the opening paths (`place_limit_order_v2`/`place_market_order_v2` → `assert_post_trade_solvent`) use `min_open`; the borrow gate and the v2 TP/SL post-fill check still use `min_borrow`. From 0d0235f5d6b45d2f42b67a8c18a30a8f30a2a687 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Thu, 25 Jun 2026 11:32:34 -0400 Subject: [PATCH 20/27] margin: relax reduce-only bid to direction-only (drop the net-debt size cap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per integrator (Deeptrade) feedback: the net-debt bid size cap guarded a non-risk and wasn't enforceable. - Over-buying past the debt on a reduce-only bid converts quote into base, which for a base-denominated debt is price-invariant (28 SUI held vs 25 SUI debt is ratio 28/25 at every price) — it *reduces*, not increases, ratio exposure. - The cap was bypassable anyway (stacking reduce-only orders in one PTB exceeds the net debt), so it was friction, not protection. Across all four reduce-only entries the bid predicate becomes `base_debt > 0` (direction guard only, symmetric with the ask's `quote_debt > 0 && quantity <= base_asset`); funding + the monotonic / post-repay gate bound the size. Delete the orphaned reduce_only_bid_cap helper. The direction guard is the real boundary — it blocks a long (base_debt == 0) from bidding, the only genuinely exposure-increasing misuse. Tests: - Repurpose reduce_only_and_repay_quantity_exceeds_debt_aborts -> reduce_only_and_repay_bid_over_net_debt_fully_closes (over-cover now OK). - Repurpose the two *_not_reduce_only_quantity_bid tests -> *_bid_requires_base_debt (direction-guard regression: a long can't bid). - Rename the dust tests (no longer about the removed floor/round-up). Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 6 +- .../deepbook_margin/sources/pool_proxy.move | 75 ++++----- .../tests/pool_proxy_tests.move | 148 +++++++----------- 3 files changed, 91 insertions(+), 138 deletions(-) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index 958a84f28..df12beb72 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -260,13 +260,15 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - Slippage tolerance and solvency are orthogonal knobs. Allowable slippage belongs in the per-pool `price_tolerance` band enforced by `margin_registry.move::assert_price` (bids capped above oracle, asks capped below; 1%–50%, default 5%) — not in the risk-ratio check. Do not relax the risk-ratio floor to make room for slippage. -- The reduce-only **quantity** cap is **reduce-only *semantics*, not a solvency guard**. Solvency is the monotonic check while debt remains, plus the fact that zero debt has no bad-debt risk (a full close drives `risk_ratio` to MAX — which is why the close path *skips* the monotonic check once debt hits 0). So the cap's only job is "don't let a reduce-only order open fresh opposite-side exposure." It is asymmetric: the *ask* (closing a long) caps on **gross holdings** (`quantity <= base_asset`) — selling base can only shrink base exposure, never flip short; the *bid* (covering a short) caps on the **net short rounded up to the next lot, floored at one `min_size`** (`reduce_only_bid_cap` = ceil `net` up to the next `lot_size`, then `.max(min_size)`; lot/min from `pool.pool_book_params()`. **Compute the lot ceiling with modulo arithmetic** (`net - net % lot + lot`), *not* `u64.div_ceil` — that method is new in recent Sui std and CI's older toolchain fails to build with `No known method 'div_ceil' on type 'u64'`. Also do **not** reach for `deepbook::math::div_round_up`: it's `FLOAT_SCALING`-scaled (a fixed-point divide), not a plain integer ceiling) — net so a buy can't overshoot into a fresh long *while debt remains*, but rounded up so a *non-lot-aligned* debt (accrued interest, e.g. owe 40.001 with lot 0.1) can be cleared by buying the next lot, and floored so a *sub-`min_size`* net debt isn't stuck. The over-cover is a fixed bound — one lot (round-up branch) or one `min_size` (floor branch, **which can exceed a lot when `min_size > lot_size`**, so don't claim "always under one lot") — not open-ended exposure. The monotonic check (or, in the and-repay entries, the post-repay solvency gate) is what actually guards value; this cap only enforces reduce-only *semantics*. Guarded in `pool_proxy.move`'s reduce-only entries. +- The reduce-only order check is a **direction guard, not a size cap** — the debt-relative size cap was **removed** (see the design journal; superseded the earlier net-debt / round-up-to-lot bid cap and its `reduce_only_bid_cap` helper). It enforces reduce-only *semantics* ("only trade the position-reducing direction") and is symmetric across all four reduce-only entries: a **bid** requires base (short-side) debt (`base_debt > 0`); an **ask** requires quote (long-side) debt and sells up to gross base held (`quote_debt > 0 && quantity <= base_asset`). Neither side carries a debt-relative quantity cap — size is bounded by funding (the balance manager) plus the monotonic / post-repay solvency gate. **Why dropping the bid size cap is safe:** over-buying past the debt on the bid converts quote into base, and for a *base-denominated* debt that is **price-invariant** — 28 SUI held against 25 SUI debt is `ratio = 28/25` at *every* price, so it drives price exposure toward **zero** rather than increasing it. The direction guard is the real boundary: it blocks a *long* (`base_debt = 0`) from bidding, which is the only genuinely exposure-*increasing* misuse. (The old cap was also bypassable anyway — stacking reduce-only orders in one PTB exceeds the net debt — so it was friction, not enforcement.) Solvency is still the monotonic check while debt remains (a full close → `risk_ratio` MAX → check skipped). Guarded in `pool_proxy.move`'s four reduce-only entries. + +- **Sui-std CI gotcha (general).** `u64.div_ceil` is new in recent Sui std — CI's older toolchain fails to build with `No known method 'div_ceil' on type 'u64'`. For an integer ceiling use modulo (`x - x % m + m`); do **not** reach for `deepbook::math::div_round_up`, which is `FLOAT_SCALING`-scaled (a fixed-point divide), not a plain integer ceiling. - For **closing** (rather than strict reduce-only), prefer the non-reduce-only `pool_proxy::place_market_order_and_repay_loan`: it places a market order, repays the debt side *before* the gate, then applies the **net-state monotonic check** (post-repay `risk_ratio >=` the pre-trade ratio; skipped once a full repay clears the debt → `risk_ratio` MAX). **Gate history (this branch):** it first used `assert_post_trade_solvent` (the `min_open` floor), then switched to the monotonic gate so a danger-band position can improve *partially* without having to reach `min_open` (e.g. lift 1.12 → 1.15 even though 1.15 < `min_open`, which the floor rejected). Safe because a market (taker) fill settles immediately, so the monotonic check reflects the true final state: any exposure-*increasing* trade lowers the ratio and aborts, any deleveraging trade is allowed at any size. So there is **no quantity cap** — the monotonic gate *subsumes* the reduce-only cap for immediate fills — and an overshoot past the debt is fine (the repay caps debt reduction at the outstanding debt, zero debt has no bad-debt risk, surplus is the user's own holding, `assert_price` bounds slippage). Tested by `place_market_order_and_repay_loan_partial_close_below_min_open_ok`. Requires `pool_enabled`; in reduce-only mode use `place_reduce_only_market_order_and_repay_loan`. - The `…_and_repay_loan` pattern (repay before the solvency check) has a **limit** sibling, `place_reduce_only_limit_order_and_repay_loan`, for *price-bounded* reduces in the danger band. A limit order has three outcomes that matter here: **fully rests** (maker — no fill, locks balance counted in assets, ratio unchanged → the plain `place_reduce_only_limit_order_v2` already passes, no repay needed); **partial taker + maker rest** (the taker fills pay the spread, so the swap-only monotonic check in `…_v2` aborts — *this* is the gap the and-repay variant closes by repaying the settled taker proceeds first); **fully taker** (≈ a market order, covered by the market and-repay). So a `…_and_repay_loan` limit variant only earns its keep for the middle case — a crossing reduce-only limit. Note the repay uses `amount: none`, so it also pays down from *idle* balance, not only taker proceeds (intended: the `_and_repay` family deleverages; a user who just wants a resting order without deleveraging should use `place_reduce_only_limit_order_v2`). -- **Why the monotonic gate is safe for a *market* and-repay but a *limit* one must keep the cap.** The gate runs **once, at placement**. A market order is 100% taker, so check-time == final state — airtight, which is why `place_market_order_and_repay_loan` can drop the cap and rely on monotonic alone. A crossing limit is taker (checked, improved) **+** a resting maker remainder that is **ratio-neutral at placement** (locked balance counts as assets), so it passes the check *invisibly* and then fills **later, ungated and un-repaid**. That deferred maker fill can leave the position worse than the checked state in three ways: it accumulates exposure past the debt (no second repay — flips a close into a directional position), it converts a stabilizing quote cushion into volatile base exposure (same ratio, but drops `Q·x/D` faster on the next `x%` move), or in a fast move it fills at its stale limit price off-oracle (direct value leak — `assert_price` only bounded the price vs the *placement* oracle). So for limits the reduce-only **quantity cap is the substitute for the missing maker-fill gate**: it bounds the resting remainder so an ungated fill can only ever *reduce*, never establish new exposure. Net rule: monotonic alone is sufficient only when the whole order fills under the check (market / fully-taker); keep the size cap on any **resting (maker) remainder**. +- **The monotonic gate is per-placement, not a cross-transaction invariant — and that's why the *direction* guard (not a size cap) protects the resting portion of a limit.** The gate runs **once, at placement**. A market order is 100% taker (check-time == final state — airtight, why `place_market_order_and_repay_loan` rides on monotonic alone). A crossing limit is taker (checked) **+** a resting maker remainder that is ratio-neutral at placement, so it passes invisibly and then fills **later, ungated and un-repaid** — the monotonic check guarantees *nothing* about that deferred fill. An earlier draft concluded "limits must keep a size cap on the resting remainder," but that's **superseded**: what actually keeps the deferred fill safe is the **direction guard**. A reduce-only bid is restricted to a *short* (`base_debt > 0`), and a short's resting bid — even one that over-covers — only converts quote→base, which is **price-invariant** (see above), so the ungated fill can't raise ratio-exposure. The dangerous case (a *long* placing a resting bid to grow its long) is blocked outright by the direction guard, not by a size bound. (Inherent maker risk remains: off-oracle resting fills can leak value, and because the monotonic check resets its baseline each placement, repeated cycles can *ratchet* the ratio down across txs — bounded by the `assert_price` band on the taker leg and the order TTL, and self-inflicted since order placement is owner-gated. This is true of any margin limit order, not specific to dropping the cap.) - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. diff --git a/packages/deepbook_margin/sources/pool_proxy.move b/packages/deepbook_margin/sources/pool_proxy.move index d1014eb33..358678342 100644 --- a/packages/deepbook_margin/sources/pool_proxy.move +++ b/packages/deepbook_margin/sources/pool_proxy.move @@ -304,15 +304,15 @@ public fun place_reduce_only_limit_order_v2( }; let (base_asset, _) = margin_manager.calculate_assets(pool); - // Reduce-only. The ask (closing a long) may sell up to the full base the - // manager holds, so a long can be fully unwound — selling base can only - // shrink base exposure, never flip it short. The bid (covering a short) is - // capped at the net short rounded up to the next lot and floored at one - // `min_size` order (`reduce_only_bid_cap`), so a non-lot-aligned or sub-lot - // net debt (e.g. accrued-interest dust) can still be fully covered. The - // monotonic risk-ratio check below guards value leak. + // Reduce-only *direction* guard, symmetric for both sides — no debt-relative + // size cap. A bid requires base (short-side) debt and may buy any size: + // funding bounds it, and over-buying past the debt only converts quote into + // base, which for a base-denominated debt is price-invariant (it *reduces* + // price exposure, never increases it). The ask requires quote (long-side) debt + // and sells up to the gross base held. The monotonic risk-ratio check below + // guards value leak. assert!( - (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || + (is_bid && base_debt > 0) || (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -407,13 +407,13 @@ public fun place_reduce_only_market_order_v2( clock, ); - // Reduce-only, same cap as the other entries: the ask sells up to the gross - // base held; the bid covers the net short rounded up to the next lot and - // floored at one `min_size` (`reduce_only_bid_cap`). Superseded for closing - // (see the doc above) — a market taker fill can't satisfy the monotonic check - // without a repay, so use `place_reduce_only_market_order_and_repay_loan`. + // Reduce-only *direction* guard, same as the other entries — no size cap: a + // bid needs base (short-side) debt, the ask needs quote (long-side) debt and + // sells up to gross base held. Superseded for closing (see the doc above) — a + // market taker fill can't satisfy the monotonic check without a repay, so use + // `place_reduce_only_market_order_and_repay_loan`. assert!( - (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || + (is_bid && base_debt > 0) || (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -507,15 +507,15 @@ public fun place_reduce_only_market_order_and_repay_loan( clock, ); - // Reduce-only. The ask (closing a long) may sell up to the full base the - // manager holds — selling can only shrink base exposure, never flip it - // short. The bid (covering a short) is capped at the net short rounded up to - // the next lot and floored at one `min_size` order (`reduce_only_bid_cap`), - // so a non-lot-aligned or sub-lot net debt (e.g. accrued-interest dust) can - // still be fully covered. The net-state monotonic check below guards value - // leak. + // Reduce-only *direction* guard, symmetric for both sides — no debt-relative + // size cap. A bid requires base (short-side) debt and may buy any size: + // over-buying past the debt converts quote into base, price-invariant for a + // base-denominated debt (it *reduces* price exposure, never increases it), and + // the repay below caps the actual debt paydown at the outstanding debt. The + // ask requires quote (long-side) debt and sells up to gross base held. The + // net-state monotonic check below guards value leak. assert!( - (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || + (is_bid && base_debt > 0) || (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -621,11 +621,12 @@ public fun place_reduce_only_limit_order_and_repay_loan( }; let (base_asset, _) = margin_manager.calculate_assets(pool); - // Same reduce-only cap as `place_reduce_only_limit_order_v2` (see there): the - // ask sells up to gross base held; the bid covers up to the net short rounded - // up to a lot and floored at one `min_size`. + // Same reduce-only direction guard as `place_reduce_only_limit_order_v2` (see + // there) — no size cap: a bid needs base (short-side) debt (any size, funding- + // bounded), the ask needs quote (long-side) debt and sells up to gross base + // held. assert!( - (is_bid && base_debt > base_asset && quantity <= reduce_only_bid_cap(pool, base_debt - base_asset)) || + (is_bid && base_debt > 0) || (!is_bid && quote_debt > 0 && quantity <= base_asset), ENotReduceOnlyOrder, ); @@ -1031,28 +1032,6 @@ public fun claim_rebates( // === Internal Functions === -/// Reduce-only bid quantity cap for covering a short. The net short is rounded -/// *up* to the next lot so a non-lot-aligned debt (e.g. accrued interest) can be -/// fully cleared, and floored at one `min_size` so a sub-`min_size` net debt -/// isn't stuck below the orderbook minimum. The over-cover is therefore bounded -/// by one lot (round-up branch) or one `min_size` (floor branch — which can -/// exceed a lot when `min_size > lot_size`); either way it's a fixed bound, not -/// open-ended exposure. The reduce-only monotonic check — or, in the and-repay -/// entries, the post-repay solvency gate — is what actually guards value; this -/// cap only enforces reduce-only *semantics*. -fun reduce_only_bid_cap( - pool: &Pool, - net_debt: u64, -): u64 { - let (_, lot_size, min_size) = pool.pool_book_params(); - // Ceil `net_debt` up to a whole multiple of `lot_size`, then floor at one - // `min_size`. Written as modulo arithmetic (rather than `div_ceil`) to build - // on every Sui std version. - let remainder = net_debt % lot_size; - let rounded_up = if (remainder == 0) net_debt else net_debt - remainder + lot_size; - rounded_up.max(min_size) -} - /// Calculates the effective price for a market order by querying the pool. /// Returns (effective_price, quote_amount) where quote_amount is quote_in for bids and quote_out for asks. fun calculate_effective_price( diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index ac35a07fd..178cb83e7 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -999,7 +999,10 @@ fun test_place_reduce_only_limit_order_not_reduce_only() { } #[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] -fun test_place_reduce_only_limit_order_not_reduce_only_quantity_bid() { +fun test_place_reduce_only_limit_order_bid_requires_base_debt() { + // A reduce-only bid covers a *short*, so it requires base (short-side) debt. + // A manager with only quote debt (a long) placing a bid would increase its + // long, so the direction guard rejects it as not reduce-only. let ( mut scenario, clock, @@ -1014,8 +1017,8 @@ fun test_place_reduce_only_limit_order_not_reduce_only_quantity_bid() { scenario.next_tx(test_constants::user1()); let mut pool = scenario.take_shared_by_id>(pool_id); let mut registry = scenario.take_shared(); - let mut base_pool = scenario.take_shared_by_id>(base_pool_id); - let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); let deepbook_registry = scenario.take_shared_by_id(registry_id); margin_manager::new( &pool, @@ -1031,46 +1034,28 @@ fun test_place_reduce_only_limit_order_not_reduce_only_quantity_bid() { let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); - // Deposit some USDT to use as collateral - mm.deposit( - ®istry, - &usdc_price, - &usdt_price, - mint_coin(10000 * test_constants::usdt_multiplier(), scenario.ctx()), - &clock, - scenario.ctx(), - ); - destroy_2!(usdc_price, usdt_price); - - let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); - let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); - // Borrow some USDC - mm.borrow_base( + // USDC collateral + USDT (quote) debt -> a long, no base debt. + mm.deposit( ®istry, - &mut base_pool, &usdc_price, &usdt_price, - &pool, - 100 * test_constants::usdc_multiplier(), // Small borrow amount + mint_coin(10000 * test_constants::usdc_multiplier(), scenario.ctx()), &clock, scenario.ctx(), ); - - let coin = mm.withdraw( + mm.borrow_quote( ®istry, - &base_pool, - "e_pool, // Pass quote_pool since we have USDT debt + &mut quote_pool, &usdc_price, &usdt_price, &pool, - 100 * test_constants::usdc_multiplier(), // Withdraw some USDC so we have have net debt + 100 * test_constants::usdt_multiplier(), &clock, scenario.ctx(), ); - destroy(coin); + destroy_2!(usdc_price, usdt_price); - // User has USDC debt, tries to buy more USDC than debt - // This should fail because user is trying to buy more USDC than debt + // Bid (buy USDC) with no base debt -> not reduce-only (direction guard). test_helpers::place_reduce_only_limit_order_v2_for_test( &mut scenario, ®istry, @@ -1078,27 +1063,18 @@ fun test_place_reduce_only_limit_order_not_reduce_only_quantity_bid() { "e_pool, &mut mm, &mut pool, - // Pass quote_pool since we have USDT debt 1, constants::no_restriction(), constants::self_matching_allowed(), constants::float_scaling(), - // price - 101 * test_constants::usdc_multiplier(), - // quantity + 50 * test_constants::usdc_multiplier(), true, - // is_bid = true (buying USDC) false, 2000000, - // expire_timestamp &clock, ); - return_shared_2!(mm, pool); - return_shared_2!(base_pool, quote_pool); - destroy(usdc_price); - destroy(usdt_price); - cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); + abort 999 } #[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] @@ -2077,12 +2053,13 @@ fun reduce_only_and_repay_fully_closes_selling_above_net_debt() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } -// A reduce-only-and-repay bid whose quantity exceeds the net base debt is -// rejected: the bid covers a short, so it stays capped at the net short -// (500 - 200 = 300) to avoid flipping into a long. A 400 USDC bid is not -// reduce-only. -#[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] -fun reduce_only_and_repay_quantity_exceeds_debt_aborts() { +// A reduce-only-and-repay bid may now exceed the net base debt. Over-covering a +// short just converts USDC into USDC-debt coverage, which is price-invariant, so +// the size cap was dropped (only the direction guard remains). A 400 USDC bid +// against a 300 net short (500 borrowed, 200 held) over-covers, and the repay +// clears the whole 500 debt. +#[test] +fun reduce_only_and_repay_bid_over_net_debt_fully_closes() { let ( mut scenario, clock, @@ -2146,9 +2123,11 @@ fun reduce_only_and_repay_quantity_exceeds_debt_aborts() { scenario.ctx(), ); destroy(withdrawn); + destroy_2!(usdc_price, usdt_price); - // Short position: net base debt is 500 - 200 = 300, so a 400 USDC bid - // overshoots the net short and is not reduce-only. + // Net base debt is 500 - 200 = 300; the 400 USDC bid overshoots it and is now + // allowed (no size cap). Buying 400 (held 200 -> 600) and repaying clears the + // whole 500 debt. let order_info = test_helpers::place_reduce_only_market_order_and_repay_loan_for_test< USDC, USDT, @@ -2168,7 +2147,12 @@ fun reduce_only_and_repay_quantity_exceeds_debt_aborts() { ); destroy(order_info); - abort + // Over-covered past the net short and fully closed. + assert_eq!(mm.borrowed_base_shares(), 0); + + return_shared_3!(mm, pool, base_pool); + return_shared(quote_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } #[test, expected_failure(abort_code = pool_proxy::EIncorrectDeepBookPool)] @@ -2365,7 +2349,10 @@ fun test_place_reduce_only_market_order_not_reduce_only() { } #[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] -fun test_place_reduce_only_market_order_not_reduce_only_quantity_bid() { +fun test_place_reduce_only_market_order_bid_requires_base_debt() { + // Market sibling of the limit direction-guard test: a reduce-only bid needs + // base (short-side) debt. A long (quote debt, no base debt) bidding is not + // reduce-only. let ( mut scenario, clock, @@ -2382,8 +2369,8 @@ fun test_place_reduce_only_market_order_not_reduce_only_quantity_bid() { scenario.next_tx(test_constants::user1()); let mut pool = scenario.take_shared_by_id>(pool_id); let mut registry = scenario.take_shared(); - let mut base_pool = scenario.take_shared_by_id>(base_pool_id); - let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); let deepbook_registry = scenario.take_shared_by_id(registry_id); margin_manager::new( &pool, @@ -2399,40 +2386,28 @@ fun test_place_reduce_only_market_order_not_reduce_only_quantity_bid() { let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); - mm.deposit( - ®istry, - &usdc_price, - &usdt_price, - mint_coin(10000 * test_constants::usdt_multiplier(), scenario.ctx()), - &clock, - scenario.ctx(), - ); - - mm.borrow_base( + // USDC collateral + USDT (quote) debt -> a long, no base debt. + mm.deposit( ®istry, - &mut base_pool, &usdc_price, &usdt_price, - &pool, - 100 * test_constants::usdc_multiplier(), + mint_coin(10000 * test_constants::usdc_multiplier(), scenario.ctx()), &clock, scenario.ctx(), ); - - let coin = mm.withdraw( + mm.borrow_quote( ®istry, - &base_pool, - "e_pool, + &mut quote_pool, &usdc_price, &usdt_price, &pool, - 100 * test_constants::usdc_multiplier(), + 100 * test_constants::usdt_multiplier(), &clock, scenario.ctx(), ); - destroy(coin); + destroy_2!(usdc_price, usdt_price); - // User has USDC debt of 100, tries to buy 101 USDC (more than debt) + // Bid (buy USDC) with no base debt -> not reduce-only (direction guard). test_helpers::place_reduce_only_market_order_v2_for_test( &mut scenario, ®istry, @@ -2442,14 +2417,13 @@ fun test_place_reduce_only_market_order_not_reduce_only_quantity_bid() { &mut pool, 4, constants::self_matching_allowed(), - 101 * test_constants::usdc_multiplier(), + 50 * test_constants::usdc_multiplier(), true, - // is_bid false, &clock, ); - abort + abort 999 } #[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] @@ -7007,12 +6981,11 @@ fun place_market_order_and_repay_loan_aborts_when_pool_disabled() { } #[test] -fun reduce_only_bid_allows_min_size_when_net_debt_is_sub_lot() { - // Reduce-only "not stuck" floor: when the net short is below one min_size - // order, the bid is still allowed up to min_size so a dust debt can be - // covered. Borrow 100 USDC, withdraw only min_size/2 (5000) of it, leaving a - // net short of 5000 — below the 10000 min_size. A reduce-only limit bid for - // min_size is accepted; the unfloored net cap (5000) would have rejected it. +fun reduce_only_bid_covers_sub_min_size_net_debt() { + // Dust coverage: a sub-`min_size` net short can still be covered. With no size + // cap, the bid just has to clear the orderbook `min_size`. Borrow 100 USDC, + // withdraw min_size/2 (5000), leaving a net short of 5000 — below the 10000 + // `min_size`. A reduce-only limit bid for `min_size` is accepted. let ( mut scenario, clock, @@ -7107,13 +7080,12 @@ fun reduce_only_bid_allows_min_size_when_net_debt_is_sub_lot() { } #[test] -fun reduce_only_and_repay_rounds_net_debt_up_to_lot_to_fully_close() { - // Reduce-only round-up-to-lot floor: a non-lot-aligned net short can be fully - // closed by buying the next lot up. Borrow 30 USDC, withdraw all but 500 raw, - // leaving a net short of ~29_999_500 (not lot-aligned, lot = 1000). The - // reduce-only and-repay bid for 30_000_000 (the net rounded up to the next - // lot) is accepted and clears the loan; the un-rounded net cap would reject - // it, stranding the dust. +fun reduce_only_and_repay_closes_non_lot_aligned_debt() { + // A non-lot-aligned net short can be fully closed. With no size cap, the bid + // can be any valid lot-aligned quantity at or past the debt. Borrow 30 USDC, + // withdraw all but 500 raw, leaving a net short of ~29_999_500 (not lot- + // aligned, lot = 1000). A reduce-only and-repay bid for 30_000_000 (the next + // lot up) is accepted and clears the loan. let ( mut scenario, clock, From 63ca8cce8d55b0fdff355c3acb4f0be0e09dfca9 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Thu, 25 Jun 2026 12:43:44 -0400 Subject: [PATCH 21/27] margin: PoC tests for the empty-book resting-fill self-trade concern Verify the reduce-only limit resting-fill scenario (M1 rests a band-edge bid in an empty book, a counterparty takes it) is bounded, not a pool-theft exploit: - reduce_only_limit_resting_fill_at_band_edge_bounded_and_solvent: M1 (short) rests a $1.05 bid, a plain counterparty balance manager takes it -> M1 overpays 5% UNGATED (the fill doesn't re-run M1's margin check). Asserts the leak is real (ratio drops) but bounded (M1 stays solvent, > 1.0). - test_place_reduce_only_limit_order_ask_requires_quote_debt: the one-way direction guard -- a short can't ask -- which caps the leak to one conversion (~the price band) so a manager can't be ratcheted underwater. Co-Authored-By: Claude Opus 4.8 --- .../tests/pool_proxy_tests.move | 252 ++++++++++++++++++ 1 file changed, 252 insertions(+) diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index 178cb83e7..2fcce1f3f 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -1077,6 +1077,90 @@ fun test_place_reduce_only_limit_order_bid_requires_base_debt() { abort 999 } +#[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] +fun test_place_reduce_only_limit_order_ask_requires_quote_debt() { + // Mirror of the bid guard, and the *one-way* property that bounds the + // resting-fill ratchet: a reduce-only ask closes a long, so it requires quote + // (long-side) debt. A short (base debt, no quote debt) is rejected. So a short + // can only ever *bid* (convert quote->base) and a long can only *ask* + // (base->quote) — neither can trade back and forth, capping any value leak to + // one conversion (~the price band) and keeping the manager solvent. Without + // this guard, a manager could ratchet quote->base->quote... unboundedly via + // ungated resting fills and drive itself underwater. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + // USDT collateral + USDC (base) debt -> a short, no quote debt. + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(10000 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 100 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy_2!(usdc_price, usdt_price); + + // Ask (sell USDC) with no quote debt -> not reduce-only (direction guard). + test_helpers::place_reduce_only_limit_order_v2_for_test( + &mut scenario, + ®istry, + &base_pool, + "e_pool, + &mut mm, + &mut pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + constants::float_scaling(), + 50 * test_constants::usdc_multiplier(), + false, + false, + 2000000, + &clock, + ); + + abort 999 +} + #[test, expected_failure(abort_code = pool_proxy::ENotReduceOnlyOrder)] fun test_place_reduce_only_limit_order_not_reduce_only_quantity_ask() { let ( @@ -2155,6 +2239,174 @@ fun reduce_only_and_repay_bid_over_net_debt_fully_closes() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } +#[test] +fun reduce_only_limit_resting_fill_at_band_edge_bounded_and_solvent() { + // PoC for the "empty book + self-trade" concern: M1 (a short) rests a + // reduce-only bid at the +5% band edge ($1.05) — it only rests because the + // book is empty — and a counterparty M2 (a plain balance manager) takes it. + // M1 overpays 5% with NO margin re-check at the fill. We assert the leak is + // real (ratio drops) but bounded: M1 stays solvent (> 1.0), so its borrowed + // principal is still covered and it remains liquidatable. Repeating is + // impossible because a short can't ask (see + // test_place_reduce_only_limit_order_ask_requires_quote_debt), so the one-way + // direction guard caps total leak at one conversion. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + // No orderbook liquidity: the band-edge bid will rest (nothing to cross). + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + // M1 short: deposit 3000 USDT, borrow 1000 USDC, withdraw it -> holds 3000 + // USDT, owes 1000 USDC (ratio 3.0). + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(3000 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + + let rr_before = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + + // M1 rests a reduce-only-and-repay bid at $1.05 (the +5% band edge) to buy + // 1000 USDC. Empty book -> it rests (no taker fill, no repay at placement). + let order_info = test_helpers::place_reduce_only_limit_order_and_repay_loan_for_test< + USDC, + USDT, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 1_050_000_000, + 1000 * test_constants::usdc_multiplier(), + true, + false, + 2_000_000, + &clock, + ); + destroy(order_info); + destroy_2!(usdc_price, usdt_price); + + // Counterparty M2 (a plain balance manager) takes M1's resting bid at $1.05, + // selling 1000 USDC for 1050 USDT. M1 overpays 5%, with no margin re-check. + scenario.next_tx(test_constants::user2()); + let mut m2 = deepbook::balance_manager::new(scenario.ctx()); + m2.deposit( + mint_coin(2000 * test_constants::usdc_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + m2.deposit( + mint_coin(2000 * test_constants::usdt_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + m2.deposit( + mint_coin(10000 * test_constants::deep_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + let m2_proof = m2.generate_proof_as_owner(scenario.ctx()); + pool.place_limit_order( + &mut m2, + &m2_proof, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 1_050_000_000, + 1000 * test_constants::usdc_multiplier(), + false, + false, + constants::max_u64(), + &clock, + scenario.ctx(), + ); + sui::transfer::public_transfer(m2, test_constants::user2()); + + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + let rr_after = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + destroy_2!(usdc_price, usdt_price); + + // The ungated self-trade leaked value (ratio dropped)... + assert!(rr_after < rr_before); + // ...but it stays bounded: M1 remains solvent (> 1.0), borrowed principal + // covered, still liquidatable. The one-way guard stops M1 repeating it. + assert!(rr_after > constants::float_scaling()); + + return_shared_3!(mm, pool, base_pool); + return_shared(quote_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + #[test, expected_failure(abort_code = pool_proxy::EIncorrectDeepBookPool)] fun test_place_reduce_only_market_order_incorrect_pool() { let ( From f6ea961cef4a1d6eb10e07444a2a795970bdbbe4 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Thu, 25 Jun 2026 12:55:47 -0400 Subject: [PATCH 22/27] =?UTF-8?q?margin:=20adversarial=20suite=20=E2=80=94?= =?UTF-8?q?=20manager=20can't=20be=20driven=20under=20ratio=201.0=20w/o=20?= =?UTF-8?q?price=20move?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attacker-perspective tests for the 'manipulate a manager underwater via self-trades / empty book' vectors: - attack_resting_fill_from_danger_band_cannot_go_underwater: M1 drifted to *at liquidation* (1.10), a counterparty takes a maximal band-edge resting bid (ungated 5% leak) -> M1 lands at ~1.05, still > 1.0. The 5% band caps the per-conversion loss and the one-way direction guard prevents a second conversion, so it can't compound underwater. - attack_market_and_repay_empty_book_aborts: empty book -> no execution surface, aborts ENoLiquidityInOrderbook. - attack_resting_order_by_itself_is_ratio_neutral: placing a resting order with no counterparty doesn't move the manager's own ratio (locked balance counts). With the one-way guard tests (short can't ask / long can't bid) and the healthy-case resting-fill PoC, this establishes: absent real price movement, no sequence of self-trades or empty/sparse-book fills can push a manager below 1.0. Co-Authored-By: Claude Opus 4.8 --- .../tests/pool_proxy_tests.move | 383 ++++++++++++++++++ 1 file changed, 383 insertions(+) diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index 2fcce1f3f..257db932e 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -2407,6 +2407,389 @@ fun reduce_only_limit_resting_fill_at_band_edge_bounded_and_solvent() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } +#[test] +fun attack_resting_fill_from_danger_band_cannot_go_underwater() { + // Worst case for the self-trade vector: M1 starts *at liquidation* (1.10) via a + // price drift, then a counterparty M2 takes a maximal band-edge resting bid. + // Even this ungated, maximal one-shot conversion leaves M1 at ~1.05 — still + // **above 1.0** (solvent, liquidatable). It cannot be pushed underwater: the + // loss per conversion is capped by the 5% band, and the one-way direction + // guard (a short can't ask) means there's no second conversion to compound. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + // No liquidity: the band-edge bid will rest. + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + // M1 short: 2200 USDT, owe 1000 USDC (ratio 2.2 at $1, so withdraw is allowed). + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(2200 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + destroy(usdc_price); + + // Drift USDC up to $2.00 -> the short's ratio drops to 1.10 (at liquidation). + let usdc_drifted = build_demo_usdc_price_info_object_with_price( + &mut scenario, + 200_000_000, + &clock, + ); + pool_proxy::update_current_price( + &mut registry, + &pool, + &usdc_drifted, + &usdt_price, + &clock, + ); + + let rr_before = mm.risk_ratio( + ®istry, + &usdc_drifted, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + assert!(rr_before <= registry.liquidation_risk_ratio(pool.id()) + 1); + + // M1 rests a reduce-only bid at the +5% band edge ($2.10) for 1000 USDC, + // locking ~2100 USDT. Empty book -> rests. + let order_info = test_helpers::place_reduce_only_limit_order_and_repay_loan_for_test< + USDC, + USDT, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 2_100_000_000, + 1000 * test_constants::usdc_multiplier(), + true, + false, + 2_000_000, + &clock, + ); + destroy(order_info); + destroy(usdt_price); + + // Counterparty M2 takes the resting bid at $2.10 -> M1 overpays 5%, ungated. + scenario.next_tx(test_constants::user2()); + let mut m2 = deepbook::balance_manager::new(scenario.ctx()); + m2.deposit( + mint_coin(2000 * test_constants::usdc_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + m2.deposit( + mint_coin(2000 * test_constants::usdt_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + m2.deposit( + mint_coin(10000 * test_constants::deep_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + let m2_proof = m2.generate_proof_as_owner(scenario.ctx()); + pool.place_limit_order( + &mut m2, + &m2_proof, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 2_100_000_000, + 1000 * test_constants::usdc_multiplier(), + false, + false, + constants::max_u64(), + &clock, + scenario.ctx(), + ); + sui::transfer::public_transfer(m2, test_constants::user2()); + + let usdc_drifted2 = build_demo_usdc_price_info_object_with_price( + &mut scenario, + 200_000_000, + &clock, + ); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + let rr_after = mm.risk_ratio( + ®istry, + &usdc_drifted2, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + destroy(usdc_drifted); + destroy(usdc_drifted2); + destroy(usdt_price); + + // Leaked ~5% (ratio dropped below liquidation) but is NEVER under 1.0. + assert!(rr_after < rr_before); + assert!(rr_after > constants::float_scaling()); + + return_shared_3!(mm, pool, base_pool); + return_shared(quote_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + +#[test, expected_failure(abort_code = pool_proxy::ENoLiquidityInOrderbook)] +fun attack_market_and_repay_empty_book_aborts() { + // An empty book gives no execution surface: a market-and-repay aborts on + // ENoLiquidityInOrderbook (in calculate_effective_price) before any trade, so a + // manager can't be manipulated via a market order when the book is empty. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + // Deliberately no orderbook liquidity. + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(3000 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 100 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy_2!(usdc_price, usdt_price); + + // Market-and-repay against an empty book -> no liquidity, aborts. + let order_info = test_helpers::place_market_order_and_repay_loan_for_test( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::self_matching_allowed(), + 50 * test_constants::usdc_multiplier(), + true, + false, + &clock, + ); + destroy(order_info); + + abort 999 +} + +#[test] +fun attack_resting_order_by_itself_is_ratio_neutral() { + // "By itself" (no counterparty fill) a manager can't move its own ratio: + // placing a reduce-only resting order locks balance that still counts as + // assets, so the ratio is unchanged. Only a real fill can move it — and a + // taker fill is gated, while a self-match (same balance manager) is + // value-neutral. So a manager can't manipulate itself underwater. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(3000 * test_constants::usdt_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_base( + ®istry, + &mut base_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + // Withdraw the borrowed USDC so M1 is a real short (no idle USDC for the + // and-repay to sweep; the placement is then a pure resting order). + let withdrawn = mm.withdraw( + ®istry, + &base_pool, + "e_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdc_multiplier(), + &clock, + scenario.ctx(), + ); + destroy(withdrawn); + + let rr_before = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + + // Place a reduce-only resting bid; no counterparty, so it just rests. + let order_info = test_helpers::place_reduce_only_limit_order_and_repay_loan_for_test< + USDC, + USDT, + >( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 1_050_000_000, + 500 * test_constants::usdc_multiplier(), + true, + false, + 2_000_000, + &clock, + ); + destroy(order_info); + + let rr_after = mm.risk_ratio( + ®istry, + &usdc_price, + &usdt_price, + &pool, + &base_pool, + "e_pool, + &clock, + ); + destroy_2!(usdc_price, usdt_price); + + // Locked-in-order balance still counts as assets -> ratio unchanged. + assert_eq!(rr_after, rr_before); + + return_shared_3!(mm, pool, base_pool); + return_shared(quote_pool); + cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); +} + #[test, expected_failure(abort_code = pool_proxy::EIncorrectDeepBookPool)] fun test_place_reduce_only_market_order_incorrect_pool() { let ( From 414bccdc42d699cf3b74260f8ee7e81793e35c46 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Thu, 25 Jun 2026 13:04:12 -0400 Subject: [PATCH 23/27] margin: test the monotonic gate fires through place_market_order_and_repay_loan place_market_order_and_repay_loan_worsening_aborts: a long that spends nearly all its quote buying base at a +4% (in-band) premium increases exposure with too little leftover to repay, so the net post-repay ratio drops -> ERiskRatioMustNotWorsen. Covers the after-repay monotonic branch on the non-reduce-only entry (distinct from the v2 swap-only check). Co-Authored-By: Claude Opus 4.8 --- .../tests/pool_proxy_tests.move | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index 257db932e..af90f5511 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -7537,6 +7537,96 @@ fun place_market_order_and_repay_loan_overbuys_short_past_debt() { cleanup_margin_test(registry, _admin_cap, _maintainer_cap, clock, scenario); } +#[test, expected_failure(abort_code = pool_proxy::ERiskRatioMustNotWorsen)] +fun place_market_order_and_repay_loan_worsening_aborts() { + // The monotonic gate fires on this entry too (the after-repay check, distinct + // from the v2 swap-only check). A long that spends almost all its quote buying + // base at a +4% premium increases exposure with too little leftover quote to + // repay, so the net ratio drops -> abort. Confirms this entry cannot be used to + // worsen a position. + let ( + mut scenario, + clock, + _admin_cap, + _maintainer_cap, + base_pool_id, + quote_pool_id, + pool_id, + registry_id, + ) = setup_pool_proxy_test_env(); + + // Ask at $1.04 (within the 5% band) for the manager's bid to overpay into. + setup_orderbook_liquidity_at_prices( + &mut scenario, + pool_id, + 950_000_000, + 1_040_000_000, + &clock, + ); + + scenario.next_tx(test_constants::user1()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut registry = scenario.take_shared(); + let mut base_pool = scenario.take_shared_by_id>(base_pool_id); + let mut quote_pool = scenario.take_shared_by_id>(quote_pool_id); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let usdc_price = build_demo_usdc_price_info_object(&mut scenario, &clock); + let usdt_price = build_demo_usdt_price_info_object(&mut scenario, &clock); + + // M1 long: 1100 USDC collateral + 1000 USDT (quote) debt, holding the borrowed + // USDT (ratio ~2.1). + mm.deposit( + ®istry, + &usdc_price, + &usdt_price, + mint_coin(1100 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.borrow_quote( + ®istry, + &mut quote_pool, + &usdc_price, + &usdt_price, + &pool, + 1000 * test_constants::usdt_multiplier(), + &clock, + scenario.ctx(), + ); + destroy_2!(usdc_price, usdt_price); + + // Spend almost all the USDT buying USDC at the $1.04 premium -> exposure up, + // tiny repay, net ratio drops -> ERiskRatioMustNotWorsen. + let order_info = test_helpers::place_market_order_and_repay_loan_for_test( + &mut scenario, + ®istry, + &mut mm, + &mut pool, + &mut base_pool, + &mut quote_pool, + 1, + constants::self_matching_allowed(), + 950 * test_constants::usdc_multiplier(), + true, + false, + &clock, + ); + destroy(order_info); + + abort 999 +} + #[test, expected_failure(abort_code = pool_proxy::EPoolNotEnabledForMarginTrading)] fun place_market_order_and_repay_loan_aborts_when_pool_disabled() { // Unlike the reduce-only and-repay siblings, the non-reduce-only and-repay From 6d164f3e9b498b4df46498cb0538b4fa9f720f00 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Thu, 25 Jun 2026 13:06:36 -0400 Subject: [PATCH 24/27] docs(move.md): record the audited self-trade/empty-book safety model The ungated resting fill can't drive a manager underwater without real price movement: assert_price caps each fill at 5%, the direction guard makes each manager one-way (so the leak can't compound), and liquidate cancels open orders so locked collateral is recoverable. References the adversarial PoC suite. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index df12beb72..2ccc1a063 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -270,6 +270,8 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - **The monotonic gate is per-placement, not a cross-transaction invariant — and that's why the *direction* guard (not a size cap) protects the resting portion of a limit.** The gate runs **once, at placement**. A market order is 100% taker (check-time == final state — airtight, why `place_market_order_and_repay_loan` rides on monotonic alone). A crossing limit is taker (checked) **+** a resting maker remainder that is ratio-neutral at placement, so it passes invisibly and then fills **later, ungated and un-repaid** — the monotonic check guarantees *nothing* about that deferred fill. An earlier draft concluded "limits must keep a size cap on the resting remainder," but that's **superseded**: what actually keeps the deferred fill safe is the **direction guard**. A reduce-only bid is restricted to a *short* (`base_debt > 0`), and a short's resting bid — even one that over-covers — only converts quote→base, which is **price-invariant** (see above), so the ungated fill can't raise ratio-exposure. The dangerous case (a *long* placing a resting bid to grow its long) is blocked outright by the direction guard, not by a size bound. (Inherent maker risk remains: off-oracle resting fills can leak value, and because the monotonic check resets its baseline each placement, repeated cycles can *ratchet* the ratio down across txs — bounded by the `assert_price` band on the taker leg and the order TTL, and self-inflicted since order placement is owner-gated. This is true of any margin limit order, not specific to dropping the cap.) +- **Self-trade / empty-book safety (audited, with PoC tests).** Why the ungated resting fill above can't be weaponized to drive a manager underwater without real price movement: (1) `assert_price` caps any single fill at ±5% of oracle, so one conversion leaks at most ~5%; (2) the **direction guard makes each manager one-way** — a short can *only* bid (convert quote→base), a long can *only* ask (base→quote), and a manager holds at most one debt side, so after one conversion it can't trade back to compound the leak (`base_debt > 0` for bids / `quote_debt > 0` for asks; `test_place_reduce_only_limit_order_ask_requires_quote_debt` pins it). 5% off a ratio `>=` liquidation (1.10) still lands `> 1.0`, so the manager stays solvent and liquidatable. The attacker suite proves it: `attack_resting_fill_from_danger_band_cannot_go_underwater` (M1 *at* 1.10, maximal self-trade → ~1.05, still > 1.0), `reduce_only_limit_resting_fill_at_band_edge_bounded_and_solvent`, `attack_market_and_repay_empty_book_aborts` (empty book → `ENoLiquidityInOrderbook`, no execution surface), `attack_resting_order_by_itself_is_ratio_neutral` (no counterparty ⇒ no ratio change). And `margin_manager::liquidate` calls `cancel_all_orders` + `withdraw_settled_amounts`, so collateral parked in resting orders is reclaimable — locked orders can't block recovery. + - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. - The post-trade solvency floor for *opening* (risk-increasing) orders is a **separate** threshold from the borrow gate: `min_open_risk_ratio`, strictly between `liquidation` and `min_borrow`. Why they must differ: the borrow gate passes at *exactly* `min_borrow` (pre-trade), but any opening trade pays the spread, so the post-trade ratio lands just under `min_borrow` — if `assert_post_trade_solvent` reused `min_borrow`, a max-leverage open would *always* abort. `min_open` (default = midpoint of liquidation and min_borrow) gives the open room to absorb its own spread while staying above the liquidatable zone. It's a per-pool admin override stored as an optional dynamic field (the `MaxOrderTtlKey` pattern); the getter derives the midpoint default from the pool's current `liquidation`/`min_borrow` (so it tracks risk-param edits) and **ignores a stranded override** outside `(liquidation, min_borrow]` rather than returning an unsafe value (self-heal). Only the opening paths (`place_limit_order_v2`/`place_market_order_v2` → `assert_post_trade_solvent`) use `min_open`; the borrow gate and the v2 TP/SL post-fill check still use `min_borrow`. From 240fe7c64eef74485975758f57515eb76aba0955 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Thu, 25 Jun 2026 14:36:58 -0400 Subject: [PATCH 25/27] margin: enforce TPSL market-order price bounds on the actual executed fill The conditional-order price-bound pre-check simulates the market order against the whole book (get_quote_quantity_out), but execution uses the order's self_matching_option. With cancel_maker, execution removes the manager's own resting maker orders and fills deeper into worse liquidity, so a fill could settle outside the oracle bounds the pre-check approved. Add a post-execution check in place_triggered_orders (shared by the v2 and v3 executors): after a triggered MARKET order fills, re-verify the actual executed price (cumulative_quote / executed) is within the same oracle bounds, aborting EFillOutsidePriceBounds otherwise. Limit fills are already bounded by their own limit price. Regression test tpsl_cancel_maker_self_match_cannot_bypass_price_bounds reproduces the cancel_maker self-match case. Co-Authored-By: Claude Opus 4.8 --- .../sources/margin_manager.move | 22 ++ .../deepbook_margin/tests/tpsl_tests.move | 200 ++++++++++++++++++ 2 files changed, 222 insertions(+) diff --git a/packages/deepbook_margin/sources/margin_manager.move b/packages/deepbook_margin/sources/margin_manager.move index 7b470807e..d1a40a99e 100644 --- a/packages/deepbook_margin/sources/margin_manager.move +++ b/packages/deepbook_margin/sources/margin_manager.move @@ -67,6 +67,11 @@ const EInsufficientRiskRatioAfterTrade: u64 = 19; /// Deprecated v1 entry was called. Use the `_v2` variant which enforces a /// post-trade risk_ratio invariant. const EDeprecatedUseV2: u64 = 20; +/// A triggered conditional **market** order settled outside the oracle price +/// bounds. The pre-check simulates the fill against the whole book, but +/// `cancel_maker` can remove the manager's own resting orders and fill deeper +/// into worse liquidity, so we re-check the *actual* executed price. +const EFillOutsidePriceBounds: u64 = 21; // === Structs === /// Witness type for authorizing MarginManager to call protected features of the DeepBook @@ -1882,6 +1887,23 @@ fun place_triggered_orders( ); if (!pending_order.is_limit_order()) { + // Self-match-aware safety net: the price-bound pre-check above + // simulated the fill against the full book, but a market order + // with `cancel_maker` can cancel the manager's own resting orders + // and fill deeper into worse liquidity. Re-check the *actual* + // executed price so a self-match can't bypass the oracle bounds. + // (Limit fills are already bounded by their own limit price.) + let executed = order_info.executed_quantity(); + if (executed > 0) { + let actual_price = math::div( + order_info.cumulative_quote_quantity(), + executed, + ); + assert!( + (is_bid && actual_price <= upper_bound) || (!is_bid && actual_price >= lower_bound), + EFillOutsidePriceBounds, + ); + }; market_filled = true; }; order_infos.push_back(order_info); diff --git a/packages/deepbook_margin/tests/tpsl_tests.move b/packages/deepbook_margin/tests/tpsl_tests.move index 069bd3624..38a25f05b 100644 --- a/packages/deepbook_margin/tests/tpsl_tests.move +++ b/packages/deepbook_margin/tests/tpsl_tests.move @@ -3440,6 +3440,206 @@ fun setup_orderbook_liquidity_out_of_bounds( return_shared(pool); } +// Far-below counterparty liquidity: a single far-below bid at $1.00 (no ask, so the +// manager's own $1.80 bid won't cross it). +fun setup_far_below_bid( + scenario: &mut test_scenario::Scenario, + pool_id: ID, + clock: &sui::clock::Clock, +) { + use deepbook::balance_manager; + use token::deep::DEEP; + + scenario.next_tx(test_constants::user2()); + let mut pool = scenario.take_shared_by_id>(pool_id); + let mut bm = balance_manager::new(scenario.ctx()); + bm.deposit( + mint_coin(1000 * test_constants::usdc_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + bm.deposit( + mint_coin(10000 * test_constants::deep_multiplier(), scenario.ctx()), + scenario.ctx(), + ); + let proof = bm.generate_proof_as_owner(scenario.ctx()); + pool.place_limit_order( + &mut bm, + &proof, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 1_000_000, // $1.00, below the $1.71 fresh lower bound + 300 * test_constants::sui_multiplier(), + true, // is_bid + false, + constants::max_u64(), + clock, + scenario.ctx(), + ); + transfer::public_share_object(bm); + return_shared(pool); +} + +#[test, expected_failure(abort_code = margin_manager::EFillOutsidePriceBounds)] +fun tpsl_cancel_maker_self_match_cannot_bypass_price_bounds() { + // Regression for the self-match price-bound check. The manager's + // TPSL market sell uses `cancel_maker`. Its own safe bid at the fresh oracle + // price ($1.80) makes the pre-check (which simulates against the *full* book) + // see an in-bounds fill, but `cancel_maker` removes that bid during execution + // and the sell fills the far-below $1.00 bid. The post-execution + // actual-fill check catches the out-of-bounds settlement and aborts. + let ( + mut scenario, + clock, + admin_cap, + maintainer_cap, + usdc_pool_id, + sui_pool_id, + pool_id, + registry_id, + ) = setup_sui_usdc_deepbook_margin(); + + setup_far_below_bid(&mut scenario, pool_id, &clock); + + // Add-time registry price $2.00 (so the $1.90 stop-loss trigger sits below it; + // the price is dropped to a fresh $1.80 at execution). Create the manager. + scenario.next_tx(test_constants::user1()); + let mut margin_registry = scenario.take_shared(); + let pool = scenario.take_shared>(); + let sui_price = build_sui_price_info_object_with_price(&mut scenario, 200, &clock); + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + pool_proxy::update_current_price( + &mut margin_registry, + &pool, + &sui_price, + &usdc_price, + &clock, + ); + let deepbook_registry = scenario.take_shared_by_id(registry_id); + margin_manager::new( + &pool, + &deepbook_registry, + &mut margin_registry, + &clock, + scenario.ctx(), + ); + return_shared(deepbook_registry); + destroy_2!(sui_price, usdc_price); + return_shared(pool); + return_shared(margin_registry); + + // Manager deposits SUI + USDC, places its own safe bid at $1.80, and adds a + // cancel_maker stop-loss market sell triggering below $1.90. + scenario.next_tx(test_constants::user1()); + let mut mm = scenario.take_shared>(); + let mut pool = scenario.take_shared>(); + let margin_registry = scenario.take_shared(); + let sui_pool = scenario.take_shared_by_id>(sui_pool_id); + let usdc_pool = scenario.take_shared_by_id>(usdc_pool_id); + let sui_price = build_sui_price_info_object_with_price(&mut scenario, 200, &clock); + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + + mm.deposit( + &margin_registry, + &sui_price, + &usdc_price, + mint_coin(10000 * test_constants::sui_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + mm.deposit( + &margin_registry, + &sui_price, + &usdc_price, + mint_coin(1000 * test_constants::usdc_multiplier(), scenario.ctx()), + &clock, + scenario.ctx(), + ); + + let bid_info = test_helpers::place_limit_order_v2_for_test( + &mut scenario, + &margin_registry, + &sui_pool, + &usdc_pool, + &mut mm, + &mut pool, + 1, + constants::no_restriction(), + constants::self_matching_allowed(), + 1_800_000, // $1.80, within bounds, rests as the best bid + 150 * test_constants::sui_multiplier(), + true, + false, + constants::max_u64(), + &clock, + ); + destroy(bid_info); + + let condition = tpsl::new_condition(true, 1_900_000); // stop-loss: trigger below $1.90 + let pending_order = tpsl::new_pending_market_order( + 1, + constants::cancel_maker(), + 150 * test_constants::sui_multiplier(), + false, // market sell + false, + ); + mm.add_conditional_order( + &pool, + &sui_price, + &usdc_price, + &margin_registry, + 1, + condition, + pending_order, + &clock, + scenario.ctx(), + ); + + destroy_2!(sui_price, usdc_price); + return_shared(sui_pool); + return_shared(usdc_pool); + return_shared_2!(mm, pool); + return_shared(margin_registry); + + // Permissionless keeper triggers at fresh $1.80 (< $1.90). cancel_maker cancels + // the manager's $1.80 bid; the sell would settle at the far-below $1.00 bid -> + // the post-execution bound check aborts EFillOutsidePriceBounds. + scenario.next_tx(test_constants::user2()); + let sui_price_trigger = build_sui_price_info_object_with_price(&mut scenario, 180, &clock); + let usdc_price = build_usdc_price_info_object(&mut scenario, &clock); + let mut pool = scenario.take_shared>(); + let mut margin_registry = scenario.take_shared(); + let mut mm = scenario.take_shared>(); + let sui_pool = scenario.take_shared_by_id>(sui_pool_id); + let usdc_pool = scenario.take_shared_by_id>(usdc_pool_id); + + // Drop the registry price to a fresh $1.80 (aligned with the trigger): bounds + // become $1.71-$1.89 so the manager's own $1.80 bid reads as in-bounds. + pool_proxy::update_current_price( + &mut margin_registry, + &pool, + &sui_price_trigger, + &usdc_price, + &clock, + ); + + let order_infos = test_helpers::execute_conditional_orders_v2_for_test( + &mut scenario, + &sui_pool, + &usdc_pool, + &mut mm, + &mut pool, + &sui_price_trigger, + &usdc_price, + &margin_registry, + 10, + &clock, + ); + destroy(order_infos); + + abort 999 +} + #[test] fun test_tpsl_limit_order_price_below_lower_bound_cancelled() { // Test that a limit order with price below the lower bound gets cancelled From ed8793d5e4c9a2376ec89cdd4a426347e2714c72 Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Thu, 25 Jun 2026 14:36:58 -0400 Subject: [PATCH 26/27] margin: rename adversarial tests to neutral names Drop the attack_ prefix on the self-trade-bound regression tests. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 2 +- packages/deepbook_margin/tests/pool_proxy_tests.move | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index 2ccc1a063..2659c1c0d 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -270,7 +270,7 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - **The monotonic gate is per-placement, not a cross-transaction invariant — and that's why the *direction* guard (not a size cap) protects the resting portion of a limit.** The gate runs **once, at placement**. A market order is 100% taker (check-time == final state — airtight, why `place_market_order_and_repay_loan` rides on monotonic alone). A crossing limit is taker (checked) **+** a resting maker remainder that is ratio-neutral at placement, so it passes invisibly and then fills **later, ungated and un-repaid** — the monotonic check guarantees *nothing* about that deferred fill. An earlier draft concluded "limits must keep a size cap on the resting remainder," but that's **superseded**: what actually keeps the deferred fill safe is the **direction guard**. A reduce-only bid is restricted to a *short* (`base_debt > 0`), and a short's resting bid — even one that over-covers — only converts quote→base, which is **price-invariant** (see above), so the ungated fill can't raise ratio-exposure. The dangerous case (a *long* placing a resting bid to grow its long) is blocked outright by the direction guard, not by a size bound. (Inherent maker risk remains: off-oracle resting fills can leak value, and because the monotonic check resets its baseline each placement, repeated cycles can *ratchet* the ratio down across txs — bounded by the `assert_price` band on the taker leg and the order TTL, and self-inflicted since order placement is owner-gated. This is true of any margin limit order, not specific to dropping the cap.) -- **Self-trade / empty-book safety (audited, with PoC tests).** Why the ungated resting fill above can't be weaponized to drive a manager underwater without real price movement: (1) `assert_price` caps any single fill at ±5% of oracle, so one conversion leaks at most ~5%; (2) the **direction guard makes each manager one-way** — a short can *only* bid (convert quote→base), a long can *only* ask (base→quote), and a manager holds at most one debt side, so after one conversion it can't trade back to compound the leak (`base_debt > 0` for bids / `quote_debt > 0` for asks; `test_place_reduce_only_limit_order_ask_requires_quote_debt` pins it). 5% off a ratio `>=` liquidation (1.10) still lands `> 1.0`, so the manager stays solvent and liquidatable. The attacker suite proves it: `attack_resting_fill_from_danger_band_cannot_go_underwater` (M1 *at* 1.10, maximal self-trade → ~1.05, still > 1.0), `reduce_only_limit_resting_fill_at_band_edge_bounded_and_solvent`, `attack_market_and_repay_empty_book_aborts` (empty book → `ENoLiquidityInOrderbook`, no execution surface), `attack_resting_order_by_itself_is_ratio_neutral` (no counterparty ⇒ no ratio change). And `margin_manager::liquidate` calls `cancel_all_orders` + `withdraw_settled_amounts`, so collateral parked in resting orders is reclaimable — locked orders can't block recovery. +- **Self-trade / empty-book safety (audited, with PoC tests).** Why the ungated resting fill above can't be weaponized to drive a manager underwater without real price movement: (1) `assert_price` caps any single fill at ±5% of oracle, so one conversion leaks at most ~5%; (2) the **direction guard makes each manager one-way** — a short can *only* bid (convert quote→base), a long can *only* ask (base→quote), and a manager holds at most one debt side, so after one conversion it can't trade back to compound the leak (`base_debt > 0` for bids / `quote_debt > 0` for asks; `test_place_reduce_only_limit_order_ask_requires_quote_debt` pins it). 5% off a ratio `>=` liquidation (1.10) still lands `> 1.0`, so the manager stays solvent and liquidatable. The adversarial suite proves it: `resting_fill_from_danger_band_cannot_go_underwater` (M1 *at* 1.10, maximal self-trade → ~1.05, still > 1.0), `reduce_only_limit_resting_fill_at_band_edge_bounded_and_solvent`, `market_and_repay_empty_book_aborts` (empty book → `ENoLiquidityInOrderbook`, no execution surface), `resting_order_by_itself_is_ratio_neutral` (no counterparty ⇒ no ratio change). And `margin_manager::liquidate` calls `cancel_all_orders` + `withdraw_settled_amounts`, so collateral parked in resting orders is reclaimable — locked orders can't block recovery. - Risk-ratio config ordering (`calculate_risk_ratios` / `set_risk_params`): `liquidation_risk_ratio < min_borrow_risk_ratio < min_withdraw_risk_ratio`, and `liquidation_risk_ratio < target_liquidation_risk_ratio`, with `liquidation_risk_ratio >= float_scaling()`. Users in the `liquidation..min_borrow` "danger zone" cannot reach the borrow floor in a single swap; only an atomic deleveraging close (or liquidation) moves them up. diff --git a/packages/deepbook_margin/tests/pool_proxy_tests.move b/packages/deepbook_margin/tests/pool_proxy_tests.move index af90f5511..e609fff3e 100644 --- a/packages/deepbook_margin/tests/pool_proxy_tests.move +++ b/packages/deepbook_margin/tests/pool_proxy_tests.move @@ -2408,7 +2408,7 @@ fun reduce_only_limit_resting_fill_at_band_edge_bounded_and_solvent() { } #[test] -fun attack_resting_fill_from_danger_band_cannot_go_underwater() { +fun resting_fill_from_danger_band_cannot_go_underwater() { // Worst case for the self-trade vector: M1 starts *at liquidation* (1.10) via a // price drift, then a counterparty M2 takes a maximal band-edge resting bid. // Even this ungated, maximal one-shot conversion leaves M1 at ~1.05 — still @@ -2591,7 +2591,7 @@ fun attack_resting_fill_from_danger_band_cannot_go_underwater() { } #[test, expected_failure(abort_code = pool_proxy::ENoLiquidityInOrderbook)] -fun attack_market_and_repay_empty_book_aborts() { +fun market_and_repay_empty_book_aborts() { // An empty book gives no execution surface: a market-and-repay aborts on // ENoLiquidityInOrderbook (in calculate_effective_price) before any trade, so a // manager can't be manipulated via a market order when the book is empty. @@ -2668,7 +2668,7 @@ fun attack_market_and_repay_empty_book_aborts() { } #[test] -fun attack_resting_order_by_itself_is_ratio_neutral() { +fun resting_order_by_itself_is_ratio_neutral() { // "By itself" (no counterparty fill) a manager can't move its own ratio: // placing a reduce-only resting order locks balance that still counts as // assets, so the ratio is unchanged. Only a real fill can move it — and a From 714b3979a2ee450dc11feba407a6a45e18bd758d Mon Sep 17 00:00:00 2001 From: Tony Lee Date: Thu, 25 Jun 2026 14:41:20 -0400 Subject: [PATCH 27/27] docs(move.md): record the self-match pre-check vs realized-fill gotcha A pre-trade orderbook simulation (get_quote_quantity_out) is not binding under cancel_maker, which cancels the manager's own makers and fills deeper. Enforce price bounds on the realized fill (EFillOutsidePriceBounds), not the simulated one. Documents the TPSL fix's pattern + trade-off. Co-Authored-By: Claude Opus 4.8 --- .claude/rules/move.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/rules/move.md b/.claude/rules/move.md index 2659c1c0d..a80528825 100644 --- a/.claude/rules/move.md +++ b/.claude/rules/move.md @@ -278,6 +278,8 @@ Then call as `self.id.exists_(key)`, `self.id.add(key, value)`, `self.id.borrow( - TP/SL conditional execution that must fire in the danger band has to **deleverage**, not just trade — `execute_conditional_orders_v3` repays the loan with the market proceeds after each fill, then gates on net (post-repay) monotonic `risk_ratio`. The v2 executor (swap-only, `min_borrow` gate) can't fire a danger-band stop and leaves the order looping the bot; v3 fixes both. Two design constraints worth remembering: (1) the executor is **permissionless**, so to call `repay` there the owner check was moved *out* of internal `repay` into the public `repay_base`/`repay_quote` wrappers — repay is owner-neutral (only moves the manager's own funds against its own debt, can't extract value), the same reason `withdraw_without_owner_check` backs both liquidation and repay. (2) Deleverage needs the margin pools as `&mut`, but `execute_conditional_orders_v2` takes them `&` — changing a **public** param from `&T` to `&mut T` breaks Sui upgrade compatibility, so this shipped as a **new** `_v3` entry, not a v2 signature change (same reason v1→v2 added entries). General rule for this deployed package: prefer a new `_vN` entry over changing any existing public signature; private-fn bodies, new functions, and new dynamic fields are the compatible levers. +- **A pre-trade orderbook simulation is NOT binding under self-matching — enforce price bounds on the *realized* fill.** `place_triggered_orders`' price-bound check simulates a triggered market order via `pool.get_quote_quantity_out` / `get_quote_quantity_in`, which walks the **whole** book *including the manager's own resting maker orders*. But execution uses the order's `self_matching_option`: with `cancel_maker`, DeepBook cancels the manager's own makers first and the taker then fills **deeper into worse liquidity**, so the fill can settle outside the oracle bounds the simulation approved. DeepBook has **no self-match-aware quote** (`get_quote_quantity_out` takes no balance-manager param), so the fix is a **post-execution** check: after a triggered *market* order fills, re-verify the actual price `order_info.cumulative_quote_quantity() / executed_quantity()` against the same `(lower_bound, upper_bound)` and abort `EFillOutsidePriceBounds` (`margin_manager`, code 21). Limit fills are already bounded by their own limit price, so the check is market-only. Regression: `tpsl_cancel_maker_self_match_cannot_bypass_price_bounds`. General gotcha: any "simulate the fill, then place with `cancel_maker`" pattern has this pre-check/execution divergence — gate on the realized fill, not the simulated one. (Trade-off: the post-check **aborts** the conditional execution if a fill lands out of bounds, so a self-matching setup griefs the stop into staying pending — bounded, no fund loss, and the owner can cancel their own competing order; the alternative is to cancel the manager's own orders *before* the check so the simulation matches execution.) + ## Tool Calling Instructions - Use `sui move build --path packages/predict` from the repo root, or `sui move build` inside a package directory with `Move.toml`.