From bf5967a2771c0e6a1c206fdbb9ddbfdd95b2e084 Mon Sep 17 00:00:00 2001 From: Sabaun Taraki Date: Mon, 3 Jul 2023 20:12:22 +0300 Subject: [PATCH] test(pallet-gear): handling gas allowance exceed works correctly (#2872) --- pallets/gear-messenger/src/tests.rs | 4 +- pallets/gear/src/manager/journal.rs | 3 +- pallets/gear/src/tests.rs | 190 +++++++++++++++++++++++++++- 3 files changed, 191 insertions(+), 6 deletions(-) diff --git a/pallets/gear-messenger/src/tests.rs b/pallets/gear-messenger/src/tests.rs index e1c0035e894..10f88671fd2 100644 --- a/pallets/gear-messenger/src/tests.rs +++ b/pallets/gear-messenger/src/tests.rs @@ -108,9 +108,9 @@ fn dequeued_impl_works_manually() { assert_eq!(DequeuedOf::get(), 0); // Value updates for future blocks. - SentOf::increase(); + DequeuedOf::increase(); - assert_eq!(SentOf::get(), 1); + assert_eq!(DequeuedOf::get(), 1); run_to_block(2); diff --git a/pallets/gear/src/manager/journal.rs b/pallets/gear/src/manager/journal.rs index 67283c2285c..4ddd332ba7f 100644 --- a/pallets/gear/src/manager/journal.rs +++ b/pallets/gear/src/manager/journal.rs @@ -20,7 +20,7 @@ use crate::{ internal::HoldBoundBuilder, manager::{CodeInfo, ExtManager}, Config, CurrencyOf, Event, GasAllowanceOf, GasHandlerOf, GasTree, Pallet, ProgramStorageOf, - QueueOf, RentFreePeriodOf, SentOf, TaskPoolOf, WaitlistOf, + QueueOf, RentFreePeriodOf, TaskPoolOf, WaitlistOf, }; use common::{ event::*, @@ -489,7 +489,6 @@ where gas_burned, ); - SentOf::::increase(); GasAllowanceOf::::decrease(gas_burned); QueueOf::::requeue(dispatch) .unwrap_or_else(|e| unreachable!("Message queue corrupted! {:?}", e)); diff --git a/pallets/gear/src/tests.rs b/pallets/gear/src/tests.rs index b34fdfc172a..e5da71fd66a 100644 --- a/pallets/gear/src/tests.rs +++ b/pallets/gear/src/tests.rs @@ -13487,6 +13487,7 @@ fn send_reply_with_voucher_works() { #[test] fn double_read_works() { use demo_constructor::{Calls, Scheme}; + init_logger(); new_test_ext().execute_with(|| { let noop_branch = Calls::builder().noop(); @@ -13498,14 +13499,14 @@ fn double_read_works() { .if_else("is_eq", noop_branch, panic_branch); let predefined_scheme = Scheme::predefined(Default::default(), handle, Default::default()); - let (_, constructor_id) = utils::init_constructor(predefined_scheme); + let (_, pid) = utils::init_constructor(predefined_scheme); // Resetting events to check the result of the last message. System::reset_events(); assert_ok!(Gear::send_message( RuntimeOrigin::signed(USER_1), - constructor_id, + pid, b"PAYLOAD".to_vec(), BlockGasLimitOf::::get(), 100_000, @@ -13519,3 +13520,188 @@ fn double_read_works() { ); }); } + +/// Tests gas allowance exceed handling. +/// More precisely, it checks one property: +/// no context data is stored within previously +/// executed message when gas allowance exceed +/// happened. +#[test] +fn test_gas_allowance_exceed_no_context() { + use crate::QueueProcessingOf; + use common::storage::{Counted, Queue, Toggler}; + + init_logger(); + new_test_ext().execute_with(|| { + let wat = r#" + (module + (import "env" "memory" (memory 1)) + (import "env" "gr_reply" (func $reply (param i32 i32 i32 i32))) + (export "handle" (func $handle)) + (func $handle + (call $reply (i32.const 0) (i32.const 32) (i32.const 10) (i32.const 333)) + (loop (br 0)) + ) + )"#; + + let pid = upload_program_default(USER_1, ProgramCodeKind::Custom(wat)) + .expect("failed uploading program"); + run_to_next_block(None); + + assert_ok!(send_default_message(USER_1, pid)); + let mid = get_last_message_id(); + // Setting to 100 million the gas allowance ends faster than gas limit + run_to_next_block(Some(100_000_000)); + + // Execution is denied after reque + assert!(QueueProcessingOf::::denied()); + + // Low level check, that no execution context is saved after gas allowance exceeded error + assert_eq!(QueueOf::::len(), 1); + let msg = QueueOf::::dequeue() + .ok() + .flatten() + .expect("must be message after requeue"); + assert_eq!(msg.id(), mid); + assert!(msg.context().is_none()); + QueueOf::::requeue(msg).expect("requeue failed"); + + // There should be now enough gas allowance, so the message is executed + // and execution ends with `GasLimitExceeded`. + run_to_next_block(None); + + assert_failed( + mid, + ErrorReplyReason::Execution(SimpleExecutionError::RanOutOfGas), + ); + assert_last_dequeued(1); + }) +} + +/// Does pretty same test as `test_gas_allowance_exceed_no_context`, +/// but this time executed message will have non zero context. +#[test] +fn test_gas_allowance_exceed_with_context() { + use crate::QueueProcessingOf; + use common::storage::*; + use demo_constructor::{Arg, Calls, Scheme}; + + init_logger(); + + let process_task_weight = mock::get_min_weight(); + + new_test_ext().execute_with(|| { + let call_wait_key = "call_wait"; + + // Initialize a program and set `call_wait' value to `true`. + let init = Calls::builder().source("user1").bool(call_wait_key, true); + let (_, pid) = utils::init_constructor(Scheme::direct(init)); + + let execute = |calls: Calls, allowance: Option| { + assert_ok!(Gear::send_message( + RuntimeOrigin::signed(USER_1), + pid, + calls.encode(), + BlockGasLimitOf::::get(), + 0, + )); + let msg_id = get_last_message_id(); + run_to_next_block(allowance); + + msg_id + }; + + // If `call_wait` is true, we execute a `wait`. Otherwise, we `reply` and `send`. + // It's intended that the message will be executed several times. + // That's to perform checks of the msg execution context when message + // is in queue after wake (so it has some context), and after gas allowance exceeded + // error (so the context remains unchanged). + let wait_branch = Calls::builder().wait(); + let skip_wait_branch = Calls::builder().noop(); + let handle1 = Calls::builder() + .if_else( + Arg::Get(call_wait_key.to_string()), + wait_branch, + skip_wait_branch, + ) + .reply(b"random_message".to_vec()) + .send("user1", b"another_random_message".to_vec()); + let handle1_mid = execute(handle1.clone(), None); + + // Check it waits. + assert!(WaitlistOf::::contains(&pid, &handle1_mid)); + assert_eq!(WaitlistOf::::len(&pid), 1); + + // Taking the context for the check later. + let handle1_ctx = WaitlistOf::::iter_key(pid) + .next() + .and_then(|(m, _)| m.context().clone()); + assert!(handle1_ctx.is_some()); + + // This will set `call_wait` to false, wake the message with `handle1_mid` id. + // We set the weight to such a value, so only message with `handle2` + // payload is executed, That will allow us to reproduce the case in + // `test_gas_allowance_exceed_no_context` test, but with message having + // context already set. + let handle2 = Calls::builder() + .bool(call_wait_key, false) + .wake(<[u8; 32]>::from(handle1_mid)); + let gas_info = Gear::calculate_gas_info( + USER_1.into_origin(), + HandleKind::Handle(pid), + handle2.encode(), + 0, + true, + true, + ) + .expect("failed getting gas info"); + // We add `process_task_weight` as block running requires not only executing message, but + // some other read/writes. By adding such small value we guarantee that + // message with `handle2` payload is executed, but message with `handle1_mid` + // id will not reach the executor. + execute( + handle2, + Some(gas_info.min_limit + process_task_weight.ref_time()), + ); + + assert_last_dequeued(1); + assert!(QueueProcessingOf::::denied()); + + // Now we calculate a required for the execution of the `handle1_mid` message. + // The queue processing is denied from the previous execution, now allowing it, + // to calculate gas properly. + QueueProcessingOf::::allow(); + let gas_info = Gear::calculate_gas_info( + USER_1.into_origin(), + HandleKind::Handle(pid), + handle1.encode(), + 0, + true, + true, + ) + .expect("failed getting gas info"); + + // Trigger gas allowance exceeded error while executing only `handle1_mid`. + // With such gas allowance we are sure, that `reply` call from `handle1` calls set + // is called successfully executed, but there's not enough allowance to end the + // execution of the message. + run_to_next_block(Some(gas_info.min_limit - 100_000)); + + // Execution is denied after reque. + assert!(QueueProcessingOf::::denied()); + + // Low level check, that no information on reply sent is saved in the execution + // context after gas allowance exceeded error. + assert_eq!(QueueOf::::len(), 1); + let msg = QueueOf::::dequeue() + .ok() + .flatten() + .expect("must be message after requeue"); + assert_eq!(msg.id(), handle1_mid); + assert_eq!(msg.context(), &handle1_ctx); + QueueOf::::requeue(msg).expect("requeue failed"); + + run_to_next_block(None); + assert_succeed(handle1_mid); + }) +}