Skip to content

Make author inherent both required and mandatory #210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JoshOrndorff merged 2 commits into from
Jan 28, 2021
Merged

Make author inherent both required and mandatory #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d8448e
make author inherent both required and mandatory
JoshOrndorff Jan 27, 2021
a67edfe
bump runtime version
JoshOrndorff Jan 27, 2021
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
19 changes: 17 additions & 2 deletions pallets/author-inherent/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@

#![cfg_attr(not(feature = "std"), no_std)]

use frame_support::{decl_error, decl_event, decl_module, decl_storage, ensure};
use frame_support::{
decl_error, decl_event, decl_module, decl_storage, ensure, weights::DispatchClass,
};
use frame_system::{ensure_none, Config as System};
use parity_scale_codec::{Decode, Encode};
#[cfg(feature = "std")]
Expand Down Expand Up @@ -69,6 +71,7 @@ decl_error! {
pub enum Error for Module<T: Config> {
/// Author already set in block.
AuthorAlreadySet,
/// The author in the inherent is not an eligible author.
CannotBeAuthor,
}
}
Expand All @@ -86,7 +89,10 @@ decl_module! {
fn deposit_event() = default;

/// Inherent to set the author of a block
#[weight = 0]
#[weight = (
0,
DispatchClass::Mandatory
)]
fn set_author(origin, author: T::AccountId) {
ensure_none(origin)?;
ensure!(<Author<T>>::get().is_none(), Error::<T>::AuthorAlreadySet);
Expand All @@ -112,6 +118,7 @@ decl_module! {
}

fn on_finalize() {
// Do we still need this now that it is required?
assert!(<Author<T>>::take().is_some(), "Author inherent must be in the block");
}
}
Expand Down Expand Up @@ -179,6 +186,14 @@ impl<T: Config> ProvideInherent for Module<T> {
type Error = InherentError;
const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER;

fn is_inherent_required(_: &InherentData) -> Result<Option<Self::Error>, Self::Error> {
// Return Ok(Some(_)) unconditionally because this inherent is required in every block
// If it is not found, throw an AuthorInherentRequired error.
Ok(Some(InherentError::Other(
sp_runtime::RuntimeString::Borrowed("AuthorInherentRequired"),
)))
}

Comment on lines +189 to +196
Copy link
Contributor Author

@JoshOrndorff JoshOrndorff Jan 28, 2021

Choose a reason for hiding this comment

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

@4meta5 This is why I was wondering whether we still need the check in on_finalize.

See paritytech/substrate#7941 for my best understanding of how this trait works.

Copy link
Contributor

@4meta5 4meta5 Jan 28, 2021

Choose a reason for hiding this comment

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

I see. This should work, but still need to clear the storage item in on_finalize so it can be set in the next block. So can remove the assert!(<Author>::take) and replace it with <Author>::kill()

Copy link
Contributor Author

@JoshOrndorff JoshOrndorff Jan 28, 2021

Choose a reason for hiding this comment

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

@4meta5 Could you please investigate whether this cleanup is valid in another PR?

fn create_inherent(data: &InherentData) -> Option<Self::Call> {
// Grab the Vec<u8> labelled with "author__" from the map of all inherent data
let author_raw = data
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/parachain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ macro_rules! runtime_parachain {
spec_name: create_runtime_str!("moonbase-alphanet"),
impl_name: create_runtime_str!("moonbase-alphanet"),
authoring_version: 3,
spec_version: 11,
spec_version: 12,
impl_version: 1,
apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/standalone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ macro_rules! runtime_standalone {
spec_name: create_runtime_str!("moonbeam-standalone"),
impl_name: create_runtime_str!("moonbeam-standalone"),
authoring_version: 3,
spec_version: 11,
spec_version: 12,
impl_version: 1,
apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
Expand Down