From ccfcee0eb2529b327968a0262ff4dfd987c86d66 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 22 Jul 2024 12:09:12 +0200 Subject: [PATCH 1/8] batch logic implementation --- libs/traits/src/liquidity_pools.rs | 21 ++++++ pallets/liquidity-pools-gateway/src/lib.rs | 79 +++++++++++++++++----- pallets/liquidity-pools/src/message.rs | 28 ++++---- 3 files changed, 97 insertions(+), 31 deletions(-) diff --git a/libs/traits/src/liquidity_pools.rs b/libs/traits/src/liquidity_pools.rs index ab747a9ea9..a9d9ea615f 100644 --- a/libs/traits/src/liquidity_pools.rs +++ b/libs/traits/src/liquidity_pools.rs @@ -20,6 +20,15 @@ use sp_std::vec::Vec; pub trait LPEncoding: Sized { fn serialize(&self) -> Vec; fn deserialize(input: &[u8]) -> Result; + + /// Compose this message with a new one + fn pack(&self, other: Self) -> Result; + + /// Decompose the message into a list of messages + fn unpack(&self) -> Vec; + + /// Creates an empty message + fn empty() -> Self; } #[cfg(any(test, feature = "std"))] @@ -43,6 +52,18 @@ pub mod test_util { None => Err("empty message".into()), } } + + fn pack(&self, _: Self) -> Result { + unimplemented!() + } + + fn unpack(&self) -> Vec { + unimplemented!() + } + + fn empty() -> Self { + unimplemented!() + } } } diff --git a/pallets/liquidity-pools-gateway/src/lib.rs b/pallets/liquidity-pools-gateway/src/lib.rs index c8c162388a..598306a6de 100644 --- a/pallets/liquidity-pools-gateway/src/lib.rs +++ b/pallets/liquidity-pools-gateway/src/lib.rs @@ -257,6 +257,10 @@ pub mod pallet { (Domain, T::AccountId, T::Message, DispatchError), >; + #[pallet::storage] + pub(crate) type PackedMessage = + StorageMap<_, Blake2_128Concat, (T::AccountId, Domain), T::Message>; + #[pallet::error] pub enum Error { /// Router initialization failed. @@ -517,7 +521,11 @@ pub mod pallet { } }; - T::InboundQueue::submit(domain_address, incoming_msg) + for msg in incoming_msg.unpack() { + T::InboundQueue::submit(domain_address.clone(), msg)?; + } + + Ok(()) } /// Convenience method for manually processing an outbound message. @@ -600,6 +608,30 @@ pub mod pallet { } } } + + #[pallet::call_index(8)] + #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] + pub fn start_pack_batch(origin: OriginFor, destination: Domain) -> DispatchResult { + let sender = ensure_signed(origin)?; + + if PackedMessage::::get((&sender, &destination)).is_none() { + PackedMessage::::insert((sender, destination), T::Message::empty()); + } + + Ok(()) + } + + #[pallet::call_index(9)] + #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] + pub fn end_pack_batch(origin: OriginFor, destination: Domain) -> DispatchResult { + let sender = ensure_signed(origin)?; + + if let Some(msg) = PackedMessage::::take((&sender, &destination)) { + Self::submit_message(destination, msg)?; + } + + Ok(()) + } } impl Pallet { @@ -773,24 +805,8 @@ pub mod pallet { .saturating_add(Weight::from_parts(0, pov_weight)) .saturating_add(extra_weight) } - } - /// This pallet will be the `OutboundQueue` used by other pallets to send - /// outgoing messages. - /// - /// NOTE - the sender provided as an argument is not used at the moment, we - /// are using the sender specified in the pallet config so that we can - /// ensure that the account is funded. - impl OutboundQueue for Pallet { - type Destination = Domain; - type Message = T::Message; - type Sender = T::AccountId; - - fn submit( - _sender: Self::Sender, - destination: Self::Destination, - message: Self::Message, - ) -> DispatchResult { + fn submit_message(destination: Domain, message: T::Message) -> DispatchResult { ensure!( destination != Domain::Centrifuge, Error::::DomainNotSupported @@ -820,4 +836,31 @@ pub mod pallet { Ok(()) } } + + /// This pallet will be the `OutboundQueue` used by other pallets to send + /// outgoing messages. + /// + /// NOTE - the sender provided as an argument is not used at the moment, we + /// are using the sender specified in the pallet config so that we can + /// ensure that the account is funded. + impl OutboundQueue for Pallet { + type Destination = Domain; + type Message = T::Message; + type Sender = T::AccountId; + + fn submit( + sender: Self::Sender, + destination: Self::Destination, + message: Self::Message, + ) -> DispatchResult { + match PackedMessage::::get((&sender, &destination)) { + Some(packed) => { + PackedMessage::::insert((sender, destination), packed.pack(message)?) + } + None => Self::submit_message(destination, message)?, + } + + Ok(()) + } + } } diff --git a/pallets/liquidity-pools/src/message.rs b/pallets/liquidity-pools/src/message.rs index 5bb8815b73..b2b96e7d56 100644 --- a/pallets/liquidity-pools/src/message.rs +++ b/pallets/liquidity-pools/src/message.rs @@ -529,9 +529,18 @@ pub enum Message { }, } -impl Message { - /// Compose this message with a new one - pub fn pack(&self, other: Self) -> Result { +impl Message {} + +impl LPEncoding for Message { + fn serialize(&self) -> Vec { + gmpf::to_vec(self).unwrap_or_default() + } + + fn deserialize(data: &[u8]) -> Result { + gmpf::from_slice(data).map_err(|_| DispatchError::Other("LP Deserialization issue")) + } + + fn pack(&self, other: Self) -> Result { Ok(match self.clone() { Message::Batch(content) => { let mut content = content.clone(); @@ -542,22 +551,15 @@ impl Message { }) } - /// Decompose the message into a list of messages - pub fn unpack(&self) -> Vec { + fn unpack(&self) -> Vec { match self { Message::Batch(content) => content.clone().into_iter().collect(), message => vec![message.clone()], } } -} -impl LPEncoding for Message { - fn serialize(&self) -> Vec { - gmpf::to_vec(self).unwrap_or_default() - } - - fn deserialize(data: &[u8]) -> Result { - gmpf::from_slice(data).map_err(|_| DispatchError::Other("LP Deserialization issue")) + fn empty() -> Message { + Message::Batch(BatchMessages::default()) } } From 7123ab8a9cdcc6298d917193e262fc8c338a283e Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 22 Jul 2024 12:39:41 +0200 Subject: [PATCH 2/8] some details --- libs/traits/src/liquidity_pools.rs | 6 ++-- pallets/liquidity-pools-gateway/src/lib.rs | 32 +++++++++++----------- pallets/liquidity-pools/src/lib.rs | 4 +++ 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/libs/traits/src/liquidity_pools.rs b/libs/traits/src/liquidity_pools.rs index a9d9ea615f..cd9b5f4fb1 100644 --- a/libs/traits/src/liquidity_pools.rs +++ b/libs/traits/src/liquidity_pools.rs @@ -25,9 +25,11 @@ pub trait LPEncoding: Sized { fn pack(&self, other: Self) -> Result; /// Decompose the message into a list of messages + /// If the message is not decomposable, it returns the own message. fn unpack(&self) -> Vec; - /// Creates an empty message + /// Creates an empty message. + /// It's the identity message for composing messages fn empty() -> Self; } @@ -58,7 +60,7 @@ pub mod test_util { } fn unpack(&self) -> Vec { - unimplemented!() + vec![Self] } fn empty() -> Self { diff --git a/pallets/liquidity-pools-gateway/src/lib.rs b/pallets/liquidity-pools-gateway/src/lib.rs index 598306a6de..fd02da9a62 100644 --- a/pallets/liquidity-pools-gateway/src/lib.rs +++ b/pallets/liquidity-pools-gateway/src/lib.rs @@ -611,7 +611,7 @@ pub mod pallet { #[pallet::call_index(8)] #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] - pub fn start_pack_batch(origin: OriginFor, destination: Domain) -> DispatchResult { + pub fn start_pack_messages(origin: OriginFor, destination: Domain) -> DispatchResult { let sender = ensure_signed(origin)?; if PackedMessage::::get((&sender, &destination)).is_none() { @@ -622,12 +622,12 @@ pub mod pallet { } #[pallet::call_index(9)] - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] - pub fn end_pack_batch(origin: OriginFor, destination: Domain) -> DispatchResult { + #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] //TODO: benchmark me + pub fn end_pack_messages(origin: OriginFor, destination: Domain) -> DispatchResult { let sender = ensure_signed(origin)?; if let Some(msg) = PackedMessage::::take((&sender, &destination)) { - Self::submit_message(destination, msg)?; + Self::queue_message(destination, msg)?; } Ok(()) @@ -806,17 +806,7 @@ pub mod pallet { .saturating_add(extra_weight) } - fn submit_message(destination: Domain, message: T::Message) -> DispatchResult { - ensure!( - destination != Domain::Centrifuge, - Error::::DomainNotSupported - ); - - ensure!( - DomainRouters::::contains_key(destination.clone()), - Error::::RouterNotFound - ); - + fn queue_message(destination: Domain, message: T::Message) -> DispatchResult { let nonce = >::try_mutate(|n| { n.ensure_add_assign(T::OutboundMessageNonce::one())?; Ok::(*n) @@ -853,11 +843,21 @@ pub mod pallet { destination: Self::Destination, message: Self::Message, ) -> DispatchResult { + ensure!( + destination != Domain::Centrifuge, + Error::::DomainNotSupported + ); + + ensure!( + DomainRouters::::contains_key(destination.clone()), + Error::::RouterNotFound + ); + match PackedMessage::::get((&sender, &destination)) { Some(packed) => { PackedMessage::::insert((sender, destination), packed.pack(message)?) } - None => Self::submit_message(destination, message)?, + None => Self::queue_message(destination, message)?, } Ok(()) diff --git a/pallets/liquidity-pools/src/lib.rs b/pallets/liquidity-pools/src/lib.rs index 889878b42b..4d1106f621 100644 --- a/pallets/liquidity-pools/src/lib.rs +++ b/pallets/liquidity-pools/src/lib.rs @@ -347,6 +347,9 @@ pub mod pallet { InvestorDomainAddressNotAMember, /// Only the PoolAdmin can execute a given operation. NotPoolAdmin, + /// This pallet does not expect to receive direclty a batch message, + /// instead it expects several calls to it with different messages. + UnsupportedBatchMessage, } #[pallet::call] @@ -1038,6 +1041,7 @@ pub mod pallet { currency.into(), sender, ), + Message::Batch(_) => Err(Error::::UnsupportedBatchMessage.into()), _ => Err(Error::::InvalidIncomingMessage.into()), }?; From c35810ea58a730e57d84f02fb6cdc353a81f39ad Mon Sep 17 00:00:00 2001 From: lemunozm Date: Wed, 24 Jul 2024 15:09:22 +0200 Subject: [PATCH 3/8] Erroring out and mutable() --- pallets/liquidity-pools-gateway/src/lib.rs | 39 +++++++++++++++------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/pallets/liquidity-pools-gateway/src/lib.rs b/pallets/liquidity-pools-gateway/src/lib.rs index fd02da9a62..9a55f4aef8 100644 --- a/pallets/liquidity-pools-gateway/src/lib.rs +++ b/pallets/liquidity-pools-gateway/src/lib.rs @@ -305,6 +305,14 @@ pub mod pallet { /// Failed outbound message not found in storage. FailedOutboundMessageNotFound, + + /// Emitted when you call `start_pack_message()` but that was already + /// called. You should finalize the message with `end_pack_message()` + MessagePackingAlreadyStarted, + + /// Emitted when you can `end_pack_message()` but the packing process + /// was not started by `start_pack_message()`. + MessagePackingNotStarted, } #[pallet::hooks] @@ -424,6 +432,8 @@ pub mod pallet { /// Process an incoming message. #[pallet::weight(T::WeightInfo::process_msg())] #[pallet::call_index(5)] + // TODO: can we modify the name to receive_message() ? + // TODO: benchmark me again with max batch limit message pub fn process_msg( origin: OriginFor, msg: BoundedVec, @@ -543,7 +553,7 @@ pub mod pallet { let (domain, sender, message) = OutboundMessageQueue::::take(nonce) .ok_or(Error::::OutboundMessageNotFound)?; - match Self::process_message(domain.clone(), sender.clone(), message.clone()) { + match Self::send_message(domain.clone(), sender.clone(), message.clone()) { Ok(_) => { Self::deposit_event(Event::::OutboundMessageExecutionSuccess { nonce, @@ -582,7 +592,7 @@ pub mod pallet { let (domain, sender, message, _) = FailedOutboundMessages::::get(nonce) .ok_or(Error::::OutboundMessageNotFound)?; - match Self::process_message(domain.clone(), sender.clone(), message.clone()) { + match Self::send_message(domain.clone(), sender.clone(), message.clone()) { Ok(_) => { Self::deposit_event(Event::::OutboundMessageExecutionSuccess { nonce, @@ -610,13 +620,17 @@ pub mod pallet { } #[pallet::call_index(8)] - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] + #[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1, 1).ref_time())] pub fn start_pack_messages(origin: OriginFor, destination: Domain) -> DispatchResult { let sender = ensure_signed(origin)?; - if PackedMessage::::get((&sender, &destination)).is_none() { - PackedMessage::::insert((sender, destination), T::Message::empty()); - } + PackedMessage::::mutate((&sender, &destination), |msg| match msg { + Some(_) => return Err(Error::::MessagePackingAlreadyStarted), + None => { + *msg = Some(T::Message::empty()); + Ok(()) + } + })?; Ok(()) } @@ -626,11 +640,11 @@ pub mod pallet { pub fn end_pack_messages(origin: OriginFor, destination: Domain) -> DispatchResult { let sender = ensure_signed(origin)?; - if let Some(msg) = PackedMessage::::take((&sender, &destination)) { - Self::queue_message(destination, msg)?; + match PackedMessage::::take((&sender, &destination)) { + Some(msg) if msg.unpack().is_empty() => Ok(()), //No-op + Some(msg) => Self::queue_message(destination, msg), + None => Err(Error::::MessagePackingNotStarted.into()), } - - Ok(()) } } @@ -687,7 +701,7 @@ pub mod pallet { processed_entries.push(nonce); let weight = - match Self::process_message(domain.clone(), sender.clone(), message.clone()) { + match Self::send_message(domain.clone(), sender.clone(), message.clone()) { Ok(post_info) => { Self::deposit_event(Event::OutboundMessageExecutionSuccess { nonce, @@ -750,7 +764,7 @@ pub mod pallet { /// Retrieves the router stored for the provided domain and sends the /// message, calculating and returning the required weight for these /// operations in the `DispatchResultWithPostInfo`. - fn process_message( + fn send_message( domain: Domain, sender: T::AccountId, message: T::Message, @@ -794,6 +808,7 @@ pub mod pallet { router_call_weight: Option, extra_weight: Weight, ) -> Weight { + // TODO: add variable weights depending on the message size. let pov_weight: u64 = (Domain::max_encoded_len() + T::AccountId::max_encoded_len() + T::Message::max_encoded_len()) From 254289ba57f58b0b6828c5ab16a30dd565125db5 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Wed, 24 Jul 2024 22:50:56 +0200 Subject: [PATCH 4/8] correct message weight computation and simplify send_message method --- pallets/liquidity-pools-gateway/src/lib.rs | 121 ++++++++------------- 1 file changed, 44 insertions(+), 77 deletions(-) diff --git a/pallets/liquidity-pools-gateway/src/lib.rs b/pallets/liquidity-pools-gateway/src/lib.rs index 9a55f4aef8..8719765e0c 100644 --- a/pallets/liquidity-pools-gateway/src/lib.rs +++ b/pallets/liquidity-pools-gateway/src/lib.rs @@ -553,7 +553,7 @@ pub mod pallet { let (domain, sender, message) = OutboundMessageQueue::::take(nonce) .ok_or(Error::::OutboundMessageNotFound)?; - match Self::send_message(domain.clone(), sender.clone(), message.clone()) { + match Self::send_message(domain.clone(), sender.clone(), message.clone()).0 { Ok(_) => { Self::deposit_event(Event::::OutboundMessageExecutionSuccess { nonce, @@ -564,16 +564,16 @@ pub mod pallet { Ok(()) } - Err(e) => { + Err(err) => { Self::deposit_event(Event::::OutboundMessageExecutionFailure { nonce, domain: domain.clone(), sender: sender.clone(), message: message.clone(), - error: e.error, + error: err, }); - FailedOutboundMessages::::insert(nonce, (domain, sender, message, e.error)); + FailedOutboundMessages::::insert(nonce, (domain, sender, message, err)); Ok(()) } @@ -592,7 +592,7 @@ pub mod pallet { let (domain, sender, message, _) = FailedOutboundMessages::::get(nonce) .ok_or(Error::::OutboundMessageNotFound)?; - match Self::send_message(domain.clone(), sender.clone(), message.clone()) { + match Self::send_message(domain.clone(), sender.clone(), message.clone()).0 { Ok(_) => { Self::deposit_event(Event::::OutboundMessageExecutionSuccess { nonce, @@ -605,13 +605,13 @@ pub mod pallet { Ok(()) } - Err(e) => { + Err(err) => { Self::deposit_event(Event::::OutboundMessageExecutionFailure { nonce, domain: domain.clone(), sender: sender.clone(), message: message.clone(), - error: e.error, + error: err, }); Ok(()) @@ -702,7 +702,7 @@ pub mod pallet { let weight = match Self::send_message(domain.clone(), sender.clone(), message.clone()) { - Ok(post_info) => { + (Ok(()), weight) => { Self::deposit_event(Event::OutboundMessageExecutionSuccess { nonce, sender, @@ -710,40 +710,34 @@ pub mod pallet { message, }); - post_info - .actual_weight - .expect("Message processing success already ensured") - // Extra weight breakdown: - // - // 1 read for the outbound message - // 1 write for the event - // 1 write for the outbound message removal - .saturating_add(T::DbWeight::get().reads_writes(1, 2)) + // Extra weight breakdown: + // + // 1 read for the outbound message + // 1 write for the event + // 1 write for the outbound message removal + weight.saturating_add(T::DbWeight::get().reads_writes(1, 2)) } - Err(e) => { + (Err(err), weight) => { Self::deposit_event(Event::OutboundMessageExecutionFailure { nonce, sender: sender.clone(), domain: domain.clone(), message: message.clone(), - error: e.error, + error: err, }); FailedOutboundMessages::::insert( nonce, - (domain, sender, message, e.error), + (domain, sender, message, err), ); - e.post_info - .actual_weight - .expect("Message processing success already ensured") - // Extra weight breakdown: - // - // 1 read for the outbound message - // 1 write for the event - // 1 write for the failed outbound message - // 1 write for the outbound message removal - .saturating_add(T::DbWeight::get().reads_writes(1, 3)) + // Extra weight breakdown: + // + // 1 read for the outbound message + // 1 write for the event + // 1 write for the failed outbound message + // 1 write for the outbound message removal + weight.saturating_add(T::DbWeight::get().reads_writes(1, 3)) } }; @@ -768,57 +762,30 @@ pub mod pallet { domain: Domain, sender: T::AccountId, message: T::Message, - ) -> DispatchResultWithPostInfo { + ) -> (DispatchResult, Weight) { let read_weight = T::DbWeight::get().reads(1); - let router = DomainRouters::::get(domain).ok_or(DispatchErrorWithPostInfo { - post_info: PostDispatchInfo { - actual_weight: Some(read_weight), - pays_fee: Pays::Yes, - }, - error: Error::::RouterNotFound.into(), - })?; + let Some(router) = DomainRouters::::get(domain) else { + return (Err(Error::::RouterNotFound.into()), read_weight); + }; - let post_dispatch_info_fn = - |actual_weight: Option, extra_weight: Weight| -> PostDispatchInfo { - PostDispatchInfo { - actual_weight: Some(Self::get_outbound_message_processing_weight( - actual_weight, - extra_weight, - )), - pays_fee: Pays::Yes, - } - }; - - match router.send(sender, message.serialize()) { - Ok(dispatch_info) => Ok(post_dispatch_info_fn( - dispatch_info.actual_weight, - read_weight, - )), - Err(e) => Err(DispatchErrorWithPostInfo { - post_info: post_dispatch_info_fn(e.post_info.actual_weight, read_weight), - error: e.error, - }), - } - } + let serialized = message.serialize(); + + let message_weight = + read_weight.saturating_add(Weight::from_parts(0, serialized.len() as u64)); + + let (result, router_weight) = match router.send(sender, serialized) { + Ok(dispatch_info) => (Ok(()), dispatch_info.actual_weight), + Err(e) => (Err(e.error), e.post_info.actual_weight), + }; - /// Calculates the weight used by a router when processing an outbound - /// message. - fn get_outbound_message_processing_weight( - router_call_weight: Option, - extra_weight: Weight, - ) -> Weight { - // TODO: add variable weights depending on the message size. - let pov_weight: u64 = (Domain::max_encoded_len() - + T::AccountId::max_encoded_len() - + T::Message::max_encoded_len()) - .try_into() - .expect("can calculate outbound message POV weight"); - - router_call_weight - .unwrap_or(Weight::from_parts(DEFAULT_WEIGHT_REF_TIME, 0)) - .saturating_add(Weight::from_parts(0, pov_weight)) - .saturating_add(extra_weight) + ( + result, + router_weight + .unwrap_or(Weight::from_parts(DEFAULT_WEIGHT_REF_TIME, 0)) + .saturating_add(message_weight) + .saturating_add(read_weight), + ) } fn queue_message(destination: Domain, message: T::Message) -> DispatchResult { From a0801308eb9c7ce69e23325e9b8186c4c76dd928 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 29 Jul 2024 09:45:42 +0200 Subject: [PATCH 5/8] receive_message with correct weights --- libs/traits/src/liquidity_pools.rs | 4 + .../axelar-gateway-precompile/src/lib.rs | 5 +- pallets/liquidity-pools-gateway/src/lib.rs | 224 +++++++++--------- pallets/liquidity-pools-gateway/src/tests.rs | 79 +++--- .../liquidity-pools-gateway/src/weights.rs | 4 +- pallets/liquidity-pools/src/message.rs | 2 + 6 files changed, 170 insertions(+), 148 deletions(-) diff --git a/libs/traits/src/liquidity_pools.rs b/libs/traits/src/liquidity_pools.rs index cd9b5f4fb1..27ec1da7d0 100644 --- a/libs/traits/src/liquidity_pools.rs +++ b/libs/traits/src/liquidity_pools.rs @@ -18,6 +18,8 @@ use sp_std::vec::Vec; /// An encoding & decoding trait for the purpose of meeting the /// LiquidityPools General Message Passing Format pub trait LPEncoding: Sized { + const MAX_PACKED_MESSAGES: u32; + fn serialize(&self) -> Vec; fn deserialize(input: &[u8]) -> Result; @@ -43,6 +45,8 @@ pub mod test_util { #[derive(Debug, Eq, PartialEq, Clone, Encode, Decode, TypeInfo, MaxEncodedLen)] pub struct Message; impl LPEncoding for Message { + const MAX_PACKED_MESSAGES: u32 = 1; + fn serialize(&self) -> Vec { vec![0x42] } diff --git a/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/lib.rs b/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/lib.rs index dcd1d3b134..39b53e8548 100644 --- a/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/lib.rs +++ b/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/lib.rs @@ -287,8 +287,9 @@ where exit_status: ExitError::Other("account bytes mismatch for domain".into()), })?; - match pallet_liquidity_pools_gateway::Pallet::::process_msg( - pallet_liquidity_pools_gateway::GatewayOrigin::Domain(domain_address).into(), + match pallet_liquidity_pools_gateway::Pallet::::receive_message( + pallet_liquidity_pools_gateway::GatewayOrigin::Domain(domain_address.clone()).into(), + pallet_liquidity_pools_gateway::GatewayOrigin::Domain(domain_address), msg, ) .map(|_| ()) diff --git a/pallets/liquidity-pools-gateway/src/lib.rs b/pallets/liquidity-pools-gateway/src/lib.rs index 8719765e0c..4c482c6ed1 100644 --- a/pallets/liquidity-pools-gateway/src/lib.rs +++ b/pallets/liquidity-pools-gateway/src/lib.rs @@ -40,7 +40,7 @@ use frame_system::{ }; pub use pallet::*; use parity_scale_codec::{EncodeLike, FullCodec}; -use sp_runtime::traits::{AtLeast32BitUnsigned, EnsureAdd, EnsureAddAssign, One}; +use sp_runtime::traits::{AtLeast32BitUnsigned, BadOrigin, EnsureAdd, EnsureAddAssign, One}; use sp_std::{convert::TryInto, vec::Vec}; use crate::weights::WeightInfo; @@ -82,9 +82,6 @@ pub mod pallet { /// https://github.com/centrifuge/centrifuge-chain/pull/1696#discussion_r1456370592 const DEFAULT_WEIGHT_REF_TIME: u64 = 5_000_000_000; - use frame_support::dispatch::PostDispatchInfo; - use sp_runtime::DispatchErrorWithPostInfo; - use super::*; use crate::RelayerMessageDecodingError::{ MalformedMessage, MalformedSourceAddress, MalformedSourceAddressLength, @@ -272,9 +269,6 @@ pub mod pallet { /// The domain is not supported. DomainNotSupported, - /// Message decoding error. - MessageDecodingFailed, - /// Instance was already added to the domain. InstanceAlreadyAdded, @@ -430,109 +424,26 @@ pub mod pallet { } /// Process an incoming message. - #[pallet::weight(T::WeightInfo::process_msg())] + #[pallet::weight(T::WeightInfo::receive_message().saturating_mul( + Pallet::::deserialize_message(expected_gateway_origin.clone(), msg.clone()) + .map(|(_, msg)| msg.unpack().len() as u64) + .unwrap_or(T::Message::MAX_PACKED_MESSAGES as u64) + ) + )] #[pallet::call_index(5)] - // TODO: can we modify the name to receive_message() ? - // TODO: benchmark me again with max batch limit message - pub fn process_msg( + pub fn receive_message( origin: OriginFor, + expected_gateway_origin: GatewayOrigin, msg: BoundedVec, ) -> DispatchResult { - let (domain_address, incoming_msg) = match T::LocalEVMOrigin::ensure_origin(origin)? { - GatewayOrigin::Domain(domain_address) => { - Pallet::::validate(domain_address, msg)? - } - GatewayOrigin::AxelarRelay(domain_address) => { - // Every axelar relay address has a separate storage - ensure!( - RelayerList::::contains_key(domain_address.domain(), domain_address), - Error::::UnknownRelayer - ); + let gateway_origin = T::LocalEVMOrigin::ensure_origin(origin)?; - // Every axelar relay will prepend the (sourceChain, - // sourceAddress) from actual origination chain to the - // message bytes, with a length identifier - let slice_ref = &mut msg.as_slice(); - let length_source_chain: usize = Pallet::::try_range( - slice_ref, - BYTES_U32, - Error::::from(MalformedSourceChainLength), - |be_bytes_u32| { - let mut bytes = [0u8; BYTES_U32]; - // NOTE: This can NEVER panic as the `try_range` logic ensures the given - // bytes have the right length. I.e. 4 in this case - bytes.copy_from_slice(be_bytes_u32); + ensure!(gateway_origin == expected_gateway_origin, BadOrigin); - u32::from_be_bytes(bytes).try_into().map_err(|_| { - DispatchError::Other("Expect: usize in wasm is always ge u32") - }) - }, - )?; - - let source_chain = Pallet::::try_range( - slice_ref, - length_source_chain, - Error::::from(MalformedSourceChain), - |source_chain| Ok(source_chain.to_vec()), - )?; - - let length_source_address: usize = Pallet::::try_range( - slice_ref, - BYTES_U32, - Error::::from(MalformedSourceAddressLength), - |be_bytes_u32| { - let mut bytes = [0u8; BYTES_U32]; - // NOTE: This can NEVER panic as the `try_range` logic ensures the given - // bytes have the right length. I.e. 4 in this case - bytes.copy_from_slice(be_bytes_u32); - - u32::from_be_bytes(bytes).try_into().map_err(|_| { - DispatchError::Other("Expect: usize in wasm is always ge u32") - }) - }, - )?; - - let source_address = Pallet::::try_range( - slice_ref, - length_source_address, - Error::::from(MalformedSourceAddress), - |source_address| { - // NOTE: Axelar simply provides the hexadecimal string of an EVM - // address as the `sourceAddress` argument. Solidity does on the - // other side recognize the hex-encoding and encode the hex bytes - // to utf-8 bytes. - // - // Hence, we are reverting this process here. - let source_address = - cfg_utils::decode_var_source::(source_address) - .ok_or(Error::::from(MalformedSourceAddress))?; - - Ok(source_address.to_vec()) - }, - )?; - - let origin_msg = Pallet::::try_range( - slice_ref, - slice_ref.len(), - Error::::from(MalformedMessage), - |msg| { - BoundedVec::try_from(msg.to_vec()).map_err(|_| { - DispatchError::Other( - "Remaining bytes smaller vector in the first place. qed.", - ) - }) - }, - )?; - - let origin_domain = - T::OriginRecovery::try_convert((source_chain, source_address))?; - - Pallet::::validate(origin_domain, origin_msg)? - } - }; + let (origin_address, incoming_msg) = Self::deserialize_message(gateway_origin, msg)?; for msg in incoming_msg.unpack() { - T::InboundQueue::submit(domain_address.clone(), msg)?; + T::InboundQueue::submit(origin_address.clone(), msg)?; } Ok(()) @@ -652,13 +563,13 @@ pub mod pallet { pub(crate) fn try_range<'a, D, F>( slice: &mut &'a [u8], next_steps: usize, - error: Error, + error: impl Into, transformer: F, ) -> Result where F: Fn(&'a [u8]) -> Result, { - ensure!(slice.len() >= next_steps, error); + ensure!(slice.len() >= next_steps, error.into()); let (input, new_slice) = slice.split_at(next_steps); let res = transformer(input)?; @@ -667,23 +578,112 @@ pub mod pallet { Ok(res) } - fn validate( - address: DomainAddress, + fn deserialize_message( + gateway_origin: GatewayOrigin, msg: BoundedVec, ) -> Result<(DomainAddress, T::Message), DispatchError> { - if let DomainAddress::Centrifuge(_) = address { + let (origin_address, origin_msg) = match gateway_origin { + GatewayOrigin::Domain(domain_address) => (domain_address, msg), + GatewayOrigin::AxelarRelay(domain_address) => { + // Every axelar relay address has a separate storage + ensure!( + RelayerList::::contains_key(domain_address.domain(), domain_address), + Error::::UnknownRelayer + ); + + // Every axelar relay will prepend the (sourceChain, + // sourceAddress) from actual origination chain to the + // message bytes, with a length identifier + let slice_ref = &mut msg.as_slice(); + + let length_source_chain: usize = Pallet::::try_range( + slice_ref, + BYTES_U32, + Error::::from(MalformedSourceChainLength), + |be_bytes_u32| { + let mut bytes = [0u8; BYTES_U32]; + // NOTE: This can NEVER panic as the `try_range` logic ensures the given + // bytes have the right length. I.e. 4 in this case + bytes.copy_from_slice(be_bytes_u32); + + u32::from_be_bytes(bytes).try_into().map_err(|_| { + DispatchError::Other("Expect: usize in wasm is always ge u32") + }) + }, + )?; + + let source_chain = Pallet::::try_range( + slice_ref, + length_source_chain, + Error::::from(MalformedSourceChain), + |source_chain| Ok(source_chain.to_vec()), + )?; + + let length_source_address: usize = Pallet::::try_range( + slice_ref, + BYTES_U32, + Error::::from(MalformedSourceAddressLength), + |be_bytes_u32| { + let mut bytes = [0u8; BYTES_U32]; + // NOTE: This can NEVER panic as the `try_range` logic ensures the given + // bytes have the right length. I.e. 4 in this case + bytes.copy_from_slice(be_bytes_u32); + + u32::from_be_bytes(bytes).try_into().map_err(|_| { + DispatchError::Other("Expect: usize in wasm is always ge u32") + }) + }, + )?; + + let source_address = Pallet::::try_range( + slice_ref, + length_source_address, + Error::::from(MalformedSourceAddress), + |source_address| { + // NOTE: Axelar simply provides the hexadecimal string of an EVM + // address as the `sourceAddress` argument. Solidity does on the + // other side recognize the hex-encoding and encode the hex bytes + // to utf-8 bytes. + // + // Hence, we are reverting this process here. + let source_address = + cfg_utils::decode_var_source::(source_address) + .ok_or(Error::::from(MalformedSourceAddress))?; + + Ok(source_address.to_vec()) + }, + )?; + + let origin_msg = Pallet::::try_range( + slice_ref, + slice_ref.len(), + Error::::from(MalformedMessage), + |msg| { + BoundedVec::try_from(msg.to_vec()).map_err(|_| { + DispatchError::Other( + "Remaining bytes smaller vector in the first place. qed.", + ) + }) + }, + )?; + + let origin_domain = + T::OriginRecovery::try_convert((source_chain, source_address))?; + + (origin_domain, origin_msg) + } + }; + + if let DomainAddress::Centrifuge(_) = origin_address { return Err(Error::::InvalidMessageOrigin.into()); } ensure!( - Allowlist::::contains_key(address.domain(), address.clone()), + Allowlist::::contains_key(origin_address.domain(), origin_address.clone()), Error::::UnknownInstance, ); - let incoming_msg = T::Message::deserialize(msg.as_slice()) - .map_err(|_| Error::::MessageDecodingFailed)?; - - Ok((address, incoming_msg)) + Ok((origin_address, T::Message::deserialize(&origin_msg)?)) } /// Iterates over the outbound messages stored in the queue and attempts diff --git a/pallets/liquidity-pools-gateway/src/tests.rs b/pallets/liquidity-pools-gateway/src/tests.rs index ad4630d478..931d70f4d1 100644 --- a/pallets/liquidity-pools-gateway/src/tests.rs +++ b/pallets/liquidity-pools-gateway/src/tests.rs @@ -52,10 +52,10 @@ mod pallet_internals { Pallet::::try_range( &mut three_bytes.as_slice(), steps, - Error::::MessageDecodingFailed, + DispatchError::Other("err"), |_| Ok(()) ), - Error::::MessageDecodingFailed + DispatchError::Other("err"), ); }) } @@ -69,7 +69,7 @@ mod pallet_internals { let first_section = Pallet::::try_range( slice, steps, - Error::::MessageDecodingFailed, + DispatchError::Other("err"), |first_section| Ok(first_section), ) .expect("Slice is long enough"); @@ -80,7 +80,7 @@ mod pallet_internals { let second_section = Pallet::::try_range( slice, steps, - Error::::MessageDecodingFailed, + DispatchError::Other("err"), |second_section| Ok(second_section), ) .expect("Slice is long enough"); @@ -91,7 +91,7 @@ mod pallet_internals { let third_section = Pallet::::try_range( slice, steps, - Error::::MessageDecodingFailed, + DispatchError::Other("err"), |third_section| Ok(third_section), ) .expect("Slice is long enough"); @@ -109,7 +109,7 @@ mod pallet_internals { let first_section = Pallet::::try_range( slice, steps, - Error::::MessageDecodingFailed, + DispatchError::Other("err"), |first_section| Ok(first_section), ) .expect("Slice is long enough"); @@ -120,7 +120,7 @@ mod pallet_internals { assert!(Pallet::::try_range( slice, steps, - Error::::MessageDecodingFailed, + DispatchError::Other("err"), |_| Err::<(), _>(DispatchError::Corruption) ) .is_err()); @@ -408,8 +408,9 @@ mod process_msg_axelar_relay { let solidity_header = "0000000a657468657265756d2d320000002a307838353033623434353242663632333863433736436462454532323362343664373139366231633933"; let payload = [hex::decode(solidity_header).unwrap(), Message.serialize()].concat(); - assert_ok!(LiquidityPoolsGateway::process_msg( - GatewayOrigin::AxelarRelay(relayer_address).into(), + assert_ok!(LiquidityPoolsGateway::receive_message( + GatewayOrigin::AxelarRelay(relayer_address.clone()).into(), + GatewayOrigin::AxelarRelay(relayer_address), BoundedVec::::try_from(payload).unwrap() )); }) @@ -459,8 +460,9 @@ mod process_msg_axelar_relay { Ok(expected_domain_address.clone()) }); - assert_ok!(LiquidityPoolsGateway::process_msg( - GatewayOrigin::AxelarRelay(relayer_address).into(), + assert_ok!(LiquidityPoolsGateway::receive_message( + GatewayOrigin::AxelarRelay(relayer_address.clone()).into(), + GatewayOrigin::AxelarRelay(relayer_address), BoundedVec::::try_from(msg).unwrap() )); }); @@ -497,8 +499,9 @@ mod process_msg_axelar_relay { }); assert_noop!( - LiquidityPoolsGateway::process_msg( - GatewayOrigin::AxelarRelay(relayer_address).into(), + LiquidityPoolsGateway::receive_message( + GatewayOrigin::AxelarRelay(relayer_address.clone()).into(), + GatewayOrigin::AxelarRelay(relayer_address), BoundedVec::::try_from(msg).unwrap() ), Error::::RelayerMessageDecodingFailed { @@ -541,8 +544,9 @@ mod process_msg_axelar_relay { }); assert_noop!( - LiquidityPoolsGateway::process_msg( - GatewayOrigin::AxelarRelay(relayer_address).into(), + LiquidityPoolsGateway::receive_message( + GatewayOrigin::AxelarRelay(relayer_address.clone()).into(), + GatewayOrigin::AxelarRelay(relayer_address), BoundedVec::::try_from(msg).unwrap() ), Error::::UnknownInstance @@ -580,11 +584,12 @@ mod process_msg_axelar_relay { }); assert_noop!( - LiquidityPoolsGateway::process_msg( - GatewayOrigin::Domain(domain_address).into(), + LiquidityPoolsGateway::receive_message( + GatewayOrigin::Domain(domain_address.clone()).into(), + GatewayOrigin::Domain(domain_address), BoundedVec::::try_from(encoded_msg).unwrap() ), - Error::::MessageDecodingFailed, + DispatchError::Other("unsupported message"), ); }); } @@ -618,8 +623,9 @@ mod process_msg_axelar_relay { MockLiquidityPools::mock_submit(move |_, _| Err(err)); assert_noop!( - LiquidityPoolsGateway::process_msg( - GatewayOrigin::Domain(domain_address).into(), + LiquidityPoolsGateway::receive_message( + GatewayOrigin::Domain(domain_address.clone()).into(), + GatewayOrigin::Domain(domain_address), BoundedVec::::try_from(encoded_msg).unwrap() ), err, @@ -655,8 +661,9 @@ mod process_msg_domain { Ok(()) }); - assert_ok!(LiquidityPoolsGateway::process_msg( - GatewayOrigin::Domain(domain_address).into(), + assert_ok!(LiquidityPoolsGateway::receive_message( + GatewayOrigin::Domain(domain_address.clone()).into(), + GatewayOrigin::Domain(domain_address), BoundedVec::::try_from(encoded_msg).unwrap() )); }); @@ -667,9 +674,13 @@ mod process_msg_domain { new_test_ext().execute_with(|| { let encoded_msg = Message.serialize(); + let address = H160::from_slice(&get_test_account_id().as_slice()[..20]); + let domain_address = DomainAddress::EVM(0, address.into()); + assert_noop!( - LiquidityPoolsGateway::process_msg( + LiquidityPoolsGateway::receive_message( RuntimeOrigin::root(), + GatewayOrigin::Domain(domain_address), BoundedVec::::try_from(encoded_msg).unwrap() ), BadOrigin, @@ -684,8 +695,9 @@ mod process_msg_domain { let encoded_msg = Message.serialize(); assert_noop!( - LiquidityPoolsGateway::process_msg( - GatewayOrigin::Domain(domain_address).into(), + LiquidityPoolsGateway::receive_message( + GatewayOrigin::Domain(domain_address.clone()).into(), + GatewayOrigin::Domain(domain_address), BoundedVec::::try_from(encoded_msg).unwrap() ), Error::::InvalidMessageOrigin, @@ -701,8 +713,9 @@ mod process_msg_domain { let encoded_msg = Message.serialize(); assert_noop!( - LiquidityPoolsGateway::process_msg( - GatewayOrigin::Domain(domain_address).into(), + LiquidityPoolsGateway::receive_message( + GatewayOrigin::Domain(domain_address.clone()).into(), + GatewayOrigin::Domain(domain_address), BoundedVec::::try_from(encoded_msg).unwrap() ), Error::::UnknownInstance, @@ -724,11 +737,12 @@ mod process_msg_domain { let encoded_msg: Vec = vec![11]; assert_noop!( - LiquidityPoolsGateway::process_msg( - GatewayOrigin::Domain(domain_address).into(), + LiquidityPoolsGateway::receive_message( + GatewayOrigin::Domain(domain_address.clone()).into(), + GatewayOrigin::Domain(domain_address), BoundedVec::::try_from(encoded_msg).unwrap() ), - Error::::MessageDecodingFailed, + DispatchError::Other("unsupported message"), ); }); } @@ -758,8 +772,9 @@ mod process_msg_domain { }); assert_noop!( - LiquidityPoolsGateway::process_msg( - GatewayOrigin::Domain(domain_address).into(), + LiquidityPoolsGateway::receive_message( + GatewayOrigin::Domain(domain_address.clone()).into(), + GatewayOrigin::Domain(domain_address), BoundedVec::::try_from(encoded_msg).unwrap() ), err, diff --git a/pallets/liquidity-pools-gateway/src/weights.rs b/pallets/liquidity-pools-gateway/src/weights.rs index d72e6e0f91..1a7e4b7055 100644 --- a/pallets/liquidity-pools-gateway/src/weights.rs +++ b/pallets/liquidity-pools-gateway/src/weights.rs @@ -18,7 +18,7 @@ pub trait WeightInfo { fn remove_instance() -> Weight; fn add_relayer() -> Weight; fn remove_relayer() -> Weight; - fn process_msg() -> Weight; + fn receive_message() -> Weight; fn process_outbound_message() -> Weight; fn process_failed_outbound_message() -> Weight; } @@ -84,7 +84,7 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().writes(1)) } - fn process_msg() -> Weight { + fn receive_message() -> Weight { // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will // be replaced with real benchmark soon. // diff --git a/pallets/liquidity-pools/src/message.rs b/pallets/liquidity-pools/src/message.rs index b2b96e7d56..decd5448d0 100644 --- a/pallets/liquidity-pools/src/message.rs +++ b/pallets/liquidity-pools/src/message.rs @@ -532,6 +532,8 @@ pub enum Message { impl Message {} impl LPEncoding for Message { + const MAX_PACKED_MESSAGES: u32 = MAX_BATCH_MESSAGES; + fn serialize(&self) -> Vec { gmpf::to_vec(self).unwrap_or_default() } From ecc9b2671c96cd8a0dfdcff0d9e91171bd9247b6 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 29 Jul 2024 10:52:27 +0200 Subject: [PATCH 6/8] simplify header deserialization --- libs/utils/src/lib.rs | 20 +++ pallets/liquidity-pools-gateway/src/lib.rs | 126 +++++-------------- pallets/liquidity-pools-gateway/src/tests.rs | 91 -------------- 3 files changed, 54 insertions(+), 183 deletions(-) diff --git a/libs/utils/src/lib.rs b/libs/utils/src/lib.rs index a1f8bc7412..189cfe8b7e 100644 --- a/libs/utils/src/lib.rs +++ b/libs/utils/src/lib.rs @@ -16,6 +16,26 @@ use parity_scale_codec::Encode; use sp_std::cmp::min; +pub struct BufferReader<'a>(pub &'a [u8]); + +impl<'a> BufferReader<'a> { + pub fn read_bytes(&mut self, bytes: usize) -> Option<&[u8]> { + if self.0.len() < bytes { + return None; + } + + let (consumed, remaining) = self.0.split_at(bytes); + self.0 = remaining; + Some(consumed) + } + + pub fn read_array(&mut self) -> Option<&[u8; N]> { + let (consumed, remaining) = self.0.split_first_chunk::()?; + self.0 = remaining; + Some(consumed) + } +} + /// Build a fixed-size array using as many elements from `src` as possible /// without overflowing and ensuring that the array is 0 padded in the case /// where `src.len()` is smaller than S. diff --git a/pallets/liquidity-pools-gateway/src/lib.rs b/pallets/liquidity-pools-gateway/src/lib.rs index 4c482c6ed1..1a6fedcc2f 100644 --- a/pallets/liquidity-pools-gateway/src/lib.rs +++ b/pallets/liquidity-pools-gateway/src/lib.rs @@ -560,24 +560,6 @@ pub mod pallet { } impl Pallet { - pub(crate) fn try_range<'a, D, F>( - slice: &mut &'a [u8], - next_steps: usize, - error: impl Into, - transformer: F, - ) -> Result - where - F: Fn(&'a [u8]) -> Result, - { - ensure!(slice.len() >= next_steps, error.into()); - - let (input, new_slice) = slice.split_at(next_steps); - let res = transformer(input)?; - *slice = new_slice; - - Ok(res) - } - fn deserialize_message( gateway_origin: GatewayOrigin, msg: BoundedVec, @@ -594,83 +576,43 @@ pub mod pallet { // Every axelar relay will prepend the (sourceChain, // sourceAddress) from actual origination chain to the // message bytes, with a length identifier - let slice_ref = &mut msg.as_slice(); - - let length_source_chain: usize = Pallet::::try_range( - slice_ref, - BYTES_U32, - Error::::from(MalformedSourceChainLength), - |be_bytes_u32| { - let mut bytes = [0u8; BYTES_U32]; - // NOTE: This can NEVER panic as the `try_range` logic ensures the given - // bytes have the right length. I.e. 4 in this case - bytes.copy_from_slice(be_bytes_u32); - - u32::from_be_bytes(bytes).try_into().map_err(|_| { - DispatchError::Other("Expect: usize in wasm is always ge u32") - }) - }, - )?; - - let source_chain = Pallet::::try_range( - slice_ref, - length_source_chain, - Error::::from(MalformedSourceChain), - |source_chain| Ok(source_chain.to_vec()), - )?; - - let length_source_address: usize = Pallet::::try_range( - slice_ref, - BYTES_U32, - Error::::from(MalformedSourceAddressLength), - |be_bytes_u32| { - let mut bytes = [0u8; BYTES_U32]; - // NOTE: This can NEVER panic as the `try_range` logic ensures the given - // bytes have the right length. I.e. 4 in this case - bytes.copy_from_slice(be_bytes_u32); - - u32::from_be_bytes(bytes).try_into().map_err(|_| { - DispatchError::Other("Expect: usize in wasm is always ge u32") - }) - }, - )?; - - let source_address = Pallet::::try_range( - slice_ref, - length_source_address, - Error::::from(MalformedSourceAddress), - |source_address| { + let mut input = cfg_utils::BufferReader(msg.as_slice()); + + let length_source_chain = match input.read_array::() { + Some(bytes) => u32::from_be_bytes(*bytes), + None => Err(Error::::from(MalformedSourceChainLength))?, + }; + + let source_chain = match input.read_bytes(length_source_chain as usize) { + Some(bytes) => bytes.to_vec(), + None => Err(Error::::from(MalformedSourceChain))?, + }; + + let length_source_address = match input.read_array::() { + Some(bytes) => u32::from_be_bytes(*bytes), + None => Err(Error::::from(MalformedSourceAddressLength))?, + }; + + let source_address = match input.read_bytes(length_source_address as usize) { + Some(bytes) => { // NOTE: Axelar simply provides the hexadecimal string of an EVM - // address as the `sourceAddress` argument. Solidity does on the - // other side recognize the hex-encoding and encode the hex bytes - // to utf-8 bytes. + // address as the `sourceAddress` argument. Solidity does on + // the other side recognize the hex-encoding and + // encode the hex bytes to utf-8 bytes. // // Hence, we are reverting this process here. - let source_address = - cfg_utils::decode_var_source::(source_address) - .ok_or(Error::::from(MalformedSourceAddress))?; - - Ok(source_address.to_vec()) - }, - )?; - - let origin_msg = Pallet::::try_range( - slice_ref, - slice_ref.len(), - Error::::from(MalformedMessage), - |msg| { - BoundedVec::try_from(msg.to_vec()).map_err(|_| { - DispatchError::Other( - "Remaining bytes smaller vector in the first place. qed.", - ) - }) - }, - )?; - - let origin_domain = - T::OriginRecovery::try_convert((source_chain, source_address))?; - - (origin_domain, origin_msg) + cfg_utils::decode_var_source::(bytes) + .ok_or(Error::::from(MalformedSourceAddress))? + .to_vec() + } + None => Err(Error::::from(MalformedSourceAddress))?, + }; + + ( + T::OriginRecovery::try_convert((source_chain, source_address))?, + BoundedVec::try_from(input.0.to_vec()) + .map_err(|_| Error::::from(MalformedMessage))?, + ) } }; diff --git a/pallets/liquidity-pools-gateway/src/tests.rs b/pallets/liquidity-pools-gateway/src/tests.rs index 931d70f4d1..7cdfc8e647 100644 --- a/pallets/liquidity-pools-gateway/src/tests.rs +++ b/pallets/liquidity-pools-gateway/src/tests.rs @@ -38,97 +38,6 @@ mod utils { use utils::*; -mod pallet_internals { - - use super::*; - - #[test] - fn try_range_fails_if_slice_to_short() { - new_test_ext().execute_with(|| { - let three_bytes = [0u8; 3]; - let steps = 4usize; - - assert_noop!( - Pallet::::try_range( - &mut three_bytes.as_slice(), - steps, - DispatchError::Other("err"), - |_| Ok(()) - ), - DispatchError::Other("err"), - ); - }) - } - - #[test] - fn try_range_updates_slice_ref_correctly() { - new_test_ext().execute_with(|| { - let bytes = [1, 2, 3, 4, 5, 6, 7u8]; - let slice = &mut bytes.as_slice(); - let steps = 4; - let first_section = Pallet::::try_range( - slice, - steps, - DispatchError::Other("err"), - |first_section| Ok(first_section), - ) - .expect("Slice is long enough"); - - assert_eq!(first_section, &[1, 2, 3, 4]); - - let steps = 2; - let second_section = Pallet::::try_range( - slice, - steps, - DispatchError::Other("err"), - |second_section| Ok(second_section), - ) - .expect("Slice is long enough"); - - assert_eq!(&second_section, &[5, 6]); - - let steps = 1; - let third_section = Pallet::::try_range( - slice, - steps, - DispatchError::Other("err"), - |third_section| Ok(third_section), - ) - .expect("Slice is long enough"); - - assert_eq!(&third_section, &[7]); - }) - } - - #[test] - fn try_range_does_not_update_slice_if_transformer_errors() { - new_test_ext().execute_with(|| { - let bytes = [1, 2, 3, 4, 5, 6, 7u8]; - let slice = &mut bytes.as_slice(); - let steps = 4; - let first_section = Pallet::::try_range( - slice, - steps, - DispatchError::Other("err"), - |first_section| Ok(first_section), - ) - .expect("Slice is long enough"); - - assert_eq!(first_section, &[1, 2, 3, 4]); - - let steps = 1; - assert!(Pallet::::try_range( - slice, - steps, - DispatchError::Other("err"), - |_| Err::<(), _>(DispatchError::Corruption) - ) - .is_err()); - assert_eq!(slice, &[5, 6, 7]); - }) - } -} - mod set_domain_router { use super::*; From 1f8f074f830fdb4c0ee0fc24df803fafe3b66d95 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 29 Jul 2024 11:54:26 +0200 Subject: [PATCH 7/8] fix outbound weights --- libs/mocks/src/outbound_queue.rs | 4 + libs/traits/src/liquidity_pools.rs | 8 +- pallets/liquidity-pools-gateway/src/lib.rs | 16 +- .../liquidity-pools/src/defensive_weights.rs | 140 ------------------ pallets/liquidity-pools/src/lib.rs | 38 +++-- pallets/liquidity-pools/src/mock.rs | 1 - runtime/altair/src/lib.rs | 1 - runtime/centrifuge/src/lib.rs | 1 - runtime/development/src/lib.rs | 1 - 9 files changed, 40 insertions(+), 170 deletions(-) delete mode 100644 pallets/liquidity-pools/src/defensive_weights.rs diff --git a/libs/mocks/src/outbound_queue.rs b/libs/mocks/src/outbound_queue.rs index 986f4acd55..7a7d860f2f 100644 --- a/libs/mocks/src/outbound_queue.rs +++ b/libs/mocks/src/outbound_queue.rs @@ -33,5 +33,9 @@ pub mod pallet { fn submit(a: Self::Sender, b: Self::Destination, c: Self::Message) -> DispatchResult { execute_call!((a, b, c)) } + + fn weight() -> Weight { + Weight::zero() + } } } diff --git a/libs/traits/src/liquidity_pools.rs b/libs/traits/src/liquidity_pools.rs index 27ec1da7d0..6ce6e8d2c3 100644 --- a/libs/traits/src/liquidity_pools.rs +++ b/libs/traits/src/liquidity_pools.rs @@ -11,7 +11,10 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. -use frame_support::dispatch::{DispatchResult, DispatchResultWithPostInfo}; +use frame_support::{ + dispatch::{DispatchResult, DispatchResultWithPostInfo}, + pallet_prelude::Weight, +}; use sp_runtime::DispatchError; use sp_std::vec::Vec; @@ -102,6 +105,9 @@ pub trait OutboundQueue { destination: Self::Destination, msg: Self::Message, ) -> DispatchResult; + + /// Returns the required defensive weight to submit any message + fn weight() -> Weight; } /// The trait required for processing incoming messages. diff --git a/pallets/liquidity-pools-gateway/src/lib.rs b/pallets/liquidity-pools-gateway/src/lib.rs index 1a6fedcc2f..f20e3c7e12 100644 --- a/pallets/liquidity-pools-gateway/src/lib.rs +++ b/pallets/liquidity-pools-gateway/src/lib.rs @@ -531,7 +531,7 @@ pub mod pallet { } #[pallet::call_index(8)] - #[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1, 1).ref_time())] + #[pallet::weight(T::DbWeight::get().reads_writes(1, 1))] pub fn start_pack_messages(origin: OriginFor, destination: Domain) -> DispatchResult { let sender = ensure_signed(origin)?; @@ -547,13 +547,13 @@ pub mod pallet { } #[pallet::call_index(9)] - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] //TODO: benchmark me + #[pallet::weight(T::DbWeight::get().reads_writes(1, 1).saturating_add(Pallet::::weight()))] pub fn end_pack_messages(origin: OriginFor, destination: Domain) -> DispatchResult { let sender = ensure_signed(origin)?; match PackedMessage::::take((&sender, &destination)) { Some(msg) if msg.unpack().is_empty() => Ok(()), //No-op - Some(msg) => Self::queue_message(destination, msg), + Some(msg) => Self::enqueue_message(destination, msg), None => Err(Error::::MessagePackingNotStarted.into()), } } @@ -730,7 +730,7 @@ pub mod pallet { ) } - fn queue_message(destination: Domain, message: T::Message) -> DispatchResult { + fn enqueue_message(destination: Domain, message: T::Message) -> DispatchResult { let nonce = >::try_mutate(|n| { n.ensure_add_assign(T::OutboundMessageNonce::one())?; Ok::(*n) @@ -781,10 +781,16 @@ pub mod pallet { Some(packed) => { PackedMessage::::insert((sender, destination), packed.pack(message)?) } - None => Self::queue_message(destination, message)?, + None => Self::enqueue_message(destination, message)?, } Ok(()) } + + fn weight() -> Weight { + T::DbWeight::get() + .reads_writes(3, 4) + .saturating_add(Weight::from_parts(0, T::Message::max_encoded_len() as u64)) + } } } diff --git a/pallets/liquidity-pools/src/defensive_weights.rs b/pallets/liquidity-pools/src/defensive_weights.rs deleted file mode 100644 index 78501303a9..0000000000 --- a/pallets/liquidity-pools/src/defensive_weights.rs +++ /dev/null @@ -1,140 +0,0 @@ -// Copyright 2023 Centrifuge Foundation (centrifuge.io). -// This file is part of Centrifuge chain project. - -// Centrifuge is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version (see http://www.gnu.org/licenses). - -// Centrifuge is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -use frame_support::weights::{constants::RocksDbWeight, Weight}; - -pub trait WeightInfo { - fn add_pool() -> Weight; - fn add_tranche() -> Weight; - fn update_token_price() -> Weight; - fn update_member() -> Weight; - fn transfer() -> Weight; - fn set_domain_router() -> Weight; - fn schedule_upgrade() -> Weight; - fn cancel_upgrade() -> Weight; - fn update_tranche_token_metadata() -> Weight; -} - -// NOTE: We use temporary weights here. `execute_epoch` is by far our heaviest -// extrinsic. N denotes the number of tranches. 4 is quite heavy and -// should be enough. -const N: u64 = 4; - -impl WeightInfo for () { - fn set_domain_router() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } - - fn add_pool() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } - - fn add_tranche() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } - - fn update_token_price() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } - - fn update_member() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } - - fn transfer() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } - - fn schedule_upgrade() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } - - fn cancel_upgrade() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } - - fn update_tranche_token_metadata() -> Weight { - // NOTE: Defensive hardcoded weight taken from pool_system::execute_epoch. Will - // be replaced with real benchmark soon. - Weight::from_parts(124_979_771, 19974) - .saturating_add(Weight::from_parts(58_136_652, 0).saturating_mul(N)) - .saturating_add(RocksDbWeight::get().reads(8)) - .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) - .saturating_add(RocksDbWeight::get().writes(8)) - .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) - .saturating_add(Weight::from_parts(0, 17774).saturating_mul(N)) - } -} diff --git a/pallets/liquidity-pools/src/lib.rs b/pallets/liquidity-pools/src/lib.rs index 4d1106f621..bf7c5e990c 100644 --- a/pallets/liquidity-pools/src/lib.rs +++ b/pallets/liquidity-pools/src/lib.rs @@ -72,10 +72,6 @@ use staging_xcm::{ use crate::message::UpdateRestrictionMessage; -// NOTE: Should be replaced with generated weights in the future. For now, let's -// be defensive. -pub mod defensive_weights; - /// Serializer for the LiquidityPool's Generic Message Passing Format (GMPF) mod gmpf { mod de; @@ -125,16 +121,12 @@ pub mod pallet { use sp_runtime::{traits::Zero, DispatchError}; use super::*; - use crate::defensive_weights::WeightInfo; #[pallet::pallet] pub struct Pallet(_); #[pallet::config] pub trait Config: frame_system::Config { - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; - /// The source of truth for the balance of accounts in native currency. type Balance: Parameter + Member @@ -358,8 +350,8 @@ pub mod pallet { ::AccountId: From<[u8; 32]> + Into<[u8; 32]>, { /// Add a pool to a given domain - #[pallet::weight(T::WeightInfo::add_pool())] #[pallet::call_index(2)] + #[pallet::weight(Pallet::::outbound_weight(2, 0))] pub fn add_pool( origin: OriginFor, pool_id: T::PoolId, @@ -392,8 +384,8 @@ pub mod pallet { } /// Add a tranche to a given domain - #[pallet::weight(T::WeightInfo::add_tranche())] #[pallet::call_index(3)] + #[pallet::weight(Pallet::::outbound_weight(3, 0))] pub fn add_tranche( origin: OriginFor, pool_id: T::PoolId, @@ -444,8 +436,8 @@ pub mod pallet { /// domain, this call origin can be permissionless. /// /// The `currency_id` parameter is necessary for the EVM side. - #[pallet::weight(T::WeightInfo::update_token_price())] #[pallet::call_index(4)] + #[pallet::weight(Pallet::::outbound_weight(6, 0))] pub fn update_token_price( origin: OriginFor, pool_id: T::PoolId, @@ -489,8 +481,8 @@ pub mod pallet { } /// Update a member - #[pallet::weight(T::WeightInfo::update_member())] #[pallet::call_index(5)] + #[pallet::weight(Pallet::::outbound_weight(6, 0))] pub fn update_member( origin: OriginFor, pool_id: T::PoolId, @@ -546,8 +538,8 @@ pub mod pallet { /// /// NOTE: The transferring account is not kept alive as we allow its /// death. - #[pallet::weight(T::WeightInfo::transfer())] #[pallet::call_index(6)] + #[pallet::weight(Pallet::::outbound_weight(9, 3))] pub fn transfer_tranche_tokens( origin: OriginFor, pool_id: T::PoolId, @@ -607,8 +599,8 @@ pub mod pallet { /// /// NOTE: The transferring account is not kept alive as we allow its /// death. - #[pallet::weight(T::WeightInfo::transfer())] #[pallet::call_index(7)] + #[pallet::weight(Pallet::::outbound_weight(10, 5))] pub fn transfer( origin: OriginFor, currency_id: T::CurrencyId, @@ -674,8 +666,8 @@ pub mod pallet { /// Add a currency to the set of known currencies on the domain derived /// from the given currency. - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] #[pallet::call_index(8)] + #[pallet::weight(Pallet::::outbound_weight(4, 0))] pub fn add_currency(origin: OriginFor, currency_id: T::CurrencyId) -> DispatchResult { let who = ensure_signed(origin)?; @@ -698,7 +690,7 @@ pub mod pallet { /// Allow a currency to be used as a pool currency and to invest in a /// pool on the domain derived from the given currency. #[pallet::call_index(9)] - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] + #[pallet::weight(Pallet::::outbound_weight(4, 0))] pub fn allow_investment_currency( origin: OriginFor, pool_id: T::PoolId, @@ -733,8 +725,8 @@ pub mod pallet { } /// Schedule an upgrade of an EVM-based liquidity pool contract instance - #[pallet::weight(::WeightInfo::schedule_upgrade())] #[pallet::call_index(10)] + #[pallet::weight(Pallet::::outbound_weight(0, 0))] pub fn schedule_upgrade( origin: OriginFor, evm_chain_id: EVMChainId, @@ -750,8 +742,8 @@ pub mod pallet { } /// Schedule an upgrade of an EVM-based liquidity pool contract instance - #[pallet::weight(T::WeightInfo::cancel_upgrade())] #[pallet::call_index(11)] + #[pallet::weight(Pallet::::outbound_weight(0, 0))] pub fn cancel_upgrade( origin: OriginFor, evm_chain_id: EVMChainId, @@ -771,8 +763,8 @@ pub mod pallet { /// NOTE: Pulls the metadata from the `AssetRegistry` and thus requires /// the pool admin to have updated the tranche tokens metadata there /// beforehand. - #[pallet::weight(T::WeightInfo::update_tranche_token_metadata())] #[pallet::call_index(12)] + #[pallet::weight(Pallet::::outbound_weight(3, 0))] pub fn update_tranche_token_metadata( origin: OriginFor, pool_id: T::PoolId, @@ -807,7 +799,7 @@ pub mod pallet { /// Disallow a currency to be used as a pool currency and to invest in a /// pool on the domain derived from the given currency. #[pallet::call_index(13)] - #[pallet::weight(10_000 + T::DbWeight::get().writes(1).ref_time())] + #[pallet::weight(Pallet::::outbound_weight(4, 0))] pub fn disallow_investment_currency( origin: OriginFor, pool_id: T::PoolId, @@ -954,6 +946,12 @@ pub mod pallet { let domain_address = T::DomainAccountToDomainAddress::convert(domain_account); T::DomainAddressToAccountId::convert(domain_address) } + + fn outbound_weight(reads: u64, writes: u64) -> Weight { + Weight::from_parts(1_000_000, 256) //defensive base weights + .saturating_add(T::DbWeight::get().reads_writes(reads, writes)) + .saturating_add(T::OutboundQueue::weight()) + } } impl InboundQueue for Pallet diff --git a/pallets/liquidity-pools/src/mock.rs b/pallets/liquidity-pools/src/mock.rs index 5af36e7719..c85ee0aa91 100644 --- a/pallets/liquidity-pools/src/mock.rs +++ b/pallets/liquidity-pools/src/mock.rs @@ -149,5 +149,4 @@ impl pallet_liquidity_pools::Config for Runtime { type TrancheId = TrancheId; type TrancheTokenPrice = Pools; type TreasuryAccount = TreasuryAccount; - type WeightInfo = (); } diff --git a/runtime/altair/src/lib.rs b/runtime/altair/src/lib.rs index 2f1b8ab1e7..8aa03714fe 100644 --- a/runtime/altair/src/lib.rs +++ b/runtime/altair/src/lib.rs @@ -1807,7 +1807,6 @@ impl pallet_liquidity_pools::Config for Runtime { type TrancheId = TrancheId; type TrancheTokenPrice = PoolSystem; type TreasuryAccount = TreasuryAccount; - type WeightInfo = (); } parameter_types! { diff --git a/runtime/centrifuge/src/lib.rs b/runtime/centrifuge/src/lib.rs index bdd150d349..d1b7db241e 100644 --- a/runtime/centrifuge/src/lib.rs +++ b/runtime/centrifuge/src/lib.rs @@ -1887,7 +1887,6 @@ impl pallet_liquidity_pools::Config for Runtime { type TrancheId = TrancheId; type TrancheTokenPrice = PoolSystem; type TreasuryAccount = TreasuryAccount; - type WeightInfo = (); } parameter_types! { diff --git a/runtime/development/src/lib.rs b/runtime/development/src/lib.rs index 95daf86c25..04cb0a64c0 100644 --- a/runtime/development/src/lib.rs +++ b/runtime/development/src/lib.rs @@ -1907,7 +1907,6 @@ impl pallet_liquidity_pools::Config for Runtime { type TrancheId = TrancheId; type TrancheTokenPrice = PoolSystem; type TreasuryAccount = TreasuryAccount; - type WeightInfo = (); } parameter_types! { From e618c96e8fce3ea759404c596e7c81b3060a098c Mon Sep 17 00:00:00 2001 From: lemunozm Date: Mon, 29 Jul 2024 12:07:34 +0200 Subject: [PATCH 8/8] minor simplifications --- pallets/liquidity-pools-gateway/src/lib.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/pallets/liquidity-pools-gateway/src/lib.rs b/pallets/liquidity-pools-gateway/src/lib.rs index f20e3c7e12..dad88f12ca 100644 --- a/pallets/liquidity-pools-gateway/src/lib.rs +++ b/pallets/liquidity-pools-gateway/src/lib.rs @@ -536,14 +536,12 @@ pub mod pallet { let sender = ensure_signed(origin)?; PackedMessage::::mutate((&sender, &destination), |msg| match msg { - Some(_) => return Err(Error::::MessagePackingAlreadyStarted), + Some(_) => Err(Error::::MessagePackingAlreadyStarted.into()), None => { *msg = Some(T::Message::empty()); Ok(()) } - })?; - - Ok(()) + }) } #[pallet::call_index(9)] @@ -712,9 +710,7 @@ pub mod pallet { }; let serialized = message.serialize(); - - let message_weight = - read_weight.saturating_add(Weight::from_parts(0, serialized.len() as u64)); + let serialized_len = serialized.len() as u64; let (result, router_weight) = match router.send(sender, serialized) { Ok(dispatch_info) => (Ok(()), dispatch_info.actual_weight), @@ -725,7 +721,7 @@ pub mod pallet { result, router_weight .unwrap_or(Weight::from_parts(DEFAULT_WEIGHT_REF_TIME, 0)) - .saturating_add(message_weight) + .saturating_add(Weight::from_parts(0, serialized_len)) .saturating_add(read_weight), ) }