margin: address review — owner-gate test, surplus assert, comment fixes

- 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 <noreply@anthropic.com>

Author: tonylee08

packages/deepbook_margin/sources/margin_manager.move

@@ -2059,7 +2059,10 @@ fun process_collected_orders_v3<BaseAsset, QuoteAsset>(
 
     // 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<ID>`, 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<BaseAsset, QuoteAsset, BaseAsset>(

packages/deepbook_margin/sources/pool_proxy.move

@@ -999,9 +999,13 @@ public fun claim_rebates<BaseAsset, QuoteAsset>(
 
 /// 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<BaseAsset, QuoteAsset>(
     pool: &Pool<BaseAsset, QuoteAsset>,
     net_debt: u64,

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<MarginRegistry>();
+    let pool = scenario.take_shared<Pool<USDT, USDC>>();
+    let deepbook_registry = scenario.take_shared_by_id<Registry>(registry_id);
+    margin_manager::new<USDT, USDC>(
+        &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<MarginManager<USDT, USDC>>();
+    let mut usdt_pool = scenario.take_shared_by_id<MarginPool<USDT>>(usdt_pool_id);
+
+    mm.repay_base<USDT, USDC>(
+        &registry,
+        &mut usdt_pool,
+        option::none(),
+        &clock,
+        scenario.ctx(),
+    );
+
+    abort 999
+}
+
 #[test]
 fun test_repay_full_with_none() {
     let (

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);