ResultQuery/ custom Err type for Storage*::try_get

[benluelo]

I have noticed this common pattern throughout our codebase:

#[pallet::storage]
#[pallet::getter(fn thing)]
pub type Thing<T: Config> = StorageMap<
    _,
    Twox64Concat,
    ThingId,
    Thing,
    OptionQuery // <- provided for explicitness
>;

let market_interest_index = Self::thing(thing_id).ok_or(Error::<T>::ThingDoesNotExist)?;

This can obviously be wrapped like this:

fn get_thing(thing_id: &ThingId) -> Result<Thing, DispatchError> {
    Thing::<T>::get(market_id).ok_or_else(|| Error::<T>::ThingDoesNotExist.into())
}

But this feels like unnecessary boilerplate for something this common.

My question is: is there a way to define a fallible getter for a storage?

Something like this, perhaps?

#[pallet::getter(fn thing)]
#[pallet::getter_fallible(fn try_get_thing<DispatchError>, Error::<T>::ThingDoesNotExist.into())]
                                        // `Err` type      function body

Where the 2nd argument is an arbitrary expression to return in Err if the value isn't found.

If something else already exists that fits this use case/ issue, please let me know! I've looked around and couldn't find anything.

[xlc]

but why?

[benluelo]

Reducing boilerplate.

I personally find this:

let thing = Self::try_get_thing(thing_id)?;
let thing2 = Self::try_get_thing2(thing2_id)?;

Easier to grok than this:

let thing1 = Thing::<T>::get(thing_id).ok_or(Error::<T>::ThingDoesNotExist);
let thing2 = Thing2::<T>::get(thing2_id).ok_or(Error::<T>::Thing2DoesNotExist)?;

Especially when there are lots of storage accesses in a short period.

As I mentioned in my first comment, this is quite a simple function, and currently I use this pattern extensively:

fn get_thing(thing_id: &ThingId) -> Result<Thing, DispatchError> {
    Thing::<T>::get(market_id).ok_or_else(|| Error::<T>::ThingDoesNotExist.into())
}

I'm just wondering if there's a way to have this be auto generated. Maybe there could be a way to associate a storage with an Error variant?

[xlc]

I don't think number of characters saved is justifiable for extra macro magic especially with the help of auto completion.

[KiChjang]

I think it actually makes sense. We could create a new ResultQuery type which uses the pallet error enum as the Err type, that way we wouldn't need to fiddle with the macro as much.

[bkchr]

I think it actually makes sense. We could create a new ResultQuery type which uses the pallet error enum as the Err type, that way we wouldn't need to fiddle with the macro as much.

That sounds like a really good idea!

[benluelo]

Fantastic idea, definitely feels more in line with the rest of the pallet macro!!

How does something like this look?

#[pallet::error]
pub enum Error<T> {
    ThingNotFound,
    ...
}

#[pallet::storage]
pub type Thing<T: Config> = StorageMap<
    _,
    Twox64Concat,
    ThingId,
    Thing,
    ResultQuery<ThingNotFound>,
>;

Where the ident provided to the ResultQuery<_> is a variant of the pallet error enum?

Another option could be an attribute, a la #[pallet::query_error(ThingNotFound)].

[shawntabrizi]

In general, I dont like or use the #[pallet::getter(...)] macro in my code. I don't like the macro magic, and I don't think it really helps all that much, especially when auditing code.

However, ResultQuery seems cool and useful enough.