Liquidation Visibility - Iteration One #9

anilhelvaci · 2024-01-29T09:11:06Z

refs: #4 #3

Description

Security Considerations

No new authority is introduced to vaultManager. However, we make a new call to auctioneer inside liquidateVaults in order to fetch the liveAuctionSchedule which we expect to be fine.

Scaling Considerations

We introduced new storage nodes in vstorage which we don't plan to update once the data is written.

Testing Considerations

What tests should we implement other than the unit tests?

Upgrade Considerations

These changes can be deployed via an upgrade to vaultFactory.

…rderKits BREAKING CHANGE: Introduced the `_timestamp` as an argument for the vaultManager liquidateVaults method.

…tate to Vstorage BREAKING CHANGE: a getVaultId method was included in the Vault interface, which returns the vault `idInManager`

…tly await multiple promises

BREAKING CHANGE: the getVaultId method of vaults interface was updated to getVaultState, which will return the vault phase as well

… and names

…om the vstorage

… Vstorage and its structure The helper methods built for this purpose became unnecessary and were removed.

…uction` data fields

…rtions for running the tests

…nge to writeFinal recorder method

…/develop

…updated, #7 is fixed

anilhelvaci

What do you think? @Jorge-Lopes

anilhelvaci · 2024-01-29T09:18:36Z

packages/inter-protocol/src/vaultFactory/vault.js

@@ -131,6 +131,7 @@ export const VaultI = M.interface('Vault', {
  getCurrentDebt: M.call().returns(AmountShape),
  getNormalizedDebt: M.call().returns(AmountShape),
  getVaultSeat: M.call().returns(SeatShape),
+  getVaultState: M.call().returns(M.any()),


Might be good to introduce a type guard instead of M.any()

anilhelvaci · 2024-01-29T09:43:55Z

packages/inter-protocol/src/vaultFactory/vaultManager.js

          // This is expected to wait for the duration of the auction, which
          // is controlled by the auction parameters startFrequency, clockStep,
          // and the difference between startingRate and lowestRate.
-          const [proceeds] = await Promise.all([deposited, userSeatPromise]);
+          const [auctionSchedule, proceeds] = await Promise.all([
+            E(auctionPF).getSchedules(),


At this point in time the auction is already finished. The question is whether or not we get the schedule where liveScheduleAuction is still pointing to the auction we just finished. This is what we need to indicate the endTime of the current liquidation auction.

I'm not sure if we can depend on (E(acutionPF).getSchedule()).liveAuctionSchedule is always going to point to right value due to distributed nature of the vaultFactory and auctioneer vats. Despite not being able put my finger directly on the possibility of a race condition, I still suggest we take the conservative path and call E(acutionPF).getSchedule() somewhere before await deposited.

I agree.
In this case, we need to handle the promise rejection.
Some tests for this edge case will be required as well.

anilhelvaci · 2024-01-29T09:50:21Z

packages/inter-protocol/src/vaultFactory/vaultManager.js

@@ -1197,10 +1276,31 @@ export const prepareVaultManagerKit = (
              ),
          );

+          const [{ userSeatPromise, deposited }, liquidationRecorderKits] =
+            await Promise.all([


Should we use Promise.allSettled instead of Promise.all? The reason being, if helper.makeLiquidationRecorderKits somehow fails Promise.all is going reject all together and we won't be able to complete the liquidation operation. What are the possible chances of helper.makeLiquidationRecorderKits failing, I don't know. However, when we think about it, helper.makeLiquidationRecorderKits interacts with another vat by calling makeChildNode. So depending on chainStorage vat to work correctly when settling liquidations seems like an unnecessary dependecy.

I agree.
In this case, we need to handle the promise rejection.
Some tests for this edge case will be required as well.

…ove readability, fetch schedule during the auction itself

…r vats

…q-result-scenario-1`

Jorge-Lopes

The changes made in this PR implement the requested features and address the security concerns raised before.

Jorge-Lopes · 2024-01-29T12:40:05Z

packages/inter-protocol/src/vaultFactory/vault.js

@@ -131,6 +131,7 @@ export const VaultI = M.interface('Vault', {
  getCurrentDebt: M.call().returns(AmountShape),
  getNormalizedDebt: M.call().returns(AmountShape),
  getVaultSeat: M.call().returns(SeatShape),
+  getVaultState: M.call().returns(M.any()),


Jorge-Lopes · 2024-01-29T12:40:27Z

packages/inter-protocol/src/vaultFactory/vaultManager.js

          // This is expected to wait for the duration of the auction, which
          // is controlled by the auction parameters startFrequency, clockStep,
          // and the difference between startingRate and lowestRate.
-          const [proceeds] = await Promise.all([deposited, userSeatPromise]);
+          const [auctionSchedule, proceeds] = await Promise.all([
+            E(auctionPF).getSchedules(),


I agree.
In this case, we need to handle the promise rejection.
Some tests for this edge case will be required as well.

Jorge-Lopes · 2024-01-29T12:41:52Z

packages/inter-protocol/src/vaultFactory/vaultManager.js

@@ -1197,10 +1276,31 @@ export const prepareVaultManagerKit = (
              ),
          );

+          const [{ userSeatPromise, deposited }, liquidationRecorderKits] =
+            await Promise.all([


I agree.
In this case, we need to handle the promise rejection.
Some tests for this edge case will be required as well.

# This is the 1st commit message: create dedicated files for test assertions and tools # This is the commit message #2: Add new test file for liquidation visibility # This is the commit message #3: auctioneer snapshot # This is the commit message #4: Update visibility tests and helper functions # This is the commit message #5: add tools and assertions to match subscriber with vstorage data # This is the commit message #6: update test visibility of vault liquidation # This is the commit message #7: fix(liquidationVisibility): fix linting errors # This is the commit message #8: fix(liquidationVisibility): type error # This is the commit message #9: chore(liquidationVisibility): #4 testing tools setup # This is the commit message #10: chore(liquidationVisibility): #4 improve testing tools # This is the commit message #11: chore(liquidationVisibility): #4 improve testing tools # This is the commit message #12: fix(liquidationVisibility): linting fixes # This is the commit message #13: chore(liquidationVisibility): fix test names # This is the commit message #14: fix(liquidationVisibility): lint fix # This is the commit message #15: chore(liquidationVisibility): #4 implement `assertNodeInStorage` # This is the commit message #16: fix(liquidationVisibility): #4 lint fix # This is the commit message #17: chore(liquidationVisibility): #4 test skeleton is ready # This is the commit message #18: chore(liquidationVisibility): #4 add marshaller for comparing data from the vstorage # This is the commit message #19: chore(liquidationVisibility): sample test for `preAuction` and `postAuction` data fields # This is the commit message #20: chore(liquidationVisibility): make sure `assertStorageData` works # This is the commit message #21: chore(liquidationVisibility): #4 add test for case 2b, uncomment assertions for running the tests # This is the commit message #22: feat(liquidationVisibility): create liquidation storageNodes and recorderKits BREAKING CHANGE: Introduced the `_timestamp` as an argument for the vaultManager liquidateVaults method. feat(liquidationVisibility): write preAuctionState and auctionResultState to Vstorage BREAKING CHANGE: a getVaultId method was included in the Vault interface, which returns the vault `idInManager` fix(liquidationVisibility): fix type definitions errors and concurrently await multiple promises feat(liquidationVisibility): write postAuctionState to Vstorage BREAKING CHANGE: the getVaultId method of vaults interface was updated to getVaultState, which will return the vault phase as well chore(liquidationVisibility): update helper methods, type definitions and names fix(liquidationVisibility): lint fix fix(liquidationVisibility): update sequence to write auction state to Vstorage and its structure The helper methods built for this purpose became unnecessary and were removed. fix(liquidationVisibility): update postAuctionState structure and change to writeFinal recorder method fix(liquidationVisibility): remove temporary changes made to tests fix(liquidationVisibility): #4 test for scenario 2b passed, test api updated, #7 is fixed chore(liquidationVisibility): #4 test for scenario 2a passed chore(liquidationVisibility): #4 update scenario 1 Squashed commit of the following: commit 728d695 Author: anilhelvaci <anilhelvaci96@gmail.com> Date: Wed Jan 31 14:28:14 2024 +0300 chore(liquidationVisibility): uncomment post auction assertion in `liq-result-scenario-1` commit dd3fbdb Author: anilhelvaci <anilhelvaci96@gmail.com> Date: Wed Jan 31 14:25:22 2024 +0300 fix(liquidationVisibility): lint fix commit 6920d1a Author: anilhelvaci <anilhelvaci96@gmail.com> Date: Wed Jan 31 14:22:28 2024 +0300 fix(liquidationVisibility): explain Promise.allSettled commit 732e1d7 Author: anilhelvaci <anilhelvaci96@gmail.com> Date: Wed Jan 31 11:37:45 2024 +0300 feat(liquidationVisibility): handle errors that might arise from other vats commit 683f56d Author: anilhelvaci <anilhelvaci96@gmail.com> Date: Tue Jan 30 14:31:13 2024 +0300 feat(liquidationVisibility): add LiquidationVisibilityWriters to improve readability, fetch schedule during the auction itself commit 4c45f2a Author: anilhelvaci <anilhelvaci96@gmail.com> Date: Tue Jan 30 10:30:02 2024 +0300 fix(liquidationVisibility): add pattern matcher to `getVaultState` chore(liquidationVisibility): #4 add auctioneer wrapper and update setupBasics chore(liquidationVisibility): #4 add mock makeChainStorageNode chore(liquidationVisibility): #4 add tests for no vaults and rejected schedule fix(liquidationVisibility): #4 add setBlockMakeChildNode method and update file name fix(liquidationVisibility): #4 update import path chore(liquidationVisibility): #4 add liq-rejected-timestampStorageNode test fix(liquidationVisibility): #4 fix bug with at makeChildNode fix(liquidationVisibility): #4 update test names and comments fix(liquidationVisibility): #4 lint fix chore(internal): create key-value pairs for Promise.allSettled values fix(internal): make allValuesSettled a mapper for resolved promises and silently handles rejected ones fix(internal): fix doc fix(liquidationVisibility): make sure promises are assigned in key-value fashion, lint fixes. chore(liquidationVisibility): extend liq-rejected-timestampStorageNode and clean outdated comments fix(liquidationVisibility): revert update made to package.json chore(liquidationVisibility): update snapshot generated by unit tests fix(liquidationVisibility): lint fix chore(liquidationVisibility) #4 test multiple vaultManagers fix(liquidationVisibility): #4 lint fix fix(liquidationVisibility): add pattern matcher to `getVaultState` feat(liquidationVisibility): add LiquidationVisibilityWriters to improve readability, fetch schedule during the auction itself chore(liquidationVisibility): init work for bootstrap tests chore(liquidationVisibility): created test-liquidation-visibility.ts and started building a test suite chore(liquidationVisibility): upgrade tests are implemented. Refs: #15 fix(liquidationVisibility): vaults are now displayed in the correct order at `vaults.preAuction` Refs: #13 fix(liquidationVisibility): vault phases are now displayed correctly at `vaults.postAuction` Refs: #14 chore(liquidationVisibility): add storage snapshot Refs: #15 fix(liquidationVisibility): don't reverse `vaultData` for preAuction storage node Refs: #13

Jorge-Lopes and others added 30 commits January 12, 2024 11:16

create dedicated files for test assertions and tools

680330c

Add new test file for liquidation visibility

a3f6cf0

auctioneer snapshot

9d7abd6

Update visibility tests and helper functions

5d1088b

add tools and assertions to match subscriber with vstorage data

f35bda1

update test visibility of vault liquidation

26dc28f

Merge branch 'master' into feature/unit-tests

e603254

fix(liquidationVisibility): fix linting errors

ae88a64

fix(liquidationVisibility): type error

7d477ff

feat(liquidationVisibility): create liquidation storageNodes and reco…

b4c8a21

…rderKits BREAKING CHANGE: Introduced the `_timestamp` as an argument for the vaultManager liquidateVaults method.

feat(liquidationVisibility): write preAuctionState and auctionResultS…

21bb9ca

…tate to Vstorage BREAKING CHANGE: a getVaultId method was included in the Vault interface, which returns the vault `idInManager`

fix(liquidationVisibility): fix type definitions errors and concurren…

3739f25

…tly await multiple promises

chore(liquidationVisibility): #4 testing tools setup

5dfa289

chore(liquidationVisibility): #4 improve testing tools

bbd2e48

chore(liquidationVisibility): #4 improve testing tools

7292068

fix(liquidationVisibility): linting fixes

d8af7fa

chore(liquidationVisibility): fix test names

45cea3b

fix(liquidationVisibility): lint fix

89db737

chore(liquidationVisibility): #4 implement assertNodeInStorage

9fefcf0

fix(liquidationVisibility): #4 lint fix

e81df01

feat(liquidationVisibility): write postAuctionState to Vstorage

419c810

BREAKING CHANGE: the getVaultId method of vaults interface was updated to getVaultState, which will return the vault phase as well

chore(liquidationVisibility): update helper methods, type definitions…

637a282

… and names

fix(liquidationVisibility): lint fix

4089361

chore(liquidationVisibility): #4 test skeleton is ready

a5815ae

chore(liquidationVisibility): #4 add marshaller for comparing data fr…

51eb929

…om the vstorage

fix(liquidationVisibility): update sequence to write auction state to…

ba95348

… Vstorage and its structure The helper methods built for this purpose became unnecessary and were removed.

chore(liquidationVisibility): sample test for preAuction and `postA…

59e155b

…uction` data fields

chore(liquidationVisibility): make sure assertStorageData works

1a17dd4

chore(liquidationVisibility): #4 add test for case 2b, uncomment asse…

b7cdb13

…rtions for running the tests

fix(liquidationVisibility): update postAuctionState structure and cha…

b906859

…nge to writeFinal recorder method

Jorge-Lopes and others added 5 commits January 25, 2024 11:02

fix(liquidationVisibility): remove temporary changes made to tests

e680466

Merge remote-tracking branch 'origin/jorge/update-contacts' into anil…

c4e2b34

…/develop

fix(liquidationVisibility): #4 test for scenario 2b passed, test api …

34ecf5c

…updated, #7 is fixed

chore(liquidationVisibility): #4 test for scenario 2a passed

348707e

chore(liquidationVisibility): #4 update scenario 1

e7259fb

anilhelvaci commented Jan 29, 2024

View reviewed changes

anilhelvaci mentioned this pull request Jan 29, 2024

Steps required for landing the PR #8

Open

anilhelvaci added 6 commits January 30, 2024 10:30

fix(liquidationVisibility): add pattern matcher to getVaultState

4c45f2a

feat(liquidationVisibility): add LiquidationVisibilityWriters to impr…

683f56d

…ove readability, fetch schedule during the auction itself

feat(liquidationVisibility): handle errors that might arise from othe…

732e1d7

…r vats

fix(liquidationVisibility): explain Promise.allSettled

6920d1a

fix(liquidationVisibility): lint fix

dd3fbdb

chore(liquidationVisibility): uncomment post auction assertion in `li…

728d695

…q-result-scenario-1`

Jorge-Lopes approved these changes Jan 31, 2024

View reviewed changes

Jorge-Lopes merged commit c60c099 into develop Feb 8, 2024
58 checks passed

Jorge-Lopes mentioned this pull request Feb 8, 2024

Revert "Liquidation Visibility - Iteration One" #18

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Liquidation Visibility - Iteration One #9

Liquidation Visibility - Iteration One #9

anilhelvaci commented Jan 29, 2024

anilhelvaci left a comment

anilhelvaci Jan 29, 2024

Jorge-Lopes Jan 29, 2024

anilhelvaci Jan 29, 2024

Jorge-Lopes Jan 29, 2024

anilhelvaci Jan 29, 2024

Jorge-Lopes Jan 29, 2024

Jorge-Lopes left a comment

Jorge-Lopes Jan 29, 2024

Jorge-Lopes Jan 29, 2024

Jorge-Lopes Jan 29, 2024

Liquidation Visibility - Iteration One #9

Liquidation Visibility - Iteration One #9

Conversation

anilhelvaci commented Jan 29, 2024

Description

Security Considerations

Scaling Considerations

Testing Considerations

Upgrade Considerations

anilhelvaci 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

Jorge-Lopes 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