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

Add Liquidity Pools related pallets to centrifuge runtime #1523

Merged
merged 10 commits into from
Sep 4, 2023

Conversation

cdamian
Copy link
Contributor

@cdamian cdamian commented Aug 31, 2023

Description

Adding the Liquidity Pools pallets to the centrifuge runtime.

Changes and Descriptions

  • Add Liquidity Pool Gateway Pallet to runtime
    • Dummy Liquidity Pool InboundQueue that triggers event with info (Runtime struct)
      • NOTE - No event being triggered at the moment
    • Include Liquidity Pools to runtime
      • NOT use LPs as InboundQueue
    • Wrapper OutboundQueue that LPs consumes
      • Only allow (AddCurrency, UpdateMember, AddPool, AddTranche, UpdateTrancheTokenPrice)
      • Uses gateway as real outbound

Checklist:

  • I have added Rust doc comments to structs, enums, traits and functions
  • I have made corresponding changes to the documentation
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works

@cdamian cdamian force-pushed the feat/add-lp-to-centrifuge-runtime branch from ebe4d1a to daad72a Compare August 31, 2023 20:15
@cdamian cdamian mentioned this pull request Aug 31, 2023
10 tasks
runtime/centrifuge/src/lib.rs Outdated Show resolved Hide resolved
parameter_types! {
// TODO(cdamian): is this correct?
// 1 DOT should be enough to cover for fees opening/accepting hrmp channels
pub MaxHrmpRelayFee: MultiAsset = (MultiLocation::parent(), 1_000_000_000_000u128).into();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we OK with this value?

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICT this is way too high: DOT has 10 decimals, so this value is actually 100 DOT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we go with 1 DOT or less?

@cdamian cdamian marked this pull request as ready for review September 1, 2023 10:37
@cdamian cdamian changed the title Feat/add lp to centrifuge runtime Add Liquidity Pools related pallets to centrifuge runtime Sep 1, 2023
type Permission = Permissions;
type PoolId = PoolId;
type PoolInspect = PoolSystem;
type Rate = Rate;
Copy link
Contributor

@lemunozm lemunozm Sep 1, 2023

Choose a reason for hiding this comment

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

Very important here to change Rate by Quantity in case #1520 is finally merged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'm not sure we wanna have that part of this release so let's keep this open until we decide that and then I'll adjust as needed.

Thank you for the catch @lemunozm! <3

cc @mustermeiszer

Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

I think the DOT decimals are off and that we should use another HRMP Encoder.

runtime/centrifuge/src/lib.rs Outdated Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
parameter_types! {
// TODO(cdamian): is this correct?
// 1 DOT should be enough to cover for fees opening/accepting hrmp channels
pub MaxHrmpRelayFee: MultiAsset = (MultiLocation::parent(), 1_000_000_000_000u128).into();
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICT this is way too high: DOT has 10 decimals, so this value is actually 100 DOT.

type CurrencyId = CurrencyId;
type CurrencyIdToMultiLocation = xcm::CurrencyIdConvert;
type DerivativeAddressRegistrationOrigin = EnsureRootOr<HalfOfCouncil>;
type HrmpEncoder = moonbeam_relay_encoder::westend::WestendEncoder;
Copy link
Contributor

Choose a reason for hiding this comment

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

I am fairly sure we want to have the Westend encoder but the Polkadot one?!

Suggested change
type HrmpEncoder = moonbeam_relay_encoder::westend::WestendEncoder;
type HrmpEncoder = moonbeam_relay_encoder::polkadot::PolkadotEncoder;

In case I am correct, we must also change the HRMP encoder for altair to moonbeam_relay_encoder::kusama::KusamaEncoder

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch! We should take a dedicated look at pallet_xcm_transactor again. I have this as part of our release list. That we check the XCM setup. I am almost certain, that we currently do not allow to transfer tokens out from our chain ^^

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should also add the axelar-gateway-precompile. I am using that one for the (source_chain, source_address) to DomainAddress conversion on the XCM level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added and updated the Centrifuge precompile as well. PTAL

runtime/centrifuge/src/lib.rs Outdated Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/Cargo.toml Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
@cdamian cdamian force-pushed the feat/add-lp-to-centrifuge-runtime branch from aa89aef to 61543a6 Compare September 1, 2023 20:36
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
Copy link
Contributor

@NunoAlexandre NunoAlexandre left a comment

Choose a reason for hiding this comment

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

Some comments, the only blocker afaik is the use of DummyInboundQueue on Centrifuge 👀

@@ -62,7 +64,7 @@ use orml_traits::{currency::MutationHooks, parameter_type_with_key};
use pallet_anchors::AnchorData;
pub use pallet_balances::Call as BalancesCall;
use pallet_collective::{EnsureMember, EnsureProportionAtLeast, EnsureProportionMoreThan};
use pallet_ethereum::Transaction as EthereumTransaction;
use pallet_ethereum::Transaction as EthTransaction;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer that we just literally refer to it with the pull path instead of aliasing the import. That's also what we do on Altair. It's only referred to in 3 places so it's fine and we save the indirection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this because EthereumTransaction is how we reference the actual pallet-ethereum-transaction that we added.

runtime/centrifuge/src/lib.rs Outdated Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/lib.rs Show resolved Hide resolved
runtime/centrifuge/src/migrations.rs Show resolved Hide resolved
@cdamian cdamian merged commit 16d9e9b into upgrade1020 Sep 4, 2023
11 checks passed
@cdamian cdamian deleted the feat/add-lp-to-centrifuge-runtime branch September 4, 2023 11:19
cdamian added a commit that referenced this pull request Sep 4, 2023
* centrifuge: Add liquidity pools pallets to runtime

* centrifuge: Add LP OutboundQueue wrapper to runtime

* taplo: Obey

* runtime: Move OutboundQueue import

* centrifuge: Allow both root and half of council as admin origins for LP pallets

* centrifuge: Add Axelar Gateway precompile

* centrifuge: Use correct HRMP encoder, update HRMP fee to 1 DOT, use XCM V3 imports

* centrifuge: Update error message for InboundQueue

* clippy: Fix warnings

* centrifuge: Rename outbound queue, fix migration import for orml asset registry
cdamian added a commit that referenced this pull request Sep 4, 2023
* centrifuge: Add liquidity pools pallets to runtime

* centrifuge: Add LP OutboundQueue wrapper to runtime

* taplo: Obey

* runtime: Move OutboundQueue import

* centrifuge: Allow both root and half of council as admin origins for LP pallets

* centrifuge: Add Axelar Gateway precompile

* centrifuge: Use correct HRMP encoder, update HRMP fee to 1 DOT, use XCM V3 imports

* centrifuge: Update error message for InboundQueue

* clippy: Fix warnings

* centrifuge: Rename outbound queue, fix migration import for orml asset registry
mustermeiszer added a commit that referenced this pull request Sep 6, 2023
* wip xcm v3 asset-registry migrations

* fix: xcm v3 altair, centrifuge

* feat: add defensive SafeCallFilter for transact

* clean up asset addition for catalyst & centrifuge

* add Altair + Algol migration

* Add Liquidity Pools related pallets to centrifuge runtime (#1523)

* centrifuge: Add liquidity pools pallets to runtime

* centrifuge: Add LP OutboundQueue wrapper to runtime

* taplo: Obey

* runtime: Move OutboundQueue import

* centrifuge: Allow both root and half of council as admin origins for LP pallets

* centrifuge: Add Axelar Gateway precompile

* centrifuge: Use correct HRMP encoder, update HRMP fee to 1 DOT, use XCM V3 imports

* centrifuge: Update error message for InboundQueue

* clippy: Fix warnings

* centrifuge: Rename outbound queue, fix migration import for orml asset registry

* Upgrade1020 LP follow-ups (#1525)

* centrifuge: Enable LP gateway XCM origin converter

* centrifuge: Update LP pallet config to use Quantity for BalanceRatio

* runtime: Update XCM origin converter, use Quantity in LP message, add OriginRecovery to LP gateway

* feat: safe XCM version migration + extend SafeCallFilter (#1526)

* fix: allow gateway process msg for xcm

* feat: add safe xcm version migration

* fix: Allow process_msg for xcm transact

* refactor: remove spec_version check

* refactor: stricter SafeCallFilter

* Upgrade1020 LP follow-ups (#1525)

* centrifuge: Enable LP gateway XCM origin converter

* centrifuge: Update LP pallet config to use Quantity for BalanceRatio

* runtime: Update XCM origin converter, use Quantity in LP message, add OriginRecovery to LP gateway

* fix: rm OrderBook safe call cause missing

* fix: OrderBook missing in both runtimes

* fix: clippy

---------

Co-authored-by: Cosmin Damian <17934949+cdamian@users.noreply.github.com>

* Feat/upgrade1020 misc 1 (#1529)

* fix: asset metadata xcm v3 migration cleanup other migrations (#1528)

* fix: centrifuge asset migration

* Apply suggestions from code review

Co-authored-by: Frederik Gartenmeister <mustermeiszer@posteo.de>

* chore: append cfg assets to catalyst

* fix: asset registry migration improvements

* feat: apply asset metadata migration to altair runtime

* fix: cleanup altair/algol migrations

* fix: less aggressive nuke pre-upgrade

* fix: finalize migrations, add context

* fix: clippy + cleanup imports

* fix: native currency altair, algol

* fix: Algol native currency name

---------

Co-authored-by: Frederik Gartenmeister <mustermeiszer@posteo.de>

* fmt

* fixup

* Upgrade1020 gateway changes (#1531)

* gateway: Use BoundedVec instead of EVMChain enum

* gateway: Add Sender type to LP gateway config

* gateway: Rename EVM chain size const, emit event when submitting message, adjust gateway sender provider

* Feat: upgrade1020 misc 2 (#1532)

* feat: account ensuring origin

* feat: bumb runtime version develpment

* fix: allow council to control gatway on Altair based chains

* fix: use EnsureSigned, make use of type and add admin

* feat: overestimated weights for lp logic

* fix: taplooo

* fix: tests and comment

* chore: bump weights for 1020 (#1533)

* chore: update centrifuge, altair weights

* feat: bump and improve dev weights

* fix: transfer_allowlist weights

* feat: normalize SafeCallFilter across runtimes

* fix: native asset key in migration

---------

Co-authored-by: William Freudenberger <w.freude@icloud.com>
Co-authored-by: Cosmin Damian <17934949+cdamian@users.noreply.github.com>
Co-authored-by: Frederik Gartenmeister <mustermeiszer@posteo.de>
Co-authored-by: nuno <hi@nunoalexandre.com>
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.

None yet

5 participants