fix(stake): #259 — RecoverFlushedInsurance undercounts realized junior loss - #262
Conversation
…cccrypto#259) process_return_insurance (admin) computes physical outstanding insurance as (total_flushed - total_returned) + realized_junior_loss, because the last-junior-exit settlement in process_withdraw adds the forfeited amount to total_returned as bookkeeping only -- no wrapper token movement -- and records it in realized_junior_loss so total_pool_value keeps treating it as dead/unclaimable. process_recover_flushed_insurance (the permissionless post-burn path) used only total_flushed - total_returned, omitting realized_junior_loss. In the fully-realized case this understates recoverable capacity down to zero, even though the tokens are still sitting in the wrapper's insurance vault -- exactly the scenario where the permissionless path is the only recovery mechanism left, since asset_admin has already been burned. Add StakePool::physical_insurance_outstanding() as the single shared formula and use it in both process_return_insurance and process_recover_flushed_insurance, so the two recovery paths can't diverge again. When realized_junior_loss is 0 (the common case with no tranche loss), this is byte-for-byte the same as the old formula -- confirmed against the v17 e2e happy-path test, which never sets it.
|
Warning Review limit reached
More reviews will be available in 52 minutes and 13 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Problem
process_return_insurance(admin path) computes physical outstanding insurance as(total_flushed - total_returned) + realized_junior_loss. That+ realized_junior_lossterm exists because when the last junior LP exits during an outstanding loss,process_withdrawsettles the forfeited portion by adding it tototal_returnedas a bookkeeping-only entry (no wrapper token movement) and records it inrealized_junior_losssototal_pool_value()keeps treating it as dead/unclaimable.process_recover_flushed_insurance(the permissionless, post-asset-admin-burn recovery path) used onlytotal_flushed - total_returned, omittingrealized_junior_loss. In the fully-realized case this understates recoverable capacity all the way down to zero — even though the tokens are still physically sitting in the wrapper's insurance vault — and this is exactly the scenario where the permissionless path is the only recovery mechanism left, sinceasset_adminhas already been burned and the admin path may no longer be reachable.Example from the issue:
total_flushed=400, total_returned=400, realized_junior_loss=400→ admin path's formula says400outstanding; permissionless path said0.Fix
Added
StakePool::physical_insurance_outstanding()as the single shared formula and used it in bothprocess_return_insuranceandprocess_recover_flushed_insurance, so the two recovery paths can't diverge again. Whenrealized_junior_loss == 0(no tranche loss has ever been realized — the common case), this is byte-for-byte identical to the old formula, confirmed against the v17 e2e happy-path test for this instruction, which never setsrealized_junior_loss.Note on scope: this fix makes the permissionless path consistent with the admin path's already-shipped formula — it does not change what happens to
total_pool_value()/senior claimability once tokens are recovered, which is a pre-existing property of the admin path. That deeper economic question (issue remediation #2: should realized-loss tokens be physically recoverable at all, and who can then claim them) is unchanged by this PR.Proof of Fix
tests/poc_recover_flushed_insurance_undercounts_realized_junior_loss.rsreproduce both examples from the issue (fully-realized and partially-realized loss) and confirmphysical_insurance_outstanding()matches the admin path's formula in both cases.state.rs(no shared helper) fails to compileprocess_recover_flushed_insurance's call site, since the method didn't exist — the two formulas were genuinely separate code before this change.cargo test --lib(142 tests) and all non-SBF-dependent integration/proptest files (197 more tests, 339 total) pass with no regressions..soviacargo build-sbf, unavailable in this sandbox — pre-existing environment limitation, not caused by this change.Related
Closes #259