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.

Sign up for GitHub

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

Jump to bottom

LPv2: Bump-up foreign investment. Fix failing investment ITs #1934

Merged
merged 13 commits into from
Aug 1, 2024
Merged

LPv2: Bump-up foreign investment. Fix failing investment ITs #1934

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5cddf8f
increase version for foreign investments
lemunozm Jul 30, 2024
b5249de
fix investment issue
lemunozm Jul 30, 2024
6ef03bd
fix solidity call for transfers_tokens
lemunozm Jul 31, 2024
3226798
fix tests
lemunozm Jul 31, 2024
a7ad1ad
minimize required tranche investors for IT
lemunozm Jul 31, 2024
c4042ec
fix docs
lemunozm Jul 31, 2024
a1a8346
Merge branch 'feat/lp-v2' into lpv2/bumb-up-version
wischli Jul 31, 2024
8a9cc0e
fix docs
lemunozm Jul 31, 2024
c8dc8ef
docs: fix hyperlink
wischli Aug 1, 2024
4e623f6
refactor: enable ITs for centrifuge + dev runtimes (#1938)
wischli Aug 1, 2024
c569e4f
fix: revert some centrifuge ITs
wischli Aug 1, 2024
94a0ea2
fmt
wischli Aug 1, 2024
3e61244
revert centrifuge addition to ITs
lemunozm Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions pallets/foreign-investments/src/entities.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ impl<T: Config> InvestmentInfo<T> {
let pool_currency = pool_currency_of::<T>(investment_id)?;

let cancel_pool_amount = T::Investment::investment(who, investment_id)?;
T::Investment::update_investment(who, investment_id, Zero::zero())?;
if !cancel_pool_amount.is_zero() {
T::Investment::update_investment(who, investment_id, Zero::zero())?;
}

if self.foreign_currency != pool_currency {
let increase_foreign = match self.order_id {
Expand Down Expand Up @@ -320,7 +322,9 @@ impl<T: Config> RedemptionInfo<T> {
investment_id: T::InvestmentId,
) -> Result<T::TrancheBalance, DispatchError> {
let cancelled = T::Investment::redemption(who, investment_id)?;
T::Investment::update_redemption(who, investment_id, Zero::zero())?;
if !cancelled.is_zero() {
T::Investment::update_redemption(who, investment_id, Zero::zero())?;
}
Ok(cancelled)
}

Expand Down
3 changes: 1 addition & 2 deletions pallets/foreign-investments/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ pub mod pallet {

use super::*;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
Expand Down
1 change: 0 additions & 1 deletion pallets/liquidity-pools-gateway/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ pub mod pallet {
};

#[pallet::pallet]

pub struct Pallet<T>(_);

#[pallet::origin]
Expand Down
14 changes: 7 additions & 7 deletions runtime/integration-tests/src/cases/lp/investments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::{
const DEFAULT_INVESTMENT_AMOUNT: Balance = 100 * DECIMALS_6;

mod utils {
use cfg_primitives::{AccountId, Balance, InvestmentId, PoolId, TrancheId};
use cfg_primitives::{AccountId, InvestmentId, PoolId, TrancheId};
use cfg_traits::HasLocalAssetRepresentation;
use ethabi::Token;
use pallet_foreign_investments::Action;
Expand Down Expand Up @@ -335,7 +335,7 @@ mod with_foreign_currency {
POOL_A,
};

#[test_runtimes([development], ignore = "solidity mismatch")]
#[test_runtimes([development])]
fn invest_cancel_full_before_swap<T: Runtime>() {
let mut env = setup_full::<T>();
env.state_mut(|evm| {
Expand All @@ -346,7 +346,7 @@ mod with_foreign_currency {
assert_eq!(
pallet_investments::InvestOrders::<T>::get(
lp::utils::remote_account_of::<T>(Keyring::TrancheInvestor(1)),
utils::investment_id::<T>(POOL_A, pool_c_tranche_1_id::<T>())
utils::investment_id::<T>(POOL_A, pool_a_tranche_1_id::<T>())
),
None,
);
Expand All @@ -371,7 +371,7 @@ mod with_foreign_currency {
assert_eq!(
pallet_investments::InvestOrders::<T>::get(
lp::utils::remote_account_of::<T>(Keyring::TrancheInvestor(1)),
utils::investment_id::<T>(POOL_A, pool_c_tranche_1_id::<T>())
utils::investment_id::<T>(POOL_A, pool_a_tranche_1_id::<T>())
),
None
);
Expand Down Expand Up @@ -589,7 +589,7 @@ mod with_foreign_currency {
});
}

#[test_runtimes([development], ignore = "solidity mismatch")]
#[test_runtimes([development])]
fn invest_cancel_full_after_swap_partially_inter_epoch_close<T: Runtime>() {
let mut env = setup_full::<T>();
let part = Quantity::checked_from_rational(1, 3).unwrap();
Expand Down Expand Up @@ -713,8 +713,8 @@ mod with_foreign_currency {
Keyring::TrancheInvestor(1)
)),
currency: utils::index_lp(evm, names::USDC),
currency_payout: DEFAULT_INVESTMENT_AMOUNT,
fulfilled_invest_amount: DEFAULT_INVESTMENT_AMOUNT,
currency_payout: DEFAULT_INVESTMENT_AMOUNT - partial_amount,
fulfilled_invest_amount: DEFAULT_INVESTMENT_AMOUNT - partial_amount,
}
)
});
Expand Down
9 changes: 6 additions & 3 deletions runtime/integration-tests/src/cases/lp/transfers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

use cfg_primitives::Balance;
use cfg_primitives::{AccountId, Balance};
use cfg_types::{
domain_address::{Domain, DomainAddress},
tokens::CurrencyId,
Expand Down Expand Up @@ -235,7 +235,7 @@ fn transfer_tranche_tokens_from_local<T: Runtime>() {
});
}

#[test_runtimes([development], ignore = "solidity mismatch")]
#[test_runtimes([development])]
fn transfer_tranche_tokens_domain_to_local_to_domain<T: Runtime>() {
let mut env = super::setup_full::<T>();
utils::prepare_hold_tt_domain::<T>(&mut env);
Expand All @@ -261,6 +261,7 @@ fn transfer_tranche_tokens_domain_to_local_to_domain<T: Runtime>() {
]),
)
.unwrap();

evm.call(
Keyring::TrancheInvestor(1),
sp_core::U256::zero(),
Expand All @@ -271,7 +272,9 @@ fn transfer_tranche_tokens_domain_to_local_to_domain<T: Runtime>() {
Token::FixedBytes(pool_a_tranche_1_id::<T>().into()),
Token::Uint(DOMAIN_EVM.into()),
Token::Uint(EVM_DOMAIN_CHAIN_ID.into()),
Token::Address(Keyring::TrancheInvestor(2).into()),
Token::FixedBytes(
AccountId::from(as_h160_32bytes(Keyring::TrancheInvestor(2))).to_raw_vec(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Damn, you were faster! Just came to the same conclusion after some debugging. Well done!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to reach the same solution, double confident about this!

),
Comment on lines +275 to +277
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes the test pass. We need an account coded in some way that later when obtaining the local representation the extra padding does not change. I think the current account conversions are quite confusing, and we should need to simplify this into the DomainAddress: ref

Copy link
Contributor

@wischli wischli Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Absolutely! I am wondering why this test has worked in the past..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously a Token::Address was passed as a parameter instead, which seems to be processed differently

Token::Uint(AMOUNT.into()),
]),
)
Expand Down
2 changes: 1 addition & 1 deletion runtime/integration-tests/src/cases/lp/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub fn pool_b_tranche_2_id<T: Runtime>() -> TrancheId {
pub fn pool_c_tranche_1_id<T: Runtime>() -> TrancheId {
*get_tranche_ids::<T>(POOL_C)
.get(0)
.expect("Pool B has two non-residuary tranches")
.expect("Pool C has one non-residuary tranche")
}

pub fn verify_outbound_failure_on_lp<T: Runtime>(to: H160) {
Expand Down
2 changes: 1 addition & 1 deletion runtime/integration-tests/src/utils/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ pub fn default_accounts() -> Vec<Keyring> {

/// Returns a Vector of default investor accounts
pub fn default_investors() -> Vec<Keyring> {
(0..=50).map(Keyring::TrancheInvestor).collect()
(1..=2).map(Keyring::TrancheInvestor).collect()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This reduces the time for each test case to more than a minute on my computer (around 70 seconds less per test case). I think we can enable again the all tag for these tests.

}

#[cfg(test)]
Expand Down
Loading