Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPB bankruptcy on settle #675

Merged
merged 1 commit into from
Mar 9, 2023
Merged

Conversation

grandizzy
Copy link
Contributor

@grandizzy grandizzy commented Mar 9, 2023

  • if there's only a tiny amount of deposit (2) backed by 2 LPs in HPB then the collateralUsed to settle is calculated as 0 due to rounding (vars.collateralUsed = Maths.wdiv(vars.scaledDeposit, vars.price))
  • this results in removing deposit (2) and not adding any collateral into bucket, leaving an amount of 2 LPs that is not backed by any asset
  • fixed by declaring bucket bankruptcy in case settle leaves bucket with 0 collateral, 0 deposit but LPs > 0
  • added check for deposit to remove to be min of available deposit in bucket / calculated deposit for settle in order to prevent any potential underflow
  • unit test

- if there's only a tiny amount of deposit (2) backed by 2 LPs in HPB then the collateralUsed to settle is calculated as 0 due to rounding (vars.collateralUsed = Maths.wdiv(vars.scaledDeposit, vars.price))
- this results in removing deposit (2) and not adding any collateral into bucket, leaving an amount of 2 LPs that is not backed by any asset
- fixed by declaring bucket bankruptcy in case settle leaves bucket with 0 collateral, 0 deposit but LPs > 0
- added check for deposit to remove to be min of available deposit in bucket / calculated deposit for settle in order to prevent any potential underflow
- unit test
Copy link
Contributor

@mattcushman mattcushman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@EdNoepel EdNoepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

uint256 depositToRemove; // [WAD] deposit used by settle auction
uint256 hpbCollateral; // [WAD] amount of collateral in HPB bucket
uint256 hpbUnscaledDeposit; // [WAD] unscaled amount of of quote tokens in HPB bucket before settle
uint256 hpbLPs; // [WAD] amount of LPs in HPB bucket
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: 3 new local vars lines 111-113

vars.hpbCollateral = hpb.collateral + vars.collateralUsed;

// set amount to remove as min of calculated amount and available deposit (to prevent rounding issues)
vars.unscaledDeposit = Maths.min(vars.hpbUnscaledDeposit, vars.unscaledDeposit);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: alignment of Maths.min call


Deposits.unscaledRemove(deposits_, vars.index, vars.unscaledDeposit); // remove amount to settle debt from bucket (could be entire deposit or only the settled debt)
}
Bucket storage hpb = buckets_[vars.index];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate naming this hpb and not settlementBucket. I reviewed and confirmed this is always the HPB; the user cannot specify the index.

@grandizzy grandizzy merged commit 55b5508 into develop Mar 9, 2023
@grandizzy grandizzy deleted the fix-settle-no-collateral-used branch March 9, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants