Skip to content

Commit

Permalink
Merge pull request #11 from ComposableFi/vault-fix
Browse files Browse the repository at this point in the history
vault account fix & property tests precision
  • Loading branch information
hussein-aitlahcen authored Sep 1, 2021
2 parents 4ea1755 + c518713 commit 9167636
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 17 deletions.
2 changes: 1 addition & 1 deletion frame/composable-traits/src/vault.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub trait Vault {

fn lp_asset_id(vault: &Self::VaultId) -> Result<Self::AssetId, DispatchError>;

fn account_id() -> Self::AccountId;
fn account_id(vault: &Self::VaultId) -> Self::AccountId;

fn create(
deposit: Deposit<Self::Balance, Self::BlockNumber>,
Expand Down
31 changes: 19 additions & 12 deletions frame/vault/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ pub mod pallet {
Allocations::<T>::insert(id, account_id, allocation);
});

Allocations::<T>::insert(id, Self::account_id(), config.reserved);
Allocations::<T>::insert(id, Self::account_id(&id), config.reserved);

let vault_info = crate::models::VaultInfo {
lp_token_id,
Expand All @@ -353,15 +353,15 @@ pub mod pallet {
})
}

fn account_id() -> T::AccountId {
PALLET_ID.into_account()
fn account_id(vault_id: &VaultIndex) -> T::AccountId {
PALLET_ID.into_sub_account(vault_id)
}

/// Computes the sum of all the assets that the vault currently controls.
fn assets_under_management(vault_id: &VaultIndex) -> Result<T::Balance, Error<T>> {
let vault =
Vaults::<T>::try_get(vault_id).map_err(|_| Error::<T>::VaultDoesNotExist)?;
let owned = T::Currency::balance(vault.asset_id, &Self::account_id());
let owned = T::Currency::balance(vault.asset_id, &Self::account_id(vault_id));
let outstanding = CapitalStructure::<T>::iter_prefix_values(vault_id)
.fold(T::Balance::zero(), |sum, item| sum + item.balance);
Ok(owned + outstanding)
Expand Down Expand Up @@ -400,7 +400,8 @@ pub mod pallet {
let lp_shares_value_amount =
<T::Convert as Convert<u128, T::Balance>>::convert(lp_shares_value);

let vault_owned_amount = T::Currency::balance(vault.asset_id, &Self::account_id());
let vault_owned_amount =
T::Currency::balance(vault.asset_id, &Self::account_id(vault_id));

// TODO(hussein-aitlahcen): should we provide what we can to reduce the available
// liquidity in order to force strategies to rebalance?
Expand All @@ -413,7 +414,7 @@ pub mod pallet {
Error::<T>::InsufficientLpTokens
);

let from = Self::account_id();
let from = Self::account_id(&vault_id);
ensure!(
T::Currency::can_withdraw(vault.asset_id, &from, lp_shares_value_amount)
.into_result()
Expand Down Expand Up @@ -441,7 +442,7 @@ pub mod pallet {
Error::<T>::TransferFromFailed
);

let to = Self::account_id();
let to = Self::account_id(&vault_id);

let vault_aum = Self::assets_under_management(vault_id)?;
if vault_aum.is_zero() {
Expand Down Expand Up @@ -504,8 +505,8 @@ pub mod pallet {
Ok(vault.asset_id)
}

fn account_id() -> Self::AccountId {
Pallet::<T>::account_id()
fn account_id(vault: &Self::VaultId) -> Self::AccountId {
Pallet::<T>::account_id(vault)
}

fn create(
Expand Down Expand Up @@ -579,7 +580,7 @@ pub mod pallet {
state.balance.checked_add(&amount).ok_or(Error::<T>::OverflowError)?;
// This can definitely overflow. Perhaps it should be a BigUint?
state.lifetime_withdrawn.checked_add(&amount).ok_or(Error::<T>::OverflowError)?;
T::Currency::transfer(vault.asset_id, &Self::account_id(), to, amount, true)
T::Currency::transfer(vault.asset_id, &Self::account_id(vault_id), to, amount, true)
.map_err(|_| Error::<T>::InsufficientFunds)
})?;
Ok(())
Expand All @@ -597,8 +598,14 @@ pub mod pallet {
state.balance.saturating_sub(&amount);
// This can definitely overflow. Perhaps it should be a BigUint?
state.lifetime_deposited.checked_add(&amount).ok_or(Error::<T>::OverflowError)?;
T::Currency::transfer(vault.asset_id, from, &Self::account_id(), amount, true)
.map_err(|_| Error::<T>::InsufficientFunds)
T::Currency::transfer(
vault.asset_id,
from,
&Self::account_id(vault_id),
amount,
true,
)
.map_err(|_| Error::<T>::InsufficientFunds)
})?;
Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions frame/vault/src/mocks/strategy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub mod pallet {
T::Currency::transfer(
asset_id,
&Self::account_id(),
&T::Vault::account_id(),
&T::Vault::account_id(&vault),
balance,
true,
)?;
Expand All @@ -101,7 +101,7 @@ pub mod pallet {
FundsAvailability::Withdrawable(balance) => {
T::Currency::transfer(
asset_id,
&T::Vault::account_id(),
&T::Vault::account_id(&vault),
&Self::account_id(),
balance,
true,
Expand All @@ -112,7 +112,7 @@ pub mod pallet {
T::Currency::transfer(
asset_id,
&Self::account_id(),
&T::Vault::account_id(),
&T::Vault::account_id(&vault),
balance,
true,
)?;
Expand Down
2 changes: 1 addition & 1 deletion frame/vault/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ macro_rules! prop_assert_ok {
/// Accept a 'dust' deviation
macro_rules! prop_assert_epsilon {
($x:expr, $y:expr) => {{
let precision = 100000;
let precision = 1000;
let epsilon = 10;
let upper = precision + epsilon;
let lower = precision - epsilon;
Expand Down

0 comments on commit 9167636

Please sign in to comment.