Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions prdoc/pr_11724.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
title: Kanas/module invalidity
doc:
- audience:
- Node Dev
- Runtime Dev
description: |-
# Description

This PR implements descriptive module invalidity support for transactions as proposed in #11337. It extends the `InvalidTransaction` enum with a new `Module(ModuleError)` variant, allowing pallets to return pallet-specific, descriptive error data during the transaction validation phase (e.g., in `TransactionExtension`).

crates:
- name: sc-rpc-api
bump: minor
- name: sc-rpc-spec-v2
bump: minor
- name: sp-runtime
bump: minor
Comment thread
Kanasjnr marked this conversation as resolved.
Outdated
7 changes: 7 additions & 0 deletions substrate/client/rpc-api/src/author/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ impl From<Error> for ErrorObjectOwned {
Some(format!("Custom error: {}", e)),
)
},
Error::Pool(PoolError::InvalidTransaction(InvalidTransaction::Module(e))) => {
ErrorObject::owned(
POOL_INVALID_TX,
"Invalid Transaction",
Some(format!("Module error: index: {}, error: {:?}", e.index, e.error)),
)
},
Error::Pool(PoolError::InvalidTransaction(e)) => {
let msg: &str = e.into();
ErrorObject::owned(
Expand Down
8 changes: 8 additions & 0 deletions substrate/client/rpc-spec-v2/src/transaction/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ impl<Hash> From<Error> for TransactionEvent<Hash> {
error: format!("Invalid transaction with custom error: {}", e),
})
},
Error::Pool(PoolError::InvalidTransaction(InvalidTransaction::Module(e))) => {
TransactionEvent::Invalid(TransactionError {
error: format!(
"Invalid transaction: Module error: index: {}, error: {:?}",
e.index, e.error
),
})
},
Error::Pool(PoolError::InvalidTransaction(e)) => {
let msg: &str = e.into();
TransactionEvent::Invalid(TransactionError {
Expand Down
28 changes: 26 additions & 2 deletions substrate/primitives/runtime/src/transaction_validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

use crate::{
codec::{Decode, Encode},
Debug,
Debug, ModuleError,
};
use alloc::{vec, vec::Vec};
use scale_info::TypeInfo;
Expand Down Expand Up @@ -89,6 +89,8 @@ pub enum InvalidTransaction {
IndeterminateImplicit,
/// The transaction extension did not authorize any origin.
UnknownOrigin,
/// An error in a module.
Module(ModuleError),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reusing the same kind of type as ModuleError is good. We want a small embedabble information. But maybe the name Error make it confusing with dispatch error. And reusing the same error type for module dispatch error and invalidity can be confusing in the future. Like using 'into' will no longer mean anything, it could convert to an invalidity or a dispatch error which has a very different consequence.

So maybe we want a new metadata information for module invalidities, etc...

It would be cleaner but it is also more work.

In the current shape it feels like pallet errors are now just a type for succint information.

}

impl InvalidTransaction {
Expand Down Expand Up @@ -129,6 +131,9 @@ impl From<InvalidTransaction> for &'static str {
InvalidTransaction::UnknownOrigin => {
"The transaction extension did not authorize any origin"
},
InvalidTransaction::Module(ModuleError { message, .. }) => {
message.unwrap_or("Unknown module error")
},
}
}
}
Expand Down Expand Up @@ -480,7 +485,7 @@ mod tests {
);

// decode back
assert_eq!(TransactionValidity::decode(&mut &*encoded), Ok(v));
assert_eq!(TransactionValidity::decode(&mut &*encoded), Ok(v.clone()));
}

#[test]
Expand All @@ -507,4 +512,23 @@ mod tests {
}
);
}

#[test]
fn module_invalidity_should_encode_and_decode() {
let v: TransactionValidity = Err(InvalidTransaction::Module(ModuleError {
index: 1,
error: [2, 0, 0, 0],
message: Some("Test error"),
})
.into());

let encoded = v.encode();
assert_eq!(TransactionValidity::decode(&mut &*encoded), Ok(v.clone()));

let error_str: &'static str = match v {
Err(TransactionValidityError::Invalid(e)) => e.into(),
_ => panic!("Expected invalid error"),
};
assert_eq!(error_str, "Test error");
}
}
Loading