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

CS Note 8.3: Delegations Are Deleted on Liquidation #497

Open
bingen opened this issue Oct 9, 2024 · 1 comment
Open

CS Note 8.3: Delegations Are Deleted on Liquidation #497

bingen opened this issue Oct 9, 2024 · 1 comment
Assignees
Labels
ChainSecurity wontfix This will not be worked on

Comments

@bingen
Copy link
Collaborator

bingen commented Oct 9, 2024

Liquidations return surplus collateral to the trove owner. However, only the owner of the trove can reclaim the surplus. Delegated accounts cannot.

Note that liquidations trigger the BorrowerOperations.onLiquidateTrove hook, so all delegations for that trove ID are deleted on liquidation.

If the trove owner is a smart contract, it must implement the function to claim collateral. It cannot rely on another contract via delegation. Not having the ability to call claimCollateral() risks losing access to the surplus collateral in case of liquidation.

@bingen bingen self-assigned this Oct 20, 2024
@bingen bingen added the wontfix This will not be worked on label Oct 20, 2024
@bingen
Copy link
Collaborator Author

bingen commented Oct 20, 2024

It’s true, but it’s the same for all other borrower operations. It would be even worse if the contract didn’t implement things like adjustTrove or closeTrove. A contract owning a trove must make sure to implement all those functions for proper functioning.
For reference, in v1 we didn’t have managers, and the process for claiming collateral was the same.

To fix this, we should also change CollSurplusPool as even if delegations were not deleted, the claim of collateral is done per account, not per trove.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ChainSecurity wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant