feat: add Slush DeFi Quickstart Provider API for margin lending

[0xaslan]

Summary

• Implements the Slush DeFi Quickstart Provider API (OpenAPI v1.1.0) as a new /slush/v1/ route module in deepbook-server, exposing DeepBook margin lending pools as LENDING strategies
• Reuses existing margin_metrics infrastructure (margin_pool_snapshots table) for pool state data (TVL, utilization, depositor counts) instead of making duplicate RPC calls
• Fetches APY from the Abyss Protocol vault indexer endpoint with 5-minute in-memory caching
• Correctly constructs deposit and withdraw PTBs including TransferObjects commands for newly-created SupplierCap and returned Coin objects
• Returns 501 Not Implemented for withdraw/cancel since DeepBook margin withdrawals are instant (no hold period)
• All configuration is env-driven: MARGIN_REGISTRY_ID for the registry shared object, SLUSH_VAULT_MAPPING (JSON) for pool-to-Abyss vault address mapping

Endpoints

Method	Path	Description
GET	/slush/v1/version	Returns spec version 1.1.0
GET	/slush/v1/provider	Provider metadata with aggregate TVL
GET	/slush/v1/strategies	List all margin pool strategies
GET	/slush/v1/strategies/{id}	Get single strategy details
GET	/slush/v1/positions?address=	List positions for a wallet
GET	/slush/v1/positions/{id}	Get single position
POST	/slush/v1/deposit	Build deposit transaction-kind bytes
POST	/slush/v1/withdraw	Build withdraw transaction-kind bytes
POST	/slush/v1/withdraw/cancel	Returns 501 (instant withdrawals)

PTB Fixes (vs previous attempt)

• Deposit: mint_supplier_cap now correctly passes registry then clock arguments
• Deposit: TransferObjects added to transfer the minted SupplierCap to sender
• Withdraw: TransferObjects added to transfer the returned Coin<Asset> to sender

Test plan

• cargo build -p deepbook-server compiles cleanly
• cargo fmt --check passes
• Verify /slush/v1/version returns {"version":"1.1.0"}
• Verify /slush/v1/provider returns provider metadata with aggregated TVL
• Verify /slush/v1/strategies returns strategies with correct APY from Abyss
• Verify /slush/v1/deposit returns valid base64-encoded transaction-kind bytes
• Verify /slush/v1/withdraw returns valid base64-encoded transaction-kind bytes
• Verify /slush/v1/withdraw/cancel returns 501 with {"_tag":"NotImplementedError"}
• Run oasdiff breaking against the OpenAPI spec to confirm zero breaking changes
• Deploy to staging and verify Slush wallet integration end-to-end

🤖 Generated with Claude Code

[0xaslan]

@claude review this

[claude]

test comment to verify gh works

[claude]

Code Review: feat/slush-defi-quickstart-api

Thanks for implementing the Slush DeFi Quickstart Provider API. The overall structure is clean and the PTB fixes look correct. Here are my findings organized by severity.

---

Critical Issues

1. Silent amount parse failure in withdraw handler - potential fund-loss footgun

In handlers.rs, if principal.amount fails to parse (e.g. a decimal like "1.5", a negative, or out-of-range value), this silently becomes None, meaning withdraw everything. Should return a 422 error on failure, not silently promote to a full withdrawal.

2. Integer overflow risk in build_positions

Both the summation and subtraction of i64 amounts can overflow silently in release builds. Use saturating_add/saturating_sub or checked_* variants instead of .sum() and -.

---

Major Issues

3. N+1 queries in build_strategies

count_active_depositors is called inside the pool loop - one DB round-trip per pool. This should be rewritten as a single aggregated query returning counts for all pools at once, similar to how get_latest_margin_pool_snapshots handles multi-pool state.

4. APY cache stampede

In apy.rs, the read lock is dropped before the write lock is acquired. During the gap, N concurrent requests can all observe a cache miss and all make outbound calls to the Abyss API in parallel. Consider a tokio::sync::Mutex-protected in-flight set to deduplicate concurrent fetches for the same vault address.

5. tvl_usd is not in USD - it is token units

Dividing the raw on-chain amount by decimals gives a token quantity, not a USD value. This is correct only for stablecoins with a 1:1 USD peg. For other assets the value will be wrong. Either rename the field to tvl_native or integrate a price oracle.

6. OR-based join in get_all_margin_pools prevents index usage

The three-way OR on asset_type prevents the query planner from using an index. Consider normalizing asset types at ingest time so a simple equality join works.

7. Missing input validation on public endpoints

• GET /positions?address=: Any string is accepted. Validate Sui address format before querying the DB.
• POST /deposit: coin_type from the request body is never validated against the pool's actual asset type - a mismatch goes undetected.
• DepositRequest.type_field and WithdrawRequest.mode are deserialized but never used - dead code.

---

Moderate Issues

8. Silent config parsing failure for SLUSH_VAULT_MAPPING

Invalid JSON silently becomes an empty map - all APY values will be 0.0 with no log output. Add a tracing::warn! (or return a startup error) when the env var is set but unparseable.

9. get_position makes redundant DB work

The handler fetches the supplier cap to find the owner address, then calls build_positions which loads all positions for that owner, then filters to the requested one. A direct lookup by cap ID would be significantly more efficient for owners with many positions.

10. Slush routes and rate limiting

Verify that the Slush sub-router is covered by the existing governor-based rate limiter. The /strategies and /positions endpoints trigger outbound HTTP calls to the Abyss API and should be protected.

11. reqwest pulls in native-tls unnecessarily

Without default-features = false, this brings in native-tls and core-foundation (visible in the Cargo.lock diff). If the workspace already uses rustls, prefer adding default-features = false and specifying rustls-tls explicitly.

---

Minor / Nits

• APY averages all identical: avg_24h, avg_7d, avg_30d all equal current_apy. Fine as a placeholder but worth a follow-up issue.
• volume_24h_usd: 0.0 hardcoded: Add a TODO comment so it is not forgotten.
• get_supplier_caps_by_cap_id returns Vec but callers always use .first(): Returning Result<Option> with a LIMIT 1 would be cleaner.
• ORDER BY amount DESC in get_supply_events_for_cap: Ordering by amount is semantically wrong for computing net supply. Consider ordering by timestamp or removing the clause.

---

Missing Tests

There are no unit tests for the PTB building logic (ptb.rs) or the handler helpers. The PTB builders are pure functions and would be straightforward to test - verifying the serialized bytes deserialize to the expected commands/arguments would prevent regressions like the PR#870 bugs this PR fixes.

---

Summary

The module structure is clean, the PTB fixes are correct, and the API shape follows the OpenAPI spec well. Top items to address before merging:

1. Silent full-withdrawal on parse failure (critical - user funds)
2. Integer overflow in position accounting (critical - correctness)
3. N+1 DB queries in build_strategies (performance)
4. TVL not in USD (API contract correctness)
5. Silent invalid JSON config (ops footgun)
6. Missing input validation on address/coin_type fields (correctness + security)

[0xaslan]

Acknowledged — this was a test comment.

[0xaslan]

Thanks for the thorough review! I've pushed a commit addressing the items that can be fixed directly. Here's a summary of what was done and what's deferred:

Fixed in this commit

1. Silent amount parse failure in withdraw handler (Critical) ✅
The withdraw handler now returns a 422 error when principal.amount is not a valid u64, instead of silently treating it as "withdraw all". Empty string is the explicit way to request a full withdrawal.

2. Integer overflow in build_positions (Critical) ✅
Replaced .sum() with .fold(0i64, |acc, x| acc.saturating_add(x)) and the subtraction with saturating_sub() to prevent silent overflow in release builds.

8. Silent config parsing failure (Moderate) ✅
Added tracing::warn! when SLUSH_VAULT_MAPPING is set but fails to parse as JSON, so operators get a clear log message instead of silent APY degradation.

5. TVL not in USD (Major) ✅ (documented)
Added TODO comments in both provider() and build_strategies() explaining this computes token quantity, not USD, and is only correct for stablecoins. The field name is kept as tvl_usd because the Slush Quickstart API spec requires it. A price oracle integration is the proper fix.

APY averages / volume_24h_usd (Nits) ✅
Added TODO comments for the APY rolling average placeholders and the hardcoded volume_24h_usd: 0.0.

Acknowledged — will address in follow-up PRs

3. N+1 queries in build_strategies — Agreed this should be a single aggregated query. Will batch the count_active_depositors calls into one query returning counts for all pools.

4. APY cache stampede — Good catch on the read→write lock gap. Will add a tokio::sync::Mutex-based in-flight deduplication set.

6. OR-based join in get_all_margin_pools — The three-way OR does prevent index usage. Will normalize asset types at ingest time.

7. Missing input validation — Will add Sui address format validation on /positions?address= and coin_type validation on /deposit. The unused type_field and mode fields are part of the Slush API contract (they're sent by the client) but agree we should validate/use them or explicitly ignore with _.

9. get_position redundant DB work — Agree a direct lookup by cap ID would be more efficient. Will refactor.

10. Rate limiting coverage — Will verify the Slush sub-router is covered by the governor rate limiter.

11. reqwest native-tls — Will add default-features = false and use rustls-tls to match the workspace.

Missing tests — Agree the PTB builders are pure functions and straightforward to test. Will add unit tests in a follow-up.

[0xaslan]

1. make sure all inputs / outputs match the criteria defined by the spec
2. address your follow ups in this PR