Skip to content

Kanas/module invalidity#11724

Open
Kanasjnr wants to merge 6 commits intoparitytech:masterfrom
Kanasjnr:kanas/module-invalidity
Open

Kanas/module invalidity#11724
Kanasjnr wants to merge 6 commits intoparitytech:masterfrom
Kanasjnr:kanas/module-invalidity

Conversation

@Kanasjnr
Copy link
Copy Markdown
Contributor

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).

Closes #11337

Integration

Downstream projects and UI tools (like subxt or polkadot-js) can now leverage descriptive invalidity reasons. When a transaction is rejected by the pool with a Module error, the RPC will return a formatted message including the pallet index and the error bytes.

Example RPC error output:
"Invalid Transaction: Module error: index: 5, error: [1, 0, 0, 0]"

Client-side libraries can use this index and byte array to look up the specific error definition in the chain's current metadata, mirroring how DispatchError::Module is handled today.

Review Notes

The implementation follows the established pattern for descriptive errors in Substrate while ensuring 100% backward compatibility for the transaction pool and node-side logic.

Key Changes:

  • sp-runtime: Appended Module(ModuleError) to the InvalidTransaction enum (index 13). This position ensures that existing SCALE-encoded variants remain unchanged.
  • String Conversion: Updated the From<InvalidTransaction> for &'static str implementation to prioritize the descriptive message from ModuleError if present.
  • RPC Layer: Enriched both the Author RPC (v1) and the Transaction RPC (v2) to explicitly handle the Module variant and return formatted debug information.
  • Verification: Added a new unit test module_invalidity_should_encode_and_decode in sp-runtime to verify the encoding/decoding and string conversion paths.

Integration Diff:

 pub enum InvalidTransaction {
     ...
     UnknownOrigin,
+    /// An error in a module.
+    Module(ModuleError),
 }

@Kanasjnr Kanasjnr requested review from a team, acatangiu and cheme as code owners April 10, 2026 14:57
@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 10, 2026 14:58
@Kanasjnr Kanasjnr force-pushed the kanas/module-invalidity branch from d5a9896 to 5e968cb Compare April 10, 2026 15:00
@Kanasjnr
Copy link
Copy Markdown
Contributor Author

/cmd label T1-FRAME T3-Client D2-substantial I5-enhancement

@Kanasjnr
Copy link
Copy Markdown
Contributor Author

/cmd prdoc --audience node_dev runtime_dev --bump minor

@paritytech-cmd-bot-polkadot-sdk paritytech-cmd-bot-polkadot-sdk bot added D2-substantial Can be fixed by an experienced coder with a working knowledge of the codebase. I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework. T3-RPC_API This PR/Issue is related to RPC APIs. labels Apr 10, 2026
@Kanasjnr
Copy link
Copy Markdown
Contributor Author

@gui1117 ready for review

Copy link
Copy Markdown
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

Some comments, I don't know the best direction, I feel like reusing pallet errors is not so clean.

Comment thread prdoc/pr_11724.prdoc Outdated
/// 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.

@Kanasjnr Kanasjnr requested a review from gui1117 April 11, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

D2-substantial Can be fixed by an experienced coder with a working knowledge of the codebase. I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework. T3-RPC_API This PR/Issue is related to RPC APIs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create ModuleInvalidity same as ModuleError but for invalidities, for descripful invalidities from pallets.

2 participants