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

feat: add oraclePrice and goverance data to liquidated vaults #20

Merged
merged 4 commits into from
May 15, 2024

Conversation

frazarshad
Copy link
Collaborator

This PR adds a snapshot of the vaultManagerGovernance and the oraclePrice at the time that a vault is liquidated. the reason for doing this is that we need to display the data at the time of liquidation in order to be accurate

example data of both a liquidated and liquidating vaults:
image

@frazarshad frazarshad force-pushed the feat/add-extra-info-to-liquidation branch from 479dbc6 to a72a70a Compare May 14, 2024 12:24
vault.coin = payload?.locked?.__brand;
vault.denom = payload?.locked?.__brand;
const vaultGovernanceId = id.split('.').slice(0, 4).join('.') + '.governance';
const vaultManagerGovernance = await VaultManagerGovernance.get(vaultGovernanceId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should add optional chaining on line 96:

const vaultGovernanceId = id?.split('.')?.slice(0, 4).join('.') + '.governance';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

id here should exist in all cases. if we put optional chaining here, we will get an invalid id. crashing in case the id format is invalid would be the preferred option here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahh, I see. I reckon it's the we're using the id declared on line 79?
Do you think we should have some sort of validation for id over there?
It's possible that id can have the value undefined-null.governance when path and payload?.vaultState have falsy values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this function is only triggered from line: src/mappings/mappingHandlers.ts:214 where we perform a regex check on the path. so there is no way it is falsy.
in any case i've added a regex check just to be safe

const oraclePrice = await OraclePrice.get(oraclPriceId);

if (vaultManagerGovernance && vault.vaultManagerGovernance === undefined)
vault.vaultManagerGovernance = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should also handle the case when vaultManagerGovernance is an empty object. Perhaps add some fallback values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

vaultManagerGovernace is not a json object object, but rather a class object. there is no empty state for it

@frazarshad frazarshad requested a review from rabi-siddique May 15, 2024 07:51
@frazarshad frazarshad merged commit f2232e6 into main May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants