Feat/lp v2 use gateway queue #1937

cdamian · 2024-07-31T14:07:08Z

Description

Use the new Gateway Queue pallet in LP and LP gateway.

Changes and Descriptions

Use pallet-liquidity-pools-gateway-queue for queueing inbound and outbound message in the LP gateway.
Add new handler traits for inbound and outbound messages that are implemented by LP and LP gateway respectively.

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
Address the benchmark weight suggestion here
Skip the LP gateway queue in as many integration tests as possible.
Replace the fudge runtime with the normal runtime in the integration tests, where possible.

* wip: add new msgs + reorder * refactor: apply renaming * fix: ITs * feat: UpdateRestriction message * fix: cancel_unprocessed_investment IT * fix: fmt * fix: clippy * tests: reanchor solidity to reorder branch * feat: apply hook to AddTranche msg * tests: fix pool_management ITs * wip: fix lp investments * fmt * fix: Tranche namespace * refactor: remove fulfilled from FulfilledRedeemRequest * chore: update lp submodule * minor cleanup * chore: update lp submodule * chore: add missing cleanup * fixes + ignore failing tests * fmt * tests: fix message indices

* add uts (#1905) * main changes for FI * fix correlation precission * minor renames * update investment UTs * update redemption UTs * miscelaneous UTs * minor renames * simplify correlation and embed to the only needed place * doc change * remove unused bound * swapping calls into entities.rs file * merge SwapDone methods into FulfilledSwapHook * fix events * working without pallet-swaps * remove boilerplate for removing entries * minor msg change * minor simplification * correct fulfilled sum after last collect * check OrderIdToSwapId storage * sending the message inside Info closure is not really a problem * send msgs from the entities * remove same currency check in impl.rs * unify hooks * remove pallet-swaps * minor fmt * add docs * add architecture diagram * return cancelled foreign amount from FI interface * update liquidity-pools * add messages to diagram * implement hooks * fix runtimes * adapt integration-tests * fix docs * fix clippy

* chore: update submodule to latest `main` 6d7f242c0dd83b1b5a4f6d506370a1f3ecbef9ce * wip: fix ITs * chore: update submodule * fix: remove sender param from `Transfer*` messages * chore: cleanup * docs: setup evm * fix: msg tests * fix: more ITs * fix: missing refactor after rebase * chore: update submodule to 223a0f36edabc675f8c74c47b20e366178df7ca3 * chore: improvements * fmt * Apply suggestions from code review * chore: bump spec_version * fmt: taplo

* custo serialize/deserialize for batch * compiling solution * serialization/deserialization working for batch message * remove gmpf changes * add batch nested protection * faster serialization for submessages * correct termination * add tests for deserialize batch of batch * add into_iter * remove unused Box and add pack methods * fix non_std compilation * non_std * fix max_encoded_len recursion issue * fix submodules * reduce batch limit

* refactor: add domain hook address storage * tests: add gateway lp admin account checks * refactor: use GetByKey * fix: outboundq mock * refactor: hook 20 bytes instead of 32

* pallets: Add LP Gateway queue pallet * lp-gateway-queue: Add benchmarks * integration-tests: Add LP gateway tests * docs: Update LP gateway queue entry * lp-gateway-queue: Use default weight for PostDispatchInfo * lp-gateway-queue: Add DEFAULT_WEIGHT_REF_TIME const, extract message processing logic, use BaseArithmetic for nonce * runtime: Add defensive weights for LP gateway queue * lint: Obey * taplo: Obey * pallet: Use DispatchResult for extrinsics * runtime: Update benchmarks and weight info * benchmarks: Add default for Message type (wip) * pallet: Add Default bound to Message type

* fix: add remark call to borrow proxy * fix: add missing message fields * chore: bump to v0.13.3 * chore: update submodule * chore: enable fixed tests --------- Co-authored-by: Frederik Gartenmeister <mustermeiszer@posteo.de>

lemunozm

Everything looks pretty good! I think gateway is much more organized now and love how the gateway interacts with the queue.

For reference to other reviewers:

MessageQueue trait is used by the gateway and implemented by the queue to enqueue both inbound and outbound messages.
MesageProcessor trait is used by the queue and implemented by the gateway to unqueue both inbound and outbound messages
OutboundMessageHandler liquidity-pools sending a outbound message to the gateway
InboundMessageHandler the gateway sending inbound messages to liquidity-pools

pallets/liquidity-pools-gateway/src/mock.rs

lemunozm · 2024-07-31T16:41:04Z

pallets/liquidity-pools-gateway/src/lib.rs

+	//TODO(cdamian): Migration?
+	// #[pallet::storage]
+	// #[pallet::getter(fn outbound_message_nonce_store)]
+	// pub type OutboundMessageNonceStore<T: Config> =
+	// 	StorageValue<_, T::OutboundMessageNonce, ValueQuery>;
+
+	//TODO(cdamian): Migration?
+	// /// Storage for outbound messages that will be processed during the
+	// /// `on_idle` hook.
+	// #[pallet::storage]
+	// #[pallet::getter(fn outbound_message_queue)]
+	// pub type OutboundMessageQueue<T: Config> = StorageMap<
+	// 	_,
+	// 	Blake2_128Concat,
+	// 	T::OutboundMessageNonce,
+	// 	(Domain, T::AccountId, T::Message),
+	// >;
+
+	//TODO(cdamian): Migration?
+	// /// Storage for failed outbound messages that can be manually re-triggered.
+	// #[pallet::storage]
+	// #[pallet::getter(fn failed_outbound_messages)]
+	// pub type FailedOutboundMessages<T: Config> = StorageMap<
+	// 	_,
+	// 	Blake2_128Concat,
+	// 	T::OutboundMessageNonce,
+	// 	(Domain, T::AccountId, T::Message, DispatchError),
+	// >;


Good question, I would say no if we're sure we do not have any content on it when the RU is executed. The message nonce is just to identify the message in order to process any pending/failing, right? I would say we can reset that number too and save us from writing an annoying migration.

Hmm, depends on how the RU update happens. I would imagine that it would happen after a block is produced, thus after an on_idle hook is called.

I meant over a human period of time. Basically we need to know if we need to maintain the message state because the users are using it at that moment or we can just assume that they will not be using it when the RU is enacted.

Asumming there're no storage or just purge it

@wischli, how do you see adding this to the current RU? THat way we avoid future migration problems because now we need to ensure we have not message there.

If we go with it, just we need to upgrade the version number of this pallet. And adding a reset migration to clean the Nonce storage. WDYT? Of course, I do not want to delay the RU, just if we have time to do it.

Forgot the above, I think we can ensure the same conditions easily in the next RU

My understanding was that we can assume there won't be any pending LP messages when we trigger the execution of the update. However, after triggering the RU, it takes one hour for it to be live. Right now, we cannot block those permissionless extrinsics which create outgoing messages. So theoretically, there is a chance. Let's discuss this in the Eng sync today.

So we're safe if I understand that correctly in the sync 😆

pallets/liquidity-pools-gateway/src/lib.rs

libs/types/src/gateway.rs

runtime/integration-tests/src/cases/liquidity_pools_gateway_queue.rs

* increase version for foreign investments * fix investment issue * fix solidity call for transfers_tokens * fix tests * minimize required tranche investors for IT * fix docs * fix docs * docs: fix hyperlink * refactor: enable ITs for centrifuge + dev runtimes (#1938) * fix: enable ITs for centrifuge + dev runtimes * fmt * fix: revert some centrifuge ITs * fmt * revert centrifuge addition to ITs --------- Co-authored-by: William Freudenberger <w.freude@icloud.com>

…es (wip)

…d specific inbound tests

lemunozm

Really really love the test improvements here. Just one minor comment in order to be more DRY

lemunozm · 2024-08-01T12:03:22Z

runtime/integration-tests/src/cases/lp/investments.rs

+					GatewayMessage::Outbound {
+						sender: <T as pallet_liquidity_pools_gateway::Config>::Sender::get(),
+						destination: Domain::EVM(EVM_DOMAIN_CHAIN_ID),
+						message: Message::FulfilledCancelDepositRequest {
+							pool_id: POOL_A,
+							tranche_id: pool_a_tranche_1_id::<T>(),
+							investor: vec_to_fixed_array(lp::utils::remote_account_of::<T>(
+								Keyring::TrancheInvestor(1)
+							)),
+							currency: utils::index_lp(evm, names::USDC),
+							currency_payout: DEFAULT_INVESTMENT_AMOUNT,
+							fulfilled_invest_amount: DEFAULT_INVESTMENT_AMOUNT,
+						},
+					},


Maybe here, the sender/destination can just be checked inside the process_outbound method, leaving each test as before without that overhead

I mean, I think for all cases the sender and destination are always the same

…Weight)

lemunozm · 2024-08-01T12:56:38Z

runtime/integration-tests/src/cases/routers.rs

 	});
 }

 #[test_runtimes(all)]
-fn submit_by_axelar_evm<T: Runtime + FudgeSupport>() {
+fn submit_by_axelar_evm<T: Runtime>() {


Thanks for realizing that this does not require xcm and then it can be done without fudge 💯

cdamian · 2024-08-01T14:57:21Z

Tons-o-conflicts 🎉 Gonna close this PR and open a new one after I manage to rebase successfully...

wischli and others added 20 commits July 12, 2024 14:09

Merge remote-tracking branch 'origin/main' into feat/lp-v2

426ae56

ignore failing tests (#1910)

fec46b1

fix clippy

8066e95

Merge remote-tracking branch 'origin/main' into feat/lp-v2

dd9f57d

fix tests after merge

6f46ba2

fix foreign investment tests (#1918)

ceda10d

ignore failing tests (#1919)

103b27e

Merge remote-tracking branch 'origin/main' into feat/lp-v2

63d7f05

fix previous merge

e7eb61c

Merge remote-tracking branch 'origin/main' into feat/lp-v2

25bfde7

feat: add domain hook storage (#1928)

020cf59

* refactor: add domain hook address storage * tests: add gateway lp admin account checks * refactor: use GetByKey * fix: outboundq mock * refactor: hook 20 bytes instead of 32

Merge remote-tracking branch 'origin/main' into feat/lp-v2

ecd8821

fix cargo fmt

5a63633

lp-v2: fix message fields (#1933)

d2118cb

* fix: add remark call to borrow proxy * fix: add missing message fields * chore: bump to v0.13.3 * chore: update submodule * chore: enable fixed tests --------- Co-authored-by: Frederik Gartenmeister <mustermeiszer@posteo.de>

refactor: cleanup my leftovers (#1935)

a38c4f9

lemunozm reviewed Jul 31, 2024

View reviewed changes

cdamian commented Jul 31, 2024

View reviewed changes

runtime/integration-tests/src/cases/liquidity_pools_gateway_queue.rs Outdated Show resolved Hide resolved

cdamian force-pushed the feat/lp-v2-use-gateway-queue branch from d9cac02 to 916c1e3 Compare July 31, 2024 21:08

cdamian marked this pull request as ready for review July 31, 2024 21:08

lemunozm reviewed Aug 1, 2024

View reviewed changes

runtime/integration-tests/src/cases/liquidity_pools_gateway_queue.rs Outdated Show resolved Hide resolved

lemunozm and others added 4 commits August 1, 2024 13:36

Merge remote-tracking branch 'origin/main' into feat/lp-v2

02a4af3

liquidity-pools: Use LP gateway queue for inbound and outbound messag…

8f08e88

…es (wip)

tests: wip

d6f8a00

cdamian added 11 commits August 1, 2024 14:46

lp-gateway-queue: Add TODO for benchmarks

c3cb7f0

lp-gateway: Add MaxEncodedLen of inbound Message

073507a

gateway: Move GatewayMessage to gateway pallet

ced49eb

integration-tests: Adapt existing tests to LP gateway queue usage, ad…

39f93f6

…d specific inbound tests

integration-tests: Add LP gateway queue tests

7795a95

unit-tests: Fix

3a0969f

integration-tests: Enable all runtimes for LP gateway queue tests

78aba4e

integration-tests: Skip the gateway queue for LP gateway router tests

423d620

integration-tests: Reduce visibility for LP utils module

24f514e

integration-tests: Drop fudge from router evm tests

8c334ea

integration-tests: Drop fudge from foreign investments tests

5b23d2a

cdamian force-pushed the feat/lp-v2-use-gateway-queue branch from 1e47f89 to 5b23d2a Compare August 1, 2024 11:51

lemunozm reviewed Aug 1, 2024

View reviewed changes

lp-gateway: Replace DispatchResultWithPostInfo with (DispatchResult, …

abf702b

…Weight)

lemunozm reviewed Aug 1, 2024

View reviewed changes

cdamian changed the base branch from feat/lp-v2 to main August 1, 2024 14:53

cdamian requested review from mustermeiszer and hieronx as code owners August 1, 2024 14:53

cdamian closed this Aug 1, 2024

lemunozm mentioned this pull request Aug 1, 2024

Feat/lp v2 use gateway queue #1943

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/lp v2 use gateway queue #1937

Feat/lp v2 use gateway queue #1937

cdamian commented Jul 31, 2024 •

edited

Loading

lemunozm left a comment

lemunozm Jul 31, 2024

cdamian Jul 31, 2024

lemunozm Aug 1, 2024

lemunozm Aug 1, 2024

lemunozm Aug 1, 2024

lemunozm Aug 1, 2024

wischli Aug 1, 2024

lemunozm Aug 1, 2024

lemunozm left a comment

lemunozm Aug 1, 2024 •

edited

Loading

lemunozm Aug 1, 2024

lemunozm Aug 1, 2024

cdamian commented Aug 1, 2024

Feat/lp v2 use gateway queue #1937

Feat/lp v2 use gateway queue #1937

Conversation

cdamian commented Jul 31, 2024 • edited Loading

Description

Changes and Descriptions

Checklist:

lemunozm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lemunozm left a comment

Choose a reason for hiding this comment

lemunozm Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdamian commented Aug 1, 2024

cdamian commented Jul 31, 2024 •

edited

Loading

lemunozm Aug 1, 2024 •

edited

Loading