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

Undercollat vault creation #33

Merged
merged 9 commits into from
Mar 20, 2024
Merged

Undercollat vault creation #33

merged 9 commits into from
Mar 20, 2024

Conversation

anihamde
Copy link
Contributor

  • adapt TokenVault to enable undercollateralized vault creation
  • adapt vault simulator to create undercollateralized vaults
  • fix various bugs in the deploy scripts and vault-simulator
  • add a bit of documentation around installing reqs

Copy link
Collaborator

@m30m m30m left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanups. Please fix the comments and bump the simulator version before merging.

@@ -23,6 +23,14 @@ This allows users to participate in liquidations without needing to set up their

Tests can be found in `test/`. These tests include checks that the protocol functions work, as well as checks around permissioning, bid conditions, and appropriate failing of components of the express relay bundle (without failing the whole bundle).

To install necessary requirements, you should first run
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this a duplicate? We have these a few lines above.

return (expressRelay, opportunityAdapter, mockPyth, vault, weth);
}

function mintTokens(address token, address to, uint256 amount) public {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand why we need this. We can just use the cast commands we have in the docs to mint right?

@@ -428,10 +455,12 @@ contract VaultScript is Script {
getNextPublishTime(idToken2Latest),
0
);
console.log("222");
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove


bytes[] memory updateData = new bytes[](2);
updateData[0] = token1UpdateData;
updateData[1] = token2UpdateData;
console.log("333");
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove

let allow_undercollateralized: bool = contract.get_allow_undercollateralized().call().await?;
if allow_undercollateralized {
collat_factor = (min_health_numerator + min_health_permissionless_numerator) / 2;
// Less than 110% to create the vault undercollateralized
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can be more specific here that this is between the min_health_ratio and min_health_permissionless_ratio
Also if the autoformatter automaically put this comment in the next line, you can put in the previous line. The comment usually precedes the logic.

collat_factor = min_health_numerator * 10_001 / 10_000; // Slightly more than 110% to make sure the vault is created
}

let amount_collateral: U256 = collateral_value_usd * precision * collat_factor
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 it's a bit more readable if you divide collat_factor by min_health_denominator before this line so this line looks similar to the debt line with the only different being this factor.

@anihamde anihamde merged commit 99bcbd8 into main Mar 20, 2024
1 check passed
@anihamde anihamde deleted the undercollat_vault_creation branch March 20, 2024 21:35
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