fix bug in _calculateSurplusAndCap() for debt redistribution case

rayeaster commented 1 year ago

The issue is found during Medusa invariant run and failure-analysis https://gist.github.com/GalloDaSballo/68046d8b089c4d29af40f31d197cc53a?permalink_comment_id=4714393#gistcomment-4714393

When a CDP is fully liquidated with some bad debt distributed (in recovery mode), there should be zero collateral as surplus for CDP owner to claim.

In previous implementation depending on Solidity multiplication and division, it might end with some rounding error that CDP ower got 1 wei collateral as surplus.

Now the issue is fixed by this PR

GalloDaSballo commented 1 year ago

Mostly trying to understand if:

Do we need to constantly ping back and forth between shares and coll?
Can we proof there's no reentrancy, hence we can just cache the math for stETH to avoid the 400+ gas per conversion back and forth (and is it worth it)

It may be worth getting more eyes on this specific change and this flow

GalloDaSballo commented 12 months ago

This merge will prob have conflicts since this doesn't use shares everywhere

dapp-whisperer commented 12 months ago

great, will merge it down and track this as an issue

ebtc-protocol / ebtc

fix bug in _calculateSurplusAndCap() for debt redistribution case #674