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

[draft] make bad debt grace period configurable per-Ilk #147

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

livnev
Copy link
Contributor

@livnev livnev commented Oct 5, 2020

Vow.wait exists in order to avoid creating unnecessary flop auctions immediately after a position has entered liquidation (creating bad debt), but before the flip auction has had time to finish and cancel the bad debt. Currently, Vow.wait has to be configured to apply to debt generated by all collateral types, meaning that it should be set to the maximum running time of any flip auction. This is a limitation, since some future collateral types may wish to allow much longer liquidation auction durations than the existing ones, which would force wait to be longer, unnecessarily delaying flop auctions for other collateral types.

This PR makes wait configurable per Ilk, as Vow.ilks(ilk).wait.

TODOs:

  • adjust tests

@livnev livnev marked this pull request as draft October 5, 2020 14:40
@gbalabasquer
Copy link
Contributor

I know this is still a very early PR, but have 2 suggestions that might make sense:

  • Keep the wait value per collateral in the vow. That way we might save replacing the cat. I haven't analysed if keeping the same cat is something convenient or which implications it might have, but at least the possibility of replacing just the vow is there (similar analysis should be done for flapper and flopper).

  • Instead of saving the tab in the 2 dimensions era and wait mapping, just save it in one dimension mapping with the addition of both values.

@livnev
Copy link
Contributor Author

livnev commented Oct 5, 2020

Hi @gbalabasquer, thanks for your attention and suggestions:

  1. It seemed better to avoid introducing the concept of Ilks (and per-Ilk data) in the Vow, seeing as currently the Vow does not concern itself with collateral-specific information. Also, even if we were to store per-Ilk wait in the Vow, we would still need a way of referencing which Ilk the fess is coming from (to know which wait to use), meaning that the interface would change and would require a change in the Cat anyway.

  2. Good point, I have further simplified fess with this idea.

Overall, the main disadvantage of this change is the need to deploy a new Vow (and new Cat), which seems unavoidable. I wanted to flag this limitation, so that it can at least be incorporated the next time it is convenient to redeploy the Vow (if ever).

@gbalabasquer
Copy link
Contributor

  1. Right, I forgot about having to identify the ilk wait time when executing fess.

  2. Great.

Yes, as you know, replacing the vow is a bit complicate process. We made some analysis about it at the moment the flop stuck auctions happened. We ended up fixing that specific problem with some work around in the flop. However a more natural fix would be in the vow.
I guess at some point vow should be updated with all these improvements.

src/vow.sol Outdated Show resolved Hide resolved
@@ -155,7 +157,7 @@ contract Cat is LibNote {
vat.grab(
ilk, urn, address(this), address(vow), -int256(dink), -int256(dart)
);
vow.fess(mul(dart, rate));
vow.fess(add(now, milk.wait), mul(dart, rate));
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure these changes make it into the dog.sol

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.

5 participants