Add pallet name as constant

[gregdhill]

For error matching we have to compare strings on the ModuleError (see related issue #313). We do this by printing the debug variant of the generated Error field (see here) but there is also no easy way to extract the pallet name which requires us to hard-code the string (see here). I'm not quite sure what better matching support would look like but a nice in-between step for our usage would be to simply extract the name into a constant.

[jsdw]

Sorry for the long delay; I was off during December!

Perhaps you could elaborate a little? ModuleError looks like:

pub struct ModuleError {
    /// The name of the pallet that the error came from.
    pub pallet: String,
    /// The name of the error.
    pub error: String,
    /// A description of the error.
    pub description: Vec<String>,
    /// A byte representation of the error.
    pub error_data: ModuleErrorData,
}

And the pallet name is available as .pallet there; is that what you're after or am I missing something?

[jsdw]

@gregdhill do you still have an issue here? I feel I may have missed something, but if not I'll close this :)

[gregdhill]

@jsdw the module error does indeed contain the pallet name but the problem is that we want to pattern match on that from the auto-generated code. Let's take runtime_types::pallet::Error::InvalidSpecName as an example, if we want to determine if ModuleError { pallet, error, .. } is equal to that enum value we have to workaround that like so by printing the debug variant of the error and hard-coding the pallet name. Ideally #313 would be implemented but an in-between step for us would be to use api::system::pallet_name() or so to get the name instead of using the hard-coded string.

[jsdw]

Ah I think I see now, so if we were to add into the generated output something like, say, an:

pub enum InvalidSpecName {
   Foo,
   Bar
}

impl InvalidSpecName {
    pub fn name(&self) -> {
        match self {
             Foo => "Foo",
             Bar => "Bar",
        }
    }
}

Would that give you what you want?

[jsdw]

Or perhaps we could do more compact codegen by having an .index(&self) -> u8 implemented on the errors (they already have a codec impl, so one can just encode the errors and take the first byte to see what index they are at, I think. This would be more compact/simple to add I suspect) and then comparing that against the module.error_data.error_index().

[jsdw]

So I've looked into this a bit more to see what can be done and try to understand it better.

So this is what I believe you want to be able to do (please lemme know if I'm wrong as I think I haven't grasped this fully):

We get error info back in this form when something goes wrong:

pub struct ModuleError {
    /// The name of the pallet that the error came from.
    pub pallet: String,
    /// The name of the error.
    pub error: String,
    /// A description of the error.
    pub description: Vec<String>,
    /// A byte representation of the error.
    pub error_data: ModuleErrorData,
}

And I think that you want to be able to match these up with the error types in the generated code that look like eg:

pub mod api {

    // ...

    pub mod runtime_types {

        // ...

        pub mod pallet_authorship {
            use super::runtime_types;
            pub mod pallet {
                // ...
                pub enum Error {
                    #[codec(index = 0)]
                    #[doc = "The uncle parent not in the chain."]
                    InvalidUncleParent,
                    #[codec(index = 1)]
                    #[doc = "Uncles already set in the block."]
                    UnclesAlreadySet,
                    #[codec(index = 2)]
                    #[doc = "Too many uncles."]
                    TooManyUncles,
                    #[codec(index = 3)]
                    #[doc = "The uncle is genesis."]
                    GenesisUncle,
                    #[codec(index = 4)]
                    #[doc = "The uncle is too high in chain."]
                    TooHighUncle,
                    #[codec(index = 5)]
                    #[doc = "The uncle is already included."]
                    UncleAlreadyIncluded,
                    #[codec(index = 6)]
                    #[doc = "The uncle isn't recent enough to be included."]
                    OldUncle,
                }
            }
        }

        // ...
    }

}

And in your code, to do that you currently:

• hardcode some pallet name strings to know which pallet the error is from and then
• Debug print the generated errors (which currently returns the variant name eg InvalidUncleParent).

It looks like you don't use the generated errors in any other way.

I guess what you'd prefer is to be able to do something more like:

fn is_module_err(&self, error: &impl PalletError) -> bool {
    matches!(
        self,
        Error::SubxtRuntimeError(SubxtError::Runtime(DispatchError::Module(ModuleError{
            pallet, error, ..
        }))) if pallet == PalletError::Pallet && error == PalletError::Name,
    )
}

// ...

pub fn is_issue_completed(&self) -> bool {
    self.is_module_err(&IssuePalletError::IssueCompleted)
}

Noting that you'd still need the manual is_x calls because you need to reference a specific rust module/enum for each one (there's no way to dynamically select the module to pull the error from afterall).

Of course, the is_module_err function could conceivably have some simple is_module_error function added to it very easily to make that bit nicer, so you'd have eg:

fn is_module_err(&self, pallet_name: &str, error_name: &str) -> bool {
    match self {
        Error::SubxtRuntimeError(subxt_error) => subxe_error.is_module_error(pallet_name, error_name),
        _ => false
    }
}

Ultimately, without a top level Error enum type, whatever solution you go for will involve manually pulling in some module/type to Debug on or whatever, so whether you do self.is_module_err(ISSUE_MODULE, &format!("{:?}", IssuePalletError::IssueCompleted)) or self.is_module_err(ISSUE_MODULE, "IssueCompleted")) or self.is_module_err(api::runtime_types::pallets::issue_pallet::NAME, "IssueCompleted")) doesn't feel to me like you gain very much.

Additionally, as the Error types are just spat out as any other generic type, it's not super easy to hook in some custom logic to ovverride the Error type or add some pallet conatnt to the module (but because you have to import the module and thus know the pallet you're looking for to see the constant, it feels not super helpful anyway?).

So perhaps we can do some small things to help, but when we get a top level Error type (see eg paritytech/frame-metadata#43) I feel like we can't make comparing ModuleError against some custom error type in the generated code very ergonomic at the moment! Perhaps I'm missing something though?

[gregdhill]

please lemme know if I'm wrong as I think I haven't grasped this fully

You have understood the problem exactly, sorry if this wasn't already clear from my initial description!

I guess what you'd prefer is to be able to do something more like

That solution looks ideal, some way to have the static metadata change result in a compile-time error would be preferred. That is the reason even api::runtime_types::pallets::issue_pallet::NAME would be good to have in my opinion - to catch when the pallet name may change.

Ultimately, without a top level Error enum type, whatever solution you go for will involve manually pulling in some module/type to Debug

I'm not sure I follow this, when Error is "spat out" we know the name of the pallet and error already right?

[jsdw]

That solution looks ideal, some way to have the static metadata change result in a compile-time error would be preferred. That is the reason even api::runtime_types::pallets::issue_pallet::NAME would be good to have in my opinion - to catch when the pallet name may change.

Aah I see; verifying that the errors you get back aline up with the generated types then is really the main driver for this all, rather than code complexity, because otherwise you could do something like this quite easily:

pub fn is_issue_completed(&self) -> bool {
    self.is_module_err("SomePalletName", "IssueCompleted")
}

If you want to catch the codegen pallet or error name changing, importing it is enough:

pub fn is_issue_completed(&self) -> bool {
    use api::runtime_types::pallet_name::Error;
    self.is_module_err("SomePalletName", Error::IssueCompleted)
}

Here, is_module_err could take something that implements Encode to turn the generated error into an error index u8 if you wanted (eg Error::IssueCompleted.encode()[0]), and this raises another question; what if the error index changes but the name remains the same? This wouldn't lead to any compile error, but if you compare the indexes you could see a runtime error.

Since the pallet name is sortof encoded into the error module path, if the pallet name were to change, just importing the error from it at all would likely start leading to a compile error. So it's not perfect in terms of API but I think you can achieve comparable compile-time safety today.

I'm not sure I follow this, when Error is "spat out" we know the name of the pallet and error already right?

Nope; alas not! Subxt just mindlessly iterates though all of the types seen in the metadata and generates code corresponding to them. Some of those types happen to correspond to errors and such, but to Subxt they are all just "types from the registry".

If the metadata pointed explicitly to some overarching Error enum that was like:

enum Error {
    Pallet1(path::to::pallet1::Error),
    Pallet2(path::to::pallet2::Error),
    // ...
}

Then we would know that they were special in some way, but also your code could just look something like:

pub fn is_issue_completed(&self) -> bool {
    use api::runtime_types::Error;
    self.is_module_err(Error::Pallet1(path::to::pallet1::IssueCompleted))
}

And by encoding that error like Error::Pallet1(path::to::pallet1::IssueCompleted).encode() we'd get back 2 bytes, 1 for the pallet index and one for the error index, and the module error could trivially be matched against those, but also since we're importing the relevant types, if they change in codegen we'll get compile errors :)

[jsdw]

Random thought that occurred to me after doing some other work around root level Events. We actually generate a root level event manually in Subxt (and then in an upcoming PR will have a trait/way to decode arbitrary things into those root level events.

Perhaps we could also manually generate a root level error like this:

enum Error {
    Pallet1(path::to::pallet1::Error),
    Pallet2(path::to::pallet2::Error),
    // ...
}

And then provide an as_root_error() type function on dispatch errors which allows decoding into it. This might actually address this issue (and be consistent with the upcoming way that as_root_event() will work for events). Would that sort of thing resolve this?

[jsdw]

So, we get back errors into this struct:

pub struct RawModuleError {
    /// Index of the pallet that the error came from.
    pub pallet_index: u8,
    /// Raw error bytes.
    pub error: [u8; 4],
}

We have the pallet index, and error index in the pallet (error[0]).

I think that the next 3 bytes might be the rest of the bytes needed to decode into one of the generated error types (not all error variants are empty; some have data, and looking at the errors generated for Polkadot, none have data more than an additional byte or so).

Steps to implement this:

• For sanity, it'd be good to have a dig around and try to work out how pallet errors are encoded (and thus whether we can assume we have all of the bytes needed to decode into any given generated error). (see subxt/testing/integration-tests/src/codegen/polkadot.rs for an example codegen file generated from metadata; each pallet has an enum Call (which all are linked from an enum RuntimeCall) and an enum Error.)
• In the codegen, see how we generate a top level enum Event in and do the same for an enum Error (see subxt/codegen/src/api/mod.rs and the #outer_event type; we want the same for the errors).
• Just as we impl #crate_path::events::RootEvent for this generated Event type, create a similar trait (eg RootError) for our Error type.
• Events have an as_root_event() function to try to decoe the event bytes into this generated Event type. Add an as_root_error() function to ModuleError to allow people to try and do the same with module errors.

Then, somebody can write a function like the following to see whether a subxt error is some specific module error, for instance:

fn is_err(error: SubxtError, target: api::runtime_types::Error) -> bool {
    let SubxtError::Runtime(DispatchError::Module(module_err) = subxt_error else {
        return false
    }
    module_err.as_root_error<api::runtime_types::Error>() == target
}