diff --git a/bridges/bin/runtime-common/src/messages.rs b/bridges/bin/runtime-common/src/messages.rs index 6f6b19595775d4417d113547cced61d07fb0d5ad..97a460716c904a86ea8be31d26192f1a2a1e71d4 100644 --- a/bridges/bin/runtime-common/src/messages.rs +++ b/bridges/bin/runtime-common/src/messages.rs @@ -22,18 +22,19 @@ pub use bp_runtime::{RangeInclusiveExt, UnderlyingChainOf, UnderlyingChainProvider}; -use bp_header_chain::{HeaderChain, HeaderChainError}; +use bp_header_chain::HeaderChain; use bp_messages::{ source_chain::{LaneMessageVerifier, TargetHeaderChain}, target_chain::{ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData, + VerificationError, }; -use bp_runtime::{Chain, RawStorageProof, Size, StorageProofChecker, StorageProofError}; +use bp_runtime::{Chain, RawStorageProof, Size, StorageProofChecker}; use codec::{Decode, Encode}; use frame_support::{traits::Get, weights::Weight, RuntimeDebug}; use hash_db::Hasher; use scale_info::TypeInfo; -use sp_std::{convert::TryFrom, fmt::Debug, marker::PhantomData, vec::Vec}; +use sp_std::{convert::TryFrom, marker::PhantomData, vec::Vec}; /// Bidirectional message bridge. pub trait MessageBridge { @@ -74,29 +75,6 @@ pub type BalanceOf<C> = bp_runtime::BalanceOf<UnderlyingChainOf<C>>; /// Type of origin that is used on the chain. pub type OriginOf<C> = <C as ThisChainWithMessages>::RuntimeOrigin; -/// Error that happens during message verification. -#[derive(Debug, PartialEq, Eq)] -pub enum Error { - /// The message proof is empty. - EmptyMessageProof, - /// Error returned by the bridged header chain. - HeaderChain(HeaderChainError), - /// Error returned while reading/decoding inbound lane data from the storage proof. - InboundLaneStorage(StorageProofError), - /// The declared message weight is incorrect. - InvalidMessageWeight, - /// Declared messages count doesn't match actual value. - MessagesCountMismatch, - /// Error returned while reading/decoding message data from the storage proof. - MessageStorage(StorageProofError), - /// The message is too large. - MessageTooLarge, - /// Error returned while reading/decoding outbound lane data from the storage proof. - OutboundLaneStorage(StorageProofError), - /// Storage proof related error. - StorageProof(StorageProofError), -} - /// Sub-module that is declaring types required for processing This -> Bridged chain messages. pub mod source { use super::*; @@ -169,14 +147,12 @@ pub mod source { + Into<Result<frame_system::RawOrigin<AccountIdOf<ThisChain<B>>>, OriginOf<ThisChain<B>>>>, AccountIdOf<ThisChain<B>>: PartialEq + Clone, { - type Error = &'static str; - fn verify_message( _submitter: &OriginOf<ThisChain<B>>, _lane: &LaneId, _lane_outbound_data: &OutboundLaneData, _payload: &FromThisChainMessagePayload, - ) -> Result<(), Self::Error> { + ) -> Result<(), VerificationError> { // IMPORTANT: any error that is returned here is fatal for the bridge, because // this code is executed at the bridge hub and message sender actually lives // at some sibling parachain. So we are failing **after** the message has been @@ -200,16 +176,15 @@ pub mod source { impl<B: MessageBridge> TargetHeaderChain<FromThisChainMessagePayload, AccountIdOf<ThisChain<B>>> for TargetHeaderChainAdapter<B> { - type Error = Error; type MessagesDeliveryProof = FromBridgedChainMessagesDeliveryProof<HashOf<BridgedChain<B>>>; - fn verify_message(payload: &FromThisChainMessagePayload) -> Result<(), Self::Error> { + fn verify_message(payload: &FromThisChainMessagePayload) -> Result<(), VerificationError> { verify_chain_message::<B>(payload) } fn verify_messages_delivery_proof( proof: Self::MessagesDeliveryProof, - ) -> Result<(LaneId, InboundLaneData<AccountIdOf<ThisChain<B>>>), Self::Error> { + ) -> Result<(LaneId, InboundLaneData<AccountIdOf<ThisChain<B>>>), VerificationError> { verify_messages_delivery_proof::<B>(proof) } } @@ -221,7 +196,7 @@ pub mod source { /// check) that would reject message (see `FromThisChainMessageVerifier`). pub fn verify_chain_message<B: MessageBridge>( payload: &FromThisChainMessagePayload, - ) -> Result<(), Error> { + ) -> Result<(), VerificationError> { // IMPORTANT: any error that is returned here is fatal for the bridge, because // this code is executed at the bridge hub and message sender actually lives // at some sibling parachain. So we are failing **after** the message has been @@ -243,7 +218,7 @@ pub mod source { // transaction also contains signatures and signed extensions. Because of this, we reserve // 1/3 of the the maximal extrinsic weight for this data. if payload.len() > maximal_message_size::<B>() as usize { - return Err(Error::MessageTooLarge) + return Err(VerificationError::MessageTooLarge) } Ok(()) @@ -255,31 +230,26 @@ pub mod source { /// parachains, please use the `verify_messages_delivery_proof_from_parachain`. pub fn verify_messages_delivery_proof<B: MessageBridge>( proof: FromBridgedChainMessagesDeliveryProof<HashOf<BridgedChain<B>>>, - ) -> Result<ParsedMessagesDeliveryProofFromBridgedChain<B>, Error> { + ) -> Result<ParsedMessagesDeliveryProofFromBridgedChain<B>, VerificationError> { let FromBridgedChainMessagesDeliveryProof { bridged_header_hash, storage_proof, lane } = proof; - B::BridgedHeaderChain::parse_finalized_storage_proof( - bridged_header_hash, - storage_proof, - |mut storage| { - // Messages delivery proof is just proof of single storage key read => any error - // is fatal. - let storage_inbound_lane_data_key = - bp_messages::storage_keys::inbound_lane_data_key( - B::BRIDGED_MESSAGES_PALLET_NAME, - &lane, - ); - let inbound_lane_data = storage - .read_and_decode_mandatory_value(storage_inbound_lane_data_key.0.as_ref()) - .map_err(Error::InboundLaneStorage)?; - - // check that the storage proof doesn't have any untouched trie nodes - storage.ensure_no_unused_nodes().map_err(Error::StorageProof)?; - - Ok((lane, inbound_lane_data)) - }, - ) - .map_err(Error::HeaderChain)? + let mut storage = + B::BridgedHeaderChain::storage_proof_checker(bridged_header_hash, storage_proof) + .map_err(VerificationError::HeaderChain)?; + // Messages delivery proof is just proof of single storage key read => any error + // is fatal. + let storage_inbound_lane_data_key = bp_messages::storage_keys::inbound_lane_data_key( + B::BRIDGED_MESSAGES_PALLET_NAME, + &lane, + ); + let inbound_lane_data = storage + .read_and_decode_mandatory_value(storage_inbound_lane_data_key.0.as_ref()) + .map_err(VerificationError::InboundLaneStorage)?; + + // check that the storage proof doesn't have any untouched trie nodes + storage.ensure_no_unused_nodes().map_err(VerificationError::StorageProof)?; + + Ok((lane, inbound_lane_data)) } } @@ -335,13 +305,12 @@ pub mod target { pub struct SourceHeaderChainAdapter<B>(PhantomData<B>); impl<B: MessageBridge> SourceHeaderChain for SourceHeaderChainAdapter<B> { - type Error = Error; type MessagesProof = FromBridgedChainMessagesProof<HashOf<BridgedChain<B>>>; fn verify_messages_proof( proof: Self::MessagesProof, messages_count: u32, - ) -> Result<ProvedMessages<Message>, Self::Error> { + ) -> Result<ProvedMessages<Message>, VerificationError> { verify_messages_proof::<B>(proof, messages_count) } } @@ -357,7 +326,7 @@ pub mod target { pub fn verify_messages_proof<B: MessageBridge>( proof: FromBridgedChainMessagesProof<HashOf<BridgedChain<B>>>, messages_count: u32, - ) -> Result<ProvedMessages<Message>, Error> { + ) -> Result<ProvedMessages<Message>, VerificationError> { let FromBridgedChainMessagesProof { bridged_header_hash, storage_proof, @@ -365,57 +334,52 @@ pub mod target { nonces_start, nonces_end, } = proof; + let storage = + B::BridgedHeaderChain::storage_proof_checker(bridged_header_hash, storage_proof) + .map_err(VerificationError::HeaderChain)?; + let mut parser = StorageProofCheckerAdapter::<_, B> { storage, _dummy: Default::default() }; let nonces_range = nonces_start..=nonces_end; - B::BridgedHeaderChain::parse_finalized_storage_proof( - bridged_header_hash, - storage_proof, - |storage| { - let mut parser = - StorageProofCheckerAdapter::<_, B> { storage, _dummy: Default::default() }; - - // receiving proofs where end < begin is ok (if proof includes outbound lane state) - let messages_in_the_proof = nonces_range.checked_len().unwrap_or(0); - if messages_in_the_proof != MessageNonce::from(messages_count) { - return Err(Error::MessagesCountMismatch) - } - - // Read messages first. All messages that are claimed to be in the proof must - // be in the proof. So any error in `read_value`, or even missing value is fatal. - // - // Mind that we allow proofs with no messages if outbound lane state is proved. - let mut messages = Vec::with_capacity(messages_in_the_proof as _); - for nonce in nonces_range { - let message_key = MessageKey { lane_id: lane, nonce }; - let message_payload = parser.read_and_decode_message_payload(&message_key)?; - messages.push(Message { key: message_key, payload: message_payload }); - } - - // Now let's check if proof contains outbound lane state proof. It is optional, so - // we simply ignore `read_value` errors and missing value. - let proved_lane_messages = ProvedLaneMessages { - lane_state: parser.read_and_decode_outbound_lane_data(&lane)?, - messages, - }; - - // Now we may actually check if the proof is empty or not. - if proved_lane_messages.lane_state.is_none() && - proved_lane_messages.messages.is_empty() - { - return Err(Error::EmptyMessageProof) - } - - // check that the storage proof doesn't have any untouched trie nodes - parser.storage.ensure_no_unused_nodes().map_err(Error::StorageProof)?; - - // We only support single lane messages in this generated_schema - let mut proved_messages = ProvedMessages::new(); - proved_messages.insert(lane, proved_lane_messages); - - Ok(proved_messages) - }, - ) - .map_err(Error::HeaderChain)? + // receiving proofs where end < begin is ok (if proof includes outbound lane state) + let messages_in_the_proof = nonces_range.checked_len().unwrap_or(0); + if messages_in_the_proof != MessageNonce::from(messages_count) { + return Err(VerificationError::MessagesCountMismatch) + } + + // Read messages first. All messages that are claimed to be in the proof must + // be in the proof. So any error in `read_value`, or even missing value is fatal. + // + // Mind that we allow proofs with no messages if outbound lane state is proved. + let mut messages = Vec::with_capacity(messages_in_the_proof as _); + for nonce in nonces_range { + let message_key = MessageKey { lane_id: lane, nonce }; + let message_payload = parser.read_and_decode_message_payload(&message_key)?; + messages.push(Message { key: message_key, payload: message_payload }); + } + + // Now let's check if proof contains outbound lane state proof. It is optional, so + // we simply ignore `read_value` errors and missing value. + let proved_lane_messages = ProvedLaneMessages { + lane_state: parser.read_and_decode_outbound_lane_data(&lane)?, + messages, + }; + + // Now we may actually check if the proof is empty or not. + if proved_lane_messages.lane_state.is_none() && proved_lane_messages.messages.is_empty() { + return Err(VerificationError::EmptyMessageProof) + } + + // check that the storage proof doesn't have any untouched trie nodes + parser + .storage + .ensure_no_unused_nodes() + .map_err(VerificationError::StorageProof)?; + + // We only support single lane messages in this generated_schema + let mut proved_messages = ProvedMessages::new(); + proved_messages.insert(lane, proved_lane_messages); + + Ok(proved_messages) } struct StorageProofCheckerAdapter<H: Hasher, B> { @@ -427,7 +391,7 @@ pub mod target { fn read_and_decode_outbound_lane_data( &mut self, lane_id: &LaneId, - ) -> Result<Option<OutboundLaneData>, Error> { + ) -> Result<Option<OutboundLaneData>, VerificationError> { let storage_outbound_lane_data_key = bp_messages::storage_keys::outbound_lane_data_key( B::BRIDGED_MESSAGES_PALLET_NAME, lane_id, @@ -435,13 +399,13 @@ pub mod target { self.storage .read_and_decode_opt_value(storage_outbound_lane_data_key.0.as_ref()) - .map_err(Error::OutboundLaneStorage) + .map_err(VerificationError::OutboundLaneStorage) } fn read_and_decode_message_payload( &mut self, message_key: &MessageKey, - ) -> Result<MessagePayload, Error> { + ) -> Result<MessagePayload, VerificationError> { let storage_message_key = bp_messages::storage_keys::message_key( B::BRIDGED_MESSAGES_PALLET_NAME, &message_key.lane_id, @@ -449,7 +413,7 @@ pub mod target { ); self.storage .read_and_decode_mandatory_value(storage_message_key.0.as_ref()) - .map_err(Error::MessageStorage) + .map_err(VerificationError::MessageStorage) } } } @@ -470,8 +434,8 @@ mod tests { }, mock::*, }; - use bp_header_chain::StoredHeaderDataBuilder; - use bp_runtime::HeaderId; + use bp_header_chain::{HeaderChainError, StoredHeaderDataBuilder}; + use bp_runtime::{HeaderId, StorageProofError}; use codec::Encode; use sp_core::H256; use sp_runtime::traits::Header as _; @@ -559,7 +523,7 @@ mod tests { using_messages_proof(10, None, encode_all_messages, encode_lane_data, |proof| { target::verify_messages_proof::<OnThisChainBridge>(proof, 5) }), - Err(Error::MessagesCountMismatch), + Err(VerificationError::MessagesCountMismatch), ); } @@ -569,7 +533,7 @@ mod tests { using_messages_proof(10, None, encode_all_messages, encode_lane_data, |proof| { target::verify_messages_proof::<OnThisChainBridge>(proof, 15) }), - Err(Error::MessagesCountMismatch), + Err(VerificationError::MessagesCountMismatch), ); } @@ -582,7 +546,7 @@ mod tests { pallet_bridge_grandpa::ImportedHeaders::<TestRuntime>::remove(bridged_header_hash); target::verify_messages_proof::<OnThisChainBridge>(proof, 10) }), - Err(Error::HeaderChain(HeaderChainError::UnknownHeader)), + Err(VerificationError::HeaderChain(HeaderChainError::UnknownHeader)), ); } @@ -605,7 +569,7 @@ mod tests { ); target::verify_messages_proof::<OnThisChainBridge>(proof, 10) }), - Err(Error::HeaderChain(HeaderChainError::StorageProof( + Err(VerificationError::HeaderChain(HeaderChainError::StorageProof( StorageProofError::StorageRootMismatch ))), ); @@ -620,7 +584,7 @@ mod tests { proof.storage_proof.push(node); target::verify_messages_proof::<OnThisChainBridge>(proof, 10) },), - Err(Error::HeaderChain(HeaderChainError::StorageProof( + Err(VerificationError::HeaderChain(HeaderChainError::StorageProof( StorageProofError::DuplicateNodesInProof ))), ); @@ -633,7 +597,7 @@ mod tests { proof.storage_proof.push(vec![42]); target::verify_messages_proof::<OnThisChainBridge>(proof, 10) },), - Err(Error::StorageProof(StorageProofError::UnusedNodesInTheProof)), + Err(VerificationError::StorageProof(StorageProofError::UnusedNodesInTheProof)), ); } @@ -647,7 +611,7 @@ mod tests { encode_lane_data, |proof| target::verify_messages_proof::<OnThisChainBridge>(proof, 10) ), - Err(Error::MessageStorage(StorageProofError::StorageValueEmpty)), + Err(VerificationError::MessageStorage(StorageProofError::StorageValueEmpty)), ); } @@ -667,7 +631,7 @@ mod tests { encode_lane_data, |proof| target::verify_messages_proof::<OnThisChainBridge>(proof, 10), ), - Err(Error::MessageStorage(StorageProofError::StorageValueDecodeFailed(_))), + Err(VerificationError::MessageStorage(StorageProofError::StorageValueDecodeFailed(_))), ); } @@ -689,7 +653,9 @@ mod tests { }, |proof| target::verify_messages_proof::<OnThisChainBridge>(proof, 10), ), - Err(Error::OutboundLaneStorage(StorageProofError::StorageValueDecodeFailed(_))), + Err(VerificationError::OutboundLaneStorage( + StorageProofError::StorageValueDecodeFailed(_) + )), ); } @@ -699,7 +665,7 @@ mod tests { using_messages_proof(0, None, encode_all_messages, encode_lane_data, |proof| { target::verify_messages_proof::<OnThisChainBridge>(proof, 0) },), - Err(Error::EmptyMessageProof), + Err(VerificationError::EmptyMessageProof), ); } @@ -773,7 +739,7 @@ mod tests { proof.nonces_end = u64::MAX; target::verify_messages_proof::<OnThisChainBridge>(proof, u32::MAX) },), - Err(Error::MessagesCountMismatch), + Err(VerificationError::MessagesCountMismatch), ); } } diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 329e4c21136775b1c8030bbf4075ec7e601ee256..50286512db89679c9c15c52143b1e877b8487c7e 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -1212,11 +1212,8 @@ mod tests { fn parse_finalized_storage_proof_rejects_proof_on_unknown_header() { run_test(|| { assert_noop!( - Pallet::<TestRuntime>::parse_finalized_storage_proof( - Default::default(), - vec![], - |_| (), - ), + Pallet::<TestRuntime>::storage_proof_checker(Default::default(), vec![],) + .map(|_| ()), bp_header_chain::HeaderChainError::UnknownHeader, ); }); @@ -1235,8 +1232,7 @@ mod tests { <ImportedHeaders<TestRuntime>>::insert(hash, header.build()); assert_ok!( - Pallet::<TestRuntime>::parse_finalized_storage_proof(hash, storage_proof, |_| (),), - (), + Pallet::<TestRuntime>::storage_proof_checker(hash, storage_proof).map(|_| ()) ); }); } diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index 045015b775125896622f42c0de14c242cec012f2..6e27c7f2f7aeb52acf19d70d369159ed24c9dc35 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -61,7 +61,7 @@ use bp_messages::{ }, total_unrewarded_messages, DeliveredMessages, InboundLaneData, InboundMessageDetails, LaneId, MessageKey, MessageNonce, MessagePayload, MessagesOperatingMode, OutboundLaneData, - OutboundMessageDetails, UnrewardedRelayersState, + OutboundMessageDetails, UnrewardedRelayersState, VerificationError, }; use bp_runtime::{BasicOperatingMode, ChainId, OwnedBridgeModule, PreComputedSize, Size}; use codec::{Decode, Encode, MaxEncodedLen}; @@ -557,9 +557,9 @@ pub mod pallet { /// The message is too large to be sent over the bridge. MessageIsTooLarge, /// Message has been treated as invalid by chain verifier. - MessageRejectedByChainVerifier, + MessageRejectedByChainVerifier(VerificationError), /// Message has been treated as invalid by lane verifier. - MessageRejectedByLaneVerifier, + MessageRejectedByLaneVerifier(VerificationError), /// Submitter has failed to pay fee for delivering and dispatching messages. FailedToWithdrawMessageFee, /// The transaction brings too many messages. @@ -732,7 +732,7 @@ fn send_message<T: Config<I>, I: 'static>( err, ); - Error::<T, I>::MessageRejectedByChainVerifier + Error::<T, I>::MessageRejectedByChainVerifier(err) })?; // now let's enforce any additional lane rules @@ -746,7 +746,7 @@ fn send_message<T: Config<I>, I: 'static>( err, ); - Error::<T, I>::MessageRejectedByLaneVerifier + Error::<T, I>::MessageRejectedByLaneVerifier(err) }, )?; @@ -918,7 +918,7 @@ impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorag fn verify_and_decode_messages_proof<Chain: SourceHeaderChain, DispatchPayload: Decode>( proof: Chain::MessagesProof, messages_count: u32, -) -> Result<ProvedMessages<DispatchMessage<DispatchPayload>>, Chain::Error> { +) -> Result<ProvedMessages<DispatchMessage<DispatchPayload>>, VerificationError> { // `receive_messages_proof` weight formula and `MaxUnconfirmedMessagesAtInboundLane` check // guarantees that the `message_count` is sane and Vec<Message> may be allocated. // (tx with too many messages will either be rejected from the pool, or will fail earlier) @@ -1177,7 +1177,9 @@ mod tests { TEST_LANE_ID, PAYLOAD_REJECTED_BY_TARGET_CHAIN, ), - Error::<TestRuntime, ()>::MessageRejectedByChainVerifier, + Error::<TestRuntime, ()>::MessageRejectedByChainVerifier(VerificationError::Other( + mock::TEST_ERROR + )), ); }); } @@ -1190,7 +1192,9 @@ mod tests { message.reject_by_lane_verifier = true; assert_noop!( send_message::<TestRuntime, ()>(RuntimeOrigin::signed(1), TEST_LANE_ID, message,), - Error::<TestRuntime, ()>::MessageRejectedByLaneVerifier, + Error::<TestRuntime, ()>::MessageRejectedByLaneVerifier(VerificationError::Other( + mock::TEST_ERROR + )), ); }); } diff --git a/bridges/modules/messages/src/mock.rs b/bridges/modules/messages/src/mock.rs index 2e45d5b601f0b2989e2347bbee393a4e6f982098..f0516fbc23f968170e293a86fa9f22aea0de40c8 100644 --- a/bridges/modules/messages/src/mock.rs +++ b/bridges/modules/messages/src/mock.rs @@ -27,7 +27,7 @@ use bp_messages::{ ProvedLaneMessages, ProvedMessages, SourceHeaderChain, }, DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload, - OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, + OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, VerificationError, }; use bp_runtime::{messages::MessageDispatchResult, Size}; use codec::{Decode, Encode}; @@ -295,13 +295,11 @@ impl Size for TestMessagesDeliveryProof { pub struct TestTargetHeaderChain; impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain { - type Error = &'static str; - type MessagesDeliveryProof = TestMessagesDeliveryProof; - fn verify_message(payload: &TestPayload) -> Result<(), Self::Error> { + fn verify_message(payload: &TestPayload) -> Result<(), VerificationError> { if *payload == PAYLOAD_REJECTED_BY_TARGET_CHAIN { - Err(TEST_ERROR) + Err(VerificationError::Other(TEST_ERROR)) } else { Ok(()) } @@ -309,8 +307,8 @@ impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain { fn verify_messages_delivery_proof( proof: Self::MessagesDeliveryProof, - ) -> Result<(LaneId, InboundLaneData<TestRelayer>), Self::Error> { - proof.0.map_err(|_| TEST_ERROR) + ) -> Result<(LaneId, InboundLaneData<TestRelayer>), VerificationError> { + proof.0.map_err(|_| VerificationError::Other(TEST_ERROR)) } } @@ -319,18 +317,16 @@ impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain { pub struct TestLaneMessageVerifier; impl LaneMessageVerifier<RuntimeOrigin, TestPayload> for TestLaneMessageVerifier { - type Error = &'static str; - fn verify_message( _submitter: &RuntimeOrigin, _lane: &LaneId, _lane_outbound_data: &OutboundLaneData, payload: &TestPayload, - ) -> Result<(), Self::Error> { + ) -> Result<(), VerificationError> { if !payload.reject_by_lane_verifier { Ok(()) } else { - Err(TEST_ERROR) + Err(VerificationError::Other(TEST_ERROR)) } } } @@ -400,15 +396,16 @@ impl DeliveryConfirmationPayments<AccountId> for TestDeliveryConfirmationPayment pub struct TestSourceHeaderChain; impl SourceHeaderChain for TestSourceHeaderChain { - type Error = &'static str; - type MessagesProof = TestMessagesProof; fn verify_messages_proof( proof: Self::MessagesProof, _messages_count: u32, - ) -> Result<ProvedMessages<Message>, Self::Error> { - proof.result.map(|proof| proof.into_iter().collect()).map_err(|_| TEST_ERROR) + ) -> Result<ProvedMessages<Message>, VerificationError> { + proof + .result + .map(|proof| proof.into_iter().collect()) + .map_err(|_| VerificationError::Other(TEST_ERROR)) } } diff --git a/bridges/modules/parachains/src/lib.rs b/bridges/modules/parachains/src/lib.rs index 6c89b09513cd6fc3d0b69abbdece323187a5a381..7670a3bacfe8d5aee2eec4dc9dfab2f9e8344205 100644 --- a/bridges/modules/parachains/src/lib.rs +++ b/bridges/modules/parachains/src/lib.rs @@ -91,6 +91,8 @@ pub mod pallet { BoundedStorageValue<<T as Config<I>>::MaxParaHeadDataSize, ParaStoredHeaderData>; /// Weight info of the given parachains pallet. pub type WeightInfoOf<T, I> = <T as Config<I>>::WeightInfo; + type GrandpaPalletOf<T, I> = + pallet_bridge_grandpa::Pallet<T, <T as Config<I>>::BridgesGrandpaPalletInstance>; #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -125,14 +127,8 @@ pub mod pallet { UnknownRelayChainBlock, /// The number of stored relay block is different from what the relayer has provided. InvalidRelayChainBlockNumber, - /// Error generated by a method defined in `bp-header-chain`. - HeaderChain(HeaderChainError), - /// Given parachain head is unknown. - UnknownParaHead, - /// The storage proof doesn't contains storage root. So it is invalid for given header. - StorageRootMismatch, - /// Failed to extract state root from given parachain head. - FailedToExtractStateRoot, + /// Parachain heads storage proof is invalid. + HeaderChainStorageProof(HeaderChainError), /// Error generated by the `OwnedBridgeModule` trait. BridgeModule(bp_runtime::OwnedBridgeModuleError), } @@ -337,112 +333,113 @@ pub mod pallet { parachains.len() as _, ); - pallet_bridge_grandpa::Pallet::<T, T::BridgesGrandpaPalletInstance>::parse_finalized_storage_proof( + let mut storage = GrandpaPalletOf::<T, I>::storage_proof_checker( relay_block_hash, parachain_heads_proof.0, - move |mut storage| { - for (parachain, parachain_head_hash) in parachains { - let parachain_head = match Pallet::<T, I>::read_parachain_head(&mut storage, parachain) { - Ok(Some(parachain_head)) => parachain_head, - Ok(None) => { - log::trace!( - target: LOG_TARGET, - "The head of parachain {:?} is None. {}", - parachain, - if ParasInfo::<T, I>::contains_key(parachain) { - "Looks like it is not yet registered at the source relay chain" - } else { - "Looks like it has been deregistered from the source relay chain" - }, - ); - Self::deposit_event(Event::MissingParachainHead { parachain }); - continue; - }, - Err(e) => { - log::trace!( - target: LOG_TARGET, - "The read of head of parachain {:?} has failed: {:?}", - parachain, - e, - ); - Self::deposit_event(Event::MissingParachainHead { parachain }); - continue; + ) + .map_err(Error::<T, I>::HeaderChainStorageProof)?; + + for (parachain, parachain_head_hash) in parachains { + let parachain_head = match Self::read_parachain_head(&mut storage, parachain) { + Ok(Some(parachain_head)) => parachain_head, + Ok(None) => { + log::trace!( + target: LOG_TARGET, + "The head of parachain {:?} is None. {}", + parachain, + if ParasInfo::<T, I>::contains_key(parachain) { + "Looks like it is not yet registered at the source relay chain" + } else { + "Looks like it has been deregistered from the source relay chain" }, - }; + ); + Self::deposit_event(Event::MissingParachainHead { parachain }); + continue + }, + Err(e) => { + log::trace!( + target: LOG_TARGET, + "The read of head of parachain {:?} has failed: {:?}", + parachain, + e, + ); + Self::deposit_event(Event::MissingParachainHead { parachain }); + continue + }, + }; + + // if relayer has specified invalid parachain head hash, ignore the head + // (this isn't strictly necessary, but better safe than sorry) + let actual_parachain_head_hash = parachain_head.hash(); + if parachain_head_hash != actual_parachain_head_hash { + log::trace!( + target: LOG_TARGET, + "The submitter has specified invalid parachain {:?} head hash: \ + {:?} vs {:?}", + parachain, + parachain_head_hash, + actual_parachain_head_hash, + ); + Self::deposit_event(Event::IncorrectParachainHeadHash { + parachain, + parachain_head_hash, + actual_parachain_head_hash, + }); + continue + } - // if relayer has specified invalid parachain head hash, ignore the head - // (this isn't strictly necessary, but better safe than sorry) - let actual_parachain_head_hash = parachain_head.hash(); - if parachain_head_hash != actual_parachain_head_hash { + // convert from parachain head into stored parachain head data + let parachain_head_data = + match T::ParaStoredHeaderDataBuilder::try_build(parachain, ¶chain_head) { + Some(parachain_head_data) => parachain_head_data, + None => { log::trace!( - target: LOG_TARGET, - "The submitter has specified invalid parachain {:?} head hash: {:?} vs {:?}", - parachain, - parachain_head_hash, - actual_parachain_head_hash, - ); - Self::deposit_event(Event::IncorrectParachainHeadHash { - parachain, - parachain_head_hash, - actual_parachain_head_hash, - }); - continue; - } - - // convert from parachain head into stored parachain head data - let parachain_head_data = match T::ParaStoredHeaderDataBuilder::try_build( - parachain, - ¶chain_head, - ) { - Some(parachain_head_data) => parachain_head_data, - None => { - log::trace!( target: LOG_TARGET, "The head of parachain {:?} has been provided, but it is not tracked by the pallet", parachain, ); - Self::deposit_event(Event::UntrackedParachainRejected { parachain }); - continue; - }, - }; - - let update_result: Result<_, ()> = ParasInfo::<T, I>::try_mutate(parachain, |stored_best_head| { - let artifacts = Pallet::<T, I>::update_parachain_head( - parachain, - stored_best_head.take(), - relay_block_number, - parachain_head_data, - parachain_head_hash, - )?; - *stored_best_head = Some(artifacts.best_head); - Ok(artifacts.prune_happened) - }); - - // we're refunding weight if update has not happened and if pruning has not happened - let is_update_happened = matches!(update_result, Ok(_)); - if !is_update_happened { - actual_weight = actual_weight - .saturating_sub(WeightInfoOf::<T, I>::parachain_head_storage_write_weight(T::DbWeight::get())); - } - let is_prune_happened = matches!(update_result, Ok(true)); - if !is_prune_happened { - actual_weight = actual_weight - .saturating_sub(WeightInfoOf::<T, I>::parachain_head_pruning_weight(T::DbWeight::get())); - } - } + Self::deposit_event(Event::UntrackedParachainRejected { parachain }); + continue + }, + }; + + let update_result: Result<_, ()> = + ParasInfo::<T, I>::try_mutate(parachain, |stored_best_head| { + let artifacts = Pallet::<T, I>::update_parachain_head( + parachain, + stored_best_head.take(), + relay_block_number, + parachain_head_data, + parachain_head_hash, + )?; + *stored_best_head = Some(artifacts.best_head); + Ok(artifacts.prune_happened) + }); + + // we're refunding weight if update has not happened and if pruning has not happened + let is_update_happened = matches!(update_result, Ok(_)); + if !is_update_happened { + actual_weight = actual_weight.saturating_sub( + WeightInfoOf::<T, I>::parachain_head_storage_write_weight( + T::DbWeight::get(), + ), + ); + } + let is_prune_happened = matches!(update_result, Ok(true)); + if !is_prune_happened { + actual_weight = actual_weight.saturating_sub( + WeightInfoOf::<T, I>::parachain_head_pruning_weight(T::DbWeight::get()), + ); + } + } - // even though we may have accepted some parachain heads, we can't allow relayers to submit - // proof with unused trie nodes - // => treat this as an error - // - // (we can throw error here, because now all our calls are transactional) - storage.ensure_no_unused_nodes() - }, - ) - .and_then(|r| r.map_err(HeaderChainError::StorageProof)) - .map_err(|e| { - log::trace!(target: LOG_TARGET, "Parachain heads storage proof is invalid: {:?}", e); - Error::<T, I>::HeaderChain(e) + // even though we may have accepted some parachain heads, we can't allow relayers to + // submit proof with unused trie nodes + // => treat this as an error + // + // (we can throw error here, because now all our calls are transactional) + storage.ensure_no_unused_nodes().map_err(|e| { + Error::<T, I>::HeaderChainStorageProof(HeaderChainError::StorageProof(e)) })?; Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes }) @@ -1438,7 +1435,7 @@ pub(crate) mod tests { // try to import head#5 of parachain#1 at relay chain block #0 assert_noop!( import_parachain_1_head(0, Default::default(), parachains, proof), - Error::<TestRuntime>::HeaderChain(HeaderChainError::StorageProof( + Error::<TestRuntime>::HeaderChainStorageProof(HeaderChainError::StorageProof( StorageProofError::StorageRootMismatch )) ); diff --git a/bridges/primitives/header-chain/src/lib.rs b/bridges/primitives/header-chain/src/lib.rs index 5e2bbad242e5b8281cda6fddcd003f5d81aecc68..5b278454728966edfffbfc726239c50341e66ce3 100644 --- a/bridges/primitives/header-chain/src/lib.rs +++ b/bridges/primitives/header-chain/src/lib.rs @@ -73,18 +73,14 @@ impl<H: HeaderT> StoredHeaderDataBuilder<H::Number, H::Hash> for H { pub trait HeaderChain<C: Chain> { /// Returns state (storage) root of given finalized header. fn finalized_header_state_root(header_hash: HashOf<C>) -> Option<HashOf<C>>; - /// Parse storage proof using finalized header. - fn parse_finalized_storage_proof<R>( + /// Get storage proof checker using finalized header. + fn storage_proof_checker( header_hash: HashOf<C>, storage_proof: RawStorageProof, - parse: impl FnOnce(StorageProofChecker<HasherOf<C>>) -> R, - ) -> Result<R, HeaderChainError> { + ) -> Result<StorageProofChecker<HasherOf<C>>, HeaderChainError> { let state_root = Self::finalized_header_state_root(header_hash) .ok_or(HeaderChainError::UnknownHeader)?; - let storage_proof_checker = bp_runtime::StorageProofChecker::new(state_root, storage_proof) - .map_err(HeaderChainError::StorageProof)?; - - Ok(parse(storage_proof_checker)) + StorageProofChecker::new(state_root, storage_proof).map_err(HeaderChainError::StorageProof) } } diff --git a/bridges/primitives/messages/Cargo.toml b/bridges/primitives/messages/Cargo.toml index 32a89f6cf78f05e89dd6d52e6fa4aa02a997b0cd..cb35b4ae65b2df4c914796b610fa2f896c29a276 100644 --- a/bridges/primitives/messages/Cargo.toml +++ b/bridges/primitives/messages/Cargo.toml @@ -14,6 +14,7 @@ serde = { version = "1.0", optional = true, features = ["derive"] } # Bridge dependencies bp-runtime = { path = "../runtime", default-features = false } +bp-header-chain = { path = "../header-chain", default-features = false } # Substrate Dependencies @@ -29,6 +30,7 @@ hex-literal = "0.4" default = ["std"] std = [ "bp-runtime/std", + "bp-header-chain/std", "codec/std", "frame-support/std", "scale-info/std", diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index e485aa2f801c3000772a5726c61629da78420145..10aee6ce97b7b487632d27e85985f68bb32a8d06 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/bridges/primitives/messages/src/lib.rs @@ -20,9 +20,15 @@ // RuntimeApi generated functions #![allow(clippy::too_many_arguments)] -use bp_runtime::{BasicOperatingMode, OperatingMode, RangeInclusiveExt}; +use bp_header_chain::HeaderChainError; +use bp_runtime::{ + messages::MessageDispatchResult, BasicOperatingMode, OperatingMode, RangeInclusiveExt, + StorageProofError, +}; use codec::{Decode, Encode, MaxEncodedLen}; -use frame_support::RuntimeDebug; +use frame_support::{PalletError, RuntimeDebug}; +// Weight is reexported to avoid additional frame-support dependencies in related crates. +pub use frame_support::weights::Weight; use scale_info::TypeInfo; use source_chain::RelayersRewards; use sp_core::TypeId; @@ -32,10 +38,6 @@ pub mod source_chain; pub mod storage_keys; pub mod target_chain; -use bp_runtime::messages::MessageDispatchResult; -// Weight is reexported to avoid additional frame-support dependencies in related crates. -pub use frame_support::weights::Weight; - /// Messages pallet operating mode. #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)] #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] @@ -414,6 +416,31 @@ pub enum BridgeMessagesCall<AccountId, MessagesProof, MessagesDeliveryProof> { }, } +/// Error that happens during message verification. +#[derive(Encode, Decode, RuntimeDebug, PartialEq, Eq, PalletError, TypeInfo)] +pub enum VerificationError { + /// The message proof is empty. + EmptyMessageProof, + /// Error returned by the bridged header chain. + HeaderChain(HeaderChainError), + /// Error returned while reading/decoding inbound lane data from the storage proof. + InboundLaneStorage(StorageProofError), + /// The declared message weight is incorrect. + InvalidMessageWeight, + /// Declared messages count doesn't match actual value. + MessagesCountMismatch, + /// Error returned while reading/decoding message data from the storage proof. + MessageStorage(StorageProofError), + /// The message is too large. + MessageTooLarge, + /// Error returned while reading/decoding outbound lane data from the storage proof. + OutboundLaneStorage(StorageProofError), + /// Storage proof related error. + StorageProof(StorageProofError), + /// Custom error + Other(#[codec(skip)] &'static str), +} + #[cfg(test)] mod tests { use super::*; diff --git a/bridges/primitives/messages/src/source_chain.rs b/bridges/primitives/messages/src/source_chain.rs index 394a934171feab441e785528584748dad9f06010..f3c50b84a09ad276229a4f4c0aa511302df16b47 100644 --- a/bridges/primitives/messages/src/source_chain.rs +++ b/bridges/primitives/messages/src/source_chain.rs @@ -16,7 +16,7 @@ //! Primitives of messages module, that are used on the source chain. -use crate::{InboundLaneData, LaneId, MessageNonce, OutboundLaneData}; +use crate::{InboundLaneData, LaneId, MessageNonce, OutboundLaneData, VerificationError}; use crate::UnrewardedRelayer; use bp_runtime::Size; @@ -40,9 +40,6 @@ pub type RelayersRewards<AccountId> = BTreeMap<AccountId, MessageNonce>; /// source chain to the target chain. The `AccountId` type here means the account /// type used by the source chain. pub trait TargetHeaderChain<Payload, AccountId> { - /// Error type. - type Error: Debug; - /// Proof that messages have been received by target chain. type MessagesDeliveryProof: Parameter + Size; @@ -58,12 +55,12 @@ pub trait TargetHeaderChain<Payload, AccountId> { /// 1MB. BTC nodes aren't accepting transactions that are larger than 1MB, so relayer /// will be unable to craft valid transaction => this (and all subsequent) messages will /// never be delivered. - fn verify_message(payload: &Payload) -> Result<(), Self::Error>; + fn verify_message(payload: &Payload) -> Result<(), VerificationError>; /// Verify messages delivery proof and return lane && nonce of the latest received message. fn verify_messages_delivery_proof( proof: Self::MessagesDeliveryProof, - ) -> Result<(LaneId, InboundLaneData<AccountId>), Self::Error>; + ) -> Result<(LaneId, InboundLaneData<AccountId>), VerificationError>; } /// Lane message verifier. @@ -75,9 +72,6 @@ pub trait TargetHeaderChain<Payload, AccountId> { /// /// Any fee requirements should also be enforced here. pub trait LaneMessageVerifier<SenderOrigin, Payload> { - /// Error type. - type Error: Debug + Into<&'static str>; - /// Verify message payload and return Ok(()) if message is valid and allowed to be sent over the /// lane. fn verify_message( @@ -85,7 +79,7 @@ pub trait LaneMessageVerifier<SenderOrigin, Payload> { lane: &LaneId, outbound_data: &OutboundLaneData, payload: &Payload, - ) -> Result<(), Self::Error>; + ) -> Result<(), VerificationError>; } /// Manages payments that are happening at the source chain during delivery confirmation @@ -169,31 +163,27 @@ const ALL_OUTBOUND_MESSAGES_REJECTED: &str = "This chain is configured to reject all outbound messages"; impl<Payload, AccountId> TargetHeaderChain<Payload, AccountId> for ForbidOutboundMessages { - type Error = &'static str; - type MessagesDeliveryProof = (); - fn verify_message(_payload: &Payload) -> Result<(), Self::Error> { - Err(ALL_OUTBOUND_MESSAGES_REJECTED) + fn verify_message(_payload: &Payload) -> Result<(), VerificationError> { + Err(VerificationError::Other(ALL_OUTBOUND_MESSAGES_REJECTED)) } fn verify_messages_delivery_proof( _proof: Self::MessagesDeliveryProof, - ) -> Result<(LaneId, InboundLaneData<AccountId>), Self::Error> { - Err(ALL_OUTBOUND_MESSAGES_REJECTED) + ) -> Result<(LaneId, InboundLaneData<AccountId>), VerificationError> { + Err(VerificationError::Other(ALL_OUTBOUND_MESSAGES_REJECTED)) } } impl<SenderOrigin, Payload> LaneMessageVerifier<SenderOrigin, Payload> for ForbidOutboundMessages { - type Error = &'static str; - fn verify_message( _submitter: &SenderOrigin, _lane: &LaneId, _outbound_data: &OutboundLaneData, _payload: &Payload, - ) -> Result<(), Self::Error> { - Err(ALL_OUTBOUND_MESSAGES_REJECTED) + ) -> Result<(), VerificationError> { + Err(VerificationError::Other(ALL_OUTBOUND_MESSAGES_REJECTED)) } } diff --git a/bridges/primitives/messages/src/target_chain.rs b/bridges/primitives/messages/src/target_chain.rs index 3c2e8cf0cb074da1b9ae1d71859c0ca5d32f81cf..385bc4ac373d63af2d247af4dc60e2d0c02fed97 100644 --- a/bridges/primitives/messages/src/target_chain.rs +++ b/bridges/primitives/messages/src/target_chain.rs @@ -16,7 +16,9 @@ //! Primitives of messages module, that are used on the target chain. -use crate::{LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData}; +use crate::{ + LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData, VerificationError, +}; use bp_runtime::{messages::MessageDispatchResult, Size}; use codec::{Decode, Encode, Error as CodecError}; @@ -58,9 +60,6 @@ pub struct DispatchMessage<DispatchPayload> { /// can't change. Wrong implementation may lead to invalid lane states (i.e. lane /// that's stuck) and/or processing messages without paying fees. pub trait SourceHeaderChain { - /// Error type. - type Error: Debug; - /// Proof that messages are sent from source chain. This may also include proof /// of corresponding outbound lane states. type MessagesProof: Parameter + Size; @@ -79,7 +78,7 @@ pub trait SourceHeaderChain { fn verify_messages_proof( proof: Self::MessagesProof, messages_count: u32, - ) -> Result<ProvedMessages<Message>, Self::Error>; + ) -> Result<ProvedMessages<Message>, VerificationError>; } /// Called when inbound message is received. @@ -164,21 +163,20 @@ pub struct ForbidInboundMessages<MessagesProof, DispatchPayload>( PhantomData<(MessagesProof, DispatchPayload)>, ); -/// Error message that is used in `ForbidOutboundMessages` implementation. +/// Error message that is used in `ForbidInboundMessages` implementation. const ALL_INBOUND_MESSAGES_REJECTED: &str = "This chain is configured to reject all inbound messages"; impl<MessagesProof: Parameter + Size, DispatchPayload> SourceHeaderChain for ForbidInboundMessages<MessagesProof, DispatchPayload> { - type Error = &'static str; type MessagesProof = MessagesProof; fn verify_messages_proof( _proof: Self::MessagesProof, _messages_count: u32, - ) -> Result<ProvedMessages<Message>, Self::Error> { - Err(ALL_INBOUND_MESSAGES_REJECTED) + ) -> Result<ProvedMessages<Message>, VerificationError> { + Err(VerificationError::Other(ALL_INBOUND_MESSAGES_REJECTED)) } }