Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add defensive testing extrinsic #1998

Merged
merged 12 commits into from
Oct 31, 2023
24 changes: 21 additions & 3 deletions substrate/frame/root-testing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@

#![cfg_attr(not(feature = "std"), no_std)]

use frame_support::dispatch::DispatchResult;
use sp_runtime::Perbill;
use frame_support::{dispatch::DispatchResult, sp_runtime::Perbill};

pub use pallet::*;

Expand All @@ -36,11 +35,21 @@ pub mod pallet {
use frame_system::pallet_prelude::*;

#[pallet::config]
pub trait Config: frame_system::Config {}
pub trait Config: frame_system::Config {
/// The overarching event type.
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
}

#[pallet::pallet]
pub struct Pallet<T>(_);

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
/// Event dispatched when the trigger_defensive extrinsic is called.
DefensiveTestCall,
}

#[pallet::call]
impl<T: Config> Pallet<T> {
/// A dispatch that will fill the block weight up to the given ratio.
Expand All @@ -50,5 +59,14 @@ pub mod pallet {
ensure_root(origin)?;
Ok(())
}

#[pallet::call_index(1)]
#[pallet::weight(0)]
pub fn trigger_defensive(origin: OriginFor<T>) -> DispatchResult {
ensure_root(origin)?;
frame_support::defensive!("root_testing::trigger_defensive was called.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ggwpez so you want this to panic? I don't really get what you want to achieve?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defensive is just log error and debug_assert, which is nop on release build, so this is a bit pointless. if you want to test log error, just log error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I know that this is using a debug assert, I was assuming @ggwpez wants to build with debug asserts enabled and then use it as overwrite.

Copy link
Member

@ggwpez ggwpez Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is that we are currently not sure if the monitoring correctly picks up defensive failures. And we really should be notified about them, so this is just as test.
It could also be logged, but the defensive! macro also logs a magic string (DEFENSIVE_OP_PUBLIC_ERROR) that we can filter for and alert in the grafana.
Then we can call this and check that we got notifications in chat about this failure.

Self::deposit_event(Event::DefensiveTestCall);
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
}
}
}