Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow pallet error enum variants to contain fields #10242

Merged
merged 83 commits into from
Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from 82 commits
Commits
Show all changes
83 commits
Select commit Hold shift + click to select a range
e569450
Allow pallet errors to contain at most one field
KiChjang Nov 11, 2021
c4f7b51
Update docs on pallet::error
KiChjang Nov 11, 2021
788ea42
Reword documentation
KiChjang Nov 11, 2021
0b70512
cargo fmt
KiChjang Nov 11, 2021
d3e5c7c
Introduce CompactPalletError trait and require #[pallet::error] field…
KiChjang Nov 18, 2021
e8f6714
cargo fmt
KiChjang Nov 18, 2021
815a616
Do not assume tuple variants
KiChjang Nov 19, 2021
78ff9af
Add CompactPalletError derive macro
KiChjang Nov 19, 2021
e86c2d9
Check for error type compactness in construct_runtime
KiChjang Nov 21, 2021
2ba5c5b
cargo fmt
KiChjang Nov 21, 2021
217e1a4
Derive CompactPalletError instead of implementing it directly during …
KiChjang Nov 23, 2021
3a399b6
Implement CompactPalletError on OptionBool instead of Option<bool>
KiChjang Nov 23, 2021
a583025
Check for type idents instead of variant ident
KiChjang Nov 24, 2021
e68ba71
Add doc comments for ErrorCompactnessTest
KiChjang Nov 24, 2021
ed643ff
Add an trait implementation of ErrorCompactnessTest for ()
KiChjang Dec 1, 2021
dcddc79
Merge remote-tracking branch 'origin/master' into kckyeung/nested-pal…
KiChjang Dec 4, 2021
3913e85
Convert the error field of DispatchError to a 4-element byte array
KiChjang Dec 4, 2021
26b0adf
Add static check for pallet error size
KiChjang Dec 7, 2021
82c29a9
Rename to MAX_PALLET_ERROR_ENCODED_SIZE
KiChjang Dec 8, 2021
a1a0c32
Remove ErrorCompactnessTest trait
KiChjang Dec 8, 2021
0fb5860
Remove check_compactness
KiChjang Dec 8, 2021
69753e9
Return only the most significant byte when constructing a custom Inva…
KiChjang Dec 8, 2021
c57ccb6
Rename CompactPalletError to PalletError
KiChjang Dec 8, 2021
f6f8be5
Use counter to generate unique idents for assert macros
KiChjang Dec 8, 2021
9b92379
Make declarative pallet macros compile with pallet error size checks
KiChjang Dec 14, 2021
b984433
Remove unused doc comment
KiChjang Dec 14, 2021
2fb09fb
Try and fix build errors
KiChjang Dec 14, 2021
adceb34
Fix build errors
KiChjang Dec 21, 2021
b018d42
Add macro_use for some test modules
KiChjang Dec 21, 2021
56ff09d
Test fix
KiChjang Dec 21, 2021
29841d9
Fix compilation errors
KiChjang Dec 21, 2021
1ca9680
Remove unneeded #[macro_use]
KiChjang Dec 21, 2021
cf1cbd3
Merge remote-tracking branch 'origin/master' into kckyeung/nested-pal…
KiChjang Dec 21, 2021
b7c2b9f
Resolve import ambiguity
KiChjang Dec 21, 2021
8783da5
Make path to pallet Error enum more specific
KiChjang Dec 21, 2021
1f991ff
Fix test expectation
KiChjang Dec 21, 2021
d7e3597
Disambiguate imports
KiChjang Dec 21, 2021
aea23e0
Fix test expectations
KiChjang Dec 21, 2021
1a4bcdf
Revert appending pallet module name to path
KiChjang Dec 21, 2021
d2e42ef
Rename bags_list::list::Error to BagError
KiChjang Dec 21, 2021
9ab1b56
Fixes
KiChjang Dec 21, 2021
1be3227
Fixes
KiChjang Dec 21, 2021
0342859
Fixes
KiChjang Dec 21, 2021
066ec5f
Fix test expectations
KiChjang Dec 21, 2021
1f3ab67
Fix test expectation
KiChjang Dec 21, 2021
b45ca96
Add more implementations for PalletError
KiChjang Dec 21, 2021
981a29f
Lift the 1-field requirement for nested pallet errors
KiChjang Dec 22, 2021
0ef2812
Fix UI test expectation
KiChjang Dec 22, 2021
fb1b32b
Remove PalletError impl for OptionBool
KiChjang Dec 22, 2021
fb73b33
Use saturating operations
KiChjang Dec 23, 2021
8e8a27a
cargo fmt
KiChjang Dec 23, 2021
2a54736
Delete obsolete test
KiChjang Dec 23, 2021
a250788
Fix test expectation
KiChjang Dec 24, 2021
7d80dae
Merge remote-tracking branch 'origin/master' into kckyeung/nested-pal…
KiChjang Jan 15, 2022
c44e6e4
Try and use assert macro in const context
KiChjang Jan 19, 2022
cc06206
Pull out the pallet error size check macro
KiChjang Feb 4, 2022
c037c6d
Merge remote-tracking branch 'origin/master' into kckyeung/nested-pal…
KiChjang Feb 4, 2022
e4c3f47
Fix UI test for const assertion
KiChjang Feb 4, 2022
fa0e155
cargo fmt
KiChjang Feb 4, 2022
29e3df1
Apply clippy suggestion
KiChjang Feb 4, 2022
a8b6214
Fix doc comment
KiChjang Feb 4, 2022
cca5ed6
Docs for create_tt_return_macro
KiChjang Feb 4, 2022
52a0a73
Ensure TryInto is imported in earlier Rust editions
KiChjang Feb 7, 2022
ece305e
Apply suggestions from code review
KiChjang Feb 11, 2022
29a131a
Fix up comments and names
KiChjang Feb 12, 2022
9cdf1aa
Implement PalletError for Never
KiChjang Feb 12, 2022
5070335
cargo fmt
KiChjang Feb 12, 2022
ac1f198
Don't compile example code
KiChjang Feb 12, 2022
811e2f2
Bump API version for block builder
KiChjang Feb 16, 2022
7595123
Factor in codec attributes while derving PalletError
KiChjang Feb 17, 2022
89abef7
Rename module and fix unit test
KiChjang Feb 17, 2022
161fb88
Add missing attribute
KiChjang Feb 18, 2022
babebfc
Check API version and convert ApplyExtrinsicResult accordingly
KiChjang Feb 18, 2022
80ed571
Merge remote-tracking branch 'origin/master' into kckyeung/nested-pal…
KiChjang Feb 18, 2022
7fed725
Rename BagError to ListError
KiChjang Mar 17, 2022
8d25a2f
Merge remote-tracking branch 'origin/master' into kckyeung/nested-pal…
KiChjang Mar 20, 2022
9fef2e1
Use codec crate re-exported from frame support
KiChjang Mar 20, 2022
ddc2cce
Add links to types mentioned in doc comments
KiChjang Mar 20, 2022
23608e7
cargo fmt
KiChjang Mar 20, 2022
9d0dfa7
cargo fmt
KiChjang Mar 20, 2022
966b177
Merge branch 'master' into kckyeung/nested-pallet-error-enum
KiChjang Mar 21, 2022
2b3b730
Re-add attribute for hidden docs
KiChjang Mar 21, 2022
f4157f4
Merge remote-tracking branch 'origin/master' into kckyeung/nested-pal…
KiChjang Mar 23, 2022
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
30 changes: 25 additions & 5 deletions client/block-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use sp_blockchain::{ApplyExtrinsicFailed, Error};
use sp_core::ExecutionContext;
use sp_runtime::{
generic::BlockId,
legacy,
traits::{Block as BlockT, Hash, HashFor, Header as HeaderT, NumberFor, One},
Digest,
};
Expand Down Expand Up @@ -135,6 +136,7 @@ where
pub struct BlockBuilder<'a, Block: BlockT, A: ProvideRuntimeApi<Block>, B> {
extrinsics: Vec<Block::Extrinsic>,
api: ApiRef<'a, A::Api>,
version: u32,
block_id: BlockId<Block>,
parent_hash: Block::Hash,
backend: &'a B,
Expand Down Expand Up @@ -183,10 +185,15 @@ where

api.initialize_block_with_context(&block_id, ExecutionContext::BlockConstruction, &header)?;

let version = api
.api_version::<dyn BlockBuilderApi<Block>>(&block_id)?
.ok_or_else(|| Error::VersionInvalid("BlockBuilderApi".to_string()))?;

Ok(Self {
parent_hash,
extrinsics: Vec::new(),
api,
version,
block_id,
backend,
estimated_header_size,
Expand All @@ -199,13 +206,26 @@ where
pub fn push(&mut self, xt: <Block as BlockT>::Extrinsic) -> Result<(), Error> {
let block_id = &self.block_id;
let extrinsics = &mut self.extrinsics;
let version = self.version;

self.api.execute_in_transaction(|api| {
match api.apply_extrinsic_with_context(
block_id,
ExecutionContext::BlockConstruction,
xt.clone(),
) {
let res = if version < 6 {
#[allow(deprecated)]
api.apply_extrinsic_before_version_6_with_context(
block_id,
ExecutionContext::BlockConstruction,
xt.clone(),
)
.map(legacy::byte_sized_error::convert_to_latest)
} else {
api.apply_extrinsic_with_context(
block_id,
ExecutionContext::BlockConstruction,
xt.clone(),
)
};

match res {
Ok(Ok(_)) => {
extrinsics.push(xt);
TransactionOutcome::Commit(Ok(()))
Expand Down
2 changes: 1 addition & 1 deletion client/rpc/src/state/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ fn should_return_runtime_version() {

let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\
\"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\
[\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",5],\
[\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",6],\
[\"0xc6e9a76309f39b09\",1],[\"0xdd718d5cc53262d4\",1],[\"0xcbca25e39f142387\",2],\
[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]],\
\"transactionVersion\":1,\"stateVersion\":1}";
Expand Down
6 changes: 3 additions & 3 deletions frame/bags-list/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub mod mock;
mod tests;
pub mod weights;

pub use list::{notional_bag_for, Bag, Error, List, Node};
pub use list::{notional_bag_for, Bag, List, ListError, Node};
pub use pallet::*;
pub use weights::WeightInfo;

Expand Down Expand Up @@ -270,7 +270,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

impl<T: Config<I>, I: 'static> SortedListProvider<T::AccountId> for Pallet<T, I> {
type Error = Error;
type Error = ListError;

type Score = T::Score;

Expand All @@ -286,7 +286,7 @@ impl<T: Config<I>, I: 'static> SortedListProvider<T::AccountId> for Pallet<T, I>
List::<T, I>::contains(id)
}

fn on_insert(id: T::AccountId, score: T::Score) -> Result<(), Error> {
fn on_insert(id: T::AccountId, score: T::Score) -> Result<(), ListError> {
List::<T, I>::insert(id, score)
}

Expand Down
6 changes: 3 additions & 3 deletions frame/bags-list/src/list/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use sp_std::{
};

#[derive(Debug, PartialEq, Eq)]
pub enum Error {
pub enum ListError {
/// A duplicate id has been detected.
Duplicate,
}
Expand Down Expand Up @@ -266,9 +266,9 @@ impl<T: Config<I>, I: 'static> List<T, I> {
/// Insert a new id into the appropriate bag in the list.
///
/// Returns an error if the list already contains `id`.
pub(crate) fn insert(id: T::AccountId, score: T::Score) -> Result<(), Error> {
pub(crate) fn insert(id: T::AccountId, score: T::Score) -> Result<(), ListError> {
if Self::contains(&id) {
return Err(Error::Duplicate)
return Err(ListError::Duplicate)
}

let bag_score = notional_bag_for::<T, I>(score);
Expand Down
2 changes: 1 addition & 1 deletion frame/bags-list/src/list/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ mod list {
// then
assert_storage_noop!(assert_eq!(
List::<Runtime>::insert(3, 20).unwrap_err(),
Error::Duplicate
ListError::Duplicate
));
});
}
Expand Down
2 changes: 1 addition & 1 deletion frame/bags-list/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ mod sorted_list_provider {
// then
assert_storage_noop!(assert_eq!(
BagsList::on_insert(3, 20).unwrap_err(),
Error::Duplicate
ListError::Duplicate
));
});
}
Expand Down
2 changes: 1 addition & 1 deletion frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1583,7 +1583,7 @@ impl<T: Config> ElectionProvider for Pallet<T> {
/// number.
pub fn dispatch_error_to_invalid(error: DispatchError) -> InvalidTransaction {
let error_number = match error {
DispatchError::Module(ModuleError { error, .. }) => error,
DispatchError::Module(ModuleError { error, .. }) => error[0],
_ => 0,
};
InvalidTransaction::Custom(error_number)
Comment on lines -1586 to 1589
Copy link
Member

Choose a reason for hiding this comment

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

Feels this could be updated to support all the bytes

Expand Down
4 changes: 2 additions & 2 deletions frame/election-provider-multi-phase/src/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ mod tests {
#[test]
#[should_panic(expected = "Invalid unsigned submission must produce invalid block and \
deprive validator from their authoring reward.: \
Module(ModuleError { index: 2, error: 1, message: \
Module(ModuleError { index: 2, error: [1, 0, 0, 0], message: \
Some(\"PreDispatchWrongWinnerCount\") })")]
fn unfeasible_solution_panics() {
ExtBuilder::default().build_and_execute(|| {
Expand Down Expand Up @@ -1053,7 +1053,7 @@ mod tests {
MultiPhase::basic_checks(&solution, "mined").unwrap_err(),
MinerError::PreDispatchChecksFailed(DispatchError::Module(ModuleError {
index: 2,
error: 1,
error: [1, 0, 0, 0],
message: Some("PreDispatchWrongWinnerCount"),
})),
);
Expand Down
4 changes: 2 additions & 2 deletions frame/scheduler/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use sp_runtime::{
// Logger module to track execution.
#[frame_support::pallet]
pub mod logger {
use super::*;
use super::{OriginCaller, OriginTrait};
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;
use std::cell::RefCell;
Expand Down Expand Up @@ -71,7 +71,7 @@ pub mod logger {
#[pallet::call]
impl<T: Config> Pallet<T>
where
<T as system::Config>::Origin: OriginTrait<PalletsOrigin = OriginCaller>,
<T as frame_system::Config>::Origin: OriginTrait<PalletsOrigin = OriginCaller>,
{
#[pallet::weight(*weight)]
pub fn log(origin: OriginFor<T>, i: u32, weight: Weight) -> DispatchResult {
Expand Down
1 change: 0 additions & 1 deletion frame/sudo/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use sp_runtime::{
// Logger module to track execution.
#[frame_support::pallet]
pub mod logger {
use super::*;
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;

Expand Down
34 changes: 34 additions & 0 deletions frame/support/procedural/src/construct_runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ fn construct_runtime_final_expansion(
expand::expand_outer_inherent(&name, &block, &unchecked_extrinsic, &pallets, &scrate);
let validate_unsigned = expand::expand_outer_validate_unsigned(&name, &pallets, &scrate);
let integrity_test = decl_integrity_test(&scrate);
let static_assertions = decl_static_assertions(&name, &pallets, &scrate);

let res = quote!(
#scrate_decl
Expand Down Expand Up @@ -282,6 +283,8 @@ fn construct_runtime_final_expansion(
#validate_unsigned

#integrity_test

#static_assertions
);

Ok(res)
Expand Down Expand Up @@ -471,3 +474,34 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 {
}
)
}

fn decl_static_assertions(
runtime: &Ident,
pallet_decls: &[Pallet],
scrate: &TokenStream2,
) -> TokenStream2 {
let error_encoded_size_check = pallet_decls.iter().map(|decl| {
let path = &decl.path;
let assert_message = format!(
"The maximum encoded size of the error type in the `{}` pallet exceeds \
`MAX_MODULE_ERROR_ENCODED_SIZE`",
decl.name,
);

quote! {
#scrate::tt_call! {
Copy link
Member

Choose a reason for hiding this comment

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

I have to admit that I'm still confused by this macro :P But yeah, it works ;)

macro = [{ #path::tt_error_token }]
frame_support = [{ #scrate }]
~~> #scrate::assert_error_encoded_size! {
path = [{ #path }]
runtime = [{ #runtime }]
assert_message = [{ #assert_message }]
}
}
}
});

quote! {
#(#error_encoded_size_check)*
}
}
19 changes: 16 additions & 3 deletions frame/support/procedural/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ mod dummy_part_checker;
mod key_prefix;
mod match_and_insert;
mod pallet;
mod pallet_error;
mod partial_eq_no_bound;
mod storage;
mod transactional;
mod tt_macro;

use proc_macro::TokenStream;
use std::{cell::RefCell, str::FromStr};
Expand All @@ -41,9 +43,9 @@ thread_local! {
static COUNTER: RefCell<Counter> = RefCell::new(Counter(0));
}

/// Counter to generate a relatively unique identifier for macros querying for the existence of
/// pallet parts. This is necessary because declarative macros gets hoisted to the crate root,
/// which shares the namespace with other pallets containing the very same query macros.
/// Counter to generate a relatively unique identifier for macros. This is necessary because
/// declarative macros gets hoisted to the crate root, which shares the namespace with other pallets
/// containing the very same macros.
struct Counter(u64);

impl Counter {
Expand Down Expand Up @@ -562,3 +564,14 @@ pub fn __generate_dummy_part_checker(input: TokenStream) -> TokenStream {
pub fn match_and_insert(input: TokenStream) -> TokenStream {
match_and_insert::match_and_insert(input)
}

#[proc_macro_derive(PalletError, attributes(codec))]
pub fn derive_pallet_error(input: TokenStream) -> TokenStream {
pallet_error::derive_pallet_error(input)
}

/// Internal macro used by `frame_support` to create tt-call-compliant macros
#[proc_macro]
pub fn __create_tt_macro(input: TokenStream) -> TokenStream {
tt_macro::create_tt_return_macro(input)
}
Loading