Feat/lp v2 gateway multi router

[cdamian]

Description

This PR adds support for multiple routers in the LP gateway.

LP Gateway Pallet Changes

Extrinsics

• Added set_routers extrinsic that sets the set of valid routers for both inbound and outbound messages.
  • Apart from populating the according Routers storage, a new session ID is created to keep track of incoming messages or proofs.
• Added execute_message_recovery extrinsic that is used to manually increase the proof count for a particular message proof and router ID, and execute the message if the required proof count is met.

Message Processing

Inbound

For every inbound message that is received via the MessageReceiver implementation, we check the Allowlist storage to confirm that the origin is allowed to submit message, if successful, we queue an inbound gateway message that contains the necessary information for processing i.e. origin address, message and router identifier.

For every inbound gateway message we create an InboundEntry that specifies whether the message is a message type or proof type.

After validation, this InboundEntry is then added to storage, if a previous entry of this type is found, the expected counts are updated accordingly - for a message type, the total expected proof count is increased by the value required for that domain, similarly, for a proof type, the proof count is increased by 1.

If the PendingInboundEntries storage contains one message entry from the first router, and the required proof entries from each of the remaining routers, the counts are decreased and the message is forwarded to the InboundMessageHandler for execution.

Outbound

For every outbound message received from the called of the OutboundMessageHandler::handle we retrieve all routers for the destination Domain.

The queueing is done using the LP Gateway Queue as follows:

• the message is queued as-is using the first router.
• the proof for the message is queued using the remaining routers.

Upon eventual receipt of the queued messages/proofs from the LP Gateway Queue, they are sent for further processing to the MessageSender.

---

Other Notable Changes

• The LPEncoding trait now has 2 more methods for retrieving a message proof and converting a message type to a proof type.
• The RouterSupport trait was slightly adjusted to better fit with the requirement of providing a statically defined list of RouterIds for a specific domain.

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

[lemunozm]

I have not finished in deep the review yet. Leaving some thoughts regarding the session_id and how the entries are cleaned

[wischli]

Another beautiful PR to review! Really liking the explicit errors! A question and a few nits. I hope we have time to improve readability on the marathon matches before the audit 😅

[lemunozm]

First, amazing work here @cdamian, don't get scared for my lists of questions/NITs. I didn't find any fault 🙌🏻

My comments are mostly regarding some minor simplification of readabilidy improvements (it's already quite clear, but the topic is quite complex, so maybe we can help auditors a bit more). The algorithm itself is great and things overall are well organized! I feel like you realized or every minor detail, awesome!

[lemunozm]

Regarding the tests, I think we should simplify them and/or factorize them. 5k lines are too many lines to refactor tomorrow because something has changed 😅.

I think by using two messages and 2 routers in the test cases, we can have all the safety we're looking for.

At least, at this stage, where we actually only support 1 router 😆

[wischli]

Haven't gone through all tests yet and still feel a bit lost understanding the new intertwined connections together with #1958 and #1959.

[cdamian]

Regarding the tests, I think we should simplify them and/or factorize them. 5k lines are too many lines to refactor tomorrow because something has changed 😅.

I think by using two messages and 2 routers in the test cases, we can have all the safety we're looking for.

At least, at this stage, where we actually only support 1 router 😆

Might seem a bit of an overkill but when I initially dealt with the message processing logic, these tests helped me stay on the correct path. I would definitely keep the ones for two_routers all the way up to the four_messages ones since those confirm that multiple identical entries are processed accordingly. For the ones with the three_routers, I would only keep the three_message ones, since that covers the case where we process maximum one message.

I'm all for simplifying these and hopefully have some logic that generates messages + expected test results, but then again, that logic would also have to be tested and I'm not sure if we have time to deal with that given the remaining things that we have to do for the audit.

Maybe we can live with having ultra-verbose tests for the time being since IMO, it's a bit clear of what's tested and what's expected.

[lemunozm]

Much more readable @cdamian, thanks!! Just some thought about the message proofs methods, that maybe we can have an issue there that we're not catching in UTs

[lemunozm]

I think my naming confusion comes from here. In my mind:

// Nothing to do with proofs, just give the hash
fn message_hash(&self) -> MessageHash {
    //Independently of the kind of message, I should be able to get a hash of it
    keccak_256(&LPEncoding::serialize(self))
}

// Creates a new message containing the hash of message
// or, return the itself if it's already a proof, without recomputing the hash
fn message_proof(&self) -> Self {
    match self {
        Message::MessageProof(..) => self,
        other_msg => Message::MessageProof(other_msg.message_hash())
    }
}

[cdamian]

Not sure I follow "Nothing to do with proofs", the proof is a (keccak) hash right?

[lemunozm]

I mean, we have a hash of the message. At this point, what a proof is doesn't care. What proof is could not even be defined. Just we know about messages having hashes

Later, a new message carrying the message hash, called MessageProof, is added.

[lemunozm]

Just explaining my thoughts about this ^^

[wischli]

I think my naming confusion comes from here. In my mind:

// Nothing to do with proofs, just give the hash
fn message_hash(&self) -> MessageHash {
    //Independently of the kind of message, I should be able to get a hash of it
    keccak_256(&LPEncoding::serialize(self))
}

// Creates a new message containing the hash of message
// or, return the itself if it's already a proof, without recomputing the hash
fn message_proof(&self) -> Self {
    match self {
        Message::MessageProof(..) => self,
        other_msg => Message::MessageProof(other_msg.message_hash())
    }
}

Are you proposing this as an alternative @lemunozm ? AFAICT, this is something different than the current impl of get_proof which extracts the proof hash if it exists. Your method will always return a hash independent of the message variant.

[lemunozm]

I was just leaving thoughts of what I had in mind. No strong opinion on whether to change this or not. Enough noise for today by my side 😆.

I just saw it interesting to have the hash independent, to be able to index messages and things like that. Without the need to create first a message proof and extract the proof.

[lemunozm]

Maybe we can live with having ultra-verbose tests for the time being since IMO, it's a bit clear of what's tested and what's expected.

Agree @cdamian, let's keep it as tech-debt. Testing is not under audit.

[cdamian]

Gonna add the fixes for ITs on a different branch to reduce the noise here.

[codecov]

Codecov Report

Attention: Patch coverage is 76.73469% with 57 lines in your changes missing coverage. Please review.

Project coverage is 48.87%. Comparing base (e8f1efd) to head (89ffd0e).
Report is 1 commits behind head on main.

Files	Patch %	Lines
.../liquidity-pools-gateway/src/message_processing.rs	79.67%	38 Missing ⚠️
pallets/liquidity-pools-gateway/src/lib.rs	76.59%	11 Missing ⚠️
pallets/liquidity-pools/src/message.rs	0.00%	6 Missing ⚠️
pallets/axelar-router/src/lib.rs	0.00%	1 Missing ⚠️
runtime/common/src/routing.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1961      +/-   ##
==========================================
+ Coverage   47.76%   48.87%   +1.10%     
==========================================
  Files         183      183              
  Lines       12929    13143     +214     
==========================================
+ Hits         6176     6424     +248     
+ Misses       6753     6719      -34     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lemunozm]

Thanks @cosmin for flighting with this beast! And also for being so patient with all my questions/suggestions and NITs.

Amazing work about to be merged!

[wischli]

Thanks for all the effort you have put into slaying this enormous beast! IMO, we can put an end to this chapter and iron out any minor leftovers in a subsequent PR.