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

fix(vaults): when reconstituting vaults, report correct shortfall #7752

Merged
merged 2 commits into from
May 16, 2023

Conversation

Chris-Hibbert
Copy link
Contributor

fixes: #7474

Description

We weren't reducing the reported shortfall by the amount of debt being restored to some vaults.

Security Considerations

None.

Scaling Considerations

Not an issue.

Documentation Considerations

N/A

Testing Considerations

Corrected a test.

@Chris-Hibbert Chris-Hibbert added the Vaults VaultFactor (née Treasury) label May 16, 2023
@Chris-Hibbert Chris-Hibbert added this to the Vaults Validation milestone May 16, 2023
@Chris-Hibbert Chris-Hibbert requested a review from turadg May 16, 2023 04:07
@Chris-Hibbert Chris-Hibbert self-assigned this May 16, 2023
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

Fix LGTM. There's a lint error I made a commit for. Also I was thinking about how to have prevented this and noticed that recordShortfallAndProceeds is supposed to be called whenever it's done liquidating, so I moved the work into markDoneLiquidating.

I pushed for expediency. It has a fixup commit so still requires a rebase to merge.

facets.helper.sendToReserve(collatRemaining, liqSeat);
facets.helper.markDoneLiquidating(totalDebt, totalCollateral);
facets.helper.recordShortfallAndProceeds({
overage: accounting.overage,
Copy link
Member

Choose a reason for hiding this comment

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

without knowing the types, reviewing in web UI it looks like this could be missing some properties.

Suggested change
overage: accounting.overage,
...accounting,

Chris-Hibbert and others added 2 commits May 16, 2023 10:22
fixes: #7474

We weren't reducing the reported shortfall by the amount of debt being
restored to some vaults.
@turadg turadg enabled auto-merge May 16, 2023 17:23
@turadg turadg added this pull request to the merge queue May 16, 2023
Merged via the queue into master with commit e0c6280 May 16, 2023
@turadg turadg deleted the 7474-doubleReserve branch May 16, 2023 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Vaults VaultFactor (née Treasury)
Projects
None yet
2 participants