feat(pallet_gear): Introduce invariant: all user non-reply messages go to mailbox

core-processor/src/ext.rs

@@ -38,8 +38,8 @@ use gear_core::{
         AllocError, AllocationsContext, GrowHandler, Memory, MemoryError, MemoryInterval, PageBuf,
     },
     message::{
-        ContextOutcomeDrain, ContextStore, Dispatch, GasLimit, HandlePacket, InitPacket,
-        MessageContext, Packet, ReplyPacket,
+        ContextOutcomeDrain, ContextStore, Dispatch, DispatchKind, GasLimit, HandlePacket,
+        InitPacket, MessageContext, Packet, ReplyPacket,
     },
     pages::{
         numerated::{interval::Interval, tree::IntervalsTree},
@@ -422,66 +422,6 @@ impl<'a, LP: LazyPagesInterface> ExtMutator<'a, LP> {
         Ok(())
     }
 
-    // It's temporary fn, used to solve `core-audit/issue#22`.
-    fn safe_gasfull_sends<T: Packet>(
-        &mut self,
-        packet: &T,
-        delay: u32,
-    ) -> Result<(), FallibleExtError> {
-        // In case of delayed sending from origin message we keep some gas
-        // for it while processing outgoing sending notes so gas for
-        // previously gasless sends should appear to prevent their
-        // invasion for gas for storing delayed message.
-        match (packet.gas_limit(), delay != 0) {
-            // Zero gasfull instant.
-            //
-            // In this case there is nothing to do.
-            (Some(0), false) => Ok(()),
-
-            // Any non-zero gasfull or zero gasfull with delay.
-            //
-            // In case of zero gasfull with delay it's pretty similar to
-            // gasless with delay case.
-            //
-            // In case of any non-zero gasfull we prevent stealing for any
-            // previous gasless-es's thresholds from gas supposed to be
-            // sent with this `packet`.
-            (Some(_), _) => {
-                let prev_gasless_fee = self
-                    .outgoing_gasless
-                    .saturating_mul(self.ext.context.mailbox_threshold);
-                self.reduce_gas(prev_gasless_fee)?;
-                self.outgoing_gasless = 0;
-                Ok(())
-            }
-
-            // Gasless with delay.
-            //
-            // In this case we must give threshold for each uncovered gasless-es
-            // sent, otherwise they will steal gas from this `packet` that was
-            // supposed to pay for delay.
-            //
-            // It doesn't guarantee threshold for itself.
-            (None, true) => {
-                let prev_gasless_fee = self
-                    .outgoing_gasless
-                    .saturating_mul(self.ext.context.mailbox_threshold);
-                self.reduce_gas(prev_gasless_fee)?;
-                self.outgoing_gasless = 1;
-                Ok(())
-            }
-
-            // Gasless instant.
-            //
-            // In this case there is no need to give any thresholds for previous
-            // gasless-es: only counter should be increased.
-            (None, false) => {
-                self.outgoing_gasless = self.outgoing_gasless.saturating_add(1);
-                Ok(())
-            }
-        }
-    }
-
     fn mark_reservation_used(
         &mut self,
         reservation_id: ReservationId,
@@ -506,21 +446,47 @@ impl<'a, LP: LazyPagesInterface> ExtMutator<'a, LP> {
         Ok(())
     }
 
-    fn charge_expiring_resources<T: Packet>(
-        &mut self,
-        packet: &T,
-        check_gas_limit: bool,
-    ) -> Result<(), FallibleExtError> {
-        let gas_limit = if check_gas_limit {
-            self.check_gas_limit(packet.gas_limit())?
-        } else {
-            packet.gas_limit().unwrap_or(0)
-        };
+    fn charge_expiring_resources<T: Packet>(&mut self, packet: &T) -> Result<(), FallibleExtError> {
+        let reducing_gas_limit = self.get_reducing_gas_limit(packet)?;
 
-        self.reduce_gas(gas_limit)?;
+        self.reduce_gas(reducing_gas_limit)?;
         self.charge_message_value(packet.value())
     }
 
+    fn get_reducing_gas_limit<T: Packet>(&self, packet: &T) -> Result<u64, FallibleExtError> {
+        match T::kind() {
+            DispatchKind::Handle => {
+                // Any "handle" gasless and gasful *non zero* message must
+                // cover mailbox threshold. That's because destination
+                // of the message is unknown, so it could be a user,
+                // and if gasless message is sent, there must be a
+                // guaranteed gas to cover mailbox.
+                let mailbox_threshold = self.context.mailbox_threshold;
+                let gas_limit = packet.gas_limit().unwrap_or(mailbox_threshold);
+
+                // Zero gasful message is a special case.
+                if gas_limit != 0 && gas_limit < mailbox_threshold {
+                    return Err(MessageError::InsufficientGasLimit.into());
+                }
+
+                Ok(gas_limit)
+            }
+            DispatchKind::Init | DispatchKind::Reply => {
+                // Init and reply messages never go to mailbox.
+                //
+                // For init case, even if there's no code with a provided
+                // code id, the init message still goes to queue and then is handled as non
+                // executable, as there is no code for the destination actor.
+                //
+                // Also no reply to user messages go to mailbox, they all are emitted
+                // within events.
+
+                Ok(packet.gas_limit().unwrap_or(0))
+            }
+            DispatchKind::Signal => unreachable!("Signals can't be sent as a syscall"),
+        }
+    }
+
     fn charge_sending_fee(&mut self, delay: u32) -> Result<(), ChargeError> {
         if delay == 0 {
             self.charge_gas_if_enough(self.context.message_context.settings().sending_fee)
@@ -720,18 +686,6 @@ impl<LP: LazyPagesInterface> Ext<LP> {
         Ok(result)
     }
 
-    fn check_gas_limit(&self, gas_limit: Option<GasLimit>) -> Result<GasLimit, FallibleExtError> {
-        let mailbox_threshold = self.context.mailbox_threshold;
-        let gas_limit = gas_limit.unwrap_or(0);
-
-        // Sending gas should apply the range {0} ∪ [mailbox_threshold; +inf)
-        if gas_limit < mailbox_threshold && gas_limit != 0 {
-            Err(MessageError::InsufficientGasLimit.into())
-        } else {
-            Ok(gas_limit)
-        }
-    }
-
     /// Checking that reservation could be charged for
     /// dispatch stash with given delay.
     fn check_reservation_gas_limit_for_delayed_sending(
@@ -978,8 +932,7 @@ impl<LP: LazyPagesInterface> Externalities for Ext<LP> {
     ) -> Result<MessageId, Self::FallibleError> {
         self.with_changes(|mutator| {
             mutator.check_forbidden_destination(msg.destination())?;
-            mutator.safe_gasfull_sends(&msg, delay)?;
-            mutator.charge_expiring_resources(&msg, true)?;
+            mutator.charge_expiring_resources(&msg)?;
             mutator.charge_sending_fee(delay)?;
             mutator.charge_for_dispatch_stash_hold(delay)?;
 
@@ -1029,8 +982,7 @@ impl<LP: LazyPagesInterface> Externalities for Ext<LP> {
         self.with_changes(|mutator| {
             mutator
                 .check_forbidden_destination(mutator.context.message_context.reply_destination())?;
-            mutator.safe_gasfull_sends(&msg, 0)?;
-            mutator.charge_expiring_resources(&msg, false)?;
+            mutator.charge_expiring_resources(&msg)?;
             mutator.charge_sending_fee(0)?;
 
             mutator
@@ -1360,9 +1312,10 @@ impl<LP: LazyPagesInterface> Externalities for Ext<LP> {
     ) -> Result<(MessageId, ProgramId), Self::FallibleError> {
         let ed = self.context.existential_deposit;
         self.with_changes(|mutator| {
-            // We don't check for forbidden destination here, since dest is always unique and almost impossible to match SYSTEM_ID
-            mutator.safe_gasfull_sends(&packet, delay)?;
-            mutator.charge_expiring_resources(&packet, true)?;
+            // We don't check for forbidden destination here, since dest is always unique
+            // and almost impossible to match SYSTEM_ID
+
+            mutator.charge_expiring_resources(&packet)?;
             mutator.charge_sending_fee(delay)?;
             mutator.charge_for_dispatch_stash_hold(delay)?;
 
@@ -1501,12 +1454,6 @@ mod tests {
             self
         }
 
-        fn with_mailbox_threshold(mut self, mailbox_threshold: u64) -> Self {
-            self.0.mailbox_threshold = mailbox_threshold;
-
-            self
-        }
-
         fn with_existential_deposit(mut self, ed: u128) -> Self {
             self.0.existential_deposit = ed;
 
@@ -2109,73 +2056,6 @@ mod tests {
         ));
     }
 
-    #[test]
-    fn test_gasful_after_gasless() {
-        let gas = 1_000_000_000;
-        let mut ext = Ext::new(
-            ProcessorContextBuilder::new()
-                .with_message_context(
-                    MessageContextBuilder::new()
-                        .with_outgoing_limit(u32::MAX)
-                        .build(),
-                )
-                .with_gas(GasCounter::new(gas))
-                .with_allowance(GasAllowanceCounter::new(gas))
-                .with_mailbox_threshold(1000)
-                .build(),
-        );
-
-        // Sending some gasless messages
-        let gasless_packet = HandlePacket::new(ProgramId::zero(), Default::default(), 0);
-        assert!(ext.send(gasless_packet.clone(), 0).is_ok());
-        assert_eq!(ext.outgoing_gasless, 1);
-        assert!(ext.send(gasless_packet.clone(), 0).is_ok());
-        assert_eq!(ext.outgoing_gasless, 2);
-        assert!(ext.send(gasless_packet.clone(), 0).is_ok());
-        assert_eq!(ext.outgoing_gasless, 3);
-
-        // Sending fee is zero
-        assert_eq!(ext.current_counter_value(), gas);
-
-        // Now sending gasful message
-        let msg_gas = 1000;
-        let gasful_packet =
-            HandlePacket::new_with_gas(ProgramId::zero(), Default::default(), msg_gas, 0);
-        assert!(ext.send(gasful_packet.clone(), 0).is_ok());
-        assert_eq!(ext.outgoing_gasless, 0);
-        assert_eq!(
-            ext.current_counter_value(),
-            // reducing gas for the sent message and for mailbox threshold for each gasless
-            gas - msg_gas - 3 * ext.context.mailbox_threshold
-        );
-
-        // Sending another gasful
-        let gas = ext.current_counter_value();
-        assert!(ext.send(gasful_packet.clone(), 0).is_ok());
-        assert_eq!(ext.outgoing_gasless, 0);
-        assert_eq!(
-            ext.current_counter_value(),
-            // reducing gas the sent message only
-            gas - msg_gas
-        );
-
-        // Sending some more gasless
-        assert!(ext.send(gasless_packet.clone(), 0).is_ok());
-        assert_eq!(ext.outgoing_gasless, 1);
-        assert!(ext.send(gasless_packet.clone(), 0).is_ok());
-        assert_eq!(ext.outgoing_gasless, 2);
-
-        // And another gasful
-        let gas = ext.current_counter_value();
-        assert!(ext.send(gasful_packet.clone(), 0).is_ok());
-        assert_eq!(ext.outgoing_gasless, 0);
-        assert_eq!(
-            ext.current_counter_value(),
-            // reducing gas for the sent message and for mailbox threshold for each gasless
-            gas - msg_gas - 2 * ext.context.mailbox_threshold
-        );
-    }
-
     #[test]
     // This function tests:
     //

core/src/message/handle.rs

@@ -196,4 +196,8 @@ impl Packet for HandlePacket {
     fn value(&self) -> Value {
         self.value
     }
+
+    fn kind() -> DispatchKind {
+        DispatchKind::Handle
+    }
 }

core/src/message/init.rs

@@ -200,4 +200,8 @@ impl Packet for InitPacket {
     fn value(&self) -> Value {
         self.value
     }
+
+    fn kind() -> DispatchKind {
+        DispatchKind::Init
+    }
 }

core/src/message/mod.rs

@@ -229,6 +229,9 @@ pub trait Packet {
 
     /// Packet value.
     fn value(&self) -> Value;
+
+    /// A dispatch kind the will be generated from the packet.
+    fn kind() -> DispatchKind;
 }
 
 /// The struct contains results of read only send message RPC call.

core/src/message/reply.rs

@@ -253,4 +253,8 @@ impl Packet for ReplyPacket {
     fn value(&self) -> Value {
         self.value
     }
+
+    fn kind() -> DispatchKind {
+        DispatchKind::Reply
+    }
 }

examples/constructor/src/builder.rs

@@ -239,10 +239,30 @@ impl Calls {
     }
 
     pub fn reply(self, payload: impl Into<Arg<Vec<u8>>>) -> Self {
-        self.add_call(Call::Reply(payload.into(), None, 0.into()))
+        self.reply_value(payload, 0)
+    }
+
+    pub fn reply_value(
+        self,
+        payload: impl Into<Arg<Vec<u8>>>,
+        value: impl Into<Arg<u128>>,
+    ) -> Self {
+        self.add_call(Call::Reply(payload.into(), None, value.into()))
     }
 
     pub fn reply_wgas<T: TryInto<u64>>(self, payload: impl Into<Arg<Vec<u8>>>, gas_limit: T) -> Self
+    where
+        T::Error: Debug,
+    {
+        self.reply_value_wgas(payload, gas_limit, 0)
+    }
+
+    pub fn reply_value_wgas<T: TryInto<u64>>(
+        self,
+        payload: impl Into<Arg<Vec<u8>>>,
+        gas_limit: T,
+        value: impl Into<Arg<u128>>,
+    ) -> Self
     where
         T::Error: Debug,
     {
@@ -252,7 +272,7 @@ impl Calls {
         self.add_call(Call::Reply(
             payload.into(),
             Some(gas_limit.into()),
-            0.into(),
+            value.into(),
         ))
     }
 

pallets/gear/src/internal.rs

@@ -23,7 +23,7 @@ use crate::{
     GasBalanceOf, GasHandlerOf, GasNodeIdOf, GearBank, MailboxOf, Pallet, QueueOf,
     SchedulingCostOf, TaskPoolOf, WaitlistOf,
 };
-use alloc::collections::BTreeSet;
+use alloc::{collections::BTreeSet, format};
 use common::{
     event::{
         MessageWaitedReason, MessageWaitedRuntimeReason::*, MessageWokenReason, Reason::*,
@@ -364,10 +364,26 @@ where
 
         // Validating duration.
         if hold.expected_duration().is_zero() {
-            // TODO: Replace with unreachable call after:
-            // - `HoldBound` safety usage stabilized;
-            log::error!("Failed to figure out correct wait hold bound");
-            return;
+            let gas_limit = GasHandlerOf::<T>::get_limit(dispatch.id()).unwrap_or_else(|e| {
+                let err_msg = format!(
+                    "wait_dispatch: failed getting message gas limit. Message id - {message_id}. \
+                        Got error - {e:?}",
+                    message_id = dispatch.id()
+                );
+
+                log::error!("{err_msg}");
+                unreachable!("{err_msg}");
+            });
+
+            let err_msg = format!(
+                "wait_dispatch: message got zero duration hold bound for waitlist. \
+                Requested duration - {duration:?}, gas limit - {gas_limit}, \
+                wait reason - {reason:?}, message id - {}.",
+                dispatch.id(),
+            );
+
+            log::error!("{err_msg}");
+            unreachable!("{err_msg}");
         }
 
         // Locking funds for holding.