-
Notifications
You must be signed in to change notification settings - Fork 9
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
Feature/provault poc #605
base: main
Are you sure you want to change the base?
Feature/provault poc #605
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left initial comments on the layout/architecture of the vault
uses: bruceadams/get-release@v1.2.3 | ||
env: | ||
GITHUB_TOKEN: ${{ github.token }} | ||
- name: Upload optimized wasm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small CI improvement, we should also add the checksums of the files here
@@ -0,0 +1,42 @@ | |||
[package] | |||
authors = ["quasar labs team <quasar.fi>"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add yourself as an author ser🫡
} | ||
|
||
// Construct the bank message to transfer the funds to the contract's address | ||
let bank_msg = BankMsg::Send { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
native tokens are automatically transferred when the user calls execute, this pattern of calling the transfer would be only needed for CW20s
} | ||
|
||
//////////////////////////////////////////////////////////////// | ||
//////////////////////// VAULT QUERIES ///////////////////////// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're putting all vault related code in it's own submodule, we can move queries to its own file, this way the import would become vault::queries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are you thinking the yield manager would work? I think if we just keep an autocompounding share and return users their % ownership of the vault on withdrawal, we would not need a specific yield manager, but let that be implicit because of the below balance queries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. In that case, a yield manager could be a wrapper as a simple abstraction over this. We can write that if needed later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like any yield we generate from multiple adaptors will be tracked here on demand for queries. So could be use only for organizing the queries. And we don't actually store anything on it.
Adaptor 1 -> yield 1
Adaptor 2 -> yield 2
Adaptor 3 -> yield 3
pub struct ProVaultPosition { | ||
pub provault_share_balance: Vec<Coin>, // Total pro vault share | ||
pub last_updated: u64, | ||
pub total_unallocated: Uint128, // Remaining amount of deposited token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do not need unallocated as part of a position, but instead can keep the unallocated deposited tokens in the balance of the vault contract
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will be the use of unallocated deposit tokens when we keep it on the vault contract? One obvious option is to send that amount for staking.
// #[cw_serde] | ||
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] | ||
pub struct ProVaultPosition { | ||
pub provault_share_balance: Vec<Coin>, // Total pro vault share |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the amount of pro vault shares minted or the amount of shares in other vaults the pro vault owns?
in the first case it's probably easier to query the tokenfactory for the amount of tokens minted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Total amount of minted shares by the pro's vault. At any point in time, it should be [ Total minted on deposits - Total burned on withdrawals ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we properly tokenize the user's position and yield, keeping track of it is not necessary, also this keeping track would bring problems/exploits if people IBC their pro vault share
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are three ways for us to define positions the pro vault has in other vaults
- Keep track of allowed adapters, and require the offchain infra to connect share tokens the vault owns to the exact deposits
- Look at all shares the vault owns in it's balance off chain, and only keep track of adapters and their tokens (if they exist) in state
- Keep track of our positions and deposited funds in state, this approach is what we do here
Imo number 1 is unnecessarily difficult and approach 3 is a bit overkill. Imo a better way would be to implicitly track the positions through the underlying vaults tokenized shares and when we call withdraw from adapter the shares are automatically moved
} | ||
|
||
#[cw_serde] | ||
pub enum StrategyAction { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this approach in general but worried if this is not generic enough for the different kinds of adapters. This would work if everything is a vault but with actions like depositing collateral, taking and repaying loans or just swapping for LSTs, I'm not sure if this would work
1. Overview
2. Implementation details
3. How to test/use
4. Checklist
5. Limitations (optional)
6. Future Work (optional)