From 8fca6cf78642ebaaa3dfcb306574d2d503a36bd9 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Sun, 21 May 2023 20:20:14 +0100 Subject: [PATCH] FRAME: Allow message ID to be mutated in ProcessMessage (#14183) --- frame/message-queue/src/benchmarking.rs | 6 +++--- frame/message-queue/src/lib.rs | 22 ++++++++++------------ frame/message-queue/src/mock.rs | 2 ++ frame/message-queue/src/mock_helpers.rs | 1 + frame/message-queue/src/tests.rs | 13 +++++++------ frame/support/src/traits/messages.rs | 1 + 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/frame/message-queue/src/benchmarking.rs b/frame/message-queue/src/benchmarking.rs index b53527048ac61..53c84c3da1c07 100644 --- a/frame/message-queue/src/benchmarking.rs +++ b/frame/message-queue/src/benchmarking.rs @@ -142,7 +142,7 @@ mod benchmarks { // Check that it was processed. assert_last_event::( Event::Processed { - hash: T::Hashing::hash(&msg), + id: sp_io::hashing::blake2_256(&msg), origin: 0.into(), weight_used: 1.into_weight(), success: true, @@ -227,7 +227,7 @@ mod benchmarks { assert_last_event::( Event::Processed { - hash: T::Hashing::hash(&((msgs - 1) as u32).encode()), + id: sp_io::hashing::blake2_256(&((msgs - 1) as u32).encode()), origin: 0.into(), weight_used: Weight::from_parts(1, 1), success: true, @@ -264,7 +264,7 @@ mod benchmarks { assert_last_event::( Event::Processed { - hash: T::Hashing::hash(&((msgs - 1) as u32).encode()), + id: sp_io::hashing::blake2_256(&((msgs - 1) as u32).encode()), origin: 0.into(), weight_used: Weight::from_parts(1, 1), success: true, diff --git a/frame/message-queue/src/lib.rs b/frame/message-queue/src/lib.rs index c8e1976103ebf..37fbe85fd56be 100644 --- a/frame/message-queue/src/lib.rs +++ b/frame/message-queue/src/lib.rs @@ -204,7 +204,7 @@ pub use pallet::*; use scale_info::TypeInfo; use sp_arithmetic::traits::{BaseArithmetic, Unsigned}; use sp_runtime::{ - traits::{Hash, One, Zero}, + traits::{One, Zero}, SaturatedConversion, Saturating, }; use sp_std::{fmt::Debug, ops::Deref, prelude::*, vec}; @@ -499,16 +499,13 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { - /// Message discarded due to an inability to decode the item. Usually caused by state - /// corruption. - Discarded { hash: T::Hash }, /// Message discarded due to an error in the `MessageProcessor` (usually a format error). - ProcessingFailed { hash: T::Hash, origin: MessageOriginOf, error: ProcessMessageError }, + ProcessingFailed { id: [u8; 32], origin: MessageOriginOf, error: ProcessMessageError }, /// Message is processed. - Processed { hash: T::Hash, origin: MessageOriginOf, weight_used: Weight, success: bool }, + Processed { id: [u8; 32], origin: MessageOriginOf, weight_used: Weight, success: bool }, /// Message placed in overweight queue. OverweightEnqueued { - hash: T::Hash, + id: [u8; 32], origin: MessageOriginOf, page_index: PageIndex, message_index: T::Size, @@ -1147,15 +1144,16 @@ impl Pallet { meter: &mut WeightMeter, overweight_limit: Weight, ) -> MessageExecutionStatus { - let hash = T::Hashing::hash(message); + let hash = sp_io::hashing::blake2_256(message); use ProcessMessageError::*; let prev_consumed = meter.consumed; + let mut id = hash; - match T::MessageProcessor::process_message(message, origin.clone(), meter) { + match T::MessageProcessor::process_message(message, origin.clone(), meter, &mut id) { Err(Overweight(w)) if w.any_gt(overweight_limit) => { // Permanently overweight. Self::deposit_event(Event::::OverweightEnqueued { - hash, + id, origin, page_index, message_index, @@ -1173,13 +1171,13 @@ impl Pallet { }, Err(error @ BadFormat | error @ Corrupt | error @ Unsupported) => { // Permanent error - drop - Self::deposit_event(Event::::ProcessingFailed { hash, origin, error }); + Self::deposit_event(Event::::ProcessingFailed { id, origin, error }); MessageExecutionStatus::Unprocessable { permanent: true } }, Ok(success) => { // Success let weight_used = meter.consumed.saturating_sub(prev_consumed); - Self::deposit_event(Event::::Processed { hash, origin, weight_used, success }); + Self::deposit_event(Event::::Processed { id, origin, weight_used, success }); MessageExecutionStatus::Processed }, } diff --git a/frame/message-queue/src/mock.rs b/frame/message-queue/src/mock.rs index a0fe0105671e0..71f0b0fa20e30 100644 --- a/frame/message-queue/src/mock.rs +++ b/frame/message-queue/src/mock.rs @@ -172,6 +172,7 @@ impl ProcessMessage for RecordingMessageProcessor { message: &[u8], origin: Self::Origin, meter: &mut WeightMeter, + _id: &mut [u8; 32], ) -> Result { processing_message(message, &origin)?; @@ -239,6 +240,7 @@ impl ProcessMessage for CountingMessageProcessor { message: &[u8], origin: Self::Origin, meter: &mut WeightMeter, + _id: &mut [u8; 32], ) -> Result { if let Err(e) = processing_message(message, &origin) { NumMessagesErrored::set(NumMessagesErrored::get() + 1); diff --git a/frame/message-queue/src/mock_helpers.rs b/frame/message-queue/src/mock_helpers.rs index 257691cae4171..5a39eb0a59885 100644 --- a/frame/message-queue/src/mock_helpers.rs +++ b/frame/message-queue/src/mock_helpers.rs @@ -62,6 +62,7 @@ where _message: &[u8], _origin: Self::Origin, meter: &mut WeightMeter, + _id: &mut [u8; 32], ) -> Result { let required = Weight::from_parts(REQUIRED_WEIGHT, REQUIRED_WEIGHT); diff --git a/frame/message-queue/src/tests.rs b/frame/message-queue/src/tests.rs index 15bb905738531..10eff69c926e5 100644 --- a/frame/message-queue/src/tests.rs +++ b/frame/message-queue/src/tests.rs @@ -23,6 +23,7 @@ use crate::{mock::*, *}; use frame_support::{assert_noop, assert_ok, assert_storage_noop, StorageNoopGuard}; use rand::{rngs::StdRng, Rng, SeedableRng}; +use sp_core::blake2_256; #[test] fn mocked_weight_works() { @@ -178,7 +179,7 @@ fn service_queues_failing_messages_works() { assert_eq!(MessageQueue::service_queues(1.into_weight()), 1.into_weight()); assert_last_event::( Event::ProcessingFailed { - hash: ::Hashing::hash(b"badformat"), + id: blake2_256(b"badformat"), origin: MessageOrigin::Here, error: ProcessMessageError::BadFormat, } @@ -187,7 +188,7 @@ fn service_queues_failing_messages_works() { assert_eq!(MessageQueue::service_queues(1.into_weight()), 1.into_weight()); assert_last_event::( Event::ProcessingFailed { - hash: ::Hashing::hash(b"corrupt"), + id: blake2_256(b"corrupt"), origin: MessageOrigin::Here, error: ProcessMessageError::Corrupt, } @@ -196,7 +197,7 @@ fn service_queues_failing_messages_works() { assert_eq!(MessageQueue::service_queues(1.into_weight()), 1.into_weight()); assert_last_event::( Event::ProcessingFailed { - hash: ::Hashing::hash(b"unsupported"), + id: blake2_256(b"unsupported"), origin: MessageOrigin::Here, error: ProcessMessageError::Unsupported, } @@ -677,7 +678,7 @@ fn service_page_item_skips_perm_overweight_message() { assert_eq!(weight.consumed, 2.into_weight()); assert_last_event::( Event::OverweightEnqueued { - hash: ::Hashing::hash(b"TooMuch"), + id: blake2_256(b"TooMuch"), origin: MessageOrigin::Here, message_index: 0, page_index: 0, @@ -1050,7 +1051,7 @@ fn execute_overweight_works() { assert_eq!(QueueChanges::take(), vec![(origin, 1, 8)]); assert_last_event::( Event::OverweightEnqueued { - hash: ::Hashing::hash(b"weight=6"), + id: blake2_256(b"weight=6"), origin: MessageOrigin::Here, message_index: 0, page_index: 0, @@ -1105,7 +1106,7 @@ fn permanently_overweight_book_unknits() { assert_eq!(MessageQueue::service_queues(8.into_weight()), 4.into_weight()); assert_last_event::( Event::OverweightEnqueued { - hash: ::Hashing::hash(b"weight=9"), + id: blake2_256(b"weight=9"), origin: Here, message_index: 0, page_index: 0, diff --git a/frame/support/src/traits/messages.rs b/frame/support/src/traits/messages.rs index 781da3ed6c704..fe907b0c6d63e 100644 --- a/frame/support/src/traits/messages.rs +++ b/frame/support/src/traits/messages.rs @@ -59,6 +59,7 @@ pub trait ProcessMessage { message: &[u8], origin: Self::Origin, meter: &mut WeightMeter, + id: &mut [u8; 32], ) -> Result; }