diff --git a/core/src/message/context.rs b/core/src/message/context.rs index 32929d7b75f..cd4e5bbc124 100644 --- a/core/src/message/context.rs +++ b/core/src/message/context.rs @@ -141,34 +141,40 @@ impl ContextOutcome { } } } +/// Store of current temporary message execution context. +#[derive(Clone, Default, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Decode, Encode, TypeInfo)] +pub struct TmpStore { + outgoing: BTreeMap>, + reply: Option, +} /// Store of previous message execution context. #[derive(Clone, Default, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Decode, Encode, TypeInfo)] #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] pub struct ContextStore { - outgoing: BTreeMap>, - reply: Option, initialized: BTreeSet, reservation_nonce: ReservationNonce, system_reservation: Option, + /// Used to prevent creating messages with the same ID in DB. Before this was achieved by using `outgoing.len()` + /// but now it is moved to [TmpStore] thus we need to keep nonce here. Now to calculate nonce we do `local_nonce + tmp_store.outgoing.len()` + /// and we update `local_nonce` once message context is drained. + local_nonce: u32, } impl ContextStore { // TODO: Remove, only used in migrations (#issue 3721) /// Create a new context store with the provided parameters. pub fn new( - outgoing: BTreeMap>, - reply: Option, initialized: BTreeSet, reservation_nonce: ReservationNonce, system_reservation: Option, + local_nonce: u32, ) -> Self { Self { - outgoing, - reply, initialized, reservation_nonce, system_reservation, + local_nonce, } } @@ -208,6 +214,7 @@ pub struct MessageContext { current: IncomingMessage, outcome: ContextOutcome, store: ContextStore, + tmp_store: TmpStore, settings: ContextSettings, outgoing_bytes_counter: u32, } @@ -222,29 +229,16 @@ impl MessageContext { ) -> Option { let (kind, message, store) = dispatch.into_parts(); - let outgoing_bytes_counter = match &store { - Some(store) => { - let mut counter = 0u32; - for payload in store.outgoing.values().filter_map(|x| x.as_ref()) { - counter = counter.checked_add(payload.len_u32())?; - } - counter - } - None => 0, - }; - - if outgoing_bytes_counter > settings.outgoing_bytes_limit { - // Outgoing messages bytes limit exceeded. - return None; - } - Some(Self { kind, outcome: ContextOutcome::new(program_id, message.source(), message.id()), current: message, store: store.unwrap_or_default(), + tmp_store: TmpStore::default(), settings, - outgoing_bytes_counter, + // message context *always* starts with zero bytes outgoing. Before it could be non-zero and eventually + // overflow the current limit but now we do not save state between executions + outgoing_bytes_counter: 0, }) } @@ -288,7 +282,7 @@ impl MessageContext { return Err(Error::DuplicateInit); } - let last = self.store.outgoing.len() as u32; + let last = self.store.local_nonce + self.tmp_store.outgoing.len() as u32; if last >= self.settings.outgoing_limit { return Err(Error::OutgoingMessagesAmountLimitExceeded); @@ -297,7 +291,7 @@ impl MessageContext { let message_id = MessageId::generate_outgoing(self.current.id(), last); let message = InitMessage::from_packet(message_id, packet); - self.store.outgoing.insert(last, None); + self.tmp_store.outgoing.insert(last, None); self.store.initialized.insert(program_id); self.outcome.init.push((message, delay, None)); @@ -316,7 +310,7 @@ impl MessageContext { reservation: Option, ) -> Result { let outgoing = self - .store + .tmp_store .outgoing .get_mut(&handle) .ok_or(Error::OutOfBounds)?; @@ -362,10 +356,12 @@ impl MessageContext { /// /// Returns it's handle. pub fn send_init(&mut self) -> Result { - let last = self.store.outgoing.len() as u32; + let last = self.store.local_nonce + self.tmp_store.outgoing.len() as u32; if last < self.settings.outgoing_limit { - self.store.outgoing.insert(last, Some(Default::default())); + self.tmp_store + .outgoing + .insert(last, Some(Default::default())); Ok(last) } else { @@ -375,10 +371,13 @@ impl MessageContext { /// Pushes payload into stored payload by handle. pub fn send_push(&mut self, handle: u32, buffer: &[u8]) -> Result<(), Error> { - let data = match self.store.outgoing.get_mut(&handle) { + let data = match self.tmp_store.outgoing.get_mut(&handle) { Some(Some(data)) => data, Some(None) => return Err(Error::LateAccess), - None => return Err(Error::OutOfBounds), + None => { + // debug!("OOB: {:#?}", backtrace::Backtrace::new().frames()); + return Err(Error::OutOfBounds); + } }; let new_outgoing_bytes = Self::increase_counter( @@ -398,7 +397,7 @@ impl MessageContext { /// Pushes the incoming buffer/payload into stored payload by handle. pub fn send_push_input(&mut self, handle: u32, range: CheckedRange) -> Result<(), Error> { - let data = match self.store.outgoing.get_mut(&handle) { + let data = match self.tmp_store.outgoing.get_mut(&handle) { Some(Some(data)) => data, Some(None) => return Err(Error::LateAccess), None => return Err(Error::OutOfBounds), @@ -467,10 +466,10 @@ impl MessageContext { return Err(Error::DuplicateReply.into()); } - let data = self.store.reply.take().unwrap_or_default(); + let data = self.tmp_store.reply.take().unwrap_or_default(); if let Err(data) = packet.try_prepend(data) { - self.store.reply = Some(data); + self.tmp_store.reply = Some(data); return Err(Error::MaxMessageSizeExceed.into()); } @@ -491,7 +490,7 @@ impl MessageContext { } // NOTE: it's normal to not undone `get_or_insert_with` in case of error - self.store + self.tmp_store .reply .get_or_insert_with(Default::default) .try_extend_from_slice(buffer) @@ -517,7 +516,7 @@ impl MessageContext { } = range; // NOTE: it's normal to not undone `get_or_insert_with` in case of error - self.store + self.tmp_store .reply .get_or_insert_with(Default::default) .try_extend_from_slice(&self.current.payload_bytes()[offset..excluded_end]) @@ -584,7 +583,8 @@ impl MessageContext { } /// Destructs context after execution and returns provided outcome and store. - pub fn drain(self) -> (ContextOutcome, ContextStore) { + pub fn drain(mut self) -> (ContextOutcome, ContextStore) { + self.store.local_nonce += self.tmp_store.outgoing.len() as u32; let Self { outcome, store, .. } = self; (outcome, store) @@ -815,18 +815,13 @@ mod tests { Error::OutgoingMessagesBytesLimitExceeded, ); } - - #[test] + /* #[test] fn create_wrong_context() { let context_store = ContextStore { - outgoing: [(1, Some(vec![1, 2].try_into().unwrap()))] - .iter() - .cloned() - .collect(), - reply: None, initialized: BTreeSet::new(), reservation_nonce: ReservationNonce::default(), system_reservation: None, + local_nonce: 0, }; let incoming_dispatch = IncomingDispatch::new( @@ -844,7 +839,7 @@ mod tests { // Creating a message context must return None, // because of the outgoing messages bytes limit exceeded. assert!(ctx.is_none(), "Expect None, got {:?}", ctx); - } + }*/ #[test] fn outgoing_limit_exceeded() { @@ -979,8 +974,6 @@ mod tests { // Checking that the initial parameters of the context match the passed constants assert_eq!(context.current().id(), MessageId::from(INCOMING_MESSAGE_ID)); - assert!(context.store.reply.is_none()); - assert!(context.outcome.reply.is_none()); // Creating a reply packet let reply_packet = ReplyPacket::new(vec![0, 0].try_into().unwrap(), 0); @@ -1038,7 +1031,7 @@ mod tests { // And checking that it is not formed assert!(context - .store + .tmp_store .outgoing .get(&expected_handle) .expect("This key should be") diff --git a/gsdk/src/metadata/generated.rs b/gsdk/src/metadata/generated.rs index 4d3c64a6c0b..252980c6d3d 100644 --- a/gsdk/src/metadata/generated.rs +++ b/gsdk/src/metadata/generated.rs @@ -698,27 +698,13 @@ pub mod runtime_types { Debug, crate::gp::Decode, crate::gp::DecodeAsType, crate::gp::Encode, )] pub struct ContextStore { - pub outgoing: ::subxt::ext::subxt_core::utils::KeyedVec< - ::core::primitive::u32, - ::core::option::Option< - runtime_types::gear_core::buffer::LimitedVec< - ::core::primitive::u8, - runtime_types::gear_core::message::PayloadSizeError, - >, - >, - >, - pub reply: ::core::option::Option< - runtime_types::gear_core::buffer::LimitedVec< - ::core::primitive::u8, - runtime_types::gear_core::message::PayloadSizeError, - >, - >, pub initialized: ::subxt::ext::subxt_core::alloc::vec::Vec< runtime_types::gprimitives::ActorId, >, pub reservation_nonce: runtime_types::gear_core::reservation::ReservationNonce, pub system_reservation: ::core::option::Option<::core::primitive::u64>, + pub local_nonce: ::core::primitive::u32, } } pub mod stored { diff --git a/pallets/gear-messenger/src/lib.rs b/pallets/gear-messenger/src/lib.rs index 545bd057879..89dd4c31a8c 100644 --- a/pallets/gear-messenger/src/lib.rs +++ b/pallets/gear-messenger/src/lib.rs @@ -145,6 +145,7 @@ mod mock; #[cfg(test)] mod tests; +pub mod migrations; pub mod pallet_tests; // Public exports from pallet. @@ -166,7 +167,7 @@ pub mod pallet { use sp_std::{convert::TryInto, marker::PhantomData}; /// The current storage version. - pub(crate) const MESSENGER_STORAGE_VERSION: StorageVersion = StorageVersion::new(3); + pub(crate) const MESSENGER_STORAGE_VERSION: StorageVersion = StorageVersion::new(4); // Gear Messenger Pallet's `Config`. #[pallet::config] diff --git a/pallets/gear-messenger/src/migrations/context_store.rs b/pallets/gear-messenger/src/migrations/context_store.rs new file mode 100644 index 00000000000..db1fca2f73b --- /dev/null +++ b/pallets/gear-messenger/src/migrations/context_store.rs @@ -0,0 +1,186 @@ +// This file is part of Gear. + +// Copyright (C) 2021-2024 Gear Technologies Inc. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +use core::marker::PhantomData; + +use crate::{Config, Pallet, Waitlist}; +use common::{ + storage::{Interval, LinkedNode}, + MessageId, +}; + +#[cfg(feature = "try-runtime")] +use {frame_support::ensure, sp_runtime::TryRuntimeError, sp_std::vec::Vec}; + +use frame_support::{ + traits::{GetStorageVersion, OnRuntimeUpgrade, StorageVersion}, + weights::Weight, +}; +use frame_system::pallet_prelude::BlockNumberFor; +use gear_core::message::{ContextStore, StoredDispatch}; + +use crate::Dispatches; +use sp_runtime::traits::Get; +pub struct RemoveCommitStorage(PhantomData); + +const MIGRATE_FROM_VERSION: u16 = 3; +const MIGRATE_TO_VERSION: u16 = 4; +const ALLOWED_CURRENT_STORAGE_VERSION: u16 = 4; + +fn translate_dispatch(dispatch: v3::StoredDispatch) -> StoredDispatch { + StoredDispatch::new( + dispatch.kind, + dispatch.message, + dispatch.context.map(|ctx| { + ContextStore::new( + ctx.initialized, + ctx.reservation_nonce, + ctx.system_reservation, + // set local_none to previous number of outgoing messages to not break existing IDs. + ctx.outgoing.len() as u32, + ) + }), + ) +} + +impl OnRuntimeUpgrade for RemoveCommitStorage { + fn on_runtime_upgrade() -> Weight { + let onchain = Pallet::::on_chain_storage_version(); + + let mut weight = T::DbWeight::get().reads(1); + let mut counter = 0; + + if onchain == MIGRATE_FROM_VERSION { + let current = Pallet::::current_storage_version(); + if current != ALLOWED_CURRENT_STORAGE_VERSION { + log::error!("❌ Migration is not allowed for current storage version {current:?}."); + return weight; + } + + let update_to = StorageVersion::new(MIGRATE_TO_VERSION); + log::info!("🚚 Running migration from {onchain:?} to {update_to:?}, current storage version is {current:?}."); + + Dispatches::::translate(|_, value: LinkedNode| { + counter += 1; + Some(LinkedNode { + next: value.next, + value: translate_dispatch(value.value), + }) + }); + + Waitlist::::translate( + |_, _, (dispatch, interval): (v3::StoredDispatch, Interval>)| { + counter += 1; + Some((translate_dispatch(dispatch), interval)) + }, + ); + // each `translate` call results in read to DB to fetch dispatch and then write to DB to update it. + weight = weight.saturating_add(T::DbWeight::get().reads_writes(counter, counter)); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + + update_to.put::>(); + + log::info!("✅ Successfully migrated storage. {counter} codes have been migrated"); + } else { + log::info!("🟠 Migration requires onchain version {MIGRATE_FROM_VERSION}, so was skipped for {onchain:?}"); + } + + weight + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, TryRuntimeError> { + let current = Pallet::::current_storage_version(); + let onchain = Pallet::::on_chain_storage_version(); + + if onchain == MIGRATE_FROM_VERSION { + ensure!( + current == ALLOWED_CURRENT_STORAGE_VERSION, + "Current storage version is not allowed for migration, check migration code in order to allow it." + ); + + Ok(vec![1]) + } else { + Ok(vec![0]) + } + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { + if state[0] == 1 { + ensure!( + Pallet::::on_chain_storage_version() == MIGRATE_TO_VERSION, + "incorrect storage version after migration" + ); + } + + Ok(()) + } +} + +mod v3 { + use common::ProgramId; + + use gear_core::{ + message::{DispatchKind, Payload, StoredMessage}, + reservation::ReservationNonce, + }; + + use scale_info::{ + scale::{Decode, Encode}, + TypeInfo, + }; + + use sp_std::collections::{btree_map::BTreeMap, btree_set::BTreeSet}; + + #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Decode, Encode, TypeInfo)] + pub struct StoredDispatch { + pub kind: DispatchKind, + pub message: StoredMessage, + pub context: Option, + } + #[derive( + Clone, Default, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Decode, Encode, TypeInfo, + )] + pub struct ContextStore { + pub outgoing: BTreeMap>, + pub reply: Option, + pub initialized: BTreeSet, + pub reservation_nonce: ReservationNonce, + pub system_reservation: Option, + } +} + +#[cfg(test)] +#[cfg(feature = "try-runtime")] +mod test { + use super::*; + use crate::mock::{new_test_ext, *}; + use frame_support::traits::StorageVersion; + + #[test] + fn context_store_migration_works() { + new_test_ext().execute_with(|| { + StorageVersion::new(MIGRATE_FROM_VERSION).put::(); + let state = RemoveCommitStorage::::pre_upgrade().unwrap(); + let _ = RemoveCommitStorage::::on_runtime_upgrade(); + RemoveCommitStorage::::post_upgrade(state).unwrap(); + + assert_eq!(StorageVersion::get::(), MIGRATE_TO_VERSION); + }); + } +} diff --git a/pallets/gear-messenger/src/migrations/mod.rs b/pallets/gear-messenger/src/migrations/mod.rs new file mode 100644 index 00000000000..6fb5a3ffdfc --- /dev/null +++ b/pallets/gear-messenger/src/migrations/mod.rs @@ -0,0 +1,19 @@ +// This file is part of Gear. + +// Copyright (C) 2021-2024 Gear Technologies Inc. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +pub mod context_store; diff --git a/pallets/gear/src/tests.rs b/pallets/gear/src/tests.rs index 9d0f129d77b..4000f3383d7 100644 --- a/pallets/gear/src/tests.rs +++ b/pallets/gear/src/tests.rs @@ -12636,9 +12636,12 @@ fn incomplete_async_payloads_kept() { None, Some("OK PING"), Some("OK REPLY"), - None, - Some("STORED COMMON"), - Some("STORED REPLY"), + // payload is not kept between executions so we get a panic here + Some( + "Panic occurred: panicked with 'Failed to push payload: Ext(Message(OutOfBounds))'", + ), + // here we get `REPLY` again because its `push("REPLY")` + `commit()` + Some("REPLY"), ] .iter() .map(|v| { @@ -15484,8 +15487,8 @@ fn incorrect_store_context() { QueueOf::::queue(dispatch).unwrap(); run_to_next_block(None); - - assert_failed(mid, ActorExecutionErrorReplyReason::UnsupportedMessage); + // does not fail anymore, context does not keep state between executions + assert_succeed(mid); }); } @@ -16809,7 +16812,7 @@ pub(crate) mod utils { if messages.len() != assertions.len() { panic!( - "Expected {} messages, you assert only {} of them\n{:?}", + "Expected {} messages, you assert only {} of them\n{:#?}", messages.len(), assertions.len(), messages diff --git a/runtime/vara/src/migrations.rs b/runtime/vara/src/migrations.rs index 9b7aa1ddb0c..4b30ef74444 100644 --- a/runtime/vara/src/migrations.rs +++ b/runtime/vara/src/migrations.rs @@ -27,6 +27,7 @@ pub type Migrations = ( pallet_grandpa::migrations::MigrateV4ToV5, // move allocations to a separate storage item and remove pages_with_data field from program pallet_gear_program::migrations::allocations::MigrateAllocations, + pallet_gear_messenger::migrations::context_store::RemoveCommitStorage, ); mod staking {