diff --git a/bridges/bin/millau/runtime/src/xcm_config.rs b/bridges/bin/millau/runtime/src/xcm_config.rs index 56546f299929727a05812e62fc660ef3085b988a..4aaec83771b0411049d0c9e9749eb212cab3f662 100644 --- a/bridges/bin/millau/runtime/src/xcm_config.rs +++ b/bridges/bin/millau/runtime/src/xcm_config.rs @@ -242,7 +242,7 @@ mod tests { target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, LaneId, MessageKey, }; - use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchError; + use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchResult; use codec::Encode; use pallet_bridge_messages::OutboundLanes; use xcm_executor::XcmExecutor; @@ -352,8 +352,8 @@ mod tests { let dispatch_result = FromRialtoMessageDispatch::dispatch(&AccountId::from([0u8; 32]), incoming_message); assert!(matches!( - dispatch_result.dispatch_result, - Err(XcmBlobMessageDispatchError::NotDispatched(_)), + dispatch_result.dispatch_level_result, + XcmBlobMessageDispatchResult::NotDispatched(_), )); } @@ -366,8 +366,8 @@ mod tests { let dispatch_result = FromRialtoMessageDispatch::dispatch(&AccountId::from([0u8; 32]), incoming_message); assert!(matches!( - dispatch_result.dispatch_result, - Err(XcmBlobMessageDispatchError::NotDispatched(_)), + dispatch_result.dispatch_level_result, + XcmBlobMessageDispatchResult::NotDispatched(_), )); } } diff --git a/bridges/bin/rialto-parachain/runtime/src/lib.rs b/bridges/bin/rialto-parachain/runtime/src/lib.rs index b03f7965d7b28c9f09dddb7d964f22b0de9ca50e..cd4e256f4203e778f39d9f500b4856d986e96db5 100644 --- a/bridges/bin/rialto-parachain/runtime/src/lib.rs +++ b/bridges/bin/rialto-parachain/runtime/src/lib.rs @@ -853,7 +853,7 @@ mod tests { LaneId, MessageKey, }; use bridge_runtime_common::{ - integrity::check_additional_signed, messages_xcm_extension::XcmBlobMessageDispatchError, + integrity::check_additional_signed, messages_xcm_extension::XcmBlobMessageDispatchResult, }; use codec::Encode; use pallet_bridge_messages::OutboundLanes; @@ -928,8 +928,8 @@ mod tests { let dispatch_result = FromMillauMessageDispatch::dispatch(&AccountId::from([0u8; 32]), incoming_message); assert!(matches!( - dispatch_result.dispatch_result, - Err(XcmBlobMessageDispatchError::NotDispatched(_)), + dispatch_result.dispatch_level_result, + XcmBlobMessageDispatchResult::NotDispatched(_), )); }); } diff --git a/bridges/bin/rialto/runtime/src/xcm_config.rs b/bridges/bin/rialto/runtime/src/xcm_config.rs index 265f435645e8f0516f6a2a7aea16720b72a93b1a..9f6488b4c4d318ac58f78f2585c637ecfec8094f 100644 --- a/bridges/bin/rialto/runtime/src/xcm_config.rs +++ b/bridges/bin/rialto/runtime/src/xcm_config.rs @@ -197,7 +197,7 @@ mod tests { target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, LaneId, MessageKey, }; - use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchError; + use bridge_runtime_common::messages_xcm_extension::XcmBlobMessageDispatchResult; use codec::Encode; use pallet_bridge_messages::OutboundLanes; use xcm_executor::XcmExecutor; @@ -269,8 +269,8 @@ mod tests { let dispatch_result = FromMillauMessageDispatch::dispatch(&AccountId::from([0u8; 32]), incoming_message); assert!(matches!( - dispatch_result.dispatch_result, - Err(XcmBlobMessageDispatchError::NotDispatched(_)), + dispatch_result.dispatch_level_result, + XcmBlobMessageDispatchResult::NotDispatched(_), )); } } diff --git a/bridges/bin/runtime-common/Cargo.toml b/bridges/bin/runtime-common/Cargo.toml index 9d616bf588e45657705803af97c71a08a046f73b..3db4ae9abca6e1c03aa57eea62b587992863a5e4 100644 --- a/bridges/bin/runtime-common/Cargo.toml +++ b/bridges/bin/runtime-common/Cargo.toml @@ -47,7 +47,6 @@ xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "maste [dev-dependencies] bp-test-utils = { path = "../../primitives/test-utils" } -bitvec = { version = "1", features = ["alloc"] } pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" } [features] diff --git a/bridges/bin/runtime-common/src/integrity.rs b/bridges/bin/runtime-common/src/integrity.rs index 3a677ce43ceda13349ec70206a3ef622138d8466..5820dd99b91ca3fcb9e475e4b81852b06aafbbcc 100644 --- a/bridges/bin/runtime-common/src/integrity.rs +++ b/bridges/bin/runtime-common/src/integrity.rs @@ -307,10 +307,8 @@ pub fn check_message_lane_weights<C: Chain, T: frame_system::Config>( messages::target::maximal_incoming_message_dispatch_weight(C::max_extrinsic_weight()), ); - let max_incoming_inbound_lane_data_proof_size = InboundLaneData::<()>::encoded_size_hint_u32( - this_chain_max_unrewarded_relayers as _, - this_chain_max_unconfirmed_messages as _, - ); + let max_incoming_inbound_lane_data_proof_size = + InboundLaneData::<()>::encoded_size_hint_u32(this_chain_max_unrewarded_relayers as _); pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights<T>>( C::max_extrinsic_size(), C::max_extrinsic_weight(), diff --git a/bridges/bin/runtime-common/src/messages_call_ext.rs b/bridges/bin/runtime-common/src/messages_call_ext.rs index 1ca1bab6a351e8153f8d37d5ebeec1a1c6f2fce9..f3665a8d93b5232adba971c6d1088a3b1b202d2c 100644 --- a/bridges/bin/runtime-common/src/messages_call_ext.rs +++ b/bridges/bin/runtime-common/src/messages_call_ext.rs @@ -321,7 +321,6 @@ mod tests { TestRuntime, ThisChainRuntimeCall, }, }; - use bitvec::prelude::*; use bp_messages::{DeliveredMessages, UnrewardedRelayer, UnrewardedRelayersState}; use sp_std::ops::RangeInclusive; @@ -331,11 +330,7 @@ mod tests { for n in 0..MaxUnrewardedRelayerEntriesAtInboundLane::get() { inbound_lane_state.relayers.push_back(UnrewardedRelayer { relayer: Default::default(), - messages: DeliveredMessages { - begin: n + 1, - end: n + 1, - dispatch_results: bitvec![u8, Msb0; 1; 1], - }, + messages: DeliveredMessages { begin: n + 1, end: n + 1 }, }); } pallet_bridge_messages::InboundLanes::<TestRuntime>::insert( @@ -352,7 +347,6 @@ mod tests { messages: DeliveredMessages { begin: 1, end: MaxUnconfirmedMessagesAtInboundLane::get(), - dispatch_results: bitvec![u8, Msb0; 1; MaxUnconfirmedMessagesAtInboundLane::get() as _], }, }); pallet_bridge_messages::InboundLanes::<TestRuntime>::insert( diff --git a/bridges/bin/runtime-common/src/messages_xcm_extension.rs b/bridges/bin/runtime-common/src/messages_xcm_extension.rs index d0fdc458e958c89889bd2c7bed6e7d7404f16db6..4ccdd7a4b4df536f364ae61d068920670270e0b0 100644 --- a/bridges/bin/runtime-common/src/messages_xcm_extension.rs +++ b/bridges/bin/runtime-common/src/messages_xcm_extension.rs @@ -39,8 +39,9 @@ pub type XcmAsPlainPayload = sp_std::prelude::Vec<u8>; /// Message dispatch result type for single message #[derive(CloneNoBound, EqNoBound, PartialEqNoBound, Encode, Decode, Debug, TypeInfo)] -pub enum XcmBlobMessageDispatchError { +pub enum XcmBlobMessageDispatchResult { InvalidPayload, + Dispatched, NotDispatched(#[codec(skip)] Option<DispatchBlobError>), } @@ -64,7 +65,7 @@ impl< for XcmBlobMessageDispatch<SourceBridgeHubChain, TargetBridgeHubChain, BlobDispatcher, Weights> { type DispatchPayload = XcmAsPlainPayload; - type DispatchError = XcmBlobMessageDispatchError; + type DispatchLevelResult = XcmBlobMessageDispatchResult; fn dispatch_weight(message: &mut DispatchMessage<Self::DispatchPayload>) -> Weight { match message.data.payload { @@ -79,7 +80,7 @@ impl< fn dispatch( _relayer_account: &AccountIdOf<SourceBridgeHubChain>, message: DispatchMessage<Self::DispatchPayload>, - ) -> MessageDispatchResult<Self::DispatchError> { + ) -> MessageDispatchResult<Self::DispatchLevelResult> { let payload = match message.data.payload { Ok(payload) => payload, Err(e) => { @@ -91,7 +92,7 @@ impl< ); return MessageDispatchResult { unspent_weight: Weight::zero(), - dispatch_result: Err(XcmBlobMessageDispatchError::InvalidPayload), + dispatch_level_result: XcmBlobMessageDispatchResult::InvalidPayload, } }, }; @@ -102,7 +103,7 @@ impl< "[XcmBlobMessageDispatch] DispatchBlob::dispatch_blob was ok - message_nonce: {:?}", message.key.nonce ); - Ok(()) + XcmBlobMessageDispatchResult::Dispatched }, Err(e) => { log::error!( @@ -110,13 +111,10 @@ impl< "[XcmBlobMessageDispatch] DispatchBlob::dispatch_blob failed, error: {:?} - message_nonce: {:?}", e, message.key.nonce ); - Err(XcmBlobMessageDispatchError::NotDispatched(Some(e))) + XcmBlobMessageDispatchResult::NotDispatched(Some(e)) }, }; - MessageDispatchResult { - unspent_weight: Weight::zero(), - dispatch_result: dispatch_level_result, - } + MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_level_result } } } diff --git a/bridges/modules/messages/Cargo.toml b/bridges/modules/messages/Cargo.toml index 7dbe6e15ba342a6d0f3431f6e75682319e996679..f733d62bf64011e022706fc944f53c53971efa60 100644 --- a/bridges/modules/messages/Cargo.toml +++ b/bridges/modules/messages/Cargo.toml @@ -7,7 +7,6 @@ edition = "2021" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] -bitvec = { version = "1", default-features = false, features = ["alloc"] } codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false } log = { version = "0.4.17", default-features = false } num-traits = { version = "0.2", default-features = false } diff --git a/bridges/modules/messages/src/benchmarking.rs b/bridges/modules/messages/src/benchmarking.rs index f7a2769715fa5a9f9c5b5ee97f12bd3e92d08ccb..aab8855a729a8310dfa7f5a1b7d872e95cf1e2b3 100644 --- a/bridges/modules/messages/src/benchmarking.rs +++ b/bridges/modules/messages/src/benchmarking.rs @@ -301,7 +301,7 @@ benchmarks_instance_pallet! { inbound_lane_data: InboundLaneData { relayers: vec![UnrewardedRelayer { relayer: relayer_id.clone(), - messages: DeliveredMessages::new(1, true), + messages: DeliveredMessages::new(1), }].into_iter().collect(), last_confirmed_nonce: 0, }, @@ -333,8 +333,8 @@ benchmarks_instance_pallet! { total_messages: 2, last_delivered_nonce: 2, }; - let mut delivered_messages = DeliveredMessages::new(1, true); - delivered_messages.note_dispatched_message(true); + let mut delivered_messages = DeliveredMessages::new(1); + delivered_messages.note_dispatched_message(); let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams { lane: T::bench_lane_id(), inbound_lane_data: InboundLaneData { @@ -379,11 +379,11 @@ benchmarks_instance_pallet! { relayers: vec![ UnrewardedRelayer { relayer: relayer1_id.clone(), - messages: DeliveredMessages::new(1, true), + messages: DeliveredMessages::new(1), }, UnrewardedRelayer { relayer: relayer2_id.clone(), - messages: DeliveredMessages::new(2, true), + messages: DeliveredMessages::new(2), }, ].into_iter().collect(), last_confirmed_nonce: 0, @@ -451,7 +451,7 @@ fn receive_messages<T: Config<I>, I: 'static>(nonce: MessageNonce) { inbound_lane_storage.set_data(InboundLaneData { relayers: vec![UnrewardedRelayer { relayer: T::bridged_relayer_id(), - messages: DeliveredMessages::new(nonce, true), + messages: DeliveredMessages::new(nonce), }] .into_iter() .collect(), diff --git a/bridges/modules/messages/src/inbound_lane.rs b/bridges/modules/messages/src/inbound_lane.rs index 21d835d31c7158a5f3853228942ba003bb7fb846..3f64ab765b5dae6b69eef27f7559e8462b702165 100644 --- a/bridges/modules/messages/src/inbound_lane.rs +++ b/bridges/modules/messages/src/inbound_lane.rs @@ -101,7 +101,6 @@ impl<T: Config<I>, I: 'static> MaxEncodedLen for StoredInboundLaneData<T, I> { fn max_encoded_len() -> usize { InboundLaneData::<T::InboundRelayer>::encoded_size_hint( T::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize, - T::MaxUnconfirmedMessagesAtInboundLane::get() as usize, ) .unwrap_or(usize::MAX) } @@ -155,9 +154,6 @@ impl<S: InboundLaneStorage> InboundLane<S> { // overlap. match data.relayers.front_mut() { Some(entry) if entry.messages.begin < new_confirmed_nonce => { - entry.messages.dispatch_results = entry.messages.dispatch_results - [(new_confirmed_nonce + 1 - entry.messages.begin) as usize..] - .to_bitvec(); entry.messages.begin = new_confirmed_nonce + 1; }, _ => {}, @@ -174,7 +170,7 @@ impl<S: InboundLaneStorage> InboundLane<S> { relayer_at_this_chain: &AccountId, nonce: MessageNonce, message_data: DispatchMessageData<Dispatch::DispatchPayload>, - ) -> ReceivalResult<Dispatch::DispatchError> { + ) -> ReceivalResult<Dispatch::DispatchLevelResult> { let mut data = self.storage.data(); let is_correct_message = nonce == data.last_delivered_nonce() + 1; if !is_correct_message { @@ -202,20 +198,19 @@ impl<S: InboundLaneStorage> InboundLane<S> { ); // now let's update inbound lane storage - match data.relayers.back_mut() { + let push_new = match data.relayers.back_mut() { Some(entry) if entry.relayer == *relayer_at_bridged_chain => { - entry.messages.note_dispatched_message(dispatch_result.dispatch_result.is_ok()); - }, - _ => { - data.relayers.push_back(UnrewardedRelayer { - relayer: relayer_at_bridged_chain.clone(), - messages: DeliveredMessages::new( - nonce, - dispatch_result.dispatch_result.is_ok(), - ), - }); + entry.messages.note_dispatched_message(); + false }, + _ => true, }; + if push_new { + data.relayers.push_back(UnrewardedRelayer { + relayer: (*relayer_at_bridged_chain).clone(), + messages: DeliveredMessages::new(nonce), + }); + } self.storage.set_data(data); ReceivalResult::Dispatched(dispatch_result) diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index 5b1b98ea7a18bae71e24bd6ca58689fca1b013ab..c94f5ffa752fae7267c3e4ef3c831a48bbd41c13 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -66,7 +66,7 @@ use bp_messages::{ use bp_runtime::{BasicOperatingMode, ChainId, OwnedBridgeModule, PreComputedSize, Size}; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{dispatch::PostDispatchInfo, ensure, fail, traits::Get}; -use sp_runtime::{traits::UniqueSaturatedFrom, SaturatedConversion}; +use sp_runtime::traits::UniqueSaturatedFrom; use sp_std::{cell::RefCell, marker::PhantomData, prelude::*}; mod inbound_lane; @@ -547,7 +547,7 @@ pub mod pallet { MessagesReceived( Vec< ReceivedMessages< - <T::MessageDispatch as MessageDispatch<T::AccountId>>::DispatchError, + <T::MessageDispatch as MessageDispatch<T::AccountId>>::DispatchLevelResult, >, >, ), @@ -827,19 +827,15 @@ impl<T: Config<I>, I: 'static> RuntimeInboundLaneStorage<T, I> { /// `receive_messages_proof` call, because the actual inbound lane state is smaller than the /// maximal configured. /// - /// Maximal inbound lane state size is computed using the - /// `MaxUnrewardedRelayerEntriesAtInboundLane` and `MaxUnconfirmedMessagesAtInboundLane` - /// constants from the pallet configuration. The PoV of the call includes the maximal size - /// of the inbound lane state. If the actual size is smaller, we may subtract extra bytes - /// from this component. + /// Maximal inbound lane state set size is configured by the + /// `MaxUnrewardedRelayerEntriesAtInboundLane` constant from the pallet configuration. The PoV + /// of the call includes the maximal size of inbound lane state. If the actual size is smaller, + /// we may subtract extra bytes from this component. pub fn extra_proof_size_bytes(&self) -> u64 { let max_encoded_len = StoredInboundLaneData::<T, I>::max_encoded_len(); let relayers_count = self.data().relayers.len(); - let messages_count = self.data().relayers.iter().fold(0usize, |sum, relayer| { - sum.saturating_add(relayer.messages.total_messages().saturated_into::<usize>()) - }); let actual_encoded_len = - InboundLaneData::<T::InboundRelayer>::encoded_size_hint(relayers_count, messages_count) + InboundLaneData::<T::InboundRelayer>::encoded_size_hint(relayers_count) .unwrap_or(usize::MAX); max_encoded_len.saturating_sub(actual_encoded_len) as _ } @@ -971,7 +967,6 @@ mod tests { }; use frame_system::{EventRecord, Pallet as System, Phase}; use sp_runtime::DispatchError; - use std::collections::VecDeque; fn get_ready_for_events() { System::<TestRuntime>::set_block_number(1); @@ -1029,7 +1024,7 @@ mod tests { last_confirmed_nonce: 1, relayers: vec![UnrewardedRelayer { relayer: 0, - messages: DeliveredMessages::new(1, true), + messages: DeliveredMessages::new(1), }] .into_iter() .collect(), @@ -1049,44 +1044,13 @@ mod tests { phase: Phase::Initialization, event: TestEvent::Messages(Event::MessagesDelivered { lane_id: TEST_LANE_ID, - messages: DeliveredMessages::new(1, true), + messages: DeliveredMessages::new(1), }), topics: vec![], }], ); } - fn unrewarded_relayer_entry(msg_count: usize) -> UnrewardedRelayer<TestRelayer> { - UnrewardedRelayer { - relayer: 42u64, - messages: DeliveredMessages { - begin: 0, - end: msg_count as MessageNonce - 1, - dispatch_results: FromIterator::from_iter(vec![true; msg_count]), - }, - } - } - - fn unrewarded_relayers_vec( - entry_count: usize, - msg_count: usize, - ) -> VecDeque<UnrewardedRelayer<TestRelayer>> { - if entry_count > msg_count { - panic!("unrewarded_relayers_vec(): expecting msg_count to be >= entry_count"); - } - - let mut unrewarded_relayers = vec![]; - let mut available_msg_count = msg_count; - for _ in 0..entry_count - 1 { - unrewarded_relayers - .push(unrewarded_relayer_entry(std::cmp::min(1, available_msg_count))); - available_msg_count -= 1 - } - unrewarded_relayers.push(unrewarded_relayer_entry(available_msg_count)); - - unrewarded_relayers.into_iter().collect() - } - #[test] fn pallet_rejects_transactions_if_halted() { run_test(|| { @@ -1719,7 +1683,6 @@ mod tests { fn proof_size_refund_from_receive_messages_proof_works() { run_test(|| { let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize; - let max_msgs = crate::mock::MaxUnconfirmedMessagesAtInboundLane::get() as usize; // if there's maximal number of unrewarded relayer entries at the inbound lane, then // `proof_size` is unchanged in post-dispatch weight @@ -1734,7 +1697,15 @@ mod tests { InboundLanes::<TestRuntime>::insert( TEST_LANE_ID, StoredInboundLaneData(InboundLaneData { - relayers: unrewarded_relayers_vec(max_entries, max_msgs), + relayers: vec![ + UnrewardedRelayer { + relayer: 42, + messages: DeliveredMessages { begin: 0, end: 100 } + }; + max_entries + ] + .into_iter() + .collect(), last_confirmed_nonce: 0, }), ); @@ -1755,7 +1726,15 @@ mod tests { InboundLanes::<TestRuntime>::insert( TEST_LANE_ID, StoredInboundLaneData(InboundLaneData { - relayers: unrewarded_relayers_vec(max_entries - 1, max_msgs), + relayers: vec![ + UnrewardedRelayer { + relayer: 42, + messages: DeliveredMessages { begin: 0, end: 100 } + }; + max_entries - 1 + ] + .into_iter() + .collect(), last_confirmed_nonce: 0, }), ); @@ -1771,7 +1750,7 @@ mod tests { .unwrap(); assert!( post_dispatch_weight.proof_size() < pre_dispatch_weight.proof_size(), - "Expected post-dispatch PoV {} to be < than pre-dispatch PoV {}", + "Expected post-dispatch PoV {} to be less than pre-dispatch PoV {}", post_dispatch_weight.proof_size(), pre_dispatch_weight.proof_size(), ); @@ -1787,8 +1766,8 @@ mod tests { // messages 1+2 are confirmed in 1 tx, message 3 in a separate tx // dispatch of message 2 has failed - let mut delivered_messages_1_and_2 = DeliveredMessages::new(1, true); - delivered_messages_1_and_2.note_dispatched_message(true); + let mut delivered_messages_1_and_2 = DeliveredMessages::new(1); + delivered_messages_1_and_2.note_dispatched_message(); let messages_1_and_2_proof = Ok(( TEST_LANE_ID, InboundLaneData { @@ -1801,7 +1780,7 @@ mod tests { .collect(), }, )); - let delivered_message_3 = DeliveredMessages::new(3, true); + let delivered_message_3 = DeliveredMessages::new(3); let messages_3_proof = Ok(( TEST_LANE_ID, InboundLaneData { @@ -2092,7 +2071,7 @@ mod tests { last_confirmed_nonce: 1, relayers: vec![UnrewardedRelayer { relayer: 0, - messages: DeliveredMessages::new(1, true), + messages: DeliveredMessages::new(1), }] .into_iter() .collect(), @@ -2152,39 +2131,39 @@ mod tests { #[test] fn inbound_storage_extra_proof_size_bytes_works() { - let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize; - let max_msgs = crate::mock::MaxUnconfirmedMessagesAtInboundLane::get() as usize; + fn relayer_entry() -> UnrewardedRelayer<TestRelayer> { + UnrewardedRelayer { relayer: 42u64, messages: DeliveredMessages { begin: 0, end: 100 } } + } - fn storage( - entry_count: usize, - msg_count: usize, - ) -> RuntimeInboundLaneStorage<TestRuntime, ()> { + fn storage(relayer_entries: usize) -> RuntimeInboundLaneStorage<TestRuntime, ()> { RuntimeInboundLaneStorage { lane_id: Default::default(), cached_data: RefCell::new(Some(InboundLaneData { - relayers: unrewarded_relayers_vec(entry_count, msg_count), + relayers: vec![relayer_entry(); relayer_entries].into_iter().collect(), last_confirmed_nonce: 0, })), _phantom: Default::default(), } } + let max_entries = crate::mock::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize; + // when we have exactly `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers - assert_eq!(storage(max_entries, max_msgs).extra_proof_size_bytes(), 0); + assert_eq!(storage(max_entries).extra_proof_size_bytes(), 0); // when we have less than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers assert_eq!( - storage(max_entries - 1, max_msgs).extra_proof_size_bytes(), - unrewarded_relayer_entry(1).encoded_size() as u64 + storage(max_entries - 1).extra_proof_size_bytes(), + relayer_entry().encode().len() as u64 ); assert_eq!( - storage(max_entries - 2, max_msgs).extra_proof_size_bytes(), - 2 * unrewarded_relayer_entry(1).encoded_size() as u64 + storage(max_entries - 2).extra_proof_size_bytes(), + 2 * relayer_entry().encode().len() as u64 ); // when we have more than `MaxUnrewardedRelayerEntriesAtInboundLane` unrewarded relayers // (shall not happen in practice) - assert_eq!(storage(max_entries + 1, max_msgs).extra_proof_size_bytes(), 0); + assert_eq!(storage(max_entries + 1).extra_proof_size_bytes(), 0); } #[test] diff --git a/bridges/modules/messages/src/mock.rs b/bridges/modules/messages/src/mock.rs index d16fd4138fc3836f657f59b2dec9bf422cd6e28a..75f05b4820a8aa2331963d8048631d5d99458b74 100644 --- a/bridges/modules/messages/src/mock.rs +++ b/bridges/modules/messages/src/mock.rs @@ -19,7 +19,6 @@ use crate::Config; -use bitvec::prelude::*; use bp_messages::{ calc_relayers_rewards, source_chain::{DeliveryConfirmationPayments, LaneMessageVerifier, TargetHeaderChain}, @@ -63,13 +62,13 @@ pub struct TestPayload { /// /// Note: in correct code `dispatch_result.unspent_weight` will always be <= `declared_weight`, /// but for test purposes we'll be making it larger than `declared_weight` sometimes. - pub dispatch_result: MessageDispatchResult<TestDispatchError>, + pub dispatch_result: MessageDispatchResult<TestDispatchLevelResult>, /// Extra bytes that affect payload size. pub extra: Vec<u8>, } pub type TestMessageFee = u64; pub type TestRelayer = u64; -pub type TestDispatchError = (); +pub type TestDispatchLevelResult = (); type Block = frame_system::mocking::MockBlock<TestRuntime>; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<TestRuntime>; @@ -419,7 +418,7 @@ pub struct TestMessageDispatch; impl MessageDispatch<AccountId> for TestMessageDispatch { type DispatchPayload = TestPayload; - type DispatchError = TestDispatchError; + type DispatchLevelResult = TestDispatchLevelResult; fn dispatch_weight(message: &mut DispatchMessage<TestPayload>) -> Weight { match message.data.payload.as_ref() { @@ -431,7 +430,7 @@ impl MessageDispatch<AccountId> for TestMessageDispatch { fn dispatch( _relayer_account: &AccountId, message: DispatchMessage<TestPayload>, - ) -> MessageDispatchResult<TestDispatchError> { + ) -> MessageDispatchResult<TestDispatchLevelResult> { match message.data.payload.as_ref() { Ok(payload) => payload.dispatch_result.clone(), Err(_) => dispatch_result(0), @@ -466,10 +465,12 @@ pub const fn message_payload(id: u64, declared_weight: u64) -> TestPayload { } /// Returns message dispatch result with given unspent weight. -pub const fn dispatch_result(unspent_weight: u64) -> MessageDispatchResult<TestDispatchError> { +pub const fn dispatch_result( + unspent_weight: u64, +) -> MessageDispatchResult<TestDispatchLevelResult> { MessageDispatchResult { unspent_weight: Weight::from_parts(unspent_weight, 0), - dispatch_result: Ok(()), + dispatch_level_result: (), } } @@ -479,14 +480,7 @@ pub fn unrewarded_relayer( end: MessageNonce, relayer: TestRelayer, ) -> UnrewardedRelayer<TestRelayer> { - UnrewardedRelayer { - relayer, - messages: DeliveredMessages { - begin, - end, - dispatch_results: bitvec![u8, Msb0; 1; (end + 1).saturating_sub(begin) as _], - }, - } + UnrewardedRelayer { relayer, messages: DeliveredMessages { begin, end } } } /// Return test externalities to use in tests. diff --git a/bridges/modules/messages/src/outbound_lane.rs b/bridges/modules/messages/src/outbound_lane.rs index 5b3281b5b162fef818cfa5c7b2a16cd5a9e91b30..33a58a40400bfd6e28251fc98272da3099f4f737 100644 --- a/bridges/modules/messages/src/outbound_lane.rs +++ b/bridges/modules/messages/src/outbound_lane.rs @@ -18,10 +18,8 @@ use crate::Config; -use bitvec::prelude::*; use bp_messages::{ - DeliveredMessages, DispatchResultsBitVec, LaneId, MessageNonce, MessagePayload, - OutboundLaneData, UnrewardedRelayer, + DeliveredMessages, LaneId, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer, }; use frame_support::{ weights::{RuntimeDbWeight, Weight}, @@ -67,9 +65,6 @@ pub enum ReceivalConfirmationResult { /// The unrewarded relayers vec contains non-consecutive entries. May be a result of invalid /// bridged chain storage. NonConsecutiveUnrewardedRelayerEntries, - /// The unrewarded relayers vec contains entry with mismatched number of dispatch results. May - /// be a result of invalid bridged chain storage. - InvalidNumberOfDispatchResults, /// The chain has more messages that need to be confirmed than there is in the proof. TryingToConfirmMoreMessagesThanExpected(MessageNonce), } @@ -129,14 +124,9 @@ impl<S: OutboundLaneStorage> OutboundLane<S> { ) } - let dispatch_results = match extract_dispatch_results( - data.latest_received_nonce, - latest_delivered_nonce, - relayers, - ) { - Ok(dispatch_results) => dispatch_results, - Err(extract_error) => return extract_error, - }; + if let Err(e) = ensure_unrewarded_relayers_are_correct(latest_delivered_nonce, relayers) { + return e + } let prev_latest_received_nonce = data.latest_received_nonce; data.latest_received_nonce = latest_delivered_nonce; @@ -145,7 +135,6 @@ impl<S: OutboundLaneStorage> OutboundLane<S> { ReceivalConfirmationResult::ConfirmedMessages(DeliveredMessages { begin: prev_latest_received_nonce + 1, end: latest_delivered_nonce, - dispatch_results, }) } @@ -180,34 +169,30 @@ impl<S: OutboundLaneStorage> OutboundLane<S> { } } -/// Extract new dispatch results from the unrewarded relayers vec. +/// Verifies unrewarded relayers vec. /// /// Returns `Err(_)` if unrewarded relayers vec contains invalid data, meaning that the bridged /// chain has invalid runtime storage. -fn extract_dispatch_results<RelayerId>( - prev_latest_received_nonce: MessageNonce, +fn ensure_unrewarded_relayers_are_correct<RelayerId>( latest_received_nonce: MessageNonce, relayers: &VecDeque<UnrewardedRelayer<RelayerId>>, -) -> Result<DispatchResultsBitVec, ReceivalConfirmationResult> { - // the only caller of this functions checks that the - // prev_latest_received_nonce..=latest_received_nonce is valid, so we're ready to accept - // messages in this range => with_capacity call must succeed here or we'll be unable to receive - // confirmations at all - let mut received_dispatch_result = - BitVec::with_capacity((latest_received_nonce - prev_latest_received_nonce + 1) as _); - let mut expected_entry_begin = relayers.front().map(|entry| entry.messages.begin); +) -> Result<(), ReceivalConfirmationResult> { + let mut last_entry_end: Option<MessageNonce> = None; for entry in relayers { // unrewarded relayer entry must have at least 1 unconfirmed message // (guaranteed by the `InboundLane::receive_message()`) - if entry.messages.total_messages() == 0 { + if entry.messages.end < entry.messages.begin { return Err(ReceivalConfirmationResult::EmptyUnrewardedRelayerEntry) } // every entry must confirm range of messages that follows previous entry range // (guaranteed by the `InboundLane::receive_message()`) - if expected_entry_begin != Some(entry.messages.begin) { - return Err(ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries) + if let Some(last_entry_end) = last_entry_end { + let expected_entry_begin = last_entry_end.checked_add(1); + if expected_entry_begin != Some(entry.messages.begin) { + return Err(ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries) + } } - expected_entry_begin = entry.messages.end.checked_add(1); + last_entry_end = Some(entry.messages.end); // entry can't confirm messages larger than `inbound_lane_data.latest_received_nonce()` // (guaranteed by the `InboundLane::receive_message()`) if entry.messages.end > latest_received_nonce { @@ -216,30 +201,9 @@ fn extract_dispatch_results<RelayerId>( // this is detected now return Err(ReceivalConfirmationResult::FailedToConfirmFutureMessages) } - // entry must have single dispatch result for every message - // (guaranteed by the `InboundLane::receive_message()`) - if entry.messages.dispatch_results.len() as MessageNonce != entry.messages.total_messages() - { - return Err(ReceivalConfirmationResult::InvalidNumberOfDispatchResults) - } - - // now we know that the entry is valid - // => let's check if it brings new confirmations - let new_messages_begin = - sp_std::cmp::max(entry.messages.begin, prev_latest_received_nonce + 1); - if entry.messages.end < new_messages_begin { - continue - } - - // now we know that entry brings new confirmations - // => let's extract dispatch results - received_dispatch_result.extend_from_bitslice( - &entry.messages.dispatch_results - [(new_messages_begin - entry.messages.begin) as usize..], - ); } - Ok(received_dispatch_result) + Ok(()) } #[cfg(test)] @@ -264,11 +228,7 @@ mod tests { } fn delivered_messages(nonces: RangeInclusive<MessageNonce>) -> DeliveredMessages { - DeliveredMessages { - begin: *nonces.start(), - end: *nonces.end(), - dispatch_results: bitvec![u8, Msb0; 1; (nonces.end() - nonces.start() + 1) as _], - } + DeliveredMessages { begin: *nonces.start(), end: *nonces.end() } } fn assert_3_messages_confirmation_fails( @@ -401,20 +361,6 @@ mod tests { ); } - #[test] - fn confirm_delivery_fails_if_number_of_dispatch_results_in_entry_is_invalid() { - let mut relayers: VecDeque<_> = unrewarded_relayers(1..=1) - .into_iter() - .chain(unrewarded_relayers(2..=2).into_iter()) - .chain(unrewarded_relayers(3..=3).into_iter()) - .collect(); - relayers[0].messages.dispatch_results.clear(); - assert_eq!( - assert_3_messages_confirmation_fails(3, &relayers), - ReceivalConfirmationResult::InvalidNumberOfDispatchResults, - ); - } - #[test] fn prune_messages_works() { run_test(|| { diff --git a/bridges/primitives/messages/Cargo.toml b/bridges/primitives/messages/Cargo.toml index bc1b92c6b88e75e319b264c6f426ed301f8f6d59..32d7c65ebcbbb1c4a424c1b7603dba81c3d5dbbc 100644 --- a/bridges/primitives/messages/Cargo.toml +++ b/bridges/primitives/messages/Cargo.toml @@ -7,7 +7,6 @@ edition = "2021" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] -bitvec = { version = "1", default-features = false, features = ["alloc"] } codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false, features = ["derive", "bit-vec"] } scale-info = { version = "2.5.0", default-features = false, features = ["bit-vec", "derive"] } serde = { version = "1.0", optional = true, features = ["derive"] } @@ -29,7 +28,6 @@ hex-literal = "0.4" [features] default = ["std"] std = [ - "bitvec/std", "bp-runtime/std", "codec/std", "frame-support/std", diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index 7b7e9476a017df30198b67ebf01a1a79033a267f..3910837a442e01fef5f790c50182c0a6b6f5e9fd 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/bridges/primitives/messages/src/lib.rs @@ -20,7 +20,6 @@ // RuntimeApi generated functions #![allow(clippy::too_many_arguments)] -use bitvec::prelude::*; use bp_runtime::{BasicOperatingMode, OperatingMode, RangeInclusiveExt}; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::RuntimeDebug; @@ -157,68 +156,18 @@ impl<RelayerId> Default for InboundLaneData<RelayerId> { } impl<RelayerId> InboundLaneData<RelayerId> { - fn dispatch_results_encoded_size_hint( - relayers_entries: usize, - message_count: usize, - ) -> Option<usize> - where - RelayerId: MaxEncodedLen, - { - // The worst-case scenario for the bitvecs size is the one in which we have as many relayer - // entries as possible taking an extra 1 byte slot with just 1 bit of actual information. - // For example: - // 11111111 1------- - // 11111111 1------- - // 1------- - // 1------- - - // If there are less msgs than relayer entries, in the worst case, each dispatch result - // belongs to a different relayer slot. This means 1 byte for the len prefix and 1 byte - // for the actual data. - if relayers_entries >= message_count { - return relayers_entries.checked_add(message_count) - } - - let msgs_per_byte = 8; - // At the begining each relayer slot has 1 message, using 1 byte - let mut num_result_bytes = relayers_entries; - // Then we add batches of 8 messages to some relayer slot until there are no more messages. - // Each batch takes up 1 more byte. - num_result_bytes = - num_result_bytes.checked_add((message_count - relayers_entries) / msgs_per_byte)?; - - // The len is stored in a `Compact<u32>`. `Compact<u32>` can store a max value of - // 63 on 1 byte, 16383 on 2 bytes, etc. - let max_len_per_first_byte = 0b0011_1111; - // At the begining each relayer slot uses 1 byte for the len prefix - // (each relayer slot contains 1 message) - let mut num_len_bytes = relayers_entries; - // Then we add batches of 63 messages to as many relayer slots as possible, requiring 2 - // bytes for the `len` prefix. It's hard to believe that we'll need more than 2 bytes - // (more than 16383 messages in 1 relayer slot). - num_len_bytes = num_len_bytes.checked_add(sp_std::cmp::min( - (message_count - relayers_entries) / max_len_per_first_byte, - relayers_entries, - ))?; - - num_result_bytes.checked_add(num_len_bytes) - } - /// Returns approximate size of the struct, given a number of entries in the `relayers` set and /// size of each entry. /// /// Returns `None` if size overflows `usize` limits. - pub fn encoded_size_hint(relayers_entries: usize, message_count: usize) -> Option<usize> + pub fn encoded_size_hint(relayers_entries: usize) -> Option<usize> where RelayerId: MaxEncodedLen, { let message_nonce_size = MessageNonce::max_encoded_len(); let relayer_id_encoded_size = RelayerId::max_encoded_len(); let relayers_entry_size = relayer_id_encoded_size.checked_add(2 * message_nonce_size)?; - let relayers_size = relayers_entries.checked_mul(relayers_entry_size)?.checked_add( - Self::dispatch_results_encoded_size_hint(relayers_entries, message_count)?, - )?; - + let relayers_size = relayers_entries.checked_mul(relayers_entry_size)?; relayers_size.checked_add(message_nonce_size) } @@ -226,11 +175,11 @@ impl<RelayerId> InboundLaneData<RelayerId> { /// `relayers` set and the size of each entry. /// /// Returns `u32::MAX` if size overflows `u32` limits. - pub fn encoded_size_hint_u32(relayers_entries: usize, messages_count: usize) -> u32 + pub fn encoded_size_hint_u32(relayers_entries: usize) -> u32 where RelayerId: MaxEncodedLen, { - Self::encoded_size_hint(relayers_entries, messages_count) + Self::encoded_size_hint(relayers_entries) .and_then(|x| u32::try_from(x).ok()) .unwrap_or(u32::MAX) } @@ -270,9 +219,6 @@ pub struct InboundMessageDetails { pub dispatch_weight: Weight, } -/// Bit vector of message dispatch results. -pub type DispatchResultsBitVec = BitVec<u8, Msb0>; - /// Unrewarded relayer entry stored in the inbound lane data. /// /// This struct represents a continuous range of messages that have been delivered by the same @@ -330,19 +276,13 @@ pub struct DeliveredMessages { pub begin: MessageNonce, /// Nonce of the last message that has been delivered (inclusive). pub end: MessageNonce, - /// Dispatch result (`false`/`true`), returned by the message dispatcher for every - /// message in the `[begin; end]` range. See `dispatch_result` field of the - /// `bp_runtime::messages::MessageDispatchResult` structure for more information. - pub dispatch_results: DispatchResultsBitVec, } impl DeliveredMessages { /// Create new `DeliveredMessages` struct that confirms delivery of single nonce with given /// dispatch result. - pub fn new(nonce: MessageNonce, dispatch_result: bool) -> Self { - let mut dispatch_results = BitVec::with_capacity(1); - dispatch_results.push(dispatch_result); - DeliveredMessages { begin: nonce, end: nonce, dispatch_results } + pub fn new(nonce: MessageNonce) -> Self { + DeliveredMessages { begin: nonce, end: nonce } } /// Return total count of delivered messages. @@ -351,25 +291,14 @@ impl DeliveredMessages { } /// Note new dispatched message. - pub fn note_dispatched_message(&mut self, dispatch_result: bool) { + pub fn note_dispatched_message(&mut self) { self.end += 1; - self.dispatch_results.push(dispatch_result); } /// Returns true if delivered messages contain message with given nonce. pub fn contains_message(&self, nonce: MessageNonce) -> bool { (self.begin..=self.end).contains(&nonce) } - - /// Get dispatch result flag by message nonce. - /// - /// Dispatch result flag must be interpreted using the knowledge of dispatch mechanism - /// at the target chain. See `dispatch_result` field of the - /// `bp_runtime::messages::MessageDispatchResult` structure for more information. - pub fn message_dispatch_result(&self, nonce: MessageNonce) -> Option<bool> { - let index = nonce.checked_sub(self.begin)? as usize; - self.dispatch_results.get(index).map(|bit| *bit) - } } /// Gist of `InboundLaneData::relayers` field used by runtime APIs. @@ -481,10 +410,10 @@ mod tests { assert_eq!( total_unrewarded_messages( &vec![ - UnrewardedRelayer { relayer: 1, messages: DeliveredMessages::new(0, true) }, + UnrewardedRelayer { relayer: 1, messages: DeliveredMessages::new(0) }, UnrewardedRelayer { relayer: 2, - messages: DeliveredMessages::new(MessageNonce::MAX, true) + messages: DeliveredMessages::new(MessageNonce::MAX) }, ] .into_iter() @@ -505,21 +434,12 @@ mod tests { (13u8, 128u8), ]; for (relayer_entries, messages_count) in test_cases { - let expected_size = - InboundLaneData::<u8>::encoded_size_hint(relayer_entries as _, messages_count as _); + let expected_size = InboundLaneData::<u8>::encoded_size_hint(relayer_entries as _); let actual_size = InboundLaneData { relayers: (1u8..=relayer_entries) - .map(|i| { - let mut entry = UnrewardedRelayer { - relayer: i, - messages: DeliveredMessages::new(i as _, true), - }; - entry.messages.dispatch_results = bitvec![ - u8, Msb0; - 1; - (messages_count / relayer_entries) as _ - ]; - entry + .map(|i| UnrewardedRelayer { + relayer: i, + messages: DeliveredMessages::new(i as _), }) .collect(), last_confirmed_nonce: messages_count as _, @@ -535,16 +455,13 @@ mod tests { } #[test] - fn message_dispatch_result_works() { - let delivered_messages = - DeliveredMessages { begin: 100, end: 150, dispatch_results: bitvec![u8, Msb0; 1; 151] }; + fn contains_result_works() { + let delivered_messages = DeliveredMessages { begin: 100, end: 150 }; assert!(!delivered_messages.contains_message(99)); assert!(delivered_messages.contains_message(100)); assert!(delivered_messages.contains_message(150)); assert!(!delivered_messages.contains_message(151)); - - assert_eq!(delivered_messages.message_dispatch_result(125), Some(true)); } #[test] diff --git a/bridges/primitives/messages/src/target_chain.rs b/bridges/primitives/messages/src/target_chain.rs index da96c4e59cd959e671e5409ae5ae2fcba1b140d1..8496b90214c409bd968aa1346ca4b64984baf091 100644 --- a/bridges/primitives/messages/src/target_chain.rs +++ b/bridges/primitives/messages/src/target_chain.rs @@ -90,7 +90,7 @@ pub trait MessageDispatch<AccountId> { type DispatchPayload: Decode; /// Fine-grained result of single message dispatch (for better diagnostic purposes) - type DispatchError: Clone + sp_std::fmt::Debug + Eq; + type DispatchLevelResult: Clone + sp_std::fmt::Debug + Eq; /// Estimate dispatch weight. /// @@ -109,7 +109,7 @@ pub trait MessageDispatch<AccountId> { fn dispatch( relayer_account: &AccountId, message: DispatchMessage<Self::DispatchPayload>, - ) -> MessageDispatchResult<Self::DispatchError>; + ) -> MessageDispatchResult<Self::DispatchLevelResult>; } /// Manages payments that are happening at the target chain during message delivery transaction. @@ -190,7 +190,7 @@ impl<MessagesProof, DispatchPayload: Decode, AccountId> MessageDispatch<AccountI for ForbidInboundMessages<MessagesProof, DispatchPayload> { type DispatchPayload = DispatchPayload; - type DispatchError = (); + type DispatchLevelResult = (); fn dispatch_weight(_message: &mut DispatchMessage<Self::DispatchPayload>) -> Weight { Weight::MAX @@ -199,7 +199,7 @@ impl<MessagesProof, DispatchPayload: Decode, AccountId> MessageDispatch<AccountI fn dispatch( _: &AccountId, _: DispatchMessage<Self::DispatchPayload>, - ) -> MessageDispatchResult<Self::DispatchError> { - MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_result: Err(()) } + ) -> MessageDispatchResult<Self::DispatchLevelResult> { + MessageDispatchResult { unspent_weight: Weight::zero(), dispatch_level_result: () } } } diff --git a/bridges/primitives/runtime/src/messages.rs b/bridges/primitives/runtime/src/messages.rs index efbd751816770af20ceeda0adb746343555a32e8..9f7c8ab5ca4e3395f7777aab4e208f1fd4fa821b 100644 --- a/bridges/primitives/runtime/src/messages.rs +++ b/bridges/primitives/runtime/src/messages.rs @@ -22,7 +22,7 @@ use scale_info::TypeInfo; /// Message dispatch result. #[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq, TypeInfo)] -pub struct MessageDispatchResult<DispatchError> { +pub struct MessageDispatchResult<DispatchLevelResult> { /// Unspent dispatch weight. This weight that will be deducted from total delivery transaction /// weight, thus reducing the transaction cost. This shall not be zero in (at least) two cases: /// @@ -31,5 +31,5 @@ pub struct MessageDispatchResult<DispatchError> { /// 2) if message has not been dispatched at all. pub unspent_weight: Weight, /// Fine-grained result of single message dispatch (for better diagnostic purposes) - pub dispatch_result: Result<(), DispatchError>, + pub dispatch_level_result: DispatchLevelResult, }