feat(vault): liquidation penalty handled by liquidation contracts #5343

turadg · 2022-05-11T19:14:54Z

closes: #5167

Description

Moves the penalty charging concern from Vault Factory contract to the liquidation contracts. It's up to them what to do with the penaltyRate input.

This also removes the penaltyPoolSeat since it's raison d'être n'est plus.

Next up is #5367

Security Considerations

Empowers the liquidation contracts to charge or not charge the liquidation penalty specified by penaltyRate.

Provides them a public facet of the asset reserve.

Documentation Considerations

--

Testing Considerations

Updated test setup to accommodate wiring of the reserve instance. The test of liquidation penalty gathering (removed here) will be part of #5367.

Chris-Hibbert · 2022-05-11T21:49:03Z

packages/run-protocol/src/vaultFactory/liquidateMinimum.js

 import { AmountMath } from '@agoric/ertp';
 import { Far } from '@endo/marshal';

 import { makeTracer } from '../makeTracer.js';

 const trace = makeTracer('LiqMin', false);

+// ??? is this contract used only for tests and demo purposes?


currently only used that way, but also available to governance to fall back to.

Chris-Hibbert · 2022-05-11T21:56:59Z

packages/run-protocol/src/vaultFactory/liquidation.js

+  // all the proceeds from AMM sale are on the vault seat instead of a staging seat
+  // ??? ^ okay?


Why drop penaltyPoolSeat? That seemed like a good place from which to stage funds going to the reserve.

The requirements of the ticket are that the contract manages where funds go.

They each have their own penaltyPoolSeat now. I still have to wire up moving the assets out to the reserve.

Chris-Hibbert · 2022-05-11T21:58:21Z

packages/run-protocol/test/vaultFactory/test-vaultFactory.js

+  // ??? remove penalty pool from vault factory
+  // used to be funded from liquidation penalty
+  t.deepEqual(await E(vaultFactory).getPenaltyAllocation(), {});


I think it's correct to take the penalty funds out of the vault, but then they have to be accessible (even if we aren't yet retrieving them) from the liquidation contracts.

Yes, but I'm not sure that test-vaultFactory needs to be concerned with that. It's a concern of the liquidation contracts what they do with the liquidationPenalty argument.

I see I misread your comment. Agree completely and working on it.

dckc · 2022-05-13T20:51:15Z

packages/run-protocol/src/econ-behaviors.js

@@ -241,7 +241,8 @@ export const startVaultFactory = async (
  {
    consume: {
      chainTimerService,
-      priceAuthority,
+      priceAuthority: priceAuthorityP,
+      reservePublicFacet: reservePublicFacetP,


odd... usually we don't bother putting public facets in the produce / consume space, since we can get the instances via agoricNames and then the publicFacet via zoe

But if that's outside the scope of this PR...

to permit more stuff to be consumed, add it to https://github.com/Agoric/agoric-sdk/blob/master/packages/run-protocol/src/core-proposal.js#L40-L46

thanks! 842b068

turadg · 2022-05-13T23:29:43Z

packages/run-protocol/src/vaultFactory/liquidateMinimum.js

-  const creatorFacet = Far('debtorInvitationCreator', {
-    makeLiquidateInvitation: () => zcf.makeInvitation(debtorHook, 'Liquidate'),
+  const creatorFacet = Far('debtorInvitationCreator (minimum)', {
+    makeLiquidateInvitation: () =>


renamed à la #5179

turadg · 2022-05-13T23:30:20Z

packages/run-protocol/src/vaultFactory/liquidation.js

- * @param {Amount<'nat'>} debt - after incurring penalty
- * @param {Amount<'nat'>} penaltyPortion
- */
-const partitionProceeds = (proceeds, debt, penaltyPortion) => {


now that the math is split between two contracts, not worth having this combo fn

Chris-Hibbert

This is basically right, but I have a few questions and suggestions.

packages/run-protocol/src/vaultFactory/liquidateIncrementally.js

Chris-Hibbert · 2022-05-16T16:32:53Z

packages/run-protocol/src/vaultFactory/liquidateIncrementally.js

+    const debtPaid = debtorSeat.getAmountAllocated('Out', debtBrand);
+    const penaltyPaid = AmountMath.min(penalty, debtPaid);
+
+    // Allocate penalty portion of proceeds to a seat that will be transferred to reserve


It's not the seat that will be transferred...

Suggested change

// Allocate penalty portion of proceeds to a seat that will be transferred to reserve

// Allocate penalty portion of proceeds to a seat that will hold it for transfer to reserve

Chris-Hibbert · 2022-05-16T16:33:19Z

packages/run-protocol/src/vaultFactory/liquidateIncrementally.js

@@ -276,15 +298,28 @@ const start = async zcf => {
        });
      }
    }
+
+    // Now we need to know how much was sold so we can pay off the debt.
+    // We can use this because only liquidation adds debt brand to the seat.


Suggested change

// We can use this because only liquidation adds debt brand to the seat.

// We can use this seat because only liquidation adds debt brand to it

Chris-Hibbert · 2022-05-16T16:34:38Z

packages/run-protocol/src/vaultFactory/liquidateMinimum.js

      amountIn,
      paid: debtorSeat.getCurrentAllocation(),
      amounts,
    });
+
+    // Now we need to know how much was sold so we can pay off the debt.
+    // We can use this because only liquidation adds debt brand to the seat.


Suggested change

// We can use this because only liquidation adds debt brand to the seat.

// We can use this seat because only liquidation adds debt brand to it

Chris-Hibbert · 2022-05-16T16:41:41Z

packages/run-protocol/src/vaultFactory/liquidation.js

+    E(liqSeat).getOfferResult(),
+  ]);
+  // all the proceeds from AMM sale are on the vault seat instead of a staging seat
+  // ??? ^ okay?


Yes, it's fine that all the proceeds end up on the VaultSeat. You can drop the ??? comment.

Chris-Hibbert · 2022-05-16T16:44:42Z

packages/run-protocol/test/vaultFactory/test-vaultFactory.js

-  t.deepEqual(await E(vaultFactory).getPenaltyAllocation(), {
-    RUN: AmountMath.make(runBrand, 30n),


I expected the new PenaltySeats to have accessors like this. Why don't they?

The plan is not to expose the penalty seat. @dtribble said not to use the fee collector pattern here and instead have the liquidation contract move the assets directly to the reserve. To give input on that: #5367

turadg linked an issue May 11, 2022 that may be closed by this pull request

Liquidation penalty should be handled in the liquidation contract (not vaults) #5167

Closed

turadg requested a review from Chris-Hibbert May 11, 2022 19:15

Chris-Hibbert requested changes May 11, 2022

View reviewed changes

turadg force-pushed the 5167-liq-penalty branch from 67dd47d to 788c61b Compare May 13, 2022 20:25

dckc reviewed May 13, 2022

View reviewed changes

turadg marked this pull request as ready for review May 13, 2022 23:27

turadg requested a review from dtribble as a code owner May 13, 2022 23:27

turadg requested a review from Chris-Hibbert May 13, 2022 23:28

turadg commented May 13, 2022

View reviewed changes

turadg added 17 commits May 16, 2022 08:31

handleFooOffer style per #5179

d9c0607

chore(zoe): more typing offerTo and Invitation

024c205

plan

0cb7b4b

factored penalties into liquidation contracts

b13aba1

rm obsolete partitionProceeds

58d8555

lint and comments

276c94c

named parameters for makeGovernedTerms()

c44cd1e

wire reservePublicFacet into vaultFactory terms

6a3349e

rm obsolete penaltyPoolSeat

86dea15

fixup! wire reservePublicFacet into vaultFactory terms

db7194b

fixup! rm obsolete penaltyPoolSeat

db9e406

consume reserve instance instead of facet

4c92859

typecheck liquidator,vaultFactory tests

fff80b0

fix test typo tsc caught

9e30183

fix invitation param and typecheck that would have caught it

d8fba65

update comment docs

74460d2

work around type resolution shortcomings

bbb4ea9

turadg force-pushed the 5167-liq-penalty branch from 5eed5ff to bbb4ea9 Compare May 16, 2022 15:36

Chris-Hibbert requested changes May 16, 2022

View reviewed changes

better comments

01c3fe6

Merge branch 'master' into 5167-liq-penalty

96c9811

turadg added automerge:squash Automatically squash merge labels May 16, 2022

turadg mentioned this pull request May 16, 2022

feat(vault): econ metrics notifiers #5260

Merged

16 tasks

Chris-Hibbert approved these changes May 16, 2022

View reviewed changes

Merge branch 'master' into 5167-liq-penalty

c38cf7e

mergify bot merged commit ce1cfaf into master May 16, 2022

mergify bot deleted the 5167-liq-penalty branch May 16, 2022 23:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(vault): liquidation penalty handled by liquidation contracts #5343

feat(vault): liquidation penalty handled by liquidation contracts #5343

turadg commented May 11, 2022 •

edited

Loading

Chris-Hibbert May 11, 2022

Chris-Hibbert May 11, 2022

turadg May 11, 2022

Chris-Hibbert May 11, 2022

turadg May 11, 2022

turadg May 12, 2022 •

edited

Loading

dckc May 13, 2022

turadg May 13, 2022

turadg May 13, 2022 •

edited

Loading

turadg May 13, 2022

Chris-Hibbert left a comment

Chris-Hibbert May 16, 2022

Chris-Hibbert May 16, 2022

Chris-Hibbert May 16, 2022

Chris-Hibbert May 16, 2022

Chris-Hibbert May 16, 2022

turadg May 16, 2022

		// all the proceeds from AMM sale are on the vault seat instead of a staging seat
		// ??? ^ okay?

	// Allocate penalty portion of proceeds to a seat that will be transferred to reserve
	// Allocate penalty portion of proceeds to a seat that will hold it for transfer to reserve

	// We can use this because only liquidation adds debt brand to the seat.
	// We can use this seat because only liquidation adds debt brand to it

		t.deepEqual(await E(vaultFactory).getPenaltyAllocation(), {
		RUN: AmountMath.make(runBrand, 30n),

feat(vault): liquidation penalty handled by liquidation contracts #5343

feat(vault): liquidation penalty handled by liquidation contracts #5343

Conversation

turadg commented May 11, 2022 • edited Loading

Description

Security Considerations

Documentation Considerations

Testing Considerations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg May 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg May 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg commented May 11, 2022 •

edited

Loading

turadg May 12, 2022 •

edited

Loading

turadg May 13, 2022 •

edited

Loading