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

lp-gateway: Add extrinsics for initiating and disputing message recovery #1970

Merged
merged 11 commits into from
Aug 17, 2024
24 changes: 20 additions & 4 deletions libs/traits/src/liquidity_pools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ use frame_support::{dispatch::DispatchResult, weights::Weight};
use sp_runtime::DispatchError;
use sp_std::vec::Vec;

pub type Proof = [u8; 32];
/// Type that represents the hash of an LP message.
pub type MessageHash = [u8; 32];
cdamian marked this conversation as resolved.
Show resolved Hide resolved

/// An encoding & decoding trait for the purpose of meeting the
/// LiquidityPools General Message Passing Format
pub trait LPEncoding: Sized {
pub trait LPMessage: Sized {
cdamian marked this conversation as resolved.
Show resolved Hide resolved
fn serialize(&self) -> Vec<u8>;
fn deserialize(input: &[u8]) -> Result<Self, DispatchError>;

Expand All @@ -34,11 +35,26 @@ pub trait LPEncoding: Sized {
/// It's the identity message for composing messages with pack_with
fn empty() -> Self;

/// Retrieves the message proof hash, if the message is a proof type.
fn get_proof(&self) -> Option<Proof>;
/// Returns whether the message is a proof or not.
fn is_proof_message(&self) -> bool;
lemunozm marked this conversation as resolved.
Show resolved Hide resolved

/// Retrieves the message hash, if the message is a proof type.
fn get_message_hash(&self) -> MessageHash;

/// Converts the message into a message proof type.
fn to_proof_message(&self) -> Self;

/// Creates a message used for initiating message recovery.
///
/// Hash - hash of the message that should be recovered.
/// Router - the address of the recovery router.
fn initiate_recovery_message(hash: [u8; 32], router: [u8; 32]) -> Self;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should use MessageHash type in signature

Suggested change
fn initiate_recovery_message(hash: [u8; 32], router: [u8; 32]) -> Self;
fn initiate_recovery_message(hash: MessageHash, router: [u8; 32]) -> Self;

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'll add it in - #1974


/// Creates a message used for disputing message recovery.
///
/// Hash - hash of the message that should be disputed.
/// Router - the address of the recovery router.
fn dispute_recovery_message(hash: [u8; 32], router: [u8; 32]) -> Self;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should use MessageHash type in signature

Suggested change
fn dispute_recovery_message(hash: [u8; 32], router: [u8; 32]) -> Self;
fn dispute_recovery_message(hash: MessageHash, router: [u8; 32]) -> Self;

}

pub trait RouterProvider<Domain>: Sized {
Expand Down
165 changes: 145 additions & 20 deletions pallets/liquidity-pools-gateway/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ use core::fmt::Debug;

use cfg_primitives::LP_DEFENSIVE_WEIGHT;
use cfg_traits::liquidity_pools::{
InboundMessageHandler, LPEncoding, MessageProcessor, MessageQueue, MessageReceiver,
MessageSender, OutboundMessageHandler, Proof, RouterProvider,
InboundMessageHandler, LPMessage, MessageHash, MessageProcessor, MessageQueue, MessageReceiver,
MessageSender, OutboundMessageHandler, RouterProvider,
};
use cfg_types::domain_address::{Domain, DomainAddress};
use frame_support::{dispatch::DispatchResult, pallet_prelude::*};
Expand All @@ -44,7 +44,10 @@ use sp_arithmetic::traits::{BaseArithmetic, EnsureAddAssign, One};
use sp_runtime::SaturatedConversion;
use sp_std::{convert::TryInto, vec::Vec};

use crate::{message_processing::InboundEntry, weights::WeightInfo};
use crate::{
message_processing::{InboundEntry, ProofEntry},
weights::WeightInfo,
};

mod origin;
pub use origin::*;
Expand All @@ -63,7 +66,6 @@ mod tests;
#[frame_support::pallet]
pub mod pallet {
use super::*;
use crate::message_processing::ProofEntry;

const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

Expand Down Expand Up @@ -95,7 +97,7 @@ pub mod pallet {
type AdminOrigin: EnsureOrigin<<Self as frame_system::Config>::RuntimeOrigin>;

/// The Liquidity Pools message type.
type Message: LPEncoding
type Message: LPMessage
+ Clone
+ Debug
+ PartialEq
Expand Down Expand Up @@ -162,11 +164,54 @@ pub mod pallet {
hook_address: [u8; 20],
},

cdamian marked this conversation as resolved.
Show resolved Hide resolved
/// An inbound message was processed.
InboundMessageProcessed {
domain_address: DomainAddress,
message_hash: MessageHash,
router_id: T::RouterId,
},

/// An inbound message proof was processed.
InboundProofProcessed {
domain_address: DomainAddress,
message_hash: MessageHash,
router_id: T::RouterId,
},

/// An inbound message was executed.
InboundMessageExecuted {
domain_address: DomainAddress,
message_hash: MessageHash,
},

/// An outbound message was sent.
OutboundMessageSent {
domain_address: DomainAddress,
message_hash: MessageHash,
router_id: T::RouterId,
},

/// Message recovery was executed.
MessageRecoveryExecuted {
proof: Proof,
message_hash: MessageHash,
router_id: T::RouterId,
},

/// Message recovery was initiated.
MessageRecoveryInitiated {
domain: Domain,
message_hash: MessageHash,
recovery_router: [u8; 32],
messaging_router: T::RouterId,
},

/// Message recovery was disputed.
MessageRecoveryDisputed {
domain: Domain,
message_hash: MessageHash,
recovery_router: [u8; 32],
messaging_router: T::RouterId,
},
}

/// Storage for routers.
Expand Down Expand Up @@ -208,7 +253,7 @@ pub mod pallet {
pub type PendingInboundEntries<T: Config> = StorageDoubleMap<
_,
Blake2_128Concat,
Proof,
MessageHash,
Blake2_128Concat,
T::RouterId,
InboundEntry<T>,
Expand Down Expand Up @@ -249,6 +294,9 @@ pub mod pallet {
/// Unknown router.
UnknownRouter,

/// Messaging router not found.
MessagingRouterNotFound,

/// The router that sent the message is not the first one.
MessageExpectedFromFirstRouter,

Expand Down Expand Up @@ -431,7 +479,7 @@ pub mod pallet {
pub fn execute_message_recovery(
origin: OriginFor<T>,
domain_address: DomainAddress,
proof: Proof,
message_hash: MessageHash,
router_id: T::RouterId,
) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin)?;
Expand All @@ -448,8 +496,10 @@ pub mod pallet {

let session_id = SessionIdStore::<T>::get();

PendingInboundEntries::<T>::try_mutate(proof, router_id.clone(), |storage_entry| {
match storage_entry {
PendingInboundEntries::<T>::try_mutate(
message_hash,
router_id.clone(),
|storage_entry| match storage_entry {
Some(stored_inbound_entry) => {
stored_inbound_entry.increment_proof_count(session_id)
}
Expand All @@ -461,25 +511,103 @@ pub mod pallet {

Ok::<(), DispatchError>(())
}
}
})?;
},
)?;

let expected_proof_count = Self::get_expected_proof_count(&router_ids)?;

Self::execute_if_requirements_are_met(
proof,
message_hash,
&router_ids,
session_id,
expected_proof_count,
domain_address,
)?;

Self::deposit_event(Event::<T>::MessageRecoveryExecuted { proof, router_id });
Self::deposit_event(Event::<T>::MessageRecoveryExecuted {
message_hash,
router_id,
});

Ok(())
}

/// Sends a message that initiates a message recovery using the
/// messaging router.
///
/// Can only be called by `AdminOrigin`.
#[pallet::weight(T::WeightInfo::initiate_message_recovery())]
#[pallet::call_index(12)]
pub fn initiate_message_recovery(
cdamian marked this conversation as resolved.
Show resolved Hide resolved
origin: OriginFor<T>,
domain: Domain,
message_hash: MessageHash,
recovery_router: [u8; 32],
messaging_router: T::RouterId,
Comment on lines +542 to +546
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, and probably in any place where we're passing both domain and router_id, because router_id already carries the domain information

) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin)?;

let message = T::Message::initiate_recovery_message(message_hash, recovery_router);

Self::send_recovery_message(domain, message, messaging_router.clone())?;

Self::deposit_event(Event::<T>::MessageRecoveryInitiated {
domain,
message_hash,
recovery_router,
messaging_router,
});

Ok(())
}

/// Sends a message that disputes a message recovery using the
/// messaging router.
///
/// Can only be called by `AdminOrigin`.
#[pallet::weight(T::WeightInfo::dispute_message_recovery())]
#[pallet::call_index(13)]
pub fn dispute_message_recovery(
origin: OriginFor<T>,
domain: Domain,
message_hash: MessageHash,
recovery_router: [u8; 32],
messaging_router: T::RouterId,
Comment on lines +571 to +575
Copy link
Contributor

@lemunozm lemunozm Aug 17, 2024

Choose a reason for hiding this comment

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

Question. Should the domain field always be the domain belonging to the messaging_router passed?

If that is the case, we can extend RouterId: Into<Domain>, which is already implemented in common/routing.rs and save the user to write the domain parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

If agree, we can merge this anyways and do it in a next PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do it separately and not waste another CI run?

) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin)?;

let message = T::Message::dispute_recovery_message(message_hash, recovery_router);

Self::send_recovery_message(domain, message, messaging_router.clone())?;

Self::deposit_event(Event::<T>::MessageRecoveryDisputed {
domain,
message_hash,
recovery_router,
messaging_router,
});

Ok(())
}
}

impl<T: Config> Pallet<T> {
fn send_recovery_message(
domain: Domain,
message: T::Message,
messaging_router: T::RouterId,
) -> DispatchResult {
let router_ids = Self::get_router_ids_for_domain(domain)?;

ensure!(
router_ids.iter().any(|x| x == &messaging_router),
Error::<T>::MessagingRouterNotFound
);

T::MessageSender::send(messaging_router, T::Sender::get(), message.serialize())
cdamian marked this conversation as resolved.
Show resolved Hide resolved
}
}

impl<T: Config> OutboundMessageHandler for Pallet<T> {
type Destination = Domain;
type Message = T::Message;
Expand Down Expand Up @@ -534,12 +662,9 @@ pub mod pallet {

(res, weight)
}
GatewayMessage::Outbound {
sender,
message,
router_id,
} => {
let res = T::MessageSender::send(router_id, sender, message.serialize());
GatewayMessage::Outbound { message, router_id } => {
let res =
T::MessageSender::send(router_id, T::Sender::get(), message.serialize());

(res, LP_DEFENSIVE_WEIGHT)
}
Expand Down
1 change: 0 additions & 1 deletion pallets/liquidity-pools-gateway/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ pub enum GatewayMessage<Message, RouterId> {
router_id: RouterId,
},
Outbound {
sender: DomainAddress,
message: Message,
router_id: RouterId,
},
Expand Down
Loading
Loading