From 328563f8d42c6dcbe255546261f90a309d71d05c Mon Sep 17 00:00:00 2001
From: Stanislav Tkach <stanislav.tkach@gmail.com>
Date: Fri, 20 Dec 2019 18:12:21 +0200
Subject: [PATCH] Migrate membership, nicks, scored-pool and session to
decl_error (#4463)
* Migrate membership, nicks, scored-pool and session to decl_error
* Fix tests
* Update frame/scored-pool/src/tests.rs
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
* Remove InsufficientBalance error from scored-pool
* Replace Error::<Test, DefaultInstance> with Error::<Test, _>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
---
substrate/frame/membership/src/lib.rs | 58 +++++++++++++-----------
substrate/frame/nicks/src/lib.rs | 44 +++++++++++-------
substrate/frame/scored-pool/src/lib.rs | 35 +++++++++-----
substrate/frame/scored-pool/src/tests.rs | 25 +++++-----
substrate/frame/session/src/lib.rs | 25 +++++++---
substrate/frame/staking/src/lib.rs | 5 +-
substrate/frame/treasury/src/lib.rs | 4 +-
7 files changed, 115 insertions(+), 81 deletions(-)
diff --git a/substrate/frame/membership/src/lib.rs b/substrate/frame/membership/src/lib.rs
index 0a7f8ec7fc9..3a65f1604eb 100644
--- a/substrate/frame/membership/src/lib.rs
+++ b/substrate/frame/membership/src/lib.rs
@@ -24,7 +24,7 @@
use sp_std::prelude::*;
use frame_support::{
- decl_module, decl_storage, decl_event,
+ decl_module, decl_storage, decl_event, decl_error,
traits::{ChangeMembers, InitializeMembers},
weights::SimpleDispatchInfo,
};
@@ -93,6 +93,16 @@ decl_event!(
}
);
+decl_error! {
+ /// Error for the nicks module.
+ pub enum Error for Module<T: Trait<I>, I: Instance> {
+ /// Already a member.
+ AlreadyMember,
+ /// Not a member.
+ NotMember,
+ }
+}
+
decl_module! {
pub struct Module<T: Trait<I>, I: Instance=DefaultInstance>
for enum Call
@@ -107,11 +117,10 @@ decl_module! {
fn add_member(origin, who: T::AccountId) {
T::AddOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| "bad origin")?;
+ .or_else(ensure_root)?;
let mut members = <Members<T, I>>::get();
- let location = members.binary_search(&who).err().ok_or("already a member")?;
+ let location = members.binary_search(&who).err().ok_or(Error::<T, I>::AlreadyMember)?;
members.insert(location, who.clone());
<Members<T, I>>::put(&members);
@@ -127,11 +136,10 @@ decl_module! {
fn remove_member(origin, who: T::AccountId) {
T::RemoveOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| "bad origin")?;
+ .or_else(ensure_root)?;
let mut members = <Members<T, I>>::get();
- let location = members.binary_search(&who).ok().ok_or("not a member")?;
+ let location = members.binary_search(&who).ok().ok_or(Error::<T, I>::NotMember)?;
members.remove(location);
<Members<T, I>>::put(&members);
@@ -147,14 +155,13 @@ decl_module! {
fn swap_member(origin, remove: T::AccountId, add: T::AccountId) {
T::SwapOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| "bad origin")?;
+ .or_else(ensure_root)?;
if remove == add { return Ok(()) }
let mut members = <Members<T, I>>::get();
- let location = members.binary_search(&remove).ok().ok_or("not a member")?;
- let _ = members.binary_search(&add).err().ok_or("already a member")?;
+ let location = members.binary_search(&remove).ok().ok_or(Error::<T, I>::NotMember)?;
+ let _ = members.binary_search(&add).err().ok_or(Error::<T, I>::AlreadyMember)?;
members[location] = add.clone();
members.sort();
<Members<T, I>>::put(&members);
@@ -176,8 +183,7 @@ decl_module! {
fn reset_members(origin, members: Vec<T::AccountId>) {
T::ResetOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| "bad origin")?;
+ .or_else(ensure_root)?;
let mut members = members;
members.sort();
@@ -198,8 +204,8 @@ decl_module! {
if remove != new {
let mut members = <Members<T, I>>::get();
- let location = members.binary_search(&remove).ok().ok_or("not a member")?;
- let _ = members.binary_search(&new).err().ok_or("already a member")?;
+ let location = members.binary_search(&remove).ok().ok_or(Error::<T, I>::NotMember)?;
+ let _ = members.binary_search(&new).err().ok_or(Error::<T, I>::AlreadyMember)?;
members[location] = new.clone();
members.sort();
<Members<T, I>>::put(&members);
@@ -225,7 +231,7 @@ mod tests {
use sp_core::H256;
// The testing primitives are very useful for avoiding having to work with signatures
// or public keys. `u64` is used as the `AccountId` and no `Signature`s are requried.
- use sp_runtime::{Perbill, traits::{BlakeTwo256, IdentityLookup}, testing::Header};
+ use sp_runtime::{Perbill, traits::{BlakeTwo256, IdentityLookup}, testing::Header, traits::BadOrigin};
use frame_system::EnsureSignedBy;
impl_outer_origin! {
@@ -328,8 +334,8 @@ mod tests {
#[test]
fn add_member_works() {
new_test_ext().execute_with(|| {
- assert_noop!(Membership::add_member(Origin::signed(5), 15), "bad origin");
- assert_noop!(Membership::add_member(Origin::signed(1), 10), "already a member");
+ assert_noop!(Membership::add_member(Origin::signed(5), 15), BadOrigin);
+ assert_noop!(Membership::add_member(Origin::signed(1), 10), Error::<Test, _>::AlreadyMember);
assert_ok!(Membership::add_member(Origin::signed(1), 15));
assert_eq!(Membership::members(), vec![10, 15, 20, 30]);
assert_eq!(MEMBERS.with(|m| m.borrow().clone()), Membership::members());
@@ -339,8 +345,8 @@ mod tests {
#[test]
fn remove_member_works() {
new_test_ext().execute_with(|| {
- assert_noop!(Membership::remove_member(Origin::signed(5), 20), "bad origin");
- assert_noop!(Membership::remove_member(Origin::signed(2), 15), "not a member");
+ assert_noop!(Membership::remove_member(Origin::signed(5), 20), BadOrigin);
+ assert_noop!(Membership::remove_member(Origin::signed(2), 15), Error::<Test, _>::NotMember);
assert_ok!(Membership::remove_member(Origin::signed(2), 20));
assert_eq!(Membership::members(), vec![10, 30]);
assert_eq!(MEMBERS.with(|m| m.borrow().clone()), Membership::members());
@@ -350,9 +356,9 @@ mod tests {
#[test]
fn swap_member_works() {
new_test_ext().execute_with(|| {
- assert_noop!(Membership::swap_member(Origin::signed(5), 10, 25), "bad origin");
- assert_noop!(Membership::swap_member(Origin::signed(3), 15, 25), "not a member");
- assert_noop!(Membership::swap_member(Origin::signed(3), 10, 30), "already a member");
+ assert_noop!(Membership::swap_member(Origin::signed(5), 10, 25), BadOrigin);
+ assert_noop!(Membership::swap_member(Origin::signed(3), 15, 25), Error::<Test, _>::NotMember);
+ assert_noop!(Membership::swap_member(Origin::signed(3), 10, 30), Error::<Test, _>::AlreadyMember);
assert_ok!(Membership::swap_member(Origin::signed(3), 20, 20));
assert_eq!(Membership::members(), vec![10, 20, 30]);
assert_ok!(Membership::swap_member(Origin::signed(3), 10, 25));
@@ -373,8 +379,8 @@ mod tests {
#[test]
fn change_key_works() {
new_test_ext().execute_with(|| {
- assert_noop!(Membership::change_key(Origin::signed(3), 25), "not a member");
- assert_noop!(Membership::change_key(Origin::signed(10), 20), "already a member");
+ assert_noop!(Membership::change_key(Origin::signed(3), 25), Error::<Test, _>::NotMember);
+ assert_noop!(Membership::change_key(Origin::signed(10), 20), Error::<Test, _>::AlreadyMember);
assert_ok!(Membership::change_key(Origin::signed(10), 40));
assert_eq!(Membership::members(), vec![20, 30, 40]);
assert_eq!(MEMBERS.with(|m| m.borrow().clone()), Membership::members());
@@ -393,7 +399,7 @@ mod tests {
#[test]
fn reset_members_works() {
new_test_ext().execute_with(|| {
- assert_noop!(Membership::reset_members(Origin::signed(1), vec![20, 40, 30]), "bad origin");
+ assert_noop!(Membership::reset_members(Origin::signed(1), vec![20, 40, 30]), BadOrigin);
assert_ok!(Membership::reset_members(Origin::signed(4), vec![20, 40, 30]));
assert_eq!(Membership::members(), vec![20, 30, 40]);
assert_eq!(MEMBERS.with(|m| m.borrow().clone()), Membership::members());
diff --git a/substrate/frame/nicks/src/lib.rs b/substrate/frame/nicks/src/lib.rs
index d2f0b4d8c98..f407c4aad0d 100644
--- a/substrate/frame/nicks/src/lib.rs
+++ b/substrate/frame/nicks/src/lib.rs
@@ -43,7 +43,7 @@ use sp_runtime::{
traits::{StaticLookup, EnsureOrigin, Zero}
};
use frame_support::{
- decl_module, decl_event, decl_storage, ensure,
+ decl_module, decl_event, decl_storage, ensure, decl_error,
traits::{Currency, ReservableCurrency, OnUnbalanced, Get},
weights::SimpleDispatchInfo,
};
@@ -97,9 +97,23 @@ decl_event!(
}
);
+decl_error! {
+ /// Error for the nicks module.
+ pub enum Error for Module<T: Trait> {
+ /// A name is too short.
+ TooShort,
+ /// A name is too long.
+ TooLong,
+ /// An account in't named.
+ Unnamed,
+ }
+}
+
decl_module! {
// Simple declaration of the `Module` type. Lets the macro know what it's working on.
pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+ type Error = Error<T>;
+
fn deposit_event() = default;
/// Reservation fee.
@@ -131,8 +145,8 @@ decl_module! {
fn set_name(origin, name: Vec<u8>) {
let sender = ensure_signed(origin)?;
- ensure!(name.len() >= T::MinLength::get(), "Name too short");
- ensure!(name.len() <= T::MaxLength::get(), "Name too long");
+ ensure!(name.len() >= T::MinLength::get(), Error::<T>::TooShort,);
+ ensure!(name.len() <= T::MaxLength::get(), Error::<T>::TooLong);
let deposit = if let Some((_, deposit)) = <NameOf<T>>::get(&sender) {
Self::deposit_event(RawEvent::NameSet(sender.clone()));
@@ -160,7 +174,7 @@ decl_module! {
fn clear_name(origin) {
let sender = ensure_signed(origin)?;
- let deposit = <NameOf<T>>::take(&sender).ok_or("Not named")?.1;
+ let deposit = <NameOf<T>>::take(&sender).ok_or(Error::<T>::Unnamed)?.1;
let _ = T::Currency::unreserve(&sender, deposit.clone());
@@ -184,13 +198,12 @@ decl_module! {
fn kill_name(origin, target: <T::Lookup as StaticLookup>::Source) {
T::ForceOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| "bad origin")?;
+ .or_else(ensure_root)?;
// Figure out who we're meant to be clearing.
let target = T::Lookup::lookup(target)?;
// Grab their deposit (and check that they have one).
- let deposit = <NameOf<T>>::take(&target).ok_or("Not named")?.1;
+ let deposit = <NameOf<T>>::take(&target).ok_or(Error::<T>::Unnamed)?.1;
// Slash their deposit from them.
T::Slashed::on_unbalanced(T::Currency::slash_reserved(&target, deposit.clone()).0);
@@ -213,8 +226,7 @@ decl_module! {
fn force_name(origin, target: <T::Lookup as StaticLookup>::Source, name: Vec<u8>) {
T::ForceOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| "bad origin")?;
+ .or_else(ensure_root)?;
let target = T::Lookup::lookup(target)?;
let deposit = <NameOf<T>>::get(&target).map(|x| x.1).unwrap_or_else(Zero::zero);
@@ -235,7 +247,7 @@ mod tests {
// The testing primitives are very useful for avoiding having to work with signatures
// or public keys. `u64` is used as the `AccountId` and no `Signature`s are required.
use sp_runtime::{
- Perbill, testing::Header, traits::{BlakeTwo256, IdentityLookup},
+ Perbill, testing::Header, traits::{BlakeTwo256, IdentityLookup, BadOrigin},
};
impl_outer_origin! {
@@ -336,7 +348,7 @@ mod tests {
new_test_ext().execute_with(|| {
assert_noop!(
Nicks::set_name(Origin::signed(2), b"Dr. David Brubeck, III".to_vec()),
- "Name too long"
+ Error::<Test>::TooLong,
);
assert_ok!(Nicks::set_name(Origin::signed(2), b"Dave".to_vec()));
@@ -369,18 +381,18 @@ mod tests {
#[test]
fn error_catching_should_work() {
new_test_ext().execute_with(|| {
- assert_noop!(Nicks::clear_name(Origin::signed(1)), "Not named");
+ assert_noop!(Nicks::clear_name(Origin::signed(1)), Error::<Test>::Unnamed);
assert_noop!(Nicks::set_name(Origin::signed(3), b"Dave".to_vec()), "not enough free funds");
- assert_noop!(Nicks::set_name(Origin::signed(1), b"Ga".to_vec()), "Name too short");
+ assert_noop!(Nicks::set_name(Origin::signed(1), b"Ga".to_vec()), Error::<Test>::TooShort);
assert_noop!(
Nicks::set_name(Origin::signed(1), b"Gavin James Wood, Esquire".to_vec()),
- "Name too long"
+ Error::<Test>::TooLong
);
assert_ok!(Nicks::set_name(Origin::signed(1), b"Dave".to_vec()));
- assert_noop!(Nicks::kill_name(Origin::signed(2), 1), "bad origin");
- assert_noop!(Nicks::force_name(Origin::signed(2), 1, b"Whatever".to_vec()), "bad origin");
+ assert_noop!(Nicks::kill_name(Origin::signed(2), 1), BadOrigin);
+ assert_noop!(Nicks::force_name(Origin::signed(2), 1, b"Whatever".to_vec()), BadOrigin);
});
}
}
diff --git a/substrate/frame/scored-pool/src/lib.rs b/substrate/frame/scored-pool/src/lib.rs
index 65a867df600..685345aff6c 100644
--- a/substrate/frame/scored-pool/src/lib.rs
+++ b/substrate/frame/scored-pool/src/lib.rs
@@ -94,7 +94,7 @@ use sp_std::{
prelude::*,
};
use frame_support::{
- decl_module, decl_storage, decl_event, ensure,
+ decl_module, decl_storage, decl_event, ensure, decl_error,
traits::{ChangeMembers, InitializeMembers, Currency, Get, ReservableCurrency},
};
use frame_system::{self as system, ensure_root, ensure_signed};
@@ -222,11 +222,25 @@ decl_event!(
}
);
+decl_error! {
+ /// Error for the scored-pool module.
+ pub enum Error for Module<T: Trait<I>, I: Instance> {
+ /// Already a member.
+ AlreadyInPool,
+ /// Index out of bounds.
+ InvalidIndex,
+ /// Index does not match requested account.
+ WrongAccountIndex,
+ }
+}
+
decl_module! {
pub struct Module<T: Trait<I>, I: Instance=DefaultInstance>
for enum Call
where origin: T::Origin
{
+ type Error = Error<T, I>;
+
fn deposit_event() = default;
/// Every `Period` blocks the `Members` set is refreshed from the
@@ -251,11 +265,10 @@ decl_module! {
/// the index of the transactor in the `Pool`.
pub fn submit_candidacy(origin) {
let who = ensure_signed(origin)?;
- ensure!(!<CandidateExists<T, I>>::exists(&who), "already a member");
+ ensure!(!<CandidateExists<T, I>>::exists(&who), Error::<T, I>::AlreadyInPool);
let deposit = T::CandidateDeposit::get();
- T::Currency::reserve(&who, deposit)
- .map_err(|_| "balance too low to submit candidacy")?;
+ T::Currency::reserve(&who, deposit)?;
// can be inserted as last element in pool, since entities with
// `None` are always sorted to the end.
@@ -305,8 +318,7 @@ decl_module! {
) {
T::KickOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| "bad origin")?;
+ .or_else(ensure_root)?;
let who = T::Lookup::lookup(dest)?;
@@ -331,8 +343,7 @@ decl_module! {
) {
T::ScoreOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| "bad origin")?;
+ .or_else(ensure_root)?;
let who = T::Lookup::lookup(dest)?;
@@ -414,7 +425,7 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
mut pool: PoolT<T, I>,
remove: T::AccountId,
index: u32
- ) -> Result<(), &'static str> {
+ ) -> Result<(), Error<T, I>> {
// all callers of this function in this module also check
// the index for validity before calling this function.
// nevertheless we check again here, to assert that there was
@@ -444,11 +455,11 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
pool: &PoolT<T, I>,
who: &T::AccountId,
index: u32
- ) -> Result<(), &'static str> {
- ensure!(index < pool.len() as u32, "index out of bounds");
+ ) -> Result<(), Error<T, I>> {
+ ensure!(index < pool.len() as u32, Error::<T, I>::InvalidIndex);
let (index_who, _index_score) = &pool[index as usize];
- ensure!(index_who == who, "index does not match requested account");
+ ensure!(index_who == who, Error::<T, I>::WrongAccountIndex);
Ok(())
}
diff --git a/substrate/frame/scored-pool/src/tests.rs b/substrate/frame/scored-pool/src/tests.rs
index 0b3ede9ee04..6b6649f73cc 100644
--- a/substrate/frame/scored-pool/src/tests.rs
+++ b/substrate/frame/scored-pool/src/tests.rs
@@ -20,15 +20,12 @@ use super::*;
use mock::*;
use frame_support::{assert_ok, assert_noop};
-use sp_runtime::traits::OnInitialize;
+use sp_runtime::traits::{OnInitialize, BadOrigin};
type ScoredPool = Module<Test>;
type System = frame_system::Module<Test>;
type Balances = pallet_balances::Module<Test>;
-const OOB_ERR: &str = "index out of bounds";
-const INDEX_ERR: &str = "index does not match requested account";
-
#[test]
fn query_membership_works() {
new_test_ext().execute_with(|| {
@@ -44,11 +41,11 @@ fn submit_candidacy_must_not_work() {
new_test_ext().execute_with(|| {
assert_noop!(
ScoredPool::submit_candidacy(Origin::signed(99)),
- "balance too low to submit candidacy"
+ "not enough free funds"
);
assert_noop!(
ScoredPool::submit_candidacy(Origin::signed(40)),
- "already a member"
+ Error::<Test, _>::AlreadyInPool
);
});
}
@@ -111,7 +108,7 @@ fn kicking_works_only_for_authorized() {
new_test_ext().execute_with(|| {
let who = 40;
let index = find_in_pool(who).expect("entity must be in pool") as u32;
- assert_noop!(ScoredPool::kick(Origin::signed(99), who, index), "bad origin");
+ assert_noop!(ScoredPool::kick(Origin::signed(99), who, index), BadOrigin);
});
}
@@ -203,7 +200,7 @@ fn withdraw_candidacy_must_only_work_for_members() {
new_test_ext().execute_with(|| {
let who = 77;
let index = 0;
- assert_noop!( ScoredPool::withdraw_candidacy(Origin::signed(who), index), INDEX_ERR);
+ assert_noop!( ScoredPool::withdraw_candidacy(Origin::signed(who), index), Error::<Test, _>::WrongAccountIndex);
});
}
@@ -212,9 +209,9 @@ fn oob_index_should_abort() {
new_test_ext().execute_with(|| {
let who = 40;
let oob_index = ScoredPool::pool().len() as u32;
- assert_noop!(ScoredPool::withdraw_candidacy(Origin::signed(who), oob_index), OOB_ERR);
- assert_noop!(ScoredPool::score(Origin::signed(ScoreOrigin::get()), who, oob_index, 99), OOB_ERR);
- assert_noop!(ScoredPool::kick(Origin::signed(KickOrigin::get()), who, oob_index), OOB_ERR);
+ assert_noop!(ScoredPool::withdraw_candidacy(Origin::signed(who), oob_index), Error::<Test, _>::InvalidIndex);
+ assert_noop!(ScoredPool::score(Origin::signed(ScoreOrigin::get()), who, oob_index, 99), Error::<Test, _>::InvalidIndex);
+ assert_noop!(ScoredPool::kick(Origin::signed(KickOrigin::get()), who, oob_index), Error::<Test, _>::InvalidIndex);
});
}
@@ -223,9 +220,9 @@ fn index_mismatches_should_abort() {
new_test_ext().execute_with(|| {
let who = 40;
let index = 3;
- assert_noop!(ScoredPool::withdraw_candidacy(Origin::signed(who), index), INDEX_ERR);
- assert_noop!(ScoredPool::score(Origin::signed(ScoreOrigin::get()), who, index, 99), INDEX_ERR);
- assert_noop!(ScoredPool::kick(Origin::signed(KickOrigin::get()), who, index), INDEX_ERR);
+ assert_noop!(ScoredPool::withdraw_candidacy(Origin::signed(who), index), Error::<Test, _>::WrongAccountIndex);
+ assert_noop!(ScoredPool::score(Origin::signed(ScoreOrigin::get()), who, index, 99), Error::<Test, _>::WrongAccountIndex);
+ assert_noop!(ScoredPool::kick(Origin::signed(KickOrigin::get()), who, index), Error::<Test, _>::WrongAccountIndex);
});
}
diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs
index 5b3e4e2aeeb..79bace0d4a1 100644
--- a/substrate/frame/session/src/lib.rs
+++ b/substrate/frame/session/src/lib.rs
@@ -125,7 +125,7 @@ use sp_runtime::{KeyTypeId, Perbill, RuntimeAppPublic, BoundToRuntimeAppPublic};
use frame_support::weights::SimpleDispatchInfo;
use sp_runtime::traits::{Convert, Zero, Member, OpaqueKeys};
use sp_staking::SessionIndex;
-use frame_support::{dispatch, ConsensusEngineId, decl_module, decl_event, decl_storage};
+use frame_support::{dispatch, ConsensusEngineId, decl_module, decl_event, decl_storage, decl_error};
use frame_support::{ensure, traits::{OnFreeBalanceZero, Get, FindAuthor, ValidatorRegistration}, Parameter};
use frame_system::{self as system, ensure_signed};
@@ -464,12 +464,26 @@ decl_event!(
}
);
+decl_error! {
+ /// Error for the session module.
+ pub enum Error for Module<T: Trait> {
+ /// Invalid ownership proof.
+ InvalidProof,
+ /// No associated validator ID for account.
+ NoAssociatedValidatorId,
+ /// Registered duplicate key.
+ DuplicatedKey,
+ }
+}
+
decl_module! {
pub struct Module<T: Trait> for enum Call where origin: T::Origin {
/// Used as first key for `NextKeys` and `KeyOwner` to put all the data into the same branch
/// of the trie.
const DEDUP_KEY_PREFIX: &[u8] = DEDUP_KEY_PREFIX;
+ type Error = Error<T>;
+
fn deposit_event() = default;
/// Sets the session key(s) of the function caller to `key`.
@@ -486,12 +500,9 @@ decl_module! {
fn set_keys(origin, keys: T::Keys, proof: Vec<u8>) -> dispatch::DispatchResult {
let who = ensure_signed(origin)?;
- ensure!(keys.ownership_proof_is_valid(&proof), "invalid ownership proof");
+ ensure!(keys.ownership_proof_is_valid(&proof), Error::<T>::InvalidProof);
- let who = match T::ValidatorIdOf::convert(who) {
- Some(val_id) => val_id,
- None => Err("no associated validator ID for account.")?,
- };
+ let who = T::ValidatorIdOf::convert(who).ok_or(Error::<T>::NoAssociatedValidatorId)?;
Self::do_set_keys(&who, keys)?;
@@ -640,7 +651,7 @@ impl<T: Trait> Module<T> {
// ensure keys are without duplication.
ensure!(
Self::key_owner(*id, key).map_or(true, |owner| &owner == who),
- "registered duplicate key"
+ Error::<T>::DuplicatedKey,
);
if let Some(old) = old_keys.as_ref().map(|k| k.get_raw(*id)) {
diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs
index f8cdd270591..34c42da359b 100644
--- a/substrate/frame/staking/src/lib.rs
+++ b/substrate/frame/staking/src/lib.rs
@@ -794,8 +794,6 @@ decl_error! {
AlreadyBonded,
/// Controller is already paired.
AlreadyPaired,
- /// Should be the root origin or the `T::SlashCancelOrigin`.
- BadOrigin,
/// Targets cannot be empty.
EmptyTargets,
/// Duplicate index.
@@ -1190,8 +1188,7 @@ decl_module! {
fn cancel_deferred_slash(origin, era: EraIndex, slash_indices: Vec<u32>) {
T::SlashCancelOrigin::try_origin(origin)
.map(|_| ())
- .or_else(ensure_root)
- .map_err(|_| Error::<T>::BadOrigin)?;
+ .or_else(ensure_root)?;
let mut slash_indices = slash_indices;
slash_indices.sort_unstable();
diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs
index ee0e1adc5ee..6e1c6fb8d8a 100644
--- a/substrate/frame/treasury/src/lib.rs
+++ b/substrate/frame/treasury/src/lib.rs
@@ -166,7 +166,7 @@ decl_module! {
/// # </weight>
#[weight = SimpleDispatchInfo::FixedOperational(100_000)]
fn reject_proposal(origin, #[compact] proposal_id: ProposalIndex) {
- T::RejectOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
+ T::RejectOrigin::ensure_origin(origin)?;
let proposal = <Proposals<T>>::take(proposal_id).ok_or(Error::<T>::InvalidProposalIndex)?;
let value = proposal.bond;
@@ -184,7 +184,7 @@ decl_module! {
/// # </weight>
#[weight = SimpleDispatchInfo::FixedOperational(100_000)]
fn approve_proposal(origin, #[compact] proposal_id: ProposalIndex) {
- T::ApproveOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
+ T::ApproveOrigin::ensure_origin(origin)?;
ensure!(<Proposals<T>>::exists(proposal_id), Error::<T>::InvalidProposalIndex);
--
GitLab