Skip to content

Commit

Permalink
feat(pallet_gear): Introduce invariant: all user non-reply messages g…
Browse files Browse the repository at this point in the history 
…o to mailbox (#4060)
  • Loading branch information
@techraed
techraed authored Jul 30, 2024
1 parent e917669 commit 9ad743c
Show file tree
Hide file tree
Showing 9 changed files with 218 additions and 235 deletions.
210 changes: 45 additions & 165 deletions core-processor/src/ext.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)?;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)?;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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:
//
Expand Down
4 changes: 4 additions & 0 deletions core/src/message/handle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,4 +196,8 @@ impl Packet for HandlePacket {
fn value(&self) -> Value {
self.value
}

fn kind() -> DispatchKind {
DispatchKind::Handle
}
}
4 changes: 4 additions & 0 deletions core/src/message/init.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,8 @@ impl Packet for InitPacket {
fn value(&self) -> Value {
self.value
}

fn kind() -> DispatchKind {
DispatchKind::Init
}
}
3 changes: 3 additions & 0 deletions core/src/message/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions core/src/message/reply.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,4 +253,8 @@ impl Packet for ReplyPacket {
fn value(&self) -> Value {
self.value
}

fn kind() -> DispatchKind {
DispatchKind::Reply
}
}
24 changes: 22 additions & 2 deletions examples/constructor/src/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
{
Expand All @@ -252,7 +272,7 @@ impl Calls {
self.add_call(Call::Reply(
payload.into(),
Some(gas_limit.into()),
0.into(),
value.into(),
))
}

Expand Down
24 changes: 20 additions & 4 deletions pallets/gear/src/internal.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,10 +420,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.
Expand Down
Loading

0 comments on commit 9ad743c

Please sign in to comment.