Skip to content

Commit

Permalink
refactor(gear-builtin) Concrete error type for builtin actors (#4303)
Browse files Browse the repository at this point in the history
  • Loading branch information
Lazark0x authored Nov 4, 2024
1 parent bc78c9a commit b8c21ea
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 37 deletions.
7 changes: 4 additions & 3 deletions pallets/gear-builtin/src/bls12_381.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ const IS_VALIDATED: Validate = ark_scale::is_validated(HOST_CALL);
pub struct Actor<T: Config>(PhantomData<T>);

impl<T: Config> BuiltinActor for Actor<T> {
type Error = BuiltinActorError;

fn handle(dispatch: &StoredDispatch, gas_limit: u64) -> (Result<Payload, Self::Error>, u64) {
fn handle(
dispatch: &StoredDispatch,
gas_limit: u64,
) -> (Result<Payload, BuiltinActorError>, u64) {
let message = dispatch.message();
let payload = message.payload_bytes();
let (result, gas_spent) = match payload.first().copied() {
Expand Down
34 changes: 17 additions & 17 deletions pallets/gear-builtin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ type CallOf<T> = <T as Config>::RuntimeCall;

const LOG_TARGET: &str = "gear::builtin";

pub type ActorErrorHandleFn = HandleFn<BuiltinActorError>;

/// Built-in actor error type
#[derive(Encode, Decode, Clone, Debug, PartialEq, Eq, derive_more::Display)]
pub enum BuiltinActorError {
Expand Down Expand Up @@ -113,10 +115,11 @@ impl From<BuiltinActorError> for ActorExecutionErrorReplyReason {
/// A trait representing an interface of a builtin actor that can handle a message
/// from message queue (a `StoredDispatch`) to produce an outcome and gas spent.
pub trait BuiltinActor {
type Error;

/// Handles a message and returns a result and the actual gas spent.
fn handle(dispatch: &StoredDispatch, gas_limit: u64) -> (Result<Payload, Self::Error>, u64);
fn handle(
dispatch: &StoredDispatch,
gas_limit: u64,
) -> (Result<Payload, BuiltinActorError>, u64);
}

/// A marker struct to associate a builtin actor with its unique ID.
Expand All @@ -125,33 +128,29 @@ pub struct ActorWithId<const ID: u64, A: BuiltinActor>(PhantomData<A>);
/// Glue trait to implement `BuiltinCollection` for a tuple of `ActorWithId`.
trait BuiltinActorWithId {
const ID: u64;

type Error;
type Actor: BuiltinActor<Error = Self::Error>;
type Actor: BuiltinActor;
}

impl<const ID: u64, A: BuiltinActor> BuiltinActorWithId for ActorWithId<ID, A> {
const ID: u64 = ID;

type Error = A::Error;
type Actor = A;
}

/// A trait defining a method to convert a tuple of `BuiltinActor` types into
/// a in-memory collection of builtin actors.
pub trait BuiltinCollection<E> {
pub trait BuiltinCollection {
fn collect(
registry: &mut BTreeMap<ProgramId, Box<HandleFn<E>>>,
registry: &mut BTreeMap<ProgramId, Box<ActorErrorHandleFn>>,
id_converter: &dyn Fn(u64) -> ProgramId,
);
}

// Assuming as many as 16 builtin actors for the meantime
#[impl_for_tuples(16)]
#[tuple_types_custom_trait_bound(BuiltinActorWithId<Error = E> + 'static)]
impl<E> BuiltinCollection<E> for Tuple {
#[tuple_types_custom_trait_bound(BuiltinActorWithId + 'static)]
impl BuiltinCollection for Tuple {
fn collect(
registry: &mut BTreeMap<ProgramId, Box<HandleFn<E>>>,
registry: &mut BTreeMap<ProgramId, Box<ActorErrorHandleFn>>,
id_converter: &dyn Fn(u64) -> ProgramId,
) {
for_tuples!(
Expand Down Expand Up @@ -199,7 +198,7 @@ pub mod pallet {
+ GetDispatchInfo;

/// The builtin actor type.
type Builtins: BuiltinCollection<BuiltinActorError>;
type Builtins: BuiltinCollection;

/// Weight cost incurred by builtin actors calls.
type WeightInfo: WeightInfo;
Expand Down Expand Up @@ -261,6 +260,7 @@ pub mod pallet {

impl<T: Config> BuiltinDispatcherFactory for Pallet<T> {
type Error = BuiltinActorError;

type Output = BuiltinRegistry<T>;

fn create() -> (BuiltinRegistry<T>, u64) {
Expand All @@ -272,7 +272,7 @@ impl<T: Config> BuiltinDispatcherFactory for Pallet<T> {
}

pub struct BuiltinRegistry<T: Config> {
pub registry: BTreeMap<ProgramId, Box<HandleFn<BuiltinActorError>>>,
pub registry: BTreeMap<ProgramId, Box<ActorErrorHandleFn>>,
pub _phantom: sp_std::marker::PhantomData<T>,
}
impl<T: Config> BuiltinRegistry<T> {
Expand All @@ -290,13 +290,13 @@ impl<T: Config> BuiltinRegistry<T> {
impl<T: Config> BuiltinDispatcher for BuiltinRegistry<T> {
type Error = BuiltinActorError;

fn lookup<'a>(&'a self, id: &ProgramId) -> Option<&'a HandleFn<Self::Error>> {
fn lookup<'a>(&'a self, id: &ProgramId) -> Option<&'a ActorErrorHandleFn> {
self.registry.get(id).map(|f| &**f)
}

fn run(
&self,
f: &HandleFn<Self::Error>,
f: &ActorErrorHandleFn,
dispatch: StoredDispatch,
gas_limit: u64,
) -> Vec<JournalNote> {
Expand Down
6 changes: 0 additions & 6 deletions pallets/gear-builtin/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,6 @@ pallet_gear::impl_config!(
// A builtin actor who always returns success (even if not enough gas is provided).
pub struct SuccessBuiltinActor {}
impl BuiltinActor for SuccessBuiltinActor {
type Error = BuiltinActorError;

fn handle(
dispatch: &StoredDispatch,
_gas_limit: u64,
Expand All @@ -256,8 +254,6 @@ impl BuiltinActor for SuccessBuiltinActor {
// A builtin actor that always returns an error.
pub struct ErrorBuiltinActor {}
impl BuiltinActor for ErrorBuiltinActor {
type Error = BuiltinActorError;

fn handle(
dispatch: &StoredDispatch,
_gas_limit: u64,
Expand All @@ -279,8 +275,6 @@ impl BuiltinActor for ErrorBuiltinActor {
// An honest bulitin actor that actually checks whether the gas is sufficient.
pub struct HonestBuiltinActor {}
impl BuiltinActor for HonestBuiltinActor {
type Error = BuiltinActorError;

fn handle(
dispatch: &StoredDispatch,
gas_limit: u64,
Expand Down
7 changes: 4 additions & 3 deletions pallets/gear-builtin/src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,10 @@ where
<T as ProxyConfig>::ProxyType: From<BuiltinProxyType>,
CallOf<T>: From<pallet_proxy::Call<T>>,
{
type Error = BuiltinActorError;

fn handle(dispatch: &StoredDispatch, gas_limit: u64) -> (Result<Payload, Self::Error>, u64) {
fn handle(
dispatch: &StoredDispatch,
gas_limit: u64,
) -> (Result<Payload, BuiltinActorError>, u64) {
let Ok(request) = Request::decode(&mut dispatch.payload_bytes()) else {
return (Err(BuiltinActorError::DecodingError), 0);
};
Expand Down
7 changes: 4 additions & 3 deletions pallets/gear-builtin/src/staking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,10 @@ where
T::AccountId: Origin,
CallOf<T>: From<pallet_staking::Call<T>>,
{
type Error = BuiltinActorError;

fn handle(dispatch: &StoredDispatch, gas_limit: u64) -> (Result<Payload, Self::Error>, u64) {
fn handle(
dispatch: &StoredDispatch,
gas_limit: u64,
) -> (Result<Payload, BuiltinActorError>, u64) {
let message = dispatch.message();
let origin = dispatch.source();
let mut payload = message.payload_bytes();
Expand Down
2 changes: 0 additions & 2 deletions pallets/gear-builtin/src/tests/bad_builtin_ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ pallet_gear::impl_config!(

pub struct SomeBuiltinActor {}
impl BuiltinActor for SomeBuiltinActor {
type Error = BuiltinActorError;

fn handle(
_dispatch: &StoredDispatch,
_gas_limit: u64,
Expand Down
7 changes: 4 additions & 3 deletions pallets/gear-eth-bridge/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ impl<T: Config> BuiltinActor for Actor<T>
where
T::AccountId: Origin,
{
type Error = BuiltinActorError;

fn handle(dispatch: &StoredDispatch, gas_limit: u64) -> (Result<Payload, Self::Error>, u64) {
fn handle(
dispatch: &StoredDispatch,
gas_limit: u64,
) -> (Result<Payload, BuiltinActorError>, u64) {
if !dispatch.value().is_zero() {
return (
Err(BuiltinActorError::Custom(LimitedStr::from_small_str(
Expand Down

0 comments on commit b8c21ea

Please sign in to comment.