Reference: Model A loss-sharing for ARM withdrawal queue by clement-ux · Pull Request #219 · OriginProtocol/arm-oeth

clement-ux · 2026-05-05T14:35:43Z

Context

A recent audit finding ([OZ AI audit](/33e84d46f53c80b49635fb2609928432?pvs=25#34584d46f53c808a83acf270cb3ed2b4)) shows that the ARM withdrawal queue can become permanently unclaimable after a loss, because the FIFO gate is denominated in pre-loss assets while the payout is reduced by min(request.assets, convertToAssets(request.shares)). Before patching the bug, we need to step back and decide which loss-socialization model the ARM should follow. There are exactly two coherent options.

Model A — Pro-rata loss sharing

The redeemer's shares are escrowed (not burned) between request and claim. They remain part of totalSupply().
Any loss on ARM assets is absorbed proportionally by every share holder, queued or not.
Any gain accrued between request and claim is forfeited to the remaining LPs at claim time.
The payout at claim time is min(request.assets, convertToAssets(shares)): capped at the request-time snapshot, reduced if a loss happened.
No "first to request" advantage. No bank-run incentive on bad news. No free-timing-option for the redeemer (the cap removes any reason to delay).
Trade-off: the redeemer carries downside but keeps no upside during the wait.

Model B — Senior debt queue

requestRedeem locks in a fixed asset amount (request.assets).
That amount is paid in full at claim time, regardless of subsequent losses.
Losses are absorbed first by LPs still holding shares, then (if the loss is large enough) by the last requesters in the FIFO queue.
Simple semantics for users and integrators: "you will receive X."
Trade-off: any rumor of loss triggers a race to requestRedeem (bank run). Latecomers eat the loss in full.

The current ARM withdrawal queue uses a hybrid loss-handling model that mixes Model B accounting (burn at request, lock in request.assets) with a Model A-style payout reduction (min(request.assets, convertToAssets(request.shares))). This combination is the root cause of the audit finding "Withdrawal queue can become unclaimable after losses": the FIFO gate is denominated in pre-loss assets while the payout has been reduced, and withdrawsClaimed advances by the pre-loss amount — leaving a phantom liquidity gap that can permanently deadlock the queue.

Full discussion: https://www.notion.so/originprotocol/ARM-How-to-handle-losses-35784d46f53c80e18d70d78cab6c1d52

What this PR is

This PR is a reference implementation of Model A (pro-rata loss sharing via escrow + claim-time pricing). It is meant to make the alternative concrete so the team can compare it side by side with Model B before deciding.

Model B is the recommended fix (see the doc). This PR is not the proposed merge path — it exists to ground the discussion.

Behavioral changes

Losses (and gains) between request and claim are absorbed pro-rata across every share holder, queued or not.
No bank-run incentive on bad news: requesting redeem early no longer freezes a payout amount.
The redeemer no longer sees a fixed request.assets — that field becomes a request-time estimate only.
The FIFO deadlock from the audit finding disappears: the gate and the payout are denominated in the same unit at the same price.

Code changes (single file: `src/contracts/AbstractARM.sol`)

Storage semantics. withdrawsQueued, withdrawsClaimed, and WithdrawalRequest.queued are re-denominated from assets to shares. Storage layout preserved; meaning changed. ⚠ Upgrading a live deployment requires draining the queue first (non-zero pending values would be reinterpreted incorrectly).
requestRedeem escrows shares via _transfer(msg.sender, address(this), shares) instead of _burn. Shares stay in totalSupply() so the share price reflects all outstanding claims.
claimRedeem pays convertToAssets(request.shares) at claim time, burns the escrowed shares right after, and increments withdrawsClaimed by request.shares.
FIFO gate is now convertToAssets(pendingShares) <= claimable(). Same unit, same price, no phantom liquidity gap.
claimable() becomes a liquidity snapshot: balance + maxWithdraw(activeMarket). The cumulative withdrawsClaimed term is gone.
_availableAssets() returns gross assets (no subtraction for outstanding withdrawals). Outstanding shares are now part of totalSupply(), so the share price already accounts for them.
Liquidity reservation is dynamic: _requireLiquidityAvailable, _ensureLiquidityAvailableForSwap, getReserves, _allocate compute reserved liquidity as convertToAssets(outstandingShares) at the current share price.

Trade-off vs Model B

The main reason not to merge this PR is gas:

_ensureLiquidityAvailableForSwap now calls convertToAssets → totalAssets() → _availableAssets() → on a Morpho-backed ARM, IERC4626(morpho).convertToAssets(allShares).
Morpho's convertToAssets is expensive (full market state read, interest accrual, several storage slots).
Every buy-side swap with a non-empty queue pays this cost. The prior path was zero external calls beyond balanceOf.

For an ARM whose value proposition is cheap swaps, this is a meaningful recurring overhead.

Tests

Tests are intentionally not updated. Many will fail because:

withdrawsQueued/withdrawsClaimed/request.queued change unit (asset → share)
The RedeemRequested event's queued field changes unit
Payout behavior changes (pro-rata at claim time instead of min)

If we ever take this direction, tests would be updated in a follow-up.

Status

Reference / discussion. Do not merge. The proposed fix for the audit finding is the minimal Model B change (drop the min(...) in claimRedeem so the FIFO gate and the payout become consistent again).

Reference implementation of pro-rata loss sharing as discussed in the "ARM: How to handle losses" doc. Demonstrates an alternative to the current hybrid (which is broken — see audit finding on the FIFO deadlock after losses). Storage layout is preserved but the semantics of three fields change from asset-denominated to share-denominated: withdrawsQueued, withdrawsClaimed, and WithdrawalRequest.queued. An upgrade would only be safe with a fully drained queue. - requestRedeem escrows shares (_transfer to the contract) instead of burning them. Queued shares stay in totalSupply() so the share price reflects all outstanding claims. - claimRedeem pays convertToAssets(request.shares) at claim time, burns the escrowed shares, and increments withdrawsClaimed by shares. Gains and losses between request and claim are absorbed pro-rata. - FIFO gate compares convertToAssets(pendingShares) to claimable(), keeping the gate and the payout in the same unit and at the same price. Removes the phantom-liquidity deadlock. - claimable() becomes a snapshot (balance + maxWithdraw), without the cumulative withdrawsClaimed term. - _availableAssets() is now gross; the queued shares are already in totalSupply, so no subtraction. - _requireLiquidityAvailable, _ensureLiquidityAvailableForSwap, getReserves, _allocate compute reserved liquidity dynamically as convertToAssets(outstandingShares). Tests are not updated in this commit — many will fail (queue unit and event semantics changed, payout behavior differs).

In the previous Model A draft, escrowed shares stayed in totalSupply() during the wait, which gave the redeemer full upside until they chose to claim. That created a free timing option: the redeemer could park their request indefinitely and claim at the most favorable share price. Cap the payout at request.assets (snapshotted at request time) while still allowing the loss-adjusted current value to flow through when it is lower. Net effect on the redeemer: - Loss case: payout = convertToAssets(shares) at claim time. Loss is absorbed pro-rata exactly like before. - Gain case: payout = request.assets. Upside accrued during the wait is redistributed to the remaining LPs (early-bird-style penalty). The FIFO gate still uses convertToAssets(pendingShares) and is now conservative on the gain side: it overestimates the liquidity needed (actual payouts are capped) but never causes a deadlock.

The previous Model A draft made every buy-side swap call convertToAssets() which on a Morpho-backed ARM cascades into Morpho.convertToAssets() — an expensive market-wide read that defeated the cheap-swap value prop. Add a parallel pair of cumulative counters in assets units: - withdrawsQueuedAssets += request.assets at requestRedeem - withdrawsClaimedAssets += request.assets at claimRedeem (incremented by request.assets, NOT by the actual capped payout, so that the difference stays an upper bound on the queue's liability) The swap-side reservation (_ensureLiquidityAvailableForSwap, _requireLiquidityAvailable, getReserves, _allocate) now uses the cheap direct subtraction: reservedAssets = withdrawsQueuedAssets - withdrawsClaimedAssets This is identical in cost to Model B (no external calls beyond balanceOf) and correct because each payout is min(request.assets, convertToAssets(shares)) ≤ request.assets by construction. The reservation is a true upper bound, slightly conservative in loss scenarios but never under-reserves. The FIFO gate at claim time still uses convertToAssets(pendingShares) (shares-denominated) to keep the audit deadlock fixed. That call only runs on the rare claim path, not on every swap. Storage cost: 1 extra slot (two uint128 packed). _gap reduced from 35 to 34.

… ones Naming alignment with the original deployed contract: withdrawsQueued and withdrawsClaimed keep their original asset-denominated semantics (the upper-bound used by the cheap swap-side reservation). The shares-denominated counters added for the FIFO gate get the explicit suffix: withdrawsQueuedShares, withdrawsClaimedShares. Storage order also swapped so the original two slots stay at their deployed positions (assets), with the new shares slots placed after feesAccrued. Makes the upgrade path simpler: the legacy slots keep their meaning on a deployed contract, only the new slots and the WithdrawalRequest.queued reinterpretation require queue drainage. No behavior change.

Roll back rewordings, doc-comment expansions, and variable renames (outstandingWithdrawals, etc.) that didn't reflect any behavior change. Storage slots, function bodies, and the model semantics are unchanged. The diff vs the parent branch is now restricted to the actual changes required by the loss-sharing model: - new withdrawsQueuedShares / withdrawsClaimedShares storage - WithdrawalRequest.queued reinterpretation (assets -> shares) - requestRedeem escrows instead of burning - claimRedeem FIFO gate uses convertToAssets(pendingShares) - claimRedeem caps payout at request.assets - claimable() drops the cumulative withdrawsClaimed term - _availableAssets() returns gross (no subtraction for the queue) - _allocate() subtracts outstandingWithdrawals locally for the buffer No behavior change vs the previous commit.

naddison36 · 2026-05-06T10:37:11Z


-        // The amount of liquidity assets, eg WETH, that is still to be claimed in the withdrawal queue
+        // The amount of liquidity assets, eg WETH, reserved for the withdrawal queue (upper bound)
        outstandingWithdrawals = withdrawsQueued - withdrawsClaimed;


I think we can also remove outstandingWithdrawals from _availableAssets's return params.
outstandingWithdrawals can be calculated in _allocate instead

Yes, completely agree with you 💯

naddison36 · 2026-05-06T10:48:34Z

+    /// Compared in `claimRedeem` against `convertToAssets(pendingShares)`.
    function claimable() public view returns (uint256 claimableAmount) {
-        claimableAmount = withdrawsClaimed + IERC20(liquidityAsset).balanceOf(address(this));
+        claimableAmount = IERC20(liquidityAsset).balanceOf(address(this));


this is a functional change to how claimable works. it was a way to work out if a request could be claimed.

…ment/model-a-loss-sharing

naddison36 · 2026-05-08T06:15:19Z

    uint256 public crossPrice;

    /// @notice Cumulative total of all withdrawal requests including the ones that have already been claimed.
    uint128 public withdrawsQueued;


I think withdrawsQueued and withdrawsClaimed could be collapsed into reservedWithdrawLiquidity with the addition of withdrawsQueuedShares and withdrawsClaimedShares

I like the idea yes, but I don't know how to manage this:

// Cumulative assets queued (upper bound on liability), used for the swap-side reservation withdrawsQueued = SafeCast.toUint128(withdrawsQueued + assets);

naddison36 · 2026-05-14T06:32:33Z

Closing in preference for #223

clement-ux marked this pull request as draft May 5, 2026 14:36

clement-ux added 4 commits May 6, 2026 09:49

naddison36 reviewed May 6, 2026

View reviewed changes

Merge remote-tracking branch 'origin/nicka/withdraw-on-swap' into cle…

a795b76

…ment/model-a-loss-sharing

naddison36 reviewed May 8, 2026

View reviewed changes

naddison36 closed this May 14, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reference: Model A loss-sharing for ARM withdrawal queue#219

Reference: Model A loss-sharing for ARM withdrawal queue#219
clement-ux wants to merge 6 commits into
nicka/withdraw-on-swapfrom
clement/model-a-loss-sharing

clement-ux commented May 5, 2026 •

edited by naddison36

Loading

Uh oh!

naddison36 May 6, 2026

Uh oh!

clement-ux May 8, 2026

Uh oh!

naddison36 May 6, 2026

Uh oh!

naddison36 May 8, 2026

Uh oh!

clement-ux May 8, 2026

Uh oh!

naddison36 commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

clement-ux commented May 5, 2026 • edited by naddison36 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Model A — Pro-rata loss sharing

Model B — Senior debt queue

What this PR is

Behavioral changes

Code changes (single file: src/contracts/AbstractARM.sol)

Trade-off vs Model B

Tests

Status

Uh oh!

naddison36 May 6, 2026

Choose a reason for hiding this comment

Uh oh!

clement-ux May 8, 2026

Choose a reason for hiding this comment

Uh oh!

naddison36 May 6, 2026

Choose a reason for hiding this comment

Uh oh!

naddison36 May 8, 2026

Choose a reason for hiding this comment

Uh oh!

clement-ux May 8, 2026

Choose a reason for hiding this comment

Uh oh!

naddison36 commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clement-ux commented May 5, 2026 •

edited by naddison36

Loading

Code changes (single file: `src/contracts/AbstractARM.sol`)