diff --git a/polkadot/runtime/src/parachains.rs b/polkadot/runtime/src/parachains.rs index 1a6a77d7febd1..50da4306cb2fc 100644 --- a/polkadot/runtime/src/parachains.rs +++ b/polkadot/runtime/src/parachains.rs @@ -26,6 +26,7 @@ use polkadot_primitives::parachain::{Id, Chain, DutyRoster, CandidateReceipt}; use {system, session}; use runtime_support::{StorageValue, StorageMap}; +use runtime_support::dispatch::Result; #[cfg(any(feature = "std", test))] use rstd::marker::PhantomData; @@ -44,7 +45,7 @@ decl_module! { pub struct Module; pub enum Call where aux: ::PublicAux { // provide candidate receipts for parachains, in ascending order by id. - fn set_heads(aux, heads: Vec) = 0; + fn set_heads(aux, heads: Vec) -> Result = 0; } } @@ -137,13 +138,13 @@ impl Module { >::put(parachains); } - fn set_heads(aux: &::PublicAux, heads: Vec) { - assert!(aux.is_empty()); - assert!(!>::exists(), "Parachain heads must be updated only once in the block"); - assert!( + fn set_heads(aux: &::PublicAux, heads: Vec) -> Result { + ensure!(aux.is_empty(), "set_heads must not be signed"); + ensure!(!>::exists(), "Parachain heads must be updated only once in the block"); + ensure!( >::extrinsic_index() == T::SET_POSITION, - "Parachain heads update extrinsic must be at position {} in the block", - T::SET_POSITION + "Parachain heads update extrinsic must be at position {} in the block" +// , T::SET_POSITION ); let active_parachains = Self::active_parachains(); @@ -151,10 +152,10 @@ impl Module { // perform this check before writing to storage. for head in &heads { - assert!( + ensure!( iter.find(|&p| p == &head.parachain_index).is_some(), - "Submitted candidate for unregistered or out-of-order parachain {}", - head.parachain_index.into_inner() + "Submitted candidate for unregistered or out-of-order parachain {}" +// , head.parachain_index.into_inner() ); } @@ -164,6 +165,8 @@ impl Module { } >::put(true); + + Ok(()) } } diff --git a/substrate/runtime-std/with_std.rs b/substrate/runtime-std/with_std.rs index 92f6654bbb94e..e22e700f2b44c 100644 --- a/substrate/runtime-std/with_std.rs +++ b/substrate/runtime-std/with_std.rs @@ -29,6 +29,7 @@ pub use std::ptr; pub use std::rc; pub use std::slice; pub use std::vec; +pub use std::result; pub mod collections { pub use std::collections::btree_map; diff --git a/substrate/runtime-std/without_std.rs b/substrate/runtime-std/without_std.rs index 5e2c45bde1704..6b45bcba6f1d8 100644 --- a/substrate/runtime-std/without_std.rs +++ b/substrate/runtime-std/without_std.rs @@ -37,6 +37,7 @@ pub use core::num; pub use core::ops; pub use core::ptr; pub use core::slice; +pub use core::result; pub mod collections { pub use alloc::btree_map; diff --git a/substrate/runtime-support/src/dispatch.rs b/substrate/runtime-support/src/dispatch.rs index d5d9c97f3b9c1..683d408837898 100644 --- a/substrate/runtime-support/src/dispatch.rs +++ b/substrate/runtime-support/src/dispatch.rs @@ -19,20 +19,23 @@ pub use rstd::prelude::{Vec, Clone, Eq, PartialEq}; #[cfg(feature = "std")] pub use std::fmt; +pub use rstd::result; pub use rstd::marker::PhantomData; #[cfg(feature = "std")] use serde; pub use codec::{Slicable, Input}; +pub type Result = result::Result<(), &'static str>; + pub trait Dispatchable { type Trait; - fn dispatch(self); + fn dispatch(self) -> Result; } pub trait AuxDispatchable { type Aux; type Trait; - fn dispatch(self, aux: &Self::Aux); + fn dispatch(self, aux: &Self::Aux) -> Result; } #[cfg(feature = "std")] @@ -104,7 +107,7 @@ macro_rules! decl_dispatch { $( $param_name:ident : $param:ty ),* - ) + ) -> $result:ty = $id:expr ; )* } @@ -114,7 +117,7 @@ macro_rules! decl_dispatch { impl for $mod_type<$trait_instance: $trait_name>; pub enum $call_type; $( - fn $fn_name( $( $param_name: $param ),* ) = $id; + fn $fn_name( $( $param_name: $param ),* ) -> $result = $id; )* } decl_dispatch! { @@ -131,7 +134,7 @@ macro_rules! decl_dispatch { $( , $param_name:ident : $param:ty )* - ) + ) -> $result:ty = $id:expr ; )* } @@ -141,7 +144,7 @@ macro_rules! decl_dispatch { impl for $mod_type<$trait_instance: $trait_name>; pub enum $call_type where aux: $aux_type; $( - fn $fn_name(aux $(, $param_name: $param )*)= $id; + fn $fn_name(aux $(, $param_name: $param )*) -> $result = $id; )* } decl_dispatch! { @@ -154,11 +157,11 @@ macro_rules! decl_dispatch { impl for $mod_type:ident<$trait_instance:ident: $trait_name:ident>; ) => { impl<$trait_instance: $trait_name> $mod_type<$trait_instance> { - pub fn aux_dispatch>(d: D, aux: &D::Aux) { - d.dispatch(aux); + pub fn aux_dispatch>(d: D, aux: &D::Aux) -> $crate::dispatch::Result { + d.dispatch(aux) } - pub fn dispatch>(d: D) { - d.dispatch(); + pub fn dispatch>(d: D) -> $crate::dispatch::Result { + d.dispatch() } } } @@ -176,19 +179,20 @@ macro_rules! __decl_dispatch_module_without_aux { $param_name:ident : $param:ty ),* ) + -> $result:ty = $id:expr ; )* ) => { __decl_dispatch_module_common! { impl for $mod_type<$trait_instance: $trait_name>; pub enum $call_type; - $( fn $fn_name( $( $param_name : $param ),* ) = $id ; )* + $( fn $fn_name( $( $param_name : $param ),* ) -> $result = $id ; )* } impl<$trait_instance: $trait_name> $crate::dispatch::Dispatchable for $call_type<$trait_instance> { type Trait = $trait_instance; - fn dispatch(self) { + fn dispatch(self) -> $crate::dispatch::Result { match self { $( $call_type::$fn_name( $( $param_name ),* ) => @@ -218,20 +222,21 @@ macro_rules! __decl_dispatch_module_with_aux { , $param_name:ident : $param:ty )* ) + -> $result:ty = $id:expr ; )* ) => { __decl_dispatch_module_common! { impl for $mod_type<$trait_instance: $trait_name>; pub enum $call_type; - $( fn $fn_name( $( $param_name : $param ),* ) = $id ; )* + $( fn $fn_name( $( $param_name : $param ),* ) -> $result = $id ; )* } impl<$trait_instance: $trait_name> $crate::dispatch::AuxDispatchable for $call_type<$trait_instance> { type Trait = $trait_instance; type Aux = $aux_type; - fn dispatch(self, aux: &Self::Aux) { + fn dispatch(self, aux: &Self::Aux) -> $crate::dispatch::Result { match self { $( $call_type::$fn_name( $( $param_name ),* ) => @@ -261,6 +266,7 @@ macro_rules! __decl_dispatch_module_common { $param_name:ident : $param:ty ),* ) + -> $result:ty = $id:expr ; )* ) => { @@ -320,7 +326,7 @@ macro_rules! __decl_dispatch_module_common { impl<$trait_instance: $trait_name> $crate::dispatch::fmt::Debug for $call_type<$trait_instance> { - fn fmt(&self, f: &mut $crate::dispatch::fmt::Formatter) -> Result<(), $crate::dispatch::fmt::Error> { + fn fmt(&self, f: &mut $crate::dispatch::fmt::Formatter) -> $crate::dispatch::result::Result<(), $crate::dispatch::fmt::Error> { match *self { $( $call_type::$fn_name( $( ref $param_name ),* ) => @@ -408,7 +414,7 @@ macro_rules! impl_outer_dispatch { impl $crate::dispatch::AuxDispatchable for $call_type { type Aux = $aux; type Trait = $call_type; - fn dispatch(self, aux: &$aux) { + fn dispatch(self, aux: &$aux) -> $crate::dispatch::Result { match self { $( $call_type::$camelcase(call) => call.dispatch(&aux), @@ -448,7 +454,7 @@ macro_rules! impl_outer_dispatch { impl_outer_dispatch_common! { $call_type, $($camelcase = $id,)* } impl $crate::dispatch::Dispatchable for $call_type { type Trait = $call_type; - fn dispatch(self) { + fn dispatch(self) -> $crate::dispatch::Result { match self { $( $call_type::$camelcase(call) => call.dispatch(), diff --git a/substrate/runtime-support/src/lib.rs b/substrate/runtime-support/src/lib.rs index 514869a1b2798..5de6deb9e12cb 100644 --- a/substrate/runtime-support/src/lib.rs +++ b/substrate/runtime-support/src/lib.rs @@ -46,11 +46,11 @@ pub use self::hashable::Hashable; pub use self::dispatch::{Parameter, Dispatchable, Callable, AuxDispatchable, AuxCallable, IsSubType, IsAuxSubType}; pub use runtime_io::print; + #[macro_export] macro_rules! fail { ( $y:expr ) => {{ - $crate::print($y); - return; + return Err($y); }} } @@ -60,46 +60,31 @@ macro_rules! ensure { if !$x { fail!($y); } - }}; - ($x:expr) => {{ - if !$x { - $crate::print("Bailing! Cannot ensure: "); - $crate::print(stringify!($x)); - return; - } }} } #[macro_export] -macro_rules! ensure_unwrap { - ($x:expr, $y: expr) => { - if let Some(v) = $x { - v - } else { - fail!{$y} - } +#[cfg(feature = "std")] +macro_rules! assert_noop { + ( $x:expr , $y:expr ) => { + let h = runtime_io::storage_root(); + assert_err!($x, $y); + assert_eq!(h, runtime_io::storage_root()); } } #[macro_export] -macro_rules! ensure_unwrap_err { - ($x:expr, $y: expr) => { - if let Err(v) = $x { - v - } else { - fail!{$y} - } +#[cfg(feature = "std")] +macro_rules! assert_err { + ( $x:expr , $y:expr ) => { + assert_eq!($x, Err($y)); } } #[macro_export] #[cfg(feature = "std")] -macro_rules! assert_noop { - ( $( $x:tt )* ) => { - let h = runtime_io::storage_root(); - { - $( $x )* - } - assert_eq!(h, runtime_io::storage_root()); +macro_rules! assert_ok { + ( $x:expr ) => { + assert!($x.is_ok()); } } diff --git a/substrate/runtime/consensus/src/lib.rs b/substrate/runtime/consensus/src/lib.rs index a32c68adbc0c9..776238db7b804 100644 --- a/substrate/runtime/consensus/src/lib.rs +++ b/substrate/runtime/consensus/src/lib.rs @@ -33,6 +33,7 @@ extern crate substrate_primitives; use rstd::prelude::*; use runtime_support::{storage, Parameter}; +use runtime_support::dispatch::Result; use runtime_support::storage::unhashed::StorageVec; use primitives::traits::RefInto; use substrate_primitives::bft::MisbehaviorReport; @@ -59,11 +60,11 @@ pub trait Trait: system::Trait { decl_module! { pub struct Module; pub enum Call where aux: T::PublicAux { - fn report_misbehavior(aux, report: MisbehaviorReport) = 0; + fn report_misbehavior(aux, report: MisbehaviorReport) -> Result = 0; } pub enum PrivCall { - fn set_code(new: Vec) = 0; - fn set_storage(items: Vec) = 1; + fn set_code(new: Vec) -> Result = 0; + fn set_storage(items: Vec) -> Result = 1; } } @@ -74,20 +75,23 @@ impl Module { } /// Set the new code. - fn set_code(new: Vec) { + fn set_code(new: Vec) -> Result { storage::unhashed::put_raw(CODE, &new); + Ok(()) } /// Set some items of storage. - fn set_storage(items: Vec) { + fn set_storage(items: Vec) -> Result { for i in &items { storage::unhashed::put_raw(&i.0, &i.1); } + Ok(()) } /// Report some misbehaviour. - fn report_misbehavior(_aux: &T::PublicAux, _report: MisbehaviorReport) { + fn report_misbehavior(_aux: &T::PublicAux, _report: MisbehaviorReport) -> Result { // TODO. + Ok(()) } /// Set the current set of authorities' session keys. diff --git a/substrate/runtime/council/src/lib.rs b/substrate/runtime/council/src/lib.rs index 0a7531e076894..024ee93b81de3 100644 --- a/substrate/runtime/council/src/lib.rs +++ b/substrate/runtime/council/src/lib.rs @@ -37,6 +37,7 @@ extern crate substrate_runtime_system as system; use rstd::prelude::*; use primitives::traits::{Zero, One, RefInto, As}; use runtime_support::{StorageValue, StorageMap}; +use runtime_support::dispatch::Result; pub mod voting; @@ -101,17 +102,17 @@ pub trait Trait: democracy::Trait {} decl_module! { pub struct Module; pub enum Call where aux: T::PublicAux { - fn set_approvals(aux, votes: Vec, index: VoteIndex) = 0; - fn reap_inactive_voter(aux, signed_index: u32, who: T::AccountId, who_index: u32, assumed_vote_index: VoteIndex) = 1; - fn retract_voter(aux, index: u32) = 2; - fn submit_candidacy(aux, slot: u32) = 3; - fn present_winner(aux, candidate: T::AccountId, total: T::Balance, index: VoteIndex) = 4; + fn set_approvals(aux, votes: Vec, index: VoteIndex) -> Result = 0; + fn reap_inactive_voter(aux, signed_index: u32, who: T::AccountId, who_index: u32, assumed_vote_index: VoteIndex) -> Result = 1; + fn retract_voter(aux, index: u32) -> Result = 2; + fn submit_candidacy(aux, slot: u32) -> Result = 3; + fn present_winner(aux, candidate: T::AccountId, total: T::Balance, index: VoteIndex) -> Result = 4; } pub enum PrivCall { - fn set_desired_seats(count: u32) = 0; - fn remove_member(who: T::AccountId) = 1; - fn set_presentation_duration(count: T::BlockNumber) = 2; - fn set_term_duration(count: T::BlockNumber) = 3; + fn set_desired_seats(count: u32) -> Result = 0; + fn remove_member(who: T::AccountId) -> Result = 1; + fn set_presentation_duration(count: T::BlockNumber) -> Result = 2; + fn set_term_duration(count: T::BlockNumber) -> Result = 3; } } @@ -223,12 +224,14 @@ impl Module { /// Set candidate approvals. Approval slots stay valid as long as candidates in those slots /// are registered. - fn set_approvals(aux: &T::PublicAux, votes: Vec, index: VoteIndex) { - ensure!(!Self::presentation_active()); - ensure!(index == Self::vote_index()); + fn set_approvals(aux: &T::PublicAux, votes: Vec, index: VoteIndex) -> Result { + ensure!(!Self::presentation_active(), "no approval changes during presentation period"); + ensure!(index == Self::vote_index(), "incorrect vote index"); if !>::exists(aux.ref_into()) { // not yet a voter - deduct bond. - >::reserve_balance(aux.ref_into(), Self::voting_bond()); + // NOTE: this must be the last potential bailer, since it changes state. + >::reserve_balance(aux.ref_into(), Self::voting_bond())?; + >::put({ let mut v = Self::voters(); v.push(aux.ref_into().clone()); @@ -237,6 +240,7 @@ impl Module { } >::insert(aux.ref_into().clone(), votes); >::insert(aux.ref_into(), index); + Ok(()) } /// Remove a voter. For it not to be a bond-consuming no-op, all approved candidate indices @@ -244,10 +248,16 @@ impl Module { /// the voter gave their last approval set. /// /// May be called by anyone. Returns the voter deposit to `signed`. - fn reap_inactive_voter(aux: &T::PublicAux, signed_index: u32, who: T::AccountId, who_index: u32, assumed_vote_index: VoteIndex) { + fn reap_inactive_voter( + aux: &T::PublicAux, + signed_index: u32, + who: T::AccountId, + who_index: u32, + assumed_vote_index: VoteIndex + ) -> Result { ensure!(!Self::presentation_active(), "cannot reap during presentation period"); ensure!(Self::voter_last_active(aux.ref_into()).is_some(), "reaper must be a voter"); - let last_active = ensure_unwrap!(Self::voter_last_active(&who), "target for inactivity cleanup must be active"); + let last_active = Self::voter_last_active(&who).ok_or("target for inactivity cleanup must be active")?; ensure!(assumed_vote_index == Self::vote_index(), "vote index not current"); ensure!(last_active < assumed_vote_index - Self::inactivity_grace_period(), "cannot reap during grace perid"); let voters = Self::voters(); @@ -272,31 +282,31 @@ impl Module { voters ); if valid { - >::transfer_reserved_balance(&who, aux.ref_into(), Self::voting_bond()); + >::transfer_reserved_balance(&who, aux.ref_into(), Self::voting_bond()) } else { - >::slash_reserved(aux.ref_into(), Self::voting_bond()); + >::slash_reserved(aux.ref_into(), Self::voting_bond()) } } /// Remove a voter. All votes are cancelled and the voter deposit is returned. - fn retract_voter(aux: &T::PublicAux, index: u32) { + fn retract_voter(aux: &T::PublicAux, index: u32) -> Result { ensure!(!Self::presentation_active(), "cannot retract when presenting"); ensure!(>::exists(aux.ref_into()), "cannot retract non-voter"); let voters = Self::voters(); let index = index as usize; ensure!(index < voters.len(), "retraction index invalid"); ensure!(&voters[index] == aux.ref_into(), "retraction index mismatch"); + Self::remove_voter(aux.ref_into(), index, voters); >::unreserve_balance(aux.ref_into(), Self::voting_bond()); + Ok(()) } /// Submit oneself for candidacy. /// /// Account must have enough transferrable funds in it to pay the bond. - fn submit_candidacy(aux: &T::PublicAux, slot: u32) { + fn submit_candidacy(aux: &T::PublicAux, slot: u32) -> Result { ensure!(!Self::is_a_candidate(aux.ref_into()), "duplicate candidate submission"); - ensure!(>::can_deduct_unbonded(aux.ref_into(), Self::candidacy_bond()), "candidate has not enough funds"); - let slot = slot as usize; let count = Self::candidate_count() as usize; let candidates = Self::candidates(); @@ -305,8 +315,10 @@ impl Module { (slot < candidates.len() && candidates[slot] == T::AccountId::default()), "invalid candidate slot" ); + // NOTE: This must be last as it has side-effects. + >::deduct_unbonded(aux.ref_into(), Self::candidacy_bond()) + .map_err(|_| "candidate has not enough funds")?; - >::deduct_unbonded(aux.ref_into(), Self::candidacy_bond()); let mut candidates = candidates; if slot == candidates.len() { candidates.push(aux.ref_into().clone()); @@ -316,20 +328,26 @@ impl Module { >::put(candidates); >::put(count as u32 + 1); >::insert(aux.ref_into(), (Self::vote_index(), slot as u32)); + Ok(()) } /// Claim that `signed` is one of the top Self::carry_count() + current_vote().1 candidates. /// Only works if the `block_number >= current_vote().0` and `< current_vote().0 + presentation_duration()`` /// `signed` should have at least - fn present_winner(aux: &T::PublicAux, candidate: T::AccountId, total: T::Balance, index: VoteIndex) { + fn present_winner( + aux: &T::PublicAux, + candidate: T::AccountId, + total: T::Balance, + index: VoteIndex + ) -> Result { ensure!(index == Self::vote_index(), "index not current"); - let (_, _, expiring) = ensure_unwrap!(Self::next_finalise(), "cannot present outside of presentation period"); + let (_, _, expiring) = Self::next_finalise().ok_or("cannot present outside of presentation period")?; let stakes = Self::snapshoted_stakes(); let voters = Self::voters(); let bad_presentation_punishment = Self::present_slash_per_voter() * T::Balance::sa(voters.len()); ensure!(>::can_slash(aux.ref_into(), bad_presentation_punishment), "presenter must have sufficient slashable funds"); - let mut leaderboard = ensure_unwrap!(Self::leaderboard(), "leaderboard must exist while present phase active"); + let mut leaderboard = Self::leaderboard().ok_or("leaderboard must exist while present phase active")?; ensure!(total > leaderboard[0].0, "candidate not worthy of leaderboard"); if let Some(p) = Self::active_council().iter().position(|&(ref c, _)| c == &candidate) { @@ -337,7 +355,7 @@ impl Module { } let (registered_since, candidate_index): (VoteIndex, u32) = - ensure_unwrap!(Self::candidate_reg_info(&candidate), "presented candidate must be current"); + Self::candidate_reg_info(&candidate).ok_or("presented candidate must be current")?; let actual_total = voters.iter() .zip(stakes.iter()) .filter_map(|(voter, stake)| @@ -355,44 +373,51 @@ impl Module { leaderboard.sort_by_key(|&(t, _)| t); >::put(leaderboard); } else { - >::slash(aux.ref_into(), bad_presentation_punishment); + // we can rest assured it will be Ok since we checked `can_slash` earlier; still + // better safe than sorry. + let _ = >::slash(aux.ref_into(), bad_presentation_punishment); } + Ok(()) } /// Set the desired member count; if lower than the current count, then seats will not be up /// election when they expire. If more, then a new vote will be started if one is not already /// in progress. - fn set_desired_seats(count: u32) { + fn set_desired_seats(count: u32) -> Result { >::put(count); + Ok(()) } /// Remove a particular member. A tally will happen instantly (if not already in a presentation /// period) to fill the seat if removal means that the desired members are not met. /// This is effective immediately. - fn remove_member(who: T::AccountId) { + fn remove_member(who: T::AccountId) -> Result { let new_council: Vec<(T::AccountId, T::BlockNumber)> = Self::active_council() .into_iter() .filter(|i| i.0 != who) .collect(); >::put(new_council); + Ok(()) } /// Set the presentation duration. If there is current a vote being presented for, will /// invoke `finalise_vote`. - fn set_presentation_duration(count: T::BlockNumber) { + fn set_presentation_duration(count: T::BlockNumber) -> Result { >::put(count); + Ok(()) } /// Set the presentation duration. If there is current a vote being presented for, will /// invoke `finalise_vote`. - fn set_term_duration(count: T::BlockNumber) { + fn set_term_duration(count: T::BlockNumber) -> Result { >::put(count); + Ok(()) } // private /// Check there's nothing to do this block - fn end_block(block_number: T::BlockNumber) { + fn end_block(block_number: T::BlockNumber) -> Result { if (block_number % Self::voting_period()).is_zero() { if let Some(number) = Self::next_tally() { if block_number == number { @@ -402,9 +427,10 @@ impl Module { } if let Some((number, _, _)) = Self::next_finalise() { if block_number == number { - Self::finalise_tally(); + Self::finalise_tally()? } } + Ok(()) } /// Remove a voter from the system. Trusts that Self::voters()[index] != voter. @@ -438,10 +464,10 @@ impl Module { /// candidates in their place. If the total council members is less than the desired membership /// a new vote is started. /// Clears all presented candidates, returning the bond of the elected ones. - fn finalise_tally() { + fn finalise_tally() -> Result { >::kill(); let (_, coming, expiring): (T::BlockNumber, u32, Vec) = - ensure_unwrap!(>::take(), "finalise can only be called after a tally is started."); + >::take().ok_or("finalise can only be called after a tally is started.")?; let leaderboard: Vec<(T::Balance, T::AccountId)> = >::take().unwrap_or_default(); let new_expiry = >::block_number() + Self::term_duration(); @@ -495,6 +521,7 @@ impl Module { >::put(new_candidates); >::put(count); >::put(Self::vote_index() + 1); + Ok(()) } } @@ -702,14 +729,14 @@ mod tests { assert_eq!(Council::is_a_candidate(&1), false); assert_eq!(Council::is_a_candidate(&2), false); - Council::submit_candidacy(&1, 0); + assert_ok!(Council::submit_candidacy(&1, 0)); assert_eq!(Council::candidates(), vec![1]); assert_eq!(Council::candidate_reg_info(1), Some((0, 0))); assert_eq!(Council::candidate_reg_info(2), None); assert_eq!(Council::is_a_candidate(&1), true); assert_eq!(Council::is_a_candidate(&2), false); - Council::submit_candidacy(&2, 1); + assert_ok!(Council::submit_candidacy(&2, 1)); assert_eq!(Council::candidates(), vec![1, 2]); assert_eq!(Council::candidate_reg_info(1), Some((0, 0))); assert_eq!(Council::candidate_reg_info(2), Some((0, 1))); @@ -736,10 +763,10 @@ mod tests { System::set_block_number(1); assert_eq!(Council::candidates(), vec![0, 0, 1]); - Council::submit_candidacy(&2, 1); + assert_ok!(Council::submit_candidacy(&2, 1)); assert_eq!(Council::candidates(), vec![0, 2, 1]); - Council::submit_candidacy(&3, 0); + assert_ok!(Council::submit_candidacy(&3, 0)); assert_eq!(Council::candidates(), vec![3, 2, 1]); }); } @@ -752,10 +779,10 @@ mod tests { System::set_block_number(1); assert_eq!(Council::candidates(), vec![0, 0, 1]); - Council::submit_candidacy(&2, 0); + assert_ok!(Council::submit_candidacy(&2, 0)); assert_eq!(Council::candidates(), vec![2, 0, 1]); - Council::submit_candidacy(&3, 1); + assert_ok!(Council::submit_candidacy(&3, 1)); assert_eq!(Council::candidates(), vec![2, 3, 1]); }); } @@ -764,7 +791,7 @@ mod tests { fn candidate_submission_not_using_free_slot_should_not_work() { with_externalities(&mut new_test_ext_with_candidate_holes(), || { System::set_block_number(1); - assert_noop!{Council::submit_candidacy(&4, 3)}; // gives "invalid candidate slot" + assert_noop!(Council::submit_candidacy(&4, 3), "invalid candidate slot"); }); } @@ -773,7 +800,7 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); assert_eq!(Council::candidates(), Vec::::new()); - assert_noop!{Council::submit_candidacy(&1, 1)}; // gives "invalid candidate slot" + assert_noop!(Council::submit_candidacy(&1, 1), "invalid candidate slot"); }); } @@ -782,9 +809,9 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); assert_eq!(Council::candidates(), Vec::::new()); - Council::submit_candidacy(&1, 0); + assert_ok!(Council::submit_candidacy(&1, 0)); assert_eq!(Council::candidates(), vec![1]); - assert_noop!{Council::submit_candidacy(&2, 0)}; // gives "invalid candidate slot" + assert_noop!(Council::submit_candidacy(&2, 0), "invalid candidate slot"); }); } @@ -793,9 +820,9 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); assert_eq!(Council::candidates(), Vec::::new()); - Council::submit_candidacy(&1, 0); + assert_ok!(Council::submit_candidacy(&1, 0)); assert_eq!(Council::candidates(), vec![1]); - assert_noop!{Council::submit_candidacy(&1, 1)}; // gives "duplicate candidate submission" + assert_noop!(Council::submit_candidacy(&1, 1), "duplicate candidate submission"); }); } @@ -804,7 +831,7 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); assert_eq!(Council::candidates(), Vec::::new()); - assert_noop!{Council::submit_candidacy(&7, 0)}; // gives "candidate has not enough funds" + assert_noop!(Council::submit_candidacy(&7, 0), "candidate has not enough funds"); }); } @@ -813,20 +840,20 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); - Council::submit_candidacy(&5, 0); + assert_ok!(Council::submit_candidacy(&5, 0)); - Council::set_approvals(&1, vec![true], 0); - Council::set_approvals(&4, vec![true], 0); + assert_ok!(Council::set_approvals(&1, vec![true], 0)); + assert_ok!(Council::set_approvals(&4, vec![true], 0)); assert_eq!(Council::approvals_of(1), vec![true]); assert_eq!(Council::approvals_of(4), vec![true]); assert_eq!(Council::voters(), vec![1, 4]); - Council::submit_candidacy(&2, 1); - Council::submit_candidacy(&3, 2); + assert_ok!(Council::submit_candidacy(&2, 1)); + assert_ok!(Council::submit_candidacy(&3, 2)); - Council::set_approvals(&2, vec![false, true, true], 0); - Council::set_approvals(&3, vec![false, true, true], 0); + assert_ok!(Council::set_approvals(&2, vec![false, true, true], 0)); + assert_ok!(Council::set_approvals(&3, vec![false, true, true], 0)); assert_eq!(Council::approvals_of(1), vec![true]); assert_eq!(Council::approvals_of(4), vec![true]); @@ -842,14 +869,14 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); - Council::submit_candidacy(&5, 0); - Council::set_approvals(&4, vec![true], 0); + assert_ok!(Council::submit_candidacy(&5, 0)); + assert_ok!(Council::set_approvals(&4, vec![true], 0)); assert_eq!(Council::approvals_of(4), vec![true]); - Council::submit_candidacy(&2, 1); - Council::submit_candidacy(&3, 2); - Council::set_approvals(&4, vec![true, false, true], 0); + assert_ok!(Council::submit_candidacy(&2, 1)); + assert_ok!(Council::submit_candidacy(&3, 2)); + assert_ok!(Council::set_approvals(&4, vec![true, false, true], 0)); assert_eq!(Council::approvals_of(4), vec![true, false, true]); }); @@ -860,14 +887,14 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); - Council::submit_candidacy(&5, 0); - Council::submit_candidacy(&2, 1); - Council::submit_candidacy(&3, 2); + assert_ok!(Council::submit_candidacy(&5, 0)); + assert_ok!(Council::submit_candidacy(&2, 1)); + assert_ok!(Council::submit_candidacy(&3, 2)); - Council::set_approvals(&1, vec![true], 0); - Council::set_approvals(&2, vec![false, true, true], 0); - Council::set_approvals(&3, vec![false, true, true], 0); - Council::set_approvals(&4, vec![true, false, true], 0); + assert_ok!(Council::set_approvals(&1, vec![true], 0)); + assert_ok!(Council::set_approvals(&2, vec![false, true, true], 0)); + assert_ok!(Council::set_approvals(&3, vec![false, true, true], 0)); + assert_ok!(Council::set_approvals(&4, vec![true, false, true], 0)); assert_eq!(Council::voters(), vec![1, 2, 3, 4]); assert_eq!(Council::approvals_of(1), vec![true]); @@ -875,7 +902,7 @@ mod tests { assert_eq!(Council::approvals_of(3), vec![false, true, true]); assert_eq!(Council::approvals_of(4), vec![true, false, true]); - Council::retract_voter(&1, 0); + assert_ok!(Council::retract_voter(&1, 0)); assert_eq!(Council::voters(), vec![4, 2, 3]); assert_eq!(Council::approvals_of(1), Vec::::new()); @@ -883,7 +910,7 @@ mod tests { assert_eq!(Council::approvals_of(3), vec![false, true, true]); assert_eq!(Council::approvals_of(4), vec![true, false, true]); - Council::retract_voter(&2, 1); + assert_ok!(Council::retract_voter(&2, 1)); assert_eq!(Council::voters(), vec![4, 3]); assert_eq!(Council::approvals_of(1), Vec::::new()); @@ -891,7 +918,7 @@ mod tests { assert_eq!(Council::approvals_of(3), vec![false, true, true]); assert_eq!(Council::approvals_of(4), vec![true, false, true]); - Council::retract_voter(&3, 1); + assert_ok!(Council::retract_voter(&3, 1)); assert_eq!(Council::voters(), vec![4]); assert_eq!(Council::approvals_of(1), Vec::::new()); @@ -905,11 +932,11 @@ mod tests { fn invalid_retraction_index_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); - Council::submit_candidacy(&3, 0); - Council::set_approvals(&1, vec![true], 0); - Council::set_approvals(&2, vec![true], 0); + assert_ok!(Council::submit_candidacy(&3, 0)); + assert_ok!(Council::set_approvals(&1, vec![true], 0)); + assert_ok!(Council::set_approvals(&2, vec![true], 0)); assert_eq!(Council::voters(), vec![1, 2]); - assert_noop!{Council::retract_voter(&1, 1)}; // gives "retraction index mismatch" + assert_noop!(Council::retract_voter(&1, 1), "retraction index mismatch"); }); } @@ -917,9 +944,9 @@ mod tests { fn overflow_retraction_index_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); - Council::submit_candidacy(&3, 0); - Council::set_approvals(&1, vec![true], 0); - assert_noop!{Council::retract_voter(&1, 1)}; // gives "retraction index invalid" + assert_ok!(Council::submit_candidacy(&3, 0)); + assert_ok!(Council::set_approvals(&1, vec![true], 0)); + assert_noop!(Council::retract_voter(&1, 1), "retraction index invalid"); }); } @@ -927,9 +954,9 @@ mod tests { fn non_voter_retraction_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); - Council::submit_candidacy(&3, 0); - Council::set_approvals(&1, vec![true], 0); - assert_noop!{Council::retract_voter(&2, 0)}; // gives "cannot retract non-voter" + assert_ok!(Council::submit_candidacy(&3, 0)); + assert_ok!(Council::set_approvals(&1, vec![true], 0)); + assert_noop!(Council::retract_voter(&2, 0), "cannot retract non-voter"); }); } @@ -939,19 +966,19 @@ mod tests { System::set_block_number(4); assert!(!Council::presentation_active()); - Council::submit_candidacy(&2, 0); - Council::submit_candidacy(&5, 1); - Council::set_approvals(&2, vec![true, false], 0); - Council::set_approvals(&5, vec![false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::submit_candidacy(&5, 1)); + assert_ok!(Council::set_approvals(&2, vec![true, false], 0)); + assert_ok!(Council::set_approvals(&5, vec![false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); assert!(Council::presentation_active()); - Council::present_winner(&4, 2, 11, 0); - Council::present_winner(&4, 5, 41, 0); + assert_ok!(Council::present_winner(&4, 2, 11, 0)); + assert_ok!(Council::present_winner(&4, 5, 41, 0)); assert_eq!(Council::leaderboard(), Some(vec![(0, 0), (0, 0), (11, 2), (41, 5)])); - Council::end_block(System::block_number()); + assert_ok!(Council::end_block(System::block_number())); assert!(!Council::presentation_active()); assert_eq!(Council::active_council(), vec![(5, 11), (2, 11)]); @@ -970,17 +997,17 @@ mod tests { assert!(Staking::can_slash(&4, 10)); System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::submit_candidacy(&5, 1); - Council::set_approvals(&2, vec![true, false], 0); - Council::set_approvals(&5, vec![false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::submit_candidacy(&5, 1)); + assert_ok!(Council::set_approvals(&2, vec![true, false], 0)); + assert_ok!(Council::set_approvals(&5, vec![false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 11, 0); - Council::present_winner(&4, 5, 41, 0); - Council::present_winner(&4, 5, 41, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 11, 0)); + assert_ok!(Council::present_winner(&4, 5, 41, 0)); + assert_ok!(Council::present_winner(&4, 5, 41, 0)); + assert_ok!(Council::end_block(System::block_number())); assert_eq!(Council::active_council(), vec![(5, 11), (2, 11)]); assert_eq!(Staking::balance(&4), 38); @@ -991,28 +1018,28 @@ mod tests { fn retracting_inactive_voter_should_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::set_approvals(&2, vec![true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::set_approvals(&2, vec![true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 11, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 11, 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - Council::submit_candidacy(&5, 0); - Council::set_approvals(&5, vec![true], 1); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&5, 0)); + assert_ok!(Council::set_approvals(&5, vec![true], 1)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - Council::present_winner(&4, 5, 41, 1); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 5, 41, 1)); + assert_ok!(Council::end_block(System::block_number())); - Council::reap_inactive_voter(&5, + assert_ok!(Council::reap_inactive_voter(&5, Council::voters().iter().position(|&i| i == 5).unwrap() as u32, 2, Council::voters().iter().position(|&i| i == 2).unwrap() as u32, 2 - ); + )); assert_eq!(Council::voters(), vec![5]); assert_eq!(Council::approvals_of(2).len(), 0); @@ -1025,21 +1052,21 @@ mod tests { fn presenting_for_double_election_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::set_approvals(&2, vec![true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::set_approvals(&2, vec![true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 11, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 11, 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - Council::submit_candidacy(&2, 0); - Council::set_approvals(&2, vec![true], 1); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::set_approvals(&2, vec![true], 1)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - assert_noop!{Council::present_winner(&4, 2, 11, 1)}; // gives "candidate must not form a duplicated member if elected" + assert_noop!(Council::present_winner(&4, 2, 11, 1), "candidate must not form a duplicated member if elected"); }); } @@ -1047,31 +1074,31 @@ mod tests { fn retracting_inactive_voter_with_other_candidates_in_slots_should_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::set_approvals(&2, vec![true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::set_approvals(&2, vec![true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 11, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 11, 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - Council::submit_candidacy(&5, 0); - Council::set_approvals(&5, vec![true], 1); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&5, 0)); + assert_ok!(Council::set_approvals(&5, vec![true], 1)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - Council::present_winner(&4, 5, 41, 1); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 5, 41, 1)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(11); - Council::submit_candidacy(&1, 0); + assert_ok!(Council::submit_candidacy(&1, 0)); - Council::reap_inactive_voter(&5, + assert_ok!(Council::reap_inactive_voter(&5, Council::voters().iter().position(|&i| i == 5).unwrap() as u32, 2, Council::voters().iter().position(|&i| i == 2).unwrap() as u32, 2 - ); + )); assert_eq!(Council::voters(), vec![5]); assert_eq!(Council::approvals_of(2).len(), 0); @@ -1084,28 +1111,28 @@ mod tests { fn retracting_inactive_voter_with_bad_reporter_index_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::set_approvals(&2, vec![true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::set_approvals(&2, vec![true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 8, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 8, 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - Council::submit_candidacy(&5, 0); - Council::set_approvals(&5, vec![true], 1); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&5, 0)); + assert_ok!(Council::set_approvals(&5, vec![true], 1)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - Council::present_winner(&4, 5, 38, 1); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 5, 38, 1)); + assert_ok!(Council::end_block(System::block_number())); - assert_noop!{Council::reap_inactive_voter(&2, + assert_noop!(Council::reap_inactive_voter(&2, 42, 2, Council::voters().iter().position(|&i| i == 2).unwrap() as u32, 2 - )}; // given "bad reporter index" + ), "bad reporter index"); }); } @@ -1113,28 +1140,28 @@ mod tests { fn retracting_inactive_voter_with_bad_target_index_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::set_approvals(&2, vec![true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::set_approvals(&2, vec![true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 8, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 8, 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - Council::submit_candidacy(&5, 0); - Council::set_approvals(&5, vec![true], 1); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&5, 0)); + assert_ok!(Council::set_approvals(&5, vec![true], 1)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - Council::present_winner(&4, 5, 38, 1); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 5, 38, 1)); + assert_ok!(Council::end_block(System::block_number())); - assert_noop!{Council::reap_inactive_voter(&2, + assert_noop!(Council::reap_inactive_voter(&2, Council::voters().iter().position(|&i| i == 2).unwrap() as u32, 2, 42, 2 - )}; // gives "bad target index" + ), "bad target index"); }); } @@ -1142,37 +1169,37 @@ mod tests { fn attempting_to_retract_active_voter_should_slash_reporter() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::submit_candidacy(&3, 1); - Council::submit_candidacy(&4, 2); - Council::submit_candidacy(&5, 3); - Council::set_approvals(&2, vec![true, false, false, false], 0); - Council::set_approvals(&3, vec![false, true, false, false], 0); - Council::set_approvals(&4, vec![false, false, true, false], 0); - Council::set_approvals(&5, vec![false, false, false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::submit_candidacy(&3, 1)); + assert_ok!(Council::submit_candidacy(&4, 2)); + assert_ok!(Council::submit_candidacy(&5, 3)); + assert_ok!(Council::set_approvals(&2, vec![true, false, false, false], 0)); + assert_ok!(Council::set_approvals(&3, vec![false, true, false, false], 0)); + assert_ok!(Council::set_approvals(&4, vec![false, false, true, false], 0)); + assert_ok!(Council::set_approvals(&5, vec![false, false, false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 11, 0); - Council::present_winner(&4, 3, 21, 0); - Council::present_winner(&4, 4, 31, 0); - Council::present_winner(&4, 5, 41, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 11, 0)); + assert_ok!(Council::present_winner(&4, 3, 21, 0)); + assert_ok!(Council::present_winner(&4, 4, 31, 0)); + assert_ok!(Council::present_winner(&4, 5, 41, 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - Council::set_desired_seats(3); - Council::end_block(System::block_number()); + assert_ok!(Council::set_desired_seats(3)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - Council::present_winner(&4, 2, 11, 1); - Council::present_winner(&4, 3, 21, 1); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 11, 1)); + assert_ok!(Council::present_winner(&4, 3, 21, 1)); + assert_ok!(Council::end_block(System::block_number())); - Council::reap_inactive_voter(&4, + assert_ok!(Council::reap_inactive_voter(&4, Council::voters().iter().position(|&i| i == 4).unwrap() as u32, 2, Council::voters().iter().position(|&i| i == 2).unwrap() as u32, 2 - ); + )); assert_eq!(Council::voters(), vec![2, 3, 5]); assert_eq!(Council::approvals_of(4).len(), 0); @@ -1184,28 +1211,28 @@ mod tests { fn attempting_to_retract_inactive_voter_by_nonvoter_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::set_approvals(&2, vec![true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::set_approvals(&2, vec![true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 11, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 2, 11, 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - Council::submit_candidacy(&5, 0); - Council::set_approvals(&5, vec![true], 1); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&5, 0)); + assert_ok!(Council::set_approvals(&5, vec![true], 1)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - Council::present_winner(&4, 5, 41, 1); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 5, 41, 1)); + assert_ok!(Council::end_block(System::block_number())); - assert_noop!{Council::reap_inactive_voter(&4, + assert_noop!(Council::reap_inactive_voter(&4, 0, 2, Council::voters().iter().position(|&i| i == 2).unwrap() as u32, 2 - )}; // gives "reaper must be a voter" + ), "reaper must be a voter"); }); } @@ -1213,24 +1240,24 @@ mod tests { fn presenting_loser_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&1, 0); - Council::set_approvals(&6, vec![true], 0); - Council::submit_candidacy(&2, 1); - Council::set_approvals(&2, vec![false, true], 0); - Council::submit_candidacy(&3, 2); - Council::set_approvals(&3, vec![false, false, true], 0); - Council::submit_candidacy(&4, 3); - Council::set_approvals(&4, vec![false, false, false, true], 0); - Council::submit_candidacy(&5, 4); - Council::set_approvals(&5, vec![false, false, false, false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&1, 0)); + assert_ok!(Council::set_approvals(&6, vec![true], 0)); + assert_ok!(Council::submit_candidacy(&2, 1)); + assert_ok!(Council::set_approvals(&2, vec![false, true], 0)); + assert_ok!(Council::submit_candidacy(&3, 2)); + assert_ok!(Council::set_approvals(&3, vec![false, false, true], 0)); + assert_ok!(Council::submit_candidacy(&4, 3)); + assert_ok!(Council::set_approvals(&4, vec![false, false, false, true], 0)); + assert_ok!(Council::submit_candidacy(&5, 4)); + assert_ok!(Council::set_approvals(&5, vec![false, false, false, false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 1, 60, 0); - Council::present_winner(&4, 3, 21, 0); - Council::present_winner(&4, 4, 31, 0); - Council::present_winner(&4, 5, 41, 0); - assert_noop!{Council::present_winner(&4, 2, 11, 0)}; // gives "candidate not worthy of leaderboard" + assert_ok!(Council::present_winner(&4, 1, 60, 0)); + assert_ok!(Council::present_winner(&4, 3, 21, 0)); + assert_ok!(Council::present_winner(&4, 4, 31, 0)); + assert_ok!(Council::present_winner(&4, 5, 41, 0)); + assert_noop!(Council::present_winner(&4, 2, 11, 0), "candidate not worthy of leaderboard"); }); } @@ -1238,24 +1265,24 @@ mod tests { fn presenting_loser_first_should_not_matter() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&1, 0); - Council::set_approvals(&6, vec![true], 0); - Council::submit_candidacy(&2, 1); - Council::set_approvals(&2, vec![false, true], 0); - Council::submit_candidacy(&3, 2); - Council::set_approvals(&3, vec![false, false, true], 0); - Council::submit_candidacy(&4, 3); - Council::set_approvals(&4, vec![false, false, false, true], 0); - Council::submit_candidacy(&5, 4); - Council::set_approvals(&5, vec![false, false, false, false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&1, 0)); + assert_ok!(Council::set_approvals(&6, vec![true], 0)); + assert_ok!(Council::submit_candidacy(&2, 1)); + assert_ok!(Council::set_approvals(&2, vec![false, true], 0)); + assert_ok!(Council::submit_candidacy(&3, 2)); + assert_ok!(Council::set_approvals(&3, vec![false, false, true], 0)); + assert_ok!(Council::submit_candidacy(&4, 3)); + assert_ok!(Council::set_approvals(&4, vec![false, false, false, true], 0)); + assert_ok!(Council::submit_candidacy(&5, 4)); + assert_ok!(Council::set_approvals(&5, vec![false, false, false, false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 11, 0); - Council::present_winner(&4, 1, 60, 0); - Council::present_winner(&4, 3, 21, 0); - Council::present_winner(&4, 4, 31, 0); - Council::present_winner(&4, 5, 41, 0); + assert_ok!(Council::present_winner(&4, 2, 11, 0)); + assert_ok!(Council::present_winner(&4, 1, 60, 0)); + assert_ok!(Council::present_winner(&4, 3, 21, 0)); + assert_ok!(Council::present_winner(&4, 4, 31, 0)); + assert_ok!(Council::present_winner(&4, 5, 41, 0)); assert_eq!(Council::leaderboard(), Some(vec![ (21, 3), @@ -1271,7 +1298,7 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); assert!(!Council::presentation_active()); - assert_noop!{Council::present_winner(&5, 5, 1, 0)}; // gives "cannot present outside of presentation period" + assert_noop!(Council::present_winner(&5, 5, 1, 0), "cannot present outside of presentation period"); }); } @@ -1279,14 +1306,14 @@ mod tests { fn present_with_invalid_vote_index_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&2, 0); - Council::submit_candidacy(&5, 1); - Council::set_approvals(&2, vec![true, false], 0); - Council::set_approvals(&5, vec![false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::submit_candidacy(&5, 1)); + assert_ok!(Council::set_approvals(&2, vec![true, false], 0)); + assert_ok!(Council::set_approvals(&5, vec![false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - assert_noop!{Council::present_winner(&4, 2, 11, 1)}; // gives "index not current" + assert_noop!(Council::present_winner(&4, 2, 11, 1), "index not current"); }); } @@ -1296,15 +1323,15 @@ mod tests { System::set_block_number(4); assert!(!Council::presentation_active()); - Council::submit_candidacy(&1, 0); - Council::submit_candidacy(&5, 1); - Council::set_approvals(&2, vec![true, false], 0); - Council::set_approvals(&5, vec![false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&1, 0)); + assert_ok!(Council::submit_candidacy(&5, 1)); + assert_ok!(Council::set_approvals(&2, vec![true, false], 0)); + assert_ok!(Council::set_approvals(&5, vec![false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); assert_eq!(Staking::balance(&1), 1); - assert_noop!{Council::present_winner(&1, 1, 30, 0)}; // gives "presenter must have sufficient slashable funds" + assert_noop!(Council::present_winner(&1, 1, 30, 0), "presenter must have sufficient slashable funds"); }); } @@ -1315,14 +1342,14 @@ mod tests { assert!(!Council::presentation_active()); assert_eq!(Staking::balance(&4), 40); - Council::submit_candidacy(&2, 0); - Council::submit_candidacy(&5, 1); - Council::set_approvals(&2, vec![true, false], 0); - Council::set_approvals(&5, vec![false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&2, 0)); + assert_ok!(Council::submit_candidacy(&5, 1)); + assert_ok!(Council::set_approvals(&2, vec![true, false], 0)); + assert_ok!(Council::set_approvals(&5, vec![false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 2, 80, 0); + assert_ok!(Council::present_winner(&4, 2, 80, 0)); assert_eq!(Staking::balance(&4), 38); }); @@ -1334,31 +1361,31 @@ mod tests { System::set_block_number(4); assert!(!Council::presentation_active()); - Council::submit_candidacy(&1, 0); - Council::set_approvals(&6, vec![true], 0); - Council::submit_candidacy(&2, 1); - Council::set_approvals(&2, vec![false, true], 0); - Council::submit_candidacy(&3, 2); - Council::set_approvals(&3, vec![false, false, true], 0); - Council::submit_candidacy(&4, 3); - Council::set_approvals(&4, vec![false, false, false, true], 0); - Council::submit_candidacy(&5, 4); - Council::set_approvals(&5, vec![false, false, false, false, true], 0); + assert_ok!(Council::submit_candidacy(&1, 0)); + assert_ok!(Council::set_approvals(&6, vec![true], 0)); + assert_ok!(Council::submit_candidacy(&2, 1)); + assert_ok!(Council::set_approvals(&2, vec![false, true], 0)); + assert_ok!(Council::submit_candidacy(&3, 2)); + assert_ok!(Council::set_approvals(&3, vec![false, false, true], 0)); + assert_ok!(Council::submit_candidacy(&4, 3)); + assert_ok!(Council::set_approvals(&4, vec![false, false, false, true], 0)); + assert_ok!(Council::submit_candidacy(&5, 4)); + assert_ok!(Council::set_approvals(&5, vec![false, false, false, false, true], 0)); - Council::end_block(System::block_number()); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); assert!(Council::presentation_active()); - Council::present_winner(&4, 1, 60, 0); + assert_ok!(Council::present_winner(&4, 1, 60, 0)); assert_eq!(Council::leaderboard(), Some(vec![ (0, 0), (0, 0), (0, 0), (60, 1) ])); - Council::present_winner(&4, 3, 21, 0); - Council::present_winner(&4, 4, 31, 0); - Council::present_winner(&4, 5, 41, 0); + assert_ok!(Council::present_winner(&4, 3, 21, 0)); + assert_ok!(Council::present_winner(&4, 4, 31, 0)); + assert_ok!(Council::present_winner(&4, 5, 41, 0)); assert_eq!(Council::leaderboard(), Some(vec![ (21, 3), (31, 4), @@ -1366,7 +1393,7 @@ mod tests { (60, 1) ])); - Council::end_block(System::block_number()); + assert_ok!(Council::end_block(System::block_number())); assert!(!Council::presentation_active()); assert_eq!(Council::active_council(), vec![(1, 11), (5, 11)]); @@ -1391,34 +1418,34 @@ mod tests { fn second_tally_should_use_runners_up() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); - Council::submit_candidacy(&1, 0); - Council::set_approvals(&6, vec![true], 0); - Council::submit_candidacy(&2, 1); - Council::set_approvals(&2, vec![false, true], 0); - Council::submit_candidacy(&3, 2); - Council::set_approvals(&3, vec![false, false, true], 0); - Council::submit_candidacy(&4, 3); - Council::set_approvals(&4, vec![false, false, false, true], 0); - Council::submit_candidacy(&5, 4); - Council::set_approvals(&5, vec![false, false, false, false, true], 0); - Council::end_block(System::block_number()); + assert_ok!(Council::submit_candidacy(&1, 0)); + assert_ok!(Council::set_approvals(&6, vec![true], 0)); + assert_ok!(Council::submit_candidacy(&2, 1)); + assert_ok!(Council::set_approvals(&2, vec![false, true], 0)); + assert_ok!(Council::submit_candidacy(&3, 2)); + assert_ok!(Council::set_approvals(&3, vec![false, false, true], 0)); + assert_ok!(Council::submit_candidacy(&4, 3)); + assert_ok!(Council::set_approvals(&4, vec![false, false, false, true], 0)); + assert_ok!(Council::submit_candidacy(&5, 4)); + assert_ok!(Council::set_approvals(&5, vec![false, false, false, false, true], 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - Council::present_winner(&4, 1, 60, 0); - Council::present_winner(&4, 3, 21, 0); - Council::present_winner(&4, 4, 31, 0); - Council::present_winner(&4, 5, 41, 0); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 1, 60, 0)); + assert_ok!(Council::present_winner(&4, 3, 21, 0)); + assert_ok!(Council::present_winner(&4, 4, 31, 0)); + assert_ok!(Council::present_winner(&4, 5, 41, 0)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(8); - Council::set_approvals(&6, vec![false, false, true, false], 1); - Council::set_desired_seats(3); - Council::end_block(System::block_number()); + assert_ok!(Council::set_approvals(&6, vec![false, false, true, false], 1)); + assert_ok!(Council::set_desired_seats(3)); + assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - Council::present_winner(&4, 3, 81, 1); - Council::present_winner(&4, 4, 31, 1); - Council::end_block(System::block_number()); + assert_ok!(Council::present_winner(&4, 3, 81, 1)); + assert_ok!(Council::present_winner(&4, 4, 31, 1)); + assert_ok!(Council::end_block(System::block_number())); assert!(!Council::presentation_active()); assert_eq!(Council::active_council(), vec![(1, 11), (5, 11), (3, 15)]); diff --git a/substrate/runtime/council/src/voting.rs b/substrate/runtime/council/src/voting.rs index dedb3a23427ee..9e719776b2078 100644 --- a/substrate/runtime/council/src/voting.rs +++ b/substrate/runtime/council/src/voting.rs @@ -19,21 +19,22 @@ use rstd::prelude::*; use rstd::borrow::Borrow; use primitives::traits::{Executable, RefInto}; -use runtime_io::Hashing; +use runtime_io::{Hashing, print}; use runtime_support::{StorageValue, StorageMap, IsSubType}; +use runtime_support::dispatch::Result; use {system, democracy}; use super::{Trait, Module as Council}; decl_module! { pub struct Module; pub enum Call where aux: T::PublicAux { - fn propose(aux, proposal: Box) = 0; - fn vote(aux, proposal: T::Hash, approve: bool) = 1; - fn veto(aux, proposal_hash: T::Hash) = 2; + fn propose(aux, proposal: Box) -> Result = 0; + fn vote(aux, proposal: T::Hash, approve: bool) -> Result = 1; + fn veto(aux, proposal_hash: T::Hash) -> Result = 2; } pub enum PrivCall { - fn set_cooloff_period(blocks: T::BlockNumber) = 0; - fn set_voting_period(blocks: T::BlockNumber) = 1; + fn set_cooloff_period(blocks: T::BlockNumber) -> Result = 0; + fn set_voting_period(blocks: T::BlockNumber) -> Result = 1; } } @@ -73,14 +74,14 @@ impl Module { } // Dispatch - fn propose(aux: &T::PublicAux, proposal: Box) { + fn propose(aux: &T::PublicAux, proposal: Box) -> Result { let expiry = >::block_number() + Self::voting_period(); - ensure!(Self::will_still_be_councillor_at(aux.ref_into(), expiry)); + ensure!(Self::will_still_be_councillor_at(aux.ref_into(), expiry), "proposer would not be on council"); let proposal_hash = T::Hashing::hash_of(&proposal); - ensure!(!>::exists(proposal_hash), "No duplicate proposals allowed"); - ensure!(!Self::is_vetoed(&proposal_hash)); + ensure!(!>::exists(proposal_hash), "duplicate proposals not allowed"); + ensure!(!Self::is_vetoed(&proposal_hash), "proposal is vetoed"); let mut proposals = Self::proposals(); proposals.push((expiry, proposal_hash)); @@ -90,26 +91,28 @@ impl Module { >::insert(proposal_hash, *proposal); >::insert(proposal_hash, vec![aux.ref_into().clone()]); >::insert((proposal_hash, aux.ref_into().clone()), true); + Ok(()) } - fn vote(aux: &T::PublicAux, proposal: T::Hash, approve: bool) { + fn vote(aux: &T::PublicAux, proposal: T::Hash, approve: bool) -> Result { if Self::vote_of((proposal, aux.ref_into().clone())).is_none() { let mut voters = Self::proposal_voters(&proposal); voters.push(aux.ref_into().clone()); >::insert(proposal, voters); } >::insert((proposal, aux.ref_into().clone()), approve); + Ok(()) } - fn veto(aux: &T::PublicAux, proposal_hash: T::Hash) { + fn veto(aux: &T::PublicAux, proposal_hash: T::Hash) -> Result { ensure!(Self::is_councillor(aux.ref_into()), "only councillors may veto council proposals"); ensure!(>::exists(&proposal_hash), "proposal must exist to be vetoed"); let mut existing_vetoers = Self::veto_of(&proposal_hash) .map(|pair| pair.1) .unwrap_or_else(Vec::new); - let insert_position = ensure_unwrap_err!(existing_vetoers.binary_search(aux.ref_into()), - "a councillor may not veto a proposal twice"); + let insert_position = existing_vetoers.binary_search(aux.ref_into()) + .err().ok_or("a councillor may not veto a proposal twice")?; existing_vetoers.insert(insert_position, aux.ref_into().clone()); Self::set_veto_of(&proposal_hash, >::block_number() + Self::cooloff_period(), existing_vetoers); @@ -119,14 +122,17 @@ impl Module { for (c, _) in >::active_council() { >::remove((proposal_hash, c)); } + Ok(()) } - fn set_cooloff_period(blocks: T::BlockNumber) { + fn set_cooloff_period(blocks: T::BlockNumber) -> Result { >::put(blocks); + Ok(()) } - fn set_voting_period(blocks: T::BlockNumber) { + fn set_voting_period(blocks: T::BlockNumber) -> Result { >::put(blocks); + Ok(()) } // private @@ -169,7 +175,7 @@ impl Module { } } - fn end_block(now: T::BlockNumber) { + fn end_block(now: T::BlockNumber) -> Result { while let Some((proposal, proposal_hash)) = Self::take_proposal_if_expiring_at(now) { let tally = Self::take_tally(&proposal_hash); if let Some(&democracy::PrivCall::cancel_referendum(ref_index)) = IsSubType::>::is_sub_type(&proposal) { @@ -180,20 +186,27 @@ impl Module { if tally.0 > tally.1 + tally.2 { Self::kill_veto_of(&proposal_hash); match tally { - (_, 0, 0) => >::internal_start_referendum(proposal, democracy::VoteThreshold::SuperMajorityAgainst), - _ => >::internal_start_referendum(proposal, democracy::VoteThreshold::SimpleMajority), + (_, 0, 0) => >::internal_start_referendum(proposal, democracy::VoteThreshold::SuperMajorityAgainst).map(|_| ())?, + _ => >::internal_start_referendum(proposal, democracy::VoteThreshold::SimpleMajority).map(|_| ())?, }; } } } + Ok(()) } } impl Executable for Council { fn execute() { let n = >::block_number(); - Self::end_block(n); - >::end_block(n); + if let Err(e) = Self::end_block(n) { + print("Guru meditation"); + print(e); + } + if let Err(e) = >::end_block(n) { + print("Guru meditation"); + print(e); + } } } @@ -239,19 +252,19 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); - Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove); + assert_ok!(Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove)); assert_eq!(Democracy::active_referendums(), vec![(0, 4, proposal, VoteThreshold::SuperMajorityApprove)]); let cancellation = cancel_referendum_proposal(0); let hash = cancellation.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(cancellation)); - CouncilVoting::vote(&2, hash, true); - CouncilVoting::vote(&3, hash, true); + assert_ok!(CouncilVoting::propose(&1, Box::new(cancellation))); + assert_ok!(CouncilVoting::vote(&2, hash, true)); + assert_ok!(CouncilVoting::vote(&3, hash, true)); assert_eq!(CouncilVoting::proposals(), vec![(2, hash)]); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); System::set_block_number(2); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(Democracy::active_referendums(), vec![]); assert_eq!(Staking::bonding_duration(), 0); }); @@ -262,17 +275,17 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); - Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove); + assert_ok!(Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove)); let cancellation = cancel_referendum_proposal(0); let hash = cancellation.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(cancellation)); - CouncilVoting::vote(&2, hash, true); - CouncilVoting::vote(&3, hash, false); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::propose(&1, Box::new(cancellation))); + assert_ok!(CouncilVoting::vote(&2, hash, true)); + assert_ok!(CouncilVoting::vote(&3, hash, false)); + assert_ok!(CouncilVoting::end_block(System::block_number())); System::set_block_number(2); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(Democracy::active_referendums(), vec![(0, 4, proposal, VoteThreshold::SuperMajorityApprove)]); }); } @@ -282,16 +295,16 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); - Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove); + assert_ok!(Democracy::internal_start_referendum(proposal.clone(), VoteThreshold::SuperMajorityApprove)); let cancellation = cancel_referendum_proposal(0); let hash = cancellation.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(cancellation)); - CouncilVoting::vote(&2, hash, true); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::propose(&1, Box::new(cancellation))); + assert_ok!(CouncilVoting::vote(&2, hash, true)); + assert_ok!(CouncilVoting::end_block(System::block_number())); System::set_block_number(2); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(Democracy::active_referendums(), vec![(0, 4, proposal, VoteThreshold::SuperMajorityApprove)]); }); } @@ -302,8 +315,8 @@ mod tests { System::set_block_number(1); let proposal = bonding_duration_proposal(42); let hash = proposal.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::veto(&2, hash); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::veto(&2, hash)); assert_eq!(CouncilVoting::proposals().len(), 0); assert_eq!(Democracy::active_referendums().len(), 0); }); @@ -315,12 +328,12 @@ mod tests { System::set_block_number(1); let proposal = bonding_duration_proposal(42); let hash = proposal.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::veto(&2, hash); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::veto(&2, hash)); System::set_block_number(3); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - assert_noop!{CouncilVoting::veto(&2, hash)}; + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_noop!(CouncilVoting::veto(&2, hash), "a councillor may not veto a proposal twice"); }); } @@ -330,11 +343,11 @@ mod tests { System::set_block_number(1); let proposal = bonding_duration_proposal(42); let hash = proposal.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::veto(&2, hash); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::veto(&2, hash)); System::set_block_number(2); - assert_noop!{CouncilVoting::propose(&1, Box::new(proposal.clone()))}; + assert_noop!(CouncilVoting::propose(&1, Box::new(proposal.clone())), "proposal is vetoed"); }); } @@ -344,17 +357,17 @@ mod tests { System::set_block_number(1); let proposal = bonding_duration_proposal(42); let hash = proposal.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::veto(&2, hash); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::veto(&2, hash)); System::set_block_number(3); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::vote(&2, hash, false); - CouncilVoting::vote(&3, hash, true); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::vote(&2, hash, false)); + assert_ok!(CouncilVoting::vote(&3, hash, true)); + assert_ok!(CouncilVoting::end_block(System::block_number())); System::set_block_number(4); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(CouncilVoting::proposals().len(), 0); assert_eq!(Democracy::active_referendums(), vec![(0, 7, bonding_duration_proposal(42), VoteThreshold::SimpleMajority)]); }); @@ -366,12 +379,12 @@ mod tests { System::set_block_number(1); let proposal = bonding_duration_proposal(42); let hash = proposal.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::veto(&2, hash); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::veto(&2, hash)); System::set_block_number(3); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::veto(&3, hash); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::veto(&3, hash)); assert_eq!(CouncilVoting::proposals().len(), 0); assert_eq!(Democracy::active_referendums().len(), 0); }); @@ -383,7 +396,7 @@ mod tests { System::set_block_number(1); let proposal = bonding_duration_proposal(42); let hash = proposal.blake2_256().into(); - CouncilVoting::propose(&1, Box::new(proposal.clone())); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); assert_eq!(CouncilVoting::proposals().len(), 1); assert_eq!(CouncilVoting::proposal_voters(&hash), vec![1]); assert_eq!(CouncilVoting::vote_of((hash, 1)), Some(true)); @@ -396,12 +409,12 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); - CouncilVoting::propose(&1, Box::new(proposal.clone())); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); assert_eq!(CouncilVoting::tally(&proposal.blake2_256().into()), (1, 0, 2)); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); System::set_block_number(2); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(CouncilVoting::proposals().len(), 0); assert_eq!(Democracy::active_referendums().len(), 0); }); @@ -412,14 +425,14 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::vote(&2, proposal.blake2_256().into(), true); - CouncilVoting::vote(&3, proposal.blake2_256().into(), true); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::vote(&2, proposal.blake2_256().into(), true)); + assert_ok!(CouncilVoting::vote(&3, proposal.blake2_256().into(), true)); assert_eq!(CouncilVoting::tally(&proposal.blake2_256().into()), (3, 0, 0)); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); System::set_block_number(2); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(CouncilVoting::proposals().len(), 0); assert_eq!(Democracy::active_referendums(), vec![(0, 5, proposal, VoteThreshold::SuperMajorityAgainst)]); }); @@ -430,14 +443,14 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); - CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::vote(&2, proposal.blake2_256().into(), true); - CouncilVoting::vote(&3, proposal.blake2_256().into(), false); + assert_ok!(CouncilVoting::propose(&1, Box::new(proposal.clone()))); + assert_ok!(CouncilVoting::vote(&2, proposal.blake2_256().into(), true)); + assert_ok!(CouncilVoting::vote(&3, proposal.blake2_256().into(), false)); assert_eq!(CouncilVoting::tally(&proposal.blake2_256().into()), (2, 1, 0)); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); System::set_block_number(2); - CouncilVoting::end_block(System::block_number()); + assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(CouncilVoting::proposals().len(), 0); assert_eq!(Democracy::active_referendums(), vec![(0, 5, proposal, VoteThreshold::SimpleMajority)]); }); @@ -448,7 +461,7 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); - assert_noop!{CouncilVoting::propose(&4, Box::new(proposal))}; + assert_noop!(CouncilVoting::propose(&4, Box::new(proposal)), "proposer would not be on council"); }); } } diff --git a/substrate/runtime/democracy/src/lib.rs b/substrate/runtime/democracy/src/lib.rs index ffae2f92bcffc..730eb823bd603 100644 --- a/substrate/runtime/democracy/src/lib.rs +++ b/substrate/runtime/democracy/src/lib.rs @@ -39,8 +39,10 @@ extern crate substrate_runtime_staking as staking; extern crate substrate_runtime_system as system; use rstd::prelude::*; +use rstd::result; use primitives::traits::{Zero, Executable, RefInto, As}; use runtime_support::{StorageValue, StorageMap, Parameter, Dispatchable, IsSubType}; +use runtime_support::dispatch::Result; mod vote_threshold; pub use vote_threshold::{Approved, VoteThreshold}; @@ -57,13 +59,13 @@ pub trait Trait: staking::Trait + Sized { decl_module! { pub struct Module; pub enum Call where aux: T::PublicAux { - fn propose(aux, proposal: Box, value: T::Balance) = 0; - fn second(aux, proposal: PropIndex) = 1; - fn vote(aux, ref_index: ReferendumIndex, approve_proposal: bool) = 2; + fn propose(aux, proposal: Box, value: T::Balance) -> Result = 0; + fn second(aux, proposal: PropIndex) -> Result = 1; + fn vote(aux, ref_index: ReferendumIndex, approve_proposal: bool) -> Result = 2; } pub enum PrivCall { - fn start_referendum(proposal: Box, vote_threshold: VoteThreshold) = 0; - fn cancel_referendum(ref_index: ReferendumIndex) = 1; + fn start_referendum(proposal: Box, vote_threshold: VoteThreshold) -> Result = 0; + fn cancel_referendum(ref_index: ReferendumIndex) -> Result = 1; } } @@ -143,9 +145,10 @@ impl Module { // dispatching. /// Propose a sensitive action to be taken. - fn propose(aux: &T::PublicAux, proposal: Box, value: T::Balance) { - ensure!(value >= Self::minimum_deposit()); - ensure!(>::deduct_unbonded(aux.ref_into(), value)); + fn propose(aux: &T::PublicAux, proposal: Box, value: T::Balance) -> Result { + ensure!(value >= Self::minimum_deposit(), "value too low"); + >::deduct_unbonded(aux.ref_into(), value) + .map_err(|_| "proposer's balance too low")?; let index = Self::public_prop_count(); >::put(index + 1); @@ -154,51 +157,55 @@ impl Module { let mut props = Self::public_props(); props.push((index, (*proposal).clone(), aux.ref_into().clone())); >::put(props); + Ok(()) } /// Propose a sensitive action to be taken. - fn second(aux: &T::PublicAux, proposal: PropIndex) { - if let Some(mut deposit) = Self::deposit_of(proposal) { - ensure!(>::deduct_unbonded(aux.ref_into(), deposit.0)); - deposit.1.push(aux.ref_into().clone()); - >::insert(proposal, deposit); - } else { - fail!("can only second an existing proposal"); - } + fn second(aux: &T::PublicAux, proposal: PropIndex) -> Result { + let mut deposit = Self::deposit_of(proposal) + .ok_or("can only second an existing proposal")?; + >::deduct_unbonded(aux.ref_into(), deposit.0) + .map_err(|_| "seconder's balance too low")?; + deposit.1.push(aux.ref_into().clone()); + >::insert(proposal, deposit); + Ok(()) } /// Vote in a referendum. If `approve_proposal` is true, the vote is to enact the proposal; /// false would be a vote to keep the status quo.. - fn vote(aux: &T::PublicAux, ref_index: ReferendumIndex, approve_proposal: bool) { - if !Self::is_active_referendum(ref_index) { - fail!("vote given for invalid referendum.") - } - if >::balance(aux.ref_into()).is_zero() { - fail!("transactor must have balance to signal approval."); - } + fn vote(aux: &T::PublicAux, ref_index: ReferendumIndex, approve_proposal: bool) -> Result { + ensure!(Self::is_active_referendum(ref_index), "vote given for invalid referendum."); + ensure!(!>::balance(aux.ref_into()).is_zero(), + "transactor must have balance to signal approval."); if !>::exists(&(ref_index, aux.ref_into().clone())) { let mut voters = Self::voters_for(ref_index); voters.push(aux.ref_into().clone()); >::insert(ref_index, voters); } >::insert(&(ref_index, aux.ref_into().clone()), approve_proposal); + Ok(()) } /// Start a referendum. - fn start_referendum(proposal: Box, vote_threshold: VoteThreshold) { - let _ = Self::inject_referendum(>::block_number() + Self::voting_period(), *proposal, vote_threshold); + fn start_referendum(proposal: Box, vote_threshold: VoteThreshold) -> Result { + Self::inject_referendum( + >::block_number() + Self::voting_period(), + *proposal, + vote_threshold + ).map(|_| ()) } /// Remove a referendum. - fn cancel_referendum(ref_index: ReferendumIndex) { + fn cancel_referendum(ref_index: ReferendumIndex) -> Result { Self::clear_referendum(ref_index); + Ok(()) } // exposed mutables. /// Start a referendum. Can be called directly by the council. - pub fn internal_start_referendum(proposal: T::Proposal, vote_threshold: VoteThreshold) { - let _ = >::inject_referendum(>::block_number() + >::voting_period(), proposal, vote_threshold); + pub fn internal_start_referendum(proposal: T::Proposal, vote_threshold: VoteThreshold) -> result::Result { + >::inject_referendum(>::block_number() + >::voting_period(), proposal, vote_threshold) } /// Remove a referendum. Can be called directly by the council. @@ -213,7 +220,7 @@ impl Module { end: T::BlockNumber, proposal: T::Proposal, vote_threshold: VoteThreshold - ) -> Result { + ) -> result::Result { let ref_index = Self::referendum_count(); if ref_index > 0 && Self::referendum_info(ref_index - 1).map(|i| i.0 > end).unwrap_or(false) { Err("Cannot inject a referendum that ends earlier than preceeding referendum")? @@ -234,7 +241,7 @@ impl Module { } /// Current era is ending; we should finish up any proposals. - fn end_block(now: T::BlockNumber) -> Result<(), &'static str> { + fn end_block(now: T::BlockNumber) -> Result { // pick out another public referendum if it's time. if (now % Self::launch_period()).is_zero() { let mut public_props = Self::public_props(); @@ -251,7 +258,7 @@ impl Module { >::put(public_props); Self::inject_referendum(now + Self::voting_period(), proposal, VoteThreshold::SuperMajorityApprove)?; } else { - Err("depositors always exist for current proposals")? + return Err("depositors always exist for current proposals") } } } @@ -262,7 +269,7 @@ impl Module { let total_stake = >::total_stake(); Self::clear_referendum(index); if vote_threshold.approved(approve, against, total_stake) { - proposal.dispatch(); + proposal.dispatch()?; } >::put(index + 1); } @@ -423,17 +430,17 @@ mod tests { }); } - fn propose_sessions_per_era(who: u64, value: u64, locked: u64) { - Democracy::propose(&who, Box::new(Proposal::Staking(staking::PrivCall::set_sessions_per_era(value))), locked); + fn propose_sessions_per_era(who: u64, value: u64, locked: u64) -> super::Result { + Democracy::propose(&who, Box::new(Proposal::Staking(staking::PrivCall::set_sessions_per_era(value))), locked) } #[test] fn locked_for_should_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - propose_sessions_per_era(1, 2, 2); - propose_sessions_per_era(1, 4, 4); - propose_sessions_per_era(1, 3, 3); + assert_ok!(propose_sessions_per_era(1, 2, 2)); + assert_ok!(propose_sessions_per_era(1, 4, 4)); + assert_ok!(propose_sessions_per_era(1, 3, 3)); assert_eq!(Democracy::locked_for(0), Some(2)); assert_eq!(Democracy::locked_for(1), Some(4)); assert_eq!(Democracy::locked_for(2), Some(3)); @@ -444,12 +451,12 @@ mod tests { fn single_proposal_should_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - propose_sessions_per_era(1, 2, 1); + assert_ok!(propose_sessions_per_era(1, 2, 1)); assert_eq!(Democracy::end_block(System::block_number()), Ok(())); System::set_block_number(2); let r = 0; - Democracy::vote(&1, r, true); + assert_ok!(Democracy::vote(&1, r, true)); assert_eq!(Democracy::referendum_count(), 1); assert_eq!(Democracy::voters_for(r), vec![1]); @@ -467,11 +474,11 @@ mod tests { fn deposit_for_proposals_should_be_taken() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - propose_sessions_per_era(1, 2, 5); - Democracy::second(&2, 0); - Democracy::second(&5, 0); - Democracy::second(&5, 0); - Democracy::second(&5, 0); + assert_ok!(propose_sessions_per_era(1, 2, 5)); + assert_ok!(Democracy::second(&2, 0)); + assert_ok!(Democracy::second(&5, 0)); + assert_ok!(Democracy::second(&5, 0)); + assert_ok!(Democracy::second(&5, 0)); assert_eq!(Staking::balance(&1), 5); assert_eq!(Staking::balance(&2), 15); assert_eq!(Staking::balance(&5), 35); @@ -482,11 +489,11 @@ mod tests { fn deposit_for_proposals_should_be_returned() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - propose_sessions_per_era(1, 2, 5); - Democracy::second(&2, 0); - Democracy::second(&5, 0); - Democracy::second(&5, 0); - Democracy::second(&5, 0); + assert_ok!(propose_sessions_per_era(1, 2, 5)); + assert_ok!(Democracy::second(&2, 0)); + assert_ok!(Democracy::second(&5, 0)); + assert_ok!(Democracy::second(&5, 0)); + assert_ok!(Democracy::second(&5, 0)); assert_eq!(Democracy::end_block(System::block_number()), Ok(())); assert_eq!(Staking::balance(&1), 10); assert_eq!(Staking::balance(&2), 20); @@ -495,60 +502,57 @@ mod tests { } #[test] - fn proposal_with_deposit_below_minimum_should_panic() { + fn proposal_with_deposit_below_minimum_should_not_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - propose_sessions_per_era(1, 2, 0); - assert_eq!(Democracy::locked_for(0), None); + assert_noop!(propose_sessions_per_era(1, 2, 0), "value too low"); }); } #[test] - fn poor_proposer_should_panic() { + fn poor_proposer_should_not_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - propose_sessions_per_era(1, 2, 11); - assert_eq!(Democracy::locked_for(0), None); + assert_noop!(propose_sessions_per_era(1, 2, 11), "proposer\'s balance too low"); }); } #[test] - fn poor_seconder_should_panic() { + fn poor_seconder_should_not_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - propose_sessions_per_era(2, 2, 11); - Democracy::second(&1, 0); - assert_eq!(Democracy::locked_for(0), Some(11)); + assert_ok!(propose_sessions_per_era(2, 2, 11)); + assert_noop!(Democracy::second(&1, 0), "seconder\'s balance too low"); }); } - fn propose_bonding_duration(who: u64, value: u64, locked: u64) { - Democracy::propose(&who, Box::new(Proposal::Staking(staking::PrivCall::set_bonding_duration(value))), locked); + fn propose_bonding_duration(who: u64, value: u64, locked: u64) -> super::Result { + Democracy::propose(&who, Box::new(Proposal::Staking(staking::PrivCall::set_bonding_duration(value))), locked) } #[test] fn runners_up_should_come_after() { with_externalities(&mut new_test_ext(), || { System::set_block_number(0); - propose_bonding_duration(1, 2, 2); - propose_bonding_duration(1, 4, 4); - propose_bonding_duration(1, 3, 3); + assert_ok!(propose_bonding_duration(1, 2, 2)); + assert_ok!(propose_bonding_duration(1, 4, 4)); + assert_ok!(propose_bonding_duration(1, 3, 3)); assert_eq!(Democracy::end_block(System::block_number()), Ok(())); System::set_block_number(1); - Democracy::vote(&1, 0, true); + assert_ok!(Democracy::vote(&1, 0, true)); assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::bonding_duration(), 4); System::set_block_number(2); - Democracy::vote(&1, 1, true); + assert_ok!(Democracy::vote(&1, 1, true)); assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::bonding_duration(), 3); System::set_block_number(3); - Democracy::vote(&1, 2, true); + assert_ok!(Democracy::vote(&1, 2, true)); assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::bonding_duration(), 2); @@ -564,7 +568,7 @@ mod tests { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); - Democracy::vote(&1, r, true); + assert_ok!(Democracy::vote(&1, r, true)); assert_eq!(Democracy::voters_for(r), vec![1]); assert_eq!(Democracy::vote_of((r, 1)), Some(true)); @@ -582,8 +586,8 @@ mod tests { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); - Democracy::vote(&1, r, true); - Democracy::cancel_referendum(r); + assert_ok!(Democracy::vote(&1, r, true)); + assert_ok!(Democracy::cancel_referendum(r)); assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); @@ -597,7 +601,7 @@ mod tests { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); - Democracy::vote(&1, r, false); + assert_ok!(Democracy::vote(&1, r, false)); assert_eq!(Democracy::voters_for(r), vec![1]); assert_eq!(Democracy::vote_of((r, 1)), Some(false)); @@ -615,12 +619,12 @@ mod tests { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); - Democracy::vote(&1, r, true); - Democracy::vote(&2, r, false); - Democracy::vote(&3, r, false); - Democracy::vote(&4, r, true); - Democracy::vote(&5, r, false); - Democracy::vote(&6, r, true); + assert_ok!(Democracy::vote(&1, r, true)); + assert_ok!(Democracy::vote(&2, r, false)); + assert_ok!(Democracy::vote(&3, r, false)); + assert_ok!(Democracy::vote(&4, r, true)); + assert_ok!(Democracy::vote(&5, r, false)); + assert_ok!(Democracy::vote(&6, r, true)); assert_eq!(Democracy::tally(r), (110, 100)); @@ -636,8 +640,8 @@ mod tests { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); - Democracy::vote(&5, r, false); - Democracy::vote(&6, r, true); + assert_ok!(Democracy::vote(&5, r, false)); + assert_ok!(Democracy::vote(&6, r, true)); assert_eq!(Democracy::tally(r), (60, 50)); @@ -656,9 +660,9 @@ mod tests { System::set_block_number(1); let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); - Democracy::vote(&4, r, true); - Democracy::vote(&5, r, false); - Democracy::vote(&6, r, true); + assert_ok!(Democracy::vote(&4, r, true)); + assert_ok!(Democracy::vote(&5, r, false)); + assert_ok!(Democracy::vote(&6, r, true)); assert_eq!(Democracy::tally(r), (100, 50)); diff --git a/substrate/runtime/session/src/lib.rs b/substrate/runtime/session/src/lib.rs index ac26d6db724e8..f93c31a4b2b09 100644 --- a/substrate/runtime/session/src/lib.rs +++ b/substrate/runtime/session/src/lib.rs @@ -43,6 +43,7 @@ extern crate substrate_runtime_system as system; use rstd::prelude::*; use primitives::traits::{Zero, One, RefInto, Executable, Convert}; use runtime_support::{StorageValue, StorageMap}; +use runtime_support::dispatch::Result; pub trait Trait: consensus::Trait { type ConvertAccountIdToSessionKey: Convert; @@ -51,11 +52,11 @@ pub trait Trait: consensus::Trait { decl_module! { pub struct Module; pub enum Call where aux: T::PublicAux { - fn set_key(aux, key: T::SessionKey) = 0; + fn set_key(aux, key: T::SessionKey) -> Result = 0; } pub enum PrivCall { - fn set_length(new: T::BlockNumber) = 0; - fn force_new_session() = 1; + fn set_length(new: T::BlockNumber) -> Result = 0; + fn force_new_session() -> Result = 1; } } decl_storage! { @@ -89,19 +90,22 @@ impl Module { /// Sets the session key of `_validator` to `_key`. This doesn't take effect until the next /// session. - fn set_key(aux: &T::PublicAux, key: T::SessionKey) { + fn set_key(aux: &T::PublicAux, key: T::SessionKey) -> Result { // set new value for next session - >::insert(aux.ref_into(), key) + >::insert(aux.ref_into(), key); + Ok(()) } /// Set a new era length. Won't kick in until the next era change (at current length). - fn set_length(new: T::BlockNumber) { + fn set_length(new: T::BlockNumber) -> Result { >::put(new); + Ok(()) } /// Forces a new session. - fn force_new_session() { + fn force_new_session() -> Result { Self::rotate_session(); + Ok(()) } // INTERNAL API (available to other runtime modules) @@ -248,14 +252,14 @@ mod tests { with_externalities(&mut new_test_ext(), || { // Block 1: Change to length 3; no visible change. System::set_block_number(1); - Session::set_length(3); + assert_ok!(Session::set_length(3)); Session::check_rotate_session(); assert_eq!(Session::length(), 2); assert_eq!(Session::current_index(), 0); // Block 2: Length now changed to 3. Index incremented. System::set_block_number(2); - Session::set_length(3); + assert_ok!(Session::set_length(3)); Session::check_rotate_session(); assert_eq!(Session::length(), 3); assert_eq!(Session::current_index(), 1); @@ -268,7 +272,7 @@ mod tests { // Block 4: Change to length 2; no visible change. System::set_block_number(4); - Session::set_length(2); + assert_ok!(Session::set_length(2)); Session::check_rotate_session(); assert_eq!(Session::length(), 3); assert_eq!(Session::current_index(), 1); @@ -308,7 +312,7 @@ mod tests { // Block 3: Set new key for validator 2; no visible change. System::set_block_number(3); - Session::set_key(&2, 5); + assert_ok!(Session::set_key(&2, 5)); assert_eq!(Consensus::authorities(), vec![1, 2, 3]); Session::check_rotate_session(); diff --git a/substrate/runtime/staking/src/contract.rs b/substrate/runtime/staking/src/contract.rs index 4f03333e84756..ed6353dd461a4 100644 --- a/substrate/runtime/staking/src/contract.rs +++ b/substrate/runtime/staking/src/contract.rs @@ -551,7 +551,7 @@ mod tests { >::insert(1, code_transfer.to_vec()); - Staking::transfer(&0, 1, 11); + assert_ok!(Staking::transfer(&0, 1, 11)); assert_eq!(Staking::balance(&0), 100); assert_eq!(Staking::balance(&1), 5); @@ -616,7 +616,7 @@ r#" >::insert(1, code_create.to_vec()); // When invoked, the contract at address `1` must create a contract with 'transfer' code. - Staking::transfer(&0, 1, 11); + assert_ok!(Staking::transfer(&0, 1, 11)); let derived_address = ::DetermineContractAddress::contract_address_for(&code_transfer, &1); @@ -674,8 +674,8 @@ r#" >::insert(1, 0); >::insert(1, code_adder); - Staking::transfer(&0, 1, 1); - Staking::transfer(&0, 1, 1); + assert_ok!(Staking::transfer(&0, 1, 1)); + assert_ok!(Staking::transfer(&0, 1, 1)); let storage_addr = [0x01u8; 32]; let value = @@ -735,7 +735,7 @@ r#" // Transfer some balance from 0 to 1. This will trigger execution // of the smart-contract code at address 1. - Staking::transfer(&0, 1, 11); + assert_ok!(Staking::transfer(&0, 1, 11)); // The balance should remain unchanged since we are expecting // out-of-gas error which will revert transfer. @@ -767,7 +767,7 @@ r#" >::insert(1, code_mem.to_vec()); // Transfer some balance from 0 to 1. - Staking::transfer(&0, 1, 11); + assert_ok!(Staking::transfer(&0, 1, 11)); // The balance should remain unchanged since we are expecting // validation error caused by internal memory declaration. diff --git a/substrate/runtime/staking/src/lib.rs b/substrate/runtime/staking/src/lib.rs index 763c4fe2e7696..e63b3de7d7e14 100644 --- a/substrate/runtime/staking/src/lib.rs +++ b/substrate/runtime/staking/src/lib.rs @@ -47,11 +47,12 @@ extern crate parity_wasm; #[cfg(test)] use std::fmt::Debug; use rstd::prelude::*; -use rstd::cmp; +use rstd::{cmp, result}; use rstd::cell::RefCell; use rstd::collections::btree_map::{BTreeMap, Entry}; use codec::Slicable; use runtime_support::{StorageValue, StorageMap, Parameter}; +use runtime_support::dispatch::Result; use primitives::traits::{Zero, One, Bounded, RefInto, SimpleArithmetic, Executable, MakePayment, As}; mod contract; @@ -99,15 +100,15 @@ pub trait Trait: system::Trait + session::Trait { decl_module! { pub struct Module; pub enum Call where aux: T::PublicAux { - fn transfer(aux, dest: T::AccountId, value: T::Balance) = 0; - fn stake(aux) = 1; - fn unstake(aux) = 2; + fn transfer(aux, dest: T::AccountId, value: T::Balance) -> Result = 0; + fn stake(aux) -> Result = 1; + fn unstake(aux) -> Result = 2; } pub enum PrivCall { - fn set_sessions_per_era(new: T::BlockNumber) = 0; - fn set_bonding_duration(new: T::BlockNumber) = 1; - fn set_validator_count(new: u32) = 2; - fn force_new_era() = 3; + fn set_sessions_per_era(new: T::BlockNumber) -> Result = 0; + fn set_bonding_duration(new: T::BlockNumber) -> Result = 1; + fn set_validator_count(new: u32) -> Result = 2; + fn force_new_era() -> Result = 3; } } @@ -193,85 +194,90 @@ impl Module { } /// Create a smart-contract account. - pub fn create(aux: &T::PublicAux, code: &[u8], value: T::Balance) { + pub fn create(aux: &T::PublicAux, code: &[u8], value: T::Balance) -> Result { // commit anything that made it this far to storage - if let Ok(Some(commit)) = Self::effect_create(aux.ref_into(), code, value, &DirectAccountDb) { + if let Some(commit) = Self::effect_create(aux.ref_into(), code, value, &DirectAccountDb)? { >::merge(&mut DirectAccountDb, commit); } + Ok(()) } // PUBLIC DISPATCH /// Transfer some unlocked staking balance to another staker. /// TODO: probably want to state gas-limit and gas-price. - fn transfer(aux: &T::PublicAux, dest: T::AccountId, value: T::Balance) { + fn transfer(aux: &T::PublicAux, dest: T::AccountId, value: T::Balance) -> Result { // commit anything that made it this far to storage - if let Ok(Some(commit)) = Self::effect_transfer(aux.ref_into(), &dest, value, &DirectAccountDb) { + if let Some(commit) = Self::effect_transfer(aux.ref_into(), &dest, value, &DirectAccountDb)? { >::merge(&mut DirectAccountDb, commit); } + Ok(()) } /// Declare the desire to stake for the transactor. /// /// Effects will be felt at the beginning of the next era. - fn stake(aux: &T::PublicAux) { + fn stake(aux: &T::PublicAux) -> Result { let mut intentions = >::get(); // can't be in the list twice. ensure!(intentions.iter().find(|&t| t == aux.ref_into()).is_none(), "Cannot stake if already staked."); intentions.push(aux.ref_into().clone()); >::put(intentions); >::insert(aux.ref_into(), T::BlockNumber::max_value()); + Ok(()) } /// Retract the desire to stake for the transactor. /// /// Effects will be felt at the beginning of the next era. - fn unstake(aux: &T::PublicAux) { + fn unstake(aux: &T::PublicAux) -> Result { let mut intentions = >::get(); - if let Some(position) = intentions.iter().position(|t| t == aux.ref_into()) { - intentions.swap_remove(position); - >::put(intentions); - >::insert(aux.ref_into(), Self::current_era() + Self::bonding_duration()); - } else { - fail!("Cannot unstake if not already staked."); - } + let position = intentions.iter().position(|t| t == aux.ref_into()).ok_or("Cannot unstake if not already staked.")?; + intentions.swap_remove(position); + >::put(intentions); + >::insert(aux.ref_into(), Self::current_era() + Self::bonding_duration()); + Ok(()) } // PRIV DISPATCH /// Set the number of sessions in an era. - fn set_sessions_per_era(new: T::BlockNumber) { + fn set_sessions_per_era(new: T::BlockNumber) -> Result { >::put(&new); + Ok(()) } /// The length of the bonding duration in eras. - fn set_bonding_duration(new: T::BlockNumber) { + fn set_bonding_duration(new: T::BlockNumber) -> Result { >::put(&new); + Ok(()) } /// The length of a staking era in sessions. - fn set_validator_count(new: u32) { + fn set_validator_count(new: u32) -> Result { >::put(&new); + Ok(()) } /// Force there to be a new era. This also forces a new session immediately after. - fn force_new_era() { + fn force_new_era() -> Result { Self::new_era(); >::rotate_session(); + Ok(()) } // PUBLIC MUTABLES (DANGEROUS) /// Deduct from an unbonded balance. true if it happened. - pub fn deduct_unbonded(who: &T::AccountId, value: T::Balance) -> bool { + pub fn deduct_unbonded(who: &T::AccountId, value: T::Balance) -> Result { if let LockStatus::Liquid = Self::unlock_block(who) { let b = Self::free_balance(who); if b >= value { >::insert(who, b - value); - return true; + return Ok(()) } } - false + Err("not enough liquid funds") } /// Refund some balance. @@ -280,26 +286,27 @@ impl Module { } /// Will slash any balance, but prefer free over reserved. - pub fn slash(who: &T::AccountId, value: T::Balance) -> bool { + pub fn slash(who: &T::AccountId, value: T::Balance) -> Result { let free_balance = Self::free_balance(who); let free_slash = cmp::min(free_balance, value); >::insert(who, &(free_balance - free_slash)); if free_slash < value { Self::slash_reserved(who, value - free_slash) + .map_err(|_| "not enough funds") } else { - true + Ok(()) } } /// Moves `value` from balance to reserved balance. - pub fn reserve_balance(who: &T::AccountId, value: T::Balance) -> bool { + pub fn reserve_balance(who: &T::AccountId, value: T::Balance) -> Result { let b = Self::free_balance(who); if b < value { - return false; + return Err("not enough free funds") } >::insert(who, b - value); >::insert(who, Self::reserved_balance(who) + value); - true + Ok(()) } /// Moves `value` from reserved balance to balance. @@ -311,20 +318,28 @@ impl Module { } /// Moves `value` from reserved balance to balance. - pub fn slash_reserved(who: &T::AccountId, value: T::Balance) -> bool { + pub fn slash_reserved(who: &T::AccountId, value: T::Balance) -> Result { let b = Self::reserved_balance(who); let slash = cmp::min(b, value); >::insert(who, b - slash); - value == slash + if value == slash { + Ok(()) + } else { + Err("not enough (reserved) funds") + } } /// Moves `value` from reserved balance to balance. - pub fn transfer_reserved_balance(slashed: &T::AccountId, beneficiary: &T::AccountId, value: T::Balance) -> bool { + pub fn transfer_reserved_balance(slashed: &T::AccountId, beneficiary: &T::AccountId, value: T::Balance) -> Result { let b = Self::reserved_balance(slashed); let slash = cmp::min(b, value); >::insert(slashed, b - slash); >::insert(beneficiary, Self::free_balance(beneficiary) + slash); - slash == value + if value == slash { + Ok(()) + } else { + Err("not enough (reserved) funds") + } } /// Hook to be called after to transaction processing. @@ -551,7 +566,7 @@ impl Module { code: &[u8], value: T::Balance, account_db: &DB, - ) -> Result>, &'static str> { + ) -> result::Result>, &'static str> { let from_balance = account_db.get_balance(transactor); // TODO: a fee. if from_balance < value { @@ -580,7 +595,7 @@ impl Module { dest: &T::AccountId, value: T::Balance, account_db: &DB, - ) -> Result>, &'static str> { + ) -> result::Result>, &'static str> { let from_balance = account_db.get_balance(transactor); if from_balance < value { return Err("balance too low to send value"); @@ -756,9 +771,9 @@ mod tests { // Block 1: Add three validators. No obvious change. System::set_block_number(1); - Staking::stake(&1); - Staking::stake(&2); - Staking::stake(&4); + assert_ok!(Staking::stake(&1)); + assert_ok!(Staking::stake(&2)); + assert_ok!(Staking::stake(&4)); Staking::check_new_era(); assert_eq!(Session::validators(), vec![10, 20]); @@ -769,8 +784,8 @@ mod tests { // Block 3: Unstake highest, introduce another staker. No change yet. System::set_block_number(3); - Staking::stake(&3); - Staking::unstake(&4); + assert_ok!(Staking::stake(&3)); + assert_ok!(Staking::unstake(&4)); Staking::check_new_era(); // Block 4: New era - validators change. @@ -780,7 +795,7 @@ mod tests { // Block 5: Transfer stake from highest to lowest. No change yet. System::set_block_number(5); - Staking::transfer(&4, 1, 40); + assert_ok!(Staking::transfer(&4, 1, 40)); Staking::check_new_era(); // Block 6: Lowest now validator. @@ -790,7 +805,7 @@ mod tests { // Block 7: Unstake three. No change yet. System::set_block_number(7); - Staking::unstake(&3); + assert_ok!(Staking::unstake(&3)); Staking::check_new_era(); assert_eq!(Session::validators(), vec![1, 3]); @@ -825,7 +840,7 @@ mod tests { // Block 3: Schedule an era length change; no visible changes. System::set_block_number(3); - Staking::set_sessions_per_era(3); + assert_ok!(Staking::set_sessions_per_era(3)); Staking::check_new_era(); assert_eq!(Staking::sessions_per_era(), 2); assert_eq!(Staking::last_era_length_change(), 0); @@ -878,19 +893,18 @@ mod tests { fn staking_balance_transfer_works() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::transfer(&1, 2, 69); + assert_ok!(Staking::transfer(&1, 2, 69)); assert_eq!(Staking::balance(&1), 42); assert_eq!(Staking::balance(&2), 69); }); } #[test] - fn staking_balance_transfer_when_bonded_panics() { + fn staking_balance_transfer_when_bonded_should_not_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::stake(&1); - Staking::transfer(&1, 2, 69); - assert_eq!(Staking::balance(&1), 111); + assert_ok!(Staking::stake(&1)); + assert_noop!(Staking::transfer(&1, 2, 69), "bondage too high to send value"); }); } @@ -903,7 +917,7 @@ mod tests { assert_eq!(Staking::free_balance(&1), 111); assert_eq!(Staking::reserved_balance(&1), 0); - Staking::reserve_balance(&1, 69); + assert_ok!(Staking::reserve_balance(&1, 69)); assert_eq!(Staking::balance(&1), 111); assert_eq!(Staking::free_balance(&1), 42); @@ -911,13 +925,12 @@ mod tests { }); } - #[should_panic] - fn staking_balance_transfer_when_reserved_panics() { + #[test] + fn staking_balance_transfer_when_reserved_should_not_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::reserve_balance(&1, 69); - Staking::transfer(&1, 2, 69); - assert_eq!(Staking::balance(&1), 111); + assert_ok!(Staking::reserve_balance(&1, 69)); + assert_noop!(Staking::transfer(&1, 2, 69), "balance too low to send value"); }); } @@ -925,19 +938,19 @@ mod tests { fn deducting_balance_should_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - assert!(Staking::deduct_unbonded(&1, 69)); + assert_ok!(Staking::deduct_unbonded(&1, 69)); assert_eq!(Staking::free_balance(&1), 42); }); } #[test] - fn deducting_balance_should_fail_when_bonded() { + fn deducting_balance_when_bonded_should_not_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); >::insert(1, 2); System::set_block_number(1); assert_eq!(Staking::unlock_block(&1), LockStatus::LockedUntil(2)); - assert!(!Staking::deduct_unbonded(&1, 69)); + assert_noop!(Staking::deduct_unbonded(&1, 69), "not enough liquid funds"); }); } @@ -954,8 +967,8 @@ mod tests { fn slashing_balance_should_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::reserve_balance(&1, 69); - assert!(Staking::slash(&1, 69)); + assert_ok!(Staking::reserve_balance(&1, 69)); + assert_ok!(Staking::slash(&1, 69)); assert_eq!(Staking::free_balance(&1), 0); assert_eq!(Staking::reserved_balance(&1), 42); }); @@ -965,8 +978,8 @@ mod tests { fn slashing_incomplete_balance_should_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 42); - Staking::reserve_balance(&1, 21); - assert!(!Staking::slash(&1, 69)); + assert_ok!(Staking::reserve_balance(&1, 21)); + assert_err!(Staking::slash(&1, 69), "not enough funds"); assert_eq!(Staking::free_balance(&1), 0); assert_eq!(Staking::reserved_balance(&1), 0); }); @@ -976,7 +989,7 @@ mod tests { fn unreserving_balance_should_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::reserve_balance(&1, 111); + assert_ok!(Staking::reserve_balance(&1, 111)); Staking::unreserve_balance(&1, 42); assert_eq!(Staking::reserved_balance(&1), 69); assert_eq!(Staking::free_balance(&1), 42); @@ -987,8 +1000,8 @@ mod tests { fn slashing_reserved_balance_should_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::reserve_balance(&1, 111); - assert!(Staking::slash_reserved(&1, 42)); + assert_ok!(Staking::reserve_balance(&1, 111)); + assert_ok!(Staking::slash_reserved(&1, 42)); assert_eq!(Staking::reserved_balance(&1), 69); assert_eq!(Staking::free_balance(&1), 0); }); @@ -998,8 +1011,8 @@ mod tests { fn slashing_incomplete_reserved_balance_should_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::reserve_balance(&1, 42); - assert!(!Staking::slash_reserved(&1, 69)); + assert_ok!(Staking::reserve_balance(&1, 42)); + assert_err!(Staking::slash_reserved(&1, 69), "not enough (reserved) funds"); assert_eq!(Staking::free_balance(&1), 69); assert_eq!(Staking::reserved_balance(&1), 0); }); @@ -1009,8 +1022,8 @@ mod tests { fn transferring_reserved_balance_should_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::reserve_balance(&1, 111); - assert!(Staking::transfer_reserved_balance(&1, &2, 42)); + assert_ok!(Staking::reserve_balance(&1, 111)); + assert_ok!(Staking::transfer_reserved_balance(&1, &2, 42)); assert_eq!(Staking::reserved_balance(&1), 69); assert_eq!(Staking::free_balance(&1), 0); assert_eq!(Staking::reserved_balance(&2), 0); @@ -1022,8 +1035,8 @@ mod tests { fn transferring_incomplete_reserved_balance_should_work() { with_externalities(&mut new_test_ext(1, 3, 1, false), || { >::insert(1, 111); - Staking::reserve_balance(&1, 42); - assert!(!Staking::transfer_reserved_balance(&1, &2, 69)); + assert_ok!(Staking::reserve_balance(&1, 42)); + assert_err!(Staking::transfer_reserved_balance(&1, &2, 69), "not enough (reserved) funds"); assert_eq!(Staking::reserved_balance(&1), 0); assert_eq!(Staking::free_balance(&1), 69); assert_eq!(Staking::reserved_balance(&2), 0); diff --git a/substrate/runtime/timestamp/src/lib.rs b/substrate/runtime/timestamp/src/lib.rs index fd03fac92821f..96c953e66500e 100644 --- a/substrate/runtime/timestamp/src/lib.rs +++ b/substrate/runtime/timestamp/src/lib.rs @@ -34,6 +34,7 @@ extern crate substrate_runtime_system as system; extern crate substrate_codec as codec; use runtime_support::{StorageValue, Parameter}; +use runtime_support::dispatch::Result; use runtime_primitives::traits::{HasPublicAux, Executable, MaybeEmpty}; pub trait Trait: HasPublicAux + system::Trait { @@ -46,7 +47,7 @@ pub trait Trait: HasPublicAux + system::Trait { decl_module! { pub struct Module; pub enum Call where aux: T::PublicAux { - fn set(aux, now: T::Value) = 0; + fn set(aux, now: T::Value) -> Result = 0; } } @@ -64,7 +65,7 @@ impl Module { } /// Set the current time. - fn set(aux: &T::PublicAux, now: T::Value) { + fn set(aux: &T::PublicAux, now: T::Value) -> Result { assert!(aux.is_empty()); assert!(!::DidUpdate::exists(), "Timestamp must be updated only once in the block"); assert!( @@ -74,6 +75,7 @@ impl Module { ); ::Now::put(now); ::DidUpdate::put(true); + Ok(()) } } @@ -138,7 +140,7 @@ mod tests { with_externalities(&mut t, || { assert_eq!(::Now::get(), 42); - Timestamp::aux_dispatch(Call::set(69), &0); + assert_ok!(Timestamp::aux_dispatch(Call::set(69), &0)); assert_eq!(Timestamp::now(), 69); }); }