From 6b1e2c32aa6ffd42855a5f54addf41ee58cea8a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 23 Oct 2018 16:14:14 +0200 Subject: [PATCH] Make `decl_module!` implement calls directly --- srml/assets/src/lib.rs | 75 +++--- srml/balances/src/lib.rs | 112 ++++----- srml/consensus/src/lib.rs | 95 ++++---- srml/contract/src/lib.rs | 187 +++++++------- srml/council/src/motions.rs | 182 +++++++------- srml/council/src/seats.rs | 456 +++++++++++++++++------------------ srml/council/src/voting.rs | 152 ++++++------ srml/democracy/src/lib.rs | 137 +++++------ srml/example/src/lib.rs | 162 ++++++------- srml/session/src/lib.rs | 44 ++-- srml/staking/src/lib.rs | 291 +++++++++++----------- srml/support/src/dispatch.rs | 113 +++++---- srml/support/src/metadata.rs | 8 +- srml/timestamp/src/lib.rs | 55 +++-- srml/treasury/src/lib.rs | 146 ++++++----- 15 files changed, 1078 insertions(+), 1137 deletions(-) diff --git a/srml/assets/src/lib.rs b/srml/assets/src/lib.rs index 791907166a8db..bde551e8863ee 100644 --- a/srml/assets/src/lib.rs +++ b/srml/assets/src/lib.rs @@ -73,13 +73,43 @@ decl_module! { /// Issue a new class of fungible assets. There are, and will only ever be, `total` /// such assets and they'll all belong to the `origin` initially. It will have an /// identifier `AssetId` instance: this will be specified in the `Issued` event. - fn issue(origin, total: T::Balance) -> Result; + fn issue(origin, total: T::Balance) -> Result { + let origin = ensure_signed(origin)?; + + let id = Self::next_asset_id(); + >::mutate(|id| *id += 1); + + >::insert((id, origin.clone()), total); + + Self::deposit_event(RawEvent::Issued(id, origin, total)); + Ok(()) + } /// Move some assets from one holder to another. - fn transfer(origin, id: AssetId, target: T::AccountId, total: T::Balance) -> Result; + fn transfer(origin, id: AssetId, target: T::AccountId, amount: T::Balance) -> Result { + let origin = ensure_signed(origin)?; + let origin_account = (id, origin.clone()); + let origin_balance = >::get(&origin_account); + ensure!(origin_balance >= amount, "origin account balance must be greater than amount"); + + Self::deposit_event(RawEvent::Transfered(id, origin, target.clone(), amount)); + >::insert(origin_account, origin_balance - amount); + >::mutate((id, target), |balance| *balance += amount); + + Ok(()) + } /// Destroy any assets of `id` owned by `origin`. - fn destroy(origin, id: AssetId) -> Result; + fn destroy(origin, id: AssetId) -> Result { + let origin = ensure_signed(origin)?; + + let balance = >::take((id, origin.clone())); + ensure!(!balance.is_zero(), "origin balance should be non-zero"); + + Self::deposit_event(RawEvent::Destroyed(id, origin, balance)); + + Ok(()) + } } } @@ -114,45 +144,6 @@ impl Module { pub fn balance(id: AssetId, who: T::AccountId) -> T::Balance { >::get((id, who)) } - - // Implement Calls and add public immutables and private mutables. - - fn issue(origin: T::Origin, total: T::Balance) -> Result { - let origin = ensure_signed(origin)?; - - let id = Self::next_asset_id(); - >::mutate(|id| *id += 1); - - - >::insert((id, origin.clone()), total); - - Self::deposit_event(RawEvent::Issued(id, origin, total)); - Ok(()) - } - - fn transfer(origin: T::Origin, id: AssetId, target: T::AccountId, amount: T::Balance) -> Result { - let origin = ensure_signed(origin)?; - let origin_account = (id, origin.clone()); - let origin_balance = >::get(&origin_account); - ensure!(origin_balance >= amount, "origin account balance must be greater than amount"); - - Self::deposit_event(RawEvent::Transfered(id, origin, target.clone(), amount)); - >::insert(origin_account, origin_balance - amount); - >::mutate((id, target), |balance| *balance += amount); - - Ok(()) - } - - fn destroy(origin: T::Origin, id: AssetId) -> Result { - let origin = ensure_signed(origin)?; - - let balance = >::take((id, origin.clone())); - ensure!(!balance.is_zero(), "origin balance should be non-zero"); - - Self::deposit_event(RawEvent::Destroyed(id, origin, balance)); - - Ok(()) - } } #[cfg(test)] diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 53563f5d6636d..83eb4081327d4 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -126,8 +126,63 @@ pub trait Trait: system::Trait { decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; - fn transfer(origin, dest: RawAddress, value: ::Type) -> Result; - fn set_balance(who: RawAddress, free: ::Type, reserved: ::Type) -> Result; + + /// Transfer some liquid free balance to another staker. + pub fn transfer( + origin, + dest: RawAddress, + value: ::Type + ) -> Result { + let transactor = ensure_signed(origin)?; + + let dest = Self::lookup(dest)?; + let value = value.into(); + let from_balance = Self::free_balance(&transactor); + let to_balance = Self::free_balance(&dest); + let would_create = to_balance.is_zero(); + let fee = if would_create { Self::creation_fee() } else { Self::transfer_fee() }; + let liability = match value.checked_add(&fee) { + Some(l) => l, + None => return Err("got overflow after adding a fee to value"), + }; + + let new_from_balance = match from_balance.checked_sub(&liability) { + Some(b) => b, + None => return Err("balance too low to send value"), + }; + if would_create && value < Self::existential_deposit() { + return Err("value too low to create account"); + } + T::EnsureAccountLiquid::ensure_account_liquid(&transactor)?; + + // NOTE: total stake being stored in the same type means that this could never overflow + // but better to be safe than sorry. + let new_to_balance = match to_balance.checked_add(&value) { + Some(b) => b, + None => return Err("destination balance too high to receive value"), + }; + + if transactor != dest { + Self::set_free_balance(&transactor, new_from_balance); + Self::decrease_total_stake_by(fee); + Self::set_free_balance_creating(&dest, new_to_balance); + Self::deposit_event(RawEvent::Transfer(transactor, dest, value, fee)); + } + + Ok(()) + } + + /// Set the balances of a given account. + fn set_balance( + who: RawAddress, + free: ::Type, + reserved: ::Type + ) -> Result { + let who = Self::lookup(who)?; + Self::set_free_balance(&who, free.into()); + Self::set_reserved_balance(&who, reserved.into()); + Ok(()) + } } } @@ -280,58 +335,7 @@ impl Module { } } - // PUBLIC DISPATCH - - /// Transfer some liquid free balance to another staker. - pub fn transfer(origin: T::Origin, dest: Address, value: ::Type) -> Result { - let transactor = ensure_signed(origin)?; - - let dest = Self::lookup(dest)?; - let value = value.into(); - let from_balance = Self::free_balance(&transactor); - let to_balance = Self::free_balance(&dest); - let would_create = to_balance.is_zero(); - let fee = if would_create { Self::creation_fee() } else { Self::transfer_fee() }; - let liability = match value.checked_add(&fee) { - Some(l) => l, - None => return Err("got overflow after adding a fee to value"), - }; - - let new_from_balance = match from_balance.checked_sub(&liability) { - Some(b) => b, - None => return Err("balance too low to send value"), - }; - if would_create && value < Self::existential_deposit() { - return Err("value too low to create account"); - } - T::EnsureAccountLiquid::ensure_account_liquid(&transactor)?; - - // NOTE: total stake being stored in the same type means that this could never overflow - // but better to be safe than sorry. - let new_to_balance = match to_balance.checked_add(&value) { - Some(b) => b, - None => return Err("destination balance too high to receive value"), - }; - - if transactor != dest { - Self::set_free_balance(&transactor, new_from_balance); - Self::decrease_total_stake_by(fee); - Self::set_free_balance_creating(&dest, new_to_balance); - Self::deposit_event(RawEvent::Transfer(transactor, dest, value, fee)); - } - - Ok(()) - } - - /// Set the balances of a given account. - fn set_balance(who: Address, free: ::Type, reserved: ::Type) -> Result { - let who = Self::lookup(who)?; - Self::set_free_balance(&who, free.into()); - Self::set_reserved_balance(&who, reserved.into()); - Ok(()) - } - - // PUBLIC MUTABLES (DANGEROUS) + //PUBLIC MUTABLES (DANGEROUS) /// Set the free balance of an account to some new value. /// diff --git a/srml/consensus/src/lib.rs b/srml/consensus/src/lib.rs index ba72df18f57d1..6975fe792ca2a 100644 --- a/srml/consensus/src/lib.rs +++ b/srml/consensus/src/lib.rs @@ -143,11 +143,51 @@ decl_storage! { decl_module! { pub struct Module for enum Call where origin: T::Origin { - fn report_misbehavior(origin, report: Vec) -> Result; - fn note_offline(origin, offline_val_indices: Vec) -> Result; - fn remark(origin, remark: Vec) -> Result; - fn set_code(new: Vec) -> Result; - fn set_storage(items: Vec) -> Result; + /// Report some misbehaviour. + fn report_misbehavior(origin, _report: Vec) -> Result { + ensure_signed(origin)?; + // TODO. + Ok(()) + } + + /// Note the previous block's validator missed their opportunity to propose a block. + /// This only comes in if 2/3+1 of the validators agree that no proposal was submitted. + /// It's only relevant for the previous block. + fn note_offline(origin, offline_val_indices: Vec) -> Result { + ensure_inherent(origin)?; + assert!( + >::extrinsic_index() == Some(T::NOTE_OFFLINE_POSITION), + "note_offline extrinsic must be at position {} in the block", + T::NOTE_OFFLINE_POSITION + ); + + for validator_index in offline_val_indices.into_iter() { + T::OnOfflineValidator::on_offline_validator(validator_index as usize); + } + + Ok(()) + } + + /// Make some on-chain remark. + fn remark(origin, _remark: Vec) -> Result { + ensure_signed(origin)?; + Ok(()) + } + + /// Set the new code. + fn set_code(new: Vec) -> Result { + storage::unhashed::put_raw(well_known_keys::CODE, &new); + Ok(()) + } + + /// Set some items of storage. + fn set_storage(items: Vec) -> Result { + for i in &items { + storage::unhashed::put_raw(&i.0, &i.1); + } + Ok(()) + } + fn on_finalise() { if let Some(original_authorities) = >::take() { let current_authorities = AuthorityStorageVec::::items(); @@ -165,51 +205,6 @@ impl Module { AuthorityStorageVec::::items() } - /// Set the new code. - fn set_code(new: Vec) -> Result { - storage::unhashed::put_raw(well_known_keys::CODE, &new); - Ok(()) - } - - /// Set some items of storage. - 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(origin: T::Origin, _report: Vec) -> Result { - ensure_signed(origin)?; - // TODO. - Ok(()) - } - - /// Note the previous block's validator missed their opportunity to propose a block. This only comes in - /// if 2/3+1 of the validators agree that no proposal was submitted. It's only relevant - /// for the previous block. - fn note_offline(origin: T::Origin, offline_val_indices: Vec) -> Result { - ensure_inherent(origin)?; - assert!( - >::extrinsic_index() == Some(T::NOTE_OFFLINE_POSITION), - "note_offline extrinsic must be at position {} in the block", - T::NOTE_OFFLINE_POSITION - ); - - for validator_index in offline_val_indices.into_iter() { - T::OnOfflineValidator::on_offline_validator(validator_index as usize); - } - - Ok(()) - } - - /// Make some on-chain remark. - fn remark(origin: T::Origin, _remark: Vec) -> Result { - ensure_signed(origin)?; - Ok(()) - } - /// Set the current set of authorities' session keys. /// /// Called by `next_session` only. diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 2175ee94b1d14..30e82f4923cb5 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -153,21 +153,102 @@ decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; // TODO: Change AccountId to staking::Address + /// Make a call to a specified account, optionally transferring some balance. fn call( origin, dest: T::AccountId, value: ::Type, gas_limit: ::Type, data: Vec - ) -> Result; + ) -> Result { + let origin = ensure_signed(origin)?; + let value = value.into(); + let gas_limit = gas_limit.into(); + + // Pay for the gas upfront. + // + // NOTE: it is very important to avoid any state changes before + // paying for the gas. + let mut gas_meter = gas::buy_gas::(&origin, gas_limit)?; + + let mut ctx = ExecutionContext { + self_account: origin.clone(), + depth: 0, + overlay: OverlayAccountDb::::new(&account_db::DirectAccountDb), + events: Vec::new(), + }; + + let mut output_data = Vec::new(); + let result = ctx.call(origin.clone(), dest, value, &mut gas_meter, &data, &mut output_data); + + if let Ok(_) = result { + // Commit all changes that made it thus far into the persistant storage. + account_db::DirectAccountDb.commit(ctx.overlay.into_change_set()); + + // Then deposit all events produced. + ctx.events.into_iter().for_each(Self::deposit_event); + } + + // Refund cost of the unused gas. + // + // NOTE: this should go after the commit to the storage, since the storage changes + // can alter the balance of the caller. + gas::refund_unused_gas::(&origin, gas_meter); + + result.map(|_| ()) + } + /// Create a new contract, optionally transfering some balance to the created account. + /// + /// Creation is executed as follows: + /// + /// - the destination address is computed based on the sender and hash of the code. + /// - account is created at the computed address. + /// - the `ctor_code` is executed in the context of the newly created account. Buffer returned + /// after the execution is saved as the `code` of the account. That code will be invoked + /// upon any message received by this account. fn create( origin, - value: ::Type, + endowment: ::Type, gas_limit: ::Type, - init_code: Vec, + ctor_code: Vec, data: Vec - ) -> Result; + ) -> Result { + let origin = ensure_signed(origin)?; + let endowment = endowment.into(); + let gas_limit = gas_limit.into(); + + // Pay for the gas upfront. + // + // NOTE: it is very important to avoid any state changes before + // paying for the gas. + let mut gas_meter = gas::buy_gas::(&origin, gas_limit)?; + + let mut ctx = ExecutionContext { + self_account: origin.clone(), + depth: 0, + overlay: OverlayAccountDb::::new(&account_db::DirectAccountDb), + events: Vec::new(), + }; + let result = ctx.create(origin.clone(), endowment, &mut gas_meter, &ctor_code, &data); + + if let Ok(_) = result { + // Commit all changes that made it thus far into the persistant storage. + account_db::DirectAccountDb.commit(ctx.overlay.into_change_set()); + + // Then deposit all events produced. + ctx.events.into_iter().for_each(Self::deposit_event); + } + + // Refund cost of the unused gas. + // + // NOTE: this should go after the commit to the storage, since the storage changes + // can alter the balance of the caller. + gas::refund_unused_gas::(&origin, gas_meter); + + result.map(|_| ()) + } + fn on_finalise() { >::kill(); } @@ -221,104 +302,6 @@ impl double_map::StorageDoubleMap for StorageOf { type Value = Vec; } -impl Module { - /// Make a call to a specified account, optionally transferring some balance. - fn call( - origin: ::Origin, - dest: T::AccountId, - value: ::Type, - gas_limit: ::Type, - data: Vec, - ) -> Result { - let origin = ensure_signed(origin)?; - let value = value.into(); - let gas_limit = gas_limit.into(); - - // Pay for the gas upfront. - // - // NOTE: it is very important to avoid any state changes before - // paying for the gas. - let mut gas_meter = gas::buy_gas::(&origin, gas_limit)?; - - let mut ctx = ExecutionContext { - self_account: origin.clone(), - depth: 0, - overlay: OverlayAccountDb::::new(&account_db::DirectAccountDb), - events: Vec::new(), - }; - - let mut output_data = Vec::new(); - let result = ctx.call(origin.clone(), dest, value, &mut gas_meter, &data, &mut output_data); - - if let Ok(_) = result { - // Commit all changes that made it thus far into the persistant storage. - account_db::DirectAccountDb.commit(ctx.overlay.into_change_set()); - - // Then deposit all events produced. - ctx.events.into_iter().for_each(Self::deposit_event); - } - - // Refund cost of the unused gas. - // - // NOTE: this should go after the commit to the storage, since the storage changes - // can alter the balance of the caller. - gas::refund_unused_gas::(&origin, gas_meter); - - result.map(|_| ()) - } - - /// Create a new contract, optionally transfering some balance to the created account. - /// - /// Creation is executed as follows: - /// - /// - the destination address is computed based on the sender and hash of the code. - /// - account is created at the computed address. - /// - the `ctor_code` is executed in the context of the newly created account. Buffer returned - /// after the execution is saved as the `code` of the account. That code will be invoked - /// upon any message received by this account. - fn create( - origin: ::Origin, - endowment: ::Type, - gas_limit: ::Type, - ctor_code: Vec, - data: Vec, - ) -> Result { - let origin = ensure_signed(origin)?; - let endowment = endowment.into(); - let gas_limit = gas_limit.into(); - - // Pay for the gas upfront. - // - // NOTE: it is very important to avoid any state changes before - // paying for the gas. - let mut gas_meter = gas::buy_gas::(&origin, gas_limit)?; - - let mut ctx = ExecutionContext { - self_account: origin.clone(), - depth: 0, - overlay: OverlayAccountDb::::new(&account_db::DirectAccountDb), - events: Vec::new(), - }; - let result = ctx.create(origin.clone(), endowment, &mut gas_meter, &ctor_code, &data); - - if let Ok(_) = result { - // Commit all changes that made it thus far into the persistant storage. - account_db::DirectAccountDb.commit(ctx.overlay.into_change_set()); - - // Then deposit all events produced. - ctx.events.into_iter().for_each(Self::deposit_event); - } - - // Refund cost of the unused gas. - // - // NOTE: this should go after the commit to the storage, since the storage changes - // can alter the balance of the caller. - gas::refund_unused_gas::(&origin, gas_meter); - - result.map(|_| ()) - } -} - impl balances::OnFreeBalanceZero for Module { fn on_free_balance_zero(who: &T::AccountId) { >::remove(who); diff --git a/srml/council/src/motions.rs b/srml/council/src/motions.rs index d57af36faed88..a2c5115633f19 100644 --- a/srml/council/src/motions.rs +++ b/srml/council/src/motions.rs @@ -69,8 +69,95 @@ decl_module! { #[cfg_attr(feature = "std", serde(bound(deserialize = "::Proposal: ::serde::de::DeserializeOwned")))] pub struct Module for enum Call where origin: ::Origin { fn deposit_event() = default; - fn propose(origin, threshold: Compact, proposal: Box<::Proposal>) -> Result; - fn vote(origin, proposal: T::Hash, index: Compact, approve: bool) -> Result; + fn propose(origin, threshold: Compact, proposal: Box<::Proposal>) -> Result { + let who = ensure_signed(origin)?; + let threshold = threshold.into(); + + ensure!(Self::is_councillor(&who), "proposer not on council"); + + let proposal_hash = T::Hashing::hash_of(&proposal); + + ensure!(!>::exists(proposal_hash), "duplicate proposals not allowed"); + + if threshold < 2 { + let ok = proposal.dispatch(Origin::Members(1).into()).is_ok(); + Self::deposit_event(RawEvent::Executed(proposal_hash, ok)); + } else { + let index = Self::proposal_count(); + >::mutate(|i| *i += 1); + >::mutate(|proposals| proposals.push(proposal_hash)); + >::insert(proposal_hash, *proposal); + >::insert(proposal_hash, (index, threshold, vec![who.clone()], vec![])); + + Self::deposit_event(RawEvent::Proposed(who, index, proposal_hash, threshold)); + } + Ok(()) + } + + fn vote(origin, proposal: T::Hash, index: Compact, approve: bool) -> Result { + let who = ensure_signed(origin)?; + let index = index.into(); + + ensure!(Self::is_councillor(&who), "voter not on council"); + + let mut voting = Self::voting(&proposal).ok_or("proposal must exist")?; + ensure!(voting.0 == index, "mismatched index"); + + let position_yes = voting.2.iter().position(|a| a == &who); + let position_no = voting.3.iter().position(|a| a == &who); + + if approve { + if position_yes.is_none() { + voting.2.push(who.clone()); + } else { + return Err("duplicate vote ignored") + } + if let Some(pos) = position_no { + voting.3.swap_remove(pos); + } + } else { + if position_no.is_none() { + voting.3.push(who.clone()); + } else { + return Err("duplicate vote ignored") + } + if let Some(pos) = position_yes { + voting.2.swap_remove(pos); + } + } + + let yes_votes = voting.2.len() as u32; + let no_votes = voting.3.len() as u32; + Self::deposit_event(RawEvent::Voted(who, proposal, approve, yes_votes, no_votes)); + + let threshold = voting.1; + let potential_votes = >::active_council().len() as u32; + let approved = yes_votes >= threshold; + let disapproved = potential_votes.saturating_sub(no_votes) < threshold; + if approved || disapproved { + if approved { + Self::deposit_event(RawEvent::Approved(proposal)); + + // execute motion, assuming it exists. + if let Some(p) = >::take(&proposal) { + let ok = p.dispatch(Origin::Members(threshold).into()).is_ok(); + Self::deposit_event(RawEvent::Executed(proposal, ok)); + } + } else { + // disapproved + Self::deposit_event(RawEvent::Disapproved(proposal)); + } + + // remove vote + >::remove(&proposal); + >::mutate(|proposals| proposals.retain(|h| h != &proposal)); + } else { + // update voting + >::insert(&proposal, voting); + } + + Ok(()) + } } } @@ -96,97 +183,6 @@ impl Module { >::active_council().iter() .any(|&(ref a, _)| a == who) } - - // Dispatch - fn propose(origin: ::Origin, threshold: Compact, proposal: Box<::Proposal>) -> Result { - let who = ensure_signed(origin)?; - let threshold = threshold.into(); - - ensure!(Self::is_councillor(&who), "proposer not on council"); - - let proposal_hash = T::Hashing::hash_of(&proposal); - - ensure!(!>::exists(proposal_hash), "duplicate proposals not allowed"); - - if threshold < 2 { - let ok = proposal.dispatch(Origin::Members(1).into()).is_ok(); - Self::deposit_event(RawEvent::Executed(proposal_hash, ok)); - } else { - let index = Self::proposal_count(); - >::mutate(|i| *i += 1); - >::mutate(|proposals| proposals.push(proposal_hash)); - >::insert(proposal_hash, *proposal); - >::insert(proposal_hash, (index, threshold, vec![who.clone()], vec![])); - - Self::deposit_event(RawEvent::Proposed(who, index, proposal_hash, threshold)); - } - Ok(()) - } - - fn vote(origin: ::Origin, proposal: T::Hash, index: Compact, approve: bool) -> Result { - let who = ensure_signed(origin)?; - let index = index.into(); - - ensure!(Self::is_councillor(&who), "voter not on council"); - - let mut voting = Self::voting(&proposal).ok_or("proposal must exist")?; - ensure!(voting.0 == index, "mismatched index"); - - let position_yes = voting.2.iter().position(|a| a == &who); - let position_no = voting.3.iter().position(|a| a == &who); - - if approve { - if position_yes.is_none() { - voting.2.push(who.clone()); - } else { - return Err("duplicate vote ignored") - } - if let Some(pos) = position_no { - voting.3.swap_remove(pos); - } - } else { - if position_no.is_none() { - voting.3.push(who.clone()); - } else { - return Err("duplicate vote ignored") - } - if let Some(pos) = position_yes { - voting.2.swap_remove(pos); - } - } - - let yes_votes = voting.2.len() as u32; - let no_votes = voting.3.len() as u32; - Self::deposit_event(RawEvent::Voted(who, proposal, approve, yes_votes, no_votes)); - - let threshold = voting.1; - let potential_votes = >::active_council().len() as u32; - let approved = yes_votes >= threshold; - let disapproved = potential_votes.saturating_sub(no_votes) < threshold; - if approved || disapproved { - if approved { - Self::deposit_event(RawEvent::Approved(proposal)); - - // execute motion, assuming it exists. - if let Some(p) = >::take(&proposal) { - let ok = p.dispatch(Origin::Members(threshold).into()).is_ok(); - Self::deposit_event(RawEvent::Executed(proposal, ok)); - } - } else { - // disapproved - Self::deposit_event(RawEvent::Disapproved(proposal)); - } - - // remove vote - >::remove(&proposal); - >::mutate(|proposals| proposals.retain(|h| h != &proposal)); - } else { - // update voting - >::insert(&proposal, voting); - } - - Ok(()) - } } /// Ensure that the origin `o` represents at least `n` council members. Returns diff --git a/srml/council/src/seats.rs b/srml/council/src/seats.rs index 62e57974bb919..957b69bdfe0a8 100644 --- a/srml/council/src/seats.rs +++ b/srml/council/src/seats.rs @@ -88,16 +88,227 @@ pub trait Trait: democracy::Trait { decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; - fn set_approvals(origin, votes: Vec, index: Compact) -> Result; - fn reap_inactive_voter(origin, reporter_index: Compact, who: Address, who_index: Compact, assumed_vote_index: Compact) -> Result; - fn retract_voter(origin, index: Compact) -> Result; - fn submit_candidacy(origin, slot: Compact) -> Result; - fn present_winner(origin, candidate: Address, total: ::Type, index: Compact) -> Result; - - fn set_desired_seats(count: Compact) -> Result; - fn remove_member(who: Address) -> Result; - fn set_presentation_duration(count: ::Type) -> Result; - fn set_term_duration(count: ::Type) -> Result; + + /// Set candidate approvals. Approval slots stay valid as long as candidates in those slots + /// are registered. + fn set_approvals(origin, votes: Vec, index: Compact) -> Result { + let who = ensure_signed(origin)?; + let index: VoteIndex = index.into(); + + ensure!(!Self::presentation_active(), "no approval changes during presentation period"); + ensure!(index == Self::vote_index(), "incorrect vote index"); + if !>::exists(&who) { + // not yet a voter - deduct bond. + // NOTE: this must be the last potential bailer, since it changes state. + >::reserve(&who, Self::voting_bond())?; + + >::put({ + let mut v = Self::voters(); + v.push(who.clone()); + v + }); + } + >::insert(&who, index); + >::insert(&who, votes); + Ok(()) + } + + /// Remove a voter. For it not to be a bond-consuming no-op, all approved candidate indices + /// must now be either unregistered or registered to a candidate that registered the slot after + /// the voter gave their last approval set. + /// + /// May be called by anyone. Returns the voter deposit to `signed`. + fn reap_inactive_voter( + origin, + reporter_index: Compact, + who: Address, + who_index: Compact, + assumed_vote_index: Compact + ) -> Result { + let reporter = ensure_signed(origin)?; + let assumed_vote_index: VoteIndex = assumed_vote_index.into(); + + let who = >::lookup(who)?; + ensure!(!Self::presentation_active(), "cannot reap during presentation period"); + ensure!(Self::voter_last_active(&reporter).is_some(), "reporter must be a voter"); + 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(); + let reporter_index: u32 = reporter_index.into(); + let reporter_index = reporter_index as usize; + let who_index: u32 = who_index.into(); + let who_index = who_index as usize; + ensure!(reporter_index < voters.len() && voters[reporter_index] == reporter, "bad reporter index"); + ensure!(who_index < voters.len() && voters[who_index] == who, "bad target index"); + + // will definitely kill one of signed or who now. + + let valid = !Self::approvals_of(&who).iter() + .zip(Self::candidates().iter()) + .any(|(&appr, addr)| + appr && + *addr != T::AccountId::default() && + Self::candidate_reg_info(addr).map_or(false, |x| x.0 <= last_active)/*defensive only: all items in candidates list are registered*/ + ); + + Self::remove_voter( + if valid { &who } else { &reporter }, + if valid { who_index } else { reporter_index }, + voters + ); + if valid { + // This only fails if `who` doesn't exist, which it clearly must do since its the origin. + // Still, it's no more harmful to propagate any error at this point. + >::repatriate_reserved(&who, &reporter, Self::voting_bond())?; + Self::deposit_event(RawEvent::VoterReaped(who, reporter)); + } else { + >::slash_reserved(&reporter, Self::voting_bond()); + Self::deposit_event(RawEvent::BadReaperSlashed(reporter)); + } + Ok(()) + } + + /// Remove a voter. All votes are cancelled and the voter deposit is returned. + fn retract_voter(origin, index: Compact) -> Result { + let who = ensure_signed(origin)?; + + ensure!(!Self::presentation_active(), "cannot retract when presenting"); + ensure!(>::exists(&who), "cannot retract non-voter"); + let voters = Self::voters(); + let index: u32 = index.into(); + let index = index as usize; + ensure!(index < voters.len(), "retraction index invalid"); + ensure!(voters[index] == who, "retraction index mismatch"); + + Self::remove_voter(&who, index, voters); + >::unreserve(&who, Self::voting_bond()); + Ok(()) + } + + /// Submit oneself for candidacy. + /// + /// Account must have enough transferrable funds in it to pay the bond. + fn submit_candidacy(origin, slot: Compact) -> Result { + let who = ensure_signed(origin)?; + + ensure!(!Self::is_a_candidate(&who), "duplicate candidate submission"); + let slot: u32 = slot.into(); + let slot = slot as usize; + let count = Self::candidate_count() as usize; + let candidates = Self::candidates(); + ensure!( + (slot == count && count == candidates.len()) || + (slot < candidates.len() && candidates[slot] == T::AccountId::default()), + "invalid candidate slot" + ); + // NOTE: This must be last as it has side-effects. + >::reserve(&who, Self::candidacy_bond()) + .map_err(|_| "candidate has not enough funds")?; + + >::insert(&who, (Self::vote_index(), slot as u32)); + let mut candidates = candidates; + if slot == candidates.len() { + candidates.push(who); + } else { + candidates[slot] = who; + } + >::put(candidates); + >::put(count as u32 + 1); + 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( + origin, + candidate: Address, + total: ::Type, + index: Compact + ) -> Result { + let who = ensure_signed(origin)?; + let total = total.into(); + let index: VoteIndex = index.into(); + + let candidate = >::lookup(candidate)?; + ensure!(index == Self::vote_index(), "index not current"); + 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() as u64); + ensure!(>::can_slash(&who, bad_presentation_punishment), "presenter must have sufficient slashable funds"); + + 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) { + ensure!(p < expiring.len(), "candidate must not form a duplicated member if elected"); + } + + let (registered_since, candidate_index): (VoteIndex, u32) = + Self::candidate_reg_info(&candidate).ok_or("presented candidate must be current")?; + let actual_total = voters.iter() + .zip(stakes.iter()) + .filter_map(|(voter, stake)| + match Self::voter_last_active(voter) { + Some(b) if b >= registered_since => + Self::approvals_of(voter).get(candidate_index as usize) + .and_then(|approved| if *approved { Some(*stake) } else { None }), + _ => None, + }) + .fold(Zero::zero(), |acc, n| acc + n); + let dupe = leaderboard.iter().find(|&&(_, ref c)| c == &candidate).is_some(); + if total == actual_total && !dupe { + // insert into leaderboard + leaderboard[0] = (total, candidate); + leaderboard.sort_by_key(|&(t, _)| t); + >::put(leaderboard); + Ok(()) + } else { + // we can rest assured it will be Ok since we checked `can_slash` earlier; still + // better safe than sorry. + let _ = >::slash(&who, bad_presentation_punishment); + Err(if dupe { "duplicate presentation" } else { "incorrect total" }) + } + } + + /// 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: Compact) -> Result { + let count: u32 = count.into(); + >::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: Address) -> Result { + let who = >::lookup(who)?; + 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 currently a vote being presented for, will + /// invoke `finalise_vote`. + fn set_presentation_duration(count: ::Type) -> Result { + >::put(count.into()); + Ok(()) + } + + /// Set the presentation duration. If there is current a vote being presented for, will + /// invoke `finalise_vote`. + fn set_term_duration(count: ::Type) -> Result { + >::put(count.into()); + Ok(()) + } + fn on_finalise(n: T::BlockNumber) { if let Err(e) = Self::end_block(n) { print("Guru meditation"); @@ -225,230 +436,7 @@ impl Module { } } - // dispatch - - /// Set candidate approvals. Approval slots stay valid as long as candidates in those slots - /// are registered. - fn set_approvals(origin: T::Origin, votes: Vec, index: Compact) -> Result { - let who = ensure_signed(origin)?; - let index: VoteIndex = index.into(); - - ensure!(!Self::presentation_active(), "no approval changes during presentation period"); - ensure!(index == Self::vote_index(), "incorrect vote index"); - if !>::exists(&who) { - // not yet a voter - deduct bond. - // NOTE: this must be the last potential bailer, since it changes state. - >::reserve(&who, Self::voting_bond())?; - - >::put({ - let mut v = Self::voters(); - v.push(who.clone()); - v - }); - } - >::insert(&who, index); - >::insert(&who, votes); - Ok(()) - } - - /// Remove a voter. For it not to be a bond-consuming no-op, all approved candidate indices - /// must now be either unregistered or registered to a candidate that registered the slot after - /// the voter gave their last approval set. - /// - /// May be called by anyone. Returns the voter deposit to `signed`. - fn reap_inactive_voter( - origin: T::Origin, - reporter_index: Compact, - who: Address, - who_index: Compact, - assumed_vote_index: Compact - ) -> Result { - let reporter = ensure_signed(origin)?; - let assumed_vote_index: VoteIndex = assumed_vote_index.into(); - - let who = >::lookup(who)?; - ensure!(!Self::presentation_active(), "cannot reap during presentation period"); - ensure!(Self::voter_last_active(&reporter).is_some(), "reporter must be a voter"); - 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(); - let reporter_index: u32 = reporter_index.into(); - let reporter_index = reporter_index as usize; - let who_index: u32 = who_index.into(); - let who_index = who_index as usize; - ensure!(reporter_index < voters.len() && voters[reporter_index] == reporter, "bad reporter index"); - ensure!(who_index < voters.len() && voters[who_index] == who, "bad target index"); - - // will definitely kill one of signed or who now. - - let valid = !Self::approvals_of(&who).iter() - .zip(Self::candidates().iter()) - .any(|(&appr, addr)| - appr && - *addr != T::AccountId::default() && - Self::candidate_reg_info(addr).map_or(false, |x| x.0 <= last_active)/*defensive only: all items in candidates list are registered*/ - ); - - Self::remove_voter( - if valid { &who } else { &reporter }, - if valid { who_index } else { reporter_index }, - voters - ); - if valid { - // This only fails if `who` doesn't exist, which it clearly must do since its the origin. - // Still, it's no more harmful to propagate any error at this point. - >::repatriate_reserved(&who, &reporter, Self::voting_bond())?; - Self::deposit_event(RawEvent::VoterReaped(who, reporter)); - } else { - >::slash_reserved(&reporter, Self::voting_bond()); - Self::deposit_event(RawEvent::BadReaperSlashed(reporter)); - } - Ok(()) - } - - /// Remove a voter. All votes are cancelled and the voter deposit is returned. - fn retract_voter(origin: T::Origin, index: Compact) -> Result { - let who = ensure_signed(origin)?; - - ensure!(!Self::presentation_active(), "cannot retract when presenting"); - ensure!(>::exists(&who), "cannot retract non-voter"); - let voters = Self::voters(); - let index: u32 = index.into(); - let index = index as usize; - ensure!(index < voters.len(), "retraction index invalid"); - ensure!(voters[index] == who, "retraction index mismatch"); - - Self::remove_voter(&who, index, voters); - >::unreserve(&who, Self::voting_bond()); - Ok(()) - } - - /// Submit oneself for candidacy. - /// - /// Account must have enough transferrable funds in it to pay the bond. - fn submit_candidacy(origin: T::Origin, slot: Compact) -> Result { - let who = ensure_signed(origin)?; - - ensure!(!Self::is_a_candidate(&who), "duplicate candidate submission"); - let slot: u32 = slot.into(); - let slot = slot as usize; - let count = Self::candidate_count() as usize; - let candidates = Self::candidates(); - ensure!( - (slot == count && count == candidates.len()) || - (slot < candidates.len() && candidates[slot] == T::AccountId::default()), - "invalid candidate slot" - ); - // NOTE: This must be last as it has side-effects. - >::reserve(&who, Self::candidacy_bond()) - .map_err(|_| "candidate has not enough funds")?; - - >::insert(&who, (Self::vote_index(), slot as u32)); - let mut candidates = candidates; - if slot == candidates.len() { - candidates.push(who); - } else { - candidates[slot] = who; - } - >::put(candidates); - >::put(count as u32 + 1); - 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( - origin: T::Origin, - candidate: Address, - total: ::Type, - index: Compact - ) -> Result { - let who = ensure_signed(origin)?; - let total = total.into(); - let index: VoteIndex = index.into(); - - let candidate = >::lookup(candidate)?; - ensure!(index == Self::vote_index(), "index not current"); - 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() as u64); - ensure!(>::can_slash(&who, bad_presentation_punishment), "presenter must have sufficient slashable funds"); - - 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) { - ensure!(p < expiring.len(), "candidate must not form a duplicated member if elected"); - } - - let (registered_since, candidate_index): (VoteIndex, u32) = - Self::candidate_reg_info(&candidate).ok_or("presented candidate must be current")?; - let actual_total = voters.iter() - .zip(stakes.iter()) - .filter_map(|(voter, stake)| - match Self::voter_last_active(voter) { - Some(b) if b >= registered_since => - Self::approvals_of(voter).get(candidate_index as usize) - .and_then(|approved| if *approved { Some(*stake) } else { None }), - _ => None, - }) - .fold(Zero::zero(), |acc, n| acc + n); - let dupe = leaderboard.iter().find(|&&(_, ref c)| c == &candidate).is_some(); - if total == actual_total && !dupe { - // insert into leaderboard - leaderboard[0] = (total, candidate); - leaderboard.sort_by_key(|&(t, _)| t); - >::put(leaderboard); - Ok(()) - } else { - // we can rest assured it will be Ok since we checked `can_slash` earlier; still - // better safe than sorry. - let _ = >::slash(&who, bad_presentation_punishment); - Err(if dupe { "duplicate presentation" } else { "incorrect total" }) - } - } - - /// 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: Compact) -> Result { - let count: u32 = count.into(); - >::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: Address) -> Result { - let who = >::lookup(who)?; - 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 currently a vote being presented for, will - /// invoke `finalise_vote`. - fn set_presentation_duration(count: ::Type) -> Result { - >::put(count.into()); - Ok(()) - } - - /// Set the presentation duration. If there is current a vote being presented for, will - /// invoke `finalise_vote`. - fn set_term_duration(count: ::Type) -> Result { - >::put(count.into()); - Ok(()) - } - - // private - + // Private /// Check there's nothing to do this block fn end_block(block_number: T::BlockNumber) -> Result { if (block_number % Self::voting_period()).is_zero() { diff --git a/srml/council/src/voting.rs b/srml/council/src/voting.rs index 42363ac41defb..b598e8ca05257 100644 --- a/srml/council/src/voting.rs +++ b/srml/council/src/voting.rs @@ -34,12 +34,81 @@ pub trait Trait: CouncilTrait { decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; - fn propose(origin, proposal: Box) -> Result; - fn vote(origin, proposal: T::Hash, approve: bool) -> Result; - fn veto(origin, proposal_hash: T::Hash) -> Result; - fn set_cooloff_period(blocks: ::Type) -> Result; - fn set_voting_period(blocks: ::Type) -> Result; + fn propose(origin, proposal: Box) -> Result { + let who = ensure_signed(origin)?; + + let expiry = >::block_number() + Self::voting_period(); + ensure!(Self::will_still_be_councillor_at(&who, expiry), "proposer would not be on council"); + + let proposal_hash = T::Hashing::hash_of(&proposal); + + 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)); + proposals.sort_by_key(|&(expiry, _)| expiry); + Self::set_proposals(&proposals); + + >::insert(proposal_hash, *proposal); + >::insert(proposal_hash, vec![who.clone()]); + >::insert((proposal_hash, who.clone()), true); + + Ok(()) + } + + fn vote(origin, proposal: T::Hash, approve: bool) -> Result { + let who = ensure_signed(origin)?; + + ensure!(Self::is_councillor(&who), "only councillors may vote on council proposals"); + + if Self::vote_of((proposal, who.clone())).is_none() { + >::mutate(proposal, |voters| voters.push(who.clone())); + } + >::insert((proposal, who), approve); + Ok(()) + } + + fn veto(origin, proposal_hash: T::Hash) -> Result { + let who = ensure_signed(origin)?; + + ensure!(Self::is_councillor(&who), "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 = existing_vetoers.binary_search(&who) + .err().ok_or("a councillor may not veto a proposal twice")?; + existing_vetoers.insert(insert_position, who); + Self::set_veto_of( + &proposal_hash, + >::block_number() + Self::cooloff_period(), + existing_vetoers + ); + + Self::set_proposals( + &Self::proposals().into_iter().filter(|&(_, h)| h != proposal_hash + ).collect::>()); + >::remove(proposal_hash); + >::remove(proposal_hash); + for (c, _) in >::active_council() { + >::remove((proposal_hash, c)); + } + Ok(()) + } + + fn set_cooloff_period(blocks: ::Type) -> Result { + >::put(blocks.into()); + Ok(()) + } + + fn set_voting_period(blocks: ::Type) -> Result { + >::put(blocks.into()); + Ok(()) + } + fn on_finalise(n: T::BlockNumber) { if let Err(e) = Self::end_block(n) { print("Guru meditation"); @@ -96,78 +165,7 @@ impl Module { Self::generic_tally(proposal_hash, |w: &T::AccountId, p: &T::Hash| Self::vote_of((*p, w.clone()))) } - // Dispatch - fn propose(origin: T::Origin, proposal: Box) -> Result { - let who = ensure_signed(origin)?; - - let expiry = >::block_number() + Self::voting_period(); - ensure!(Self::will_still_be_councillor_at(&who, expiry), "proposer would not be on council"); - - let proposal_hash = T::Hashing::hash_of(&proposal); - - 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)); - proposals.sort_by_key(|&(expiry, _)| expiry); - Self::set_proposals(&proposals); - - >::insert(proposal_hash, *proposal); - >::insert(proposal_hash, vec![who.clone()]); - >::insert((proposal_hash, who.clone()), true); - - Ok(()) - } - - fn vote(origin: T::Origin, proposal: T::Hash, approve: bool) -> Result { - let who = ensure_signed(origin)?; - - ensure!(Self::is_councillor(&who), "only councillors may vote on council proposals"); - - if Self::vote_of((proposal, who.clone())).is_none() { - >::mutate(proposal, |voters| voters.push(who.clone())); - } - >::insert((proposal, who), approve); - Ok(()) - } - - fn veto(origin: T::Origin, proposal_hash: T::Hash) -> Result { - let who = ensure_signed(origin)?; - - ensure!(Self::is_councillor(&who), "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 = existing_vetoers.binary_search(&who) - .err().ok_or("a councillor may not veto a proposal twice")?; - existing_vetoers.insert(insert_position, who); - Self::set_veto_of(&proposal_hash, >::block_number() + Self::cooloff_period(), existing_vetoers); - - Self::set_proposals(&Self::proposals().into_iter().filter(|&(_, h)| h != proposal_hash).collect::>()); - >::remove(proposal_hash); - >::remove(proposal_hash); - for (c, _) in >::active_council() { - >::remove((proposal_hash, c)); - } - Ok(()) - } - - fn set_cooloff_period(blocks: ::Type) -> Result { - >::put(blocks.into()); - Ok(()) - } - - fn set_voting_period(blocks: ::Type) -> Result { - >::put(blocks.into()); - Ok(()) - } - - // private - - + // Private fn set_veto_of(proposal: &T::Hash, expiry: T::BlockNumber, vetoers: Vec) { >::insert(proposal, (expiry, vetoers)); } diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index f2abaf9581ddb..9acacb2ea9651 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -63,12 +63,73 @@ pub trait Trait: balances::Trait + Sized { decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; - fn propose(origin, proposal: Box, value: ::Type) -> Result; - fn second(origin, proposal: Compact) -> Result; - fn vote(origin, ref_index: Compact, approve_proposal: bool) -> Result; - fn start_referendum(proposal: Box, vote_threshold: VoteThreshold) -> Result; - fn cancel_referendum(ref_index: Compact) -> Result; + /// Propose a sensitive action to be taken. + fn propose( + origin, + proposal: Box, + value: ::Type + ) -> Result { + let who = ensure_signed(origin)?; + let value = value.into(); + + ensure!(value >= Self::minimum_deposit(), "value too low"); + >::reserve(&who, value) + .map_err(|_| "proposer's balance too low")?; + + let index = Self::public_prop_count(); + >::put(index + 1); + >::insert(index, (value, vec![who.clone()])); + + let mut props = Self::public_props(); + props.push((index, (*proposal).clone(), who)); + >::put(props); + Ok(()) + } + + /// Propose a sensitive action to be taken. + fn second(origin, proposal: Compact) -> Result { + let who = ensure_signed(origin)?; + let proposal: PropIndex = proposal.into(); + let mut deposit = Self::deposit_of(proposal) + .ok_or("can only second an existing proposal")?; + >::reserve(&who, deposit.0) + .map_err(|_| "seconder's balance too low")?; + deposit.1.push(who); + >::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(origin, ref_index: Compact, approve_proposal: bool) -> Result { + let who = ensure_signed(origin)?; + let ref_index = ref_index.into(); + ensure!(Self::is_active_referendum(ref_index), "vote given for invalid referendum."); + ensure!(!>::total_balance(&who).is_zero(), + "transactor must have balance to signal approval."); + if !>::exists(&(ref_index, who.clone())) { + >::mutate(ref_index, |voters| voters.push(who.clone())); + } + >::insert(&(ref_index, who), approve_proposal); + Ok(()) + } + + /// Start a referendum. + 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: Compact) -> Result { + Self::clear_referendum(ref_index.into()); + Ok(()) + } + fn on_finalise(n: T::BlockNumber) { if let Err(e) = Self::end_block(n) { runtime_io::print(e); @@ -162,71 +223,7 @@ impl Module { .fold((Zero::zero(), Zero::zero()), |(a, b), (c, d)| (a + c, b + d)) } - // dispatching. - - /// Propose a sensitive action to be taken. - fn propose(origin: T::Origin, proposal: Box, value: ::Type) -> Result { - let who = ensure_signed(origin)?; - let value = value.into(); - - ensure!(value >= Self::minimum_deposit(), "value too low"); - >::reserve(&who, value) - .map_err(|_| "proposer's balance too low")?; - - let index = Self::public_prop_count(); - >::put(index + 1); - >::insert(index, (value, vec![who.clone()])); - - let mut props = Self::public_props(); - props.push((index, (*proposal).clone(), who)); - >::put(props); - Ok(()) - } - - /// Propose a sensitive action to be taken. - fn second(origin: T::Origin, proposal: Compact) -> Result { - let who = ensure_signed(origin)?; - let proposal: PropIndex = proposal.into(); - let mut deposit = Self::deposit_of(proposal) - .ok_or("can only second an existing proposal")?; - >::reserve(&who, deposit.0) - .map_err(|_| "seconder's balance too low")?; - deposit.1.push(who); - >::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(origin: T::Origin, ref_index: Compact, approve_proposal: bool) -> Result { - let who = ensure_signed(origin)?; - let ref_index = ref_index.into(); - ensure!(Self::is_active_referendum(ref_index), "vote given for invalid referendum."); - ensure!(!>::total_balance(&who).is_zero(), - "transactor must have balance to signal approval."); - if !>::exists(&(ref_index, who.clone())) { - >::mutate(ref_index, |voters| voters.push(who.clone())); - } - >::insert(&(ref_index, who), approve_proposal); - Ok(()) - } - - /// Start a referendum. - 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: Compact) -> Result { - Self::clear_referendum(ref_index.into()); - Ok(()) - } - - // exposed mutables. + // Exposed mutables. /// Start a referendum. Can be called directly by the council. pub fn internal_start_referendum(proposal: T::Proposal, vote_threshold: VoteThreshold) -> result::Result { diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index aa643f5f769b2..64dc9d8bcd5ee 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -109,10 +109,84 @@ decl_module! { /// This is your public interface. Be extremely careful. /// This is just a simple example of how to interact with the module from the external /// world. - fn accumulate_dummy(origin, increase_by: T::Balance) -> Result; + // This just increases the value of `Dummy` by `increase_by`. + // + // Since this is a dispatched function there are two extremely important things to + // remember: + // + // - MUST NOT PANIC: Under no circumstances (save, perhaps, storage getting into an + // irreparably damaged state) must this function panic. + // - NO SIDE-EFFECTS ON ERROR: This function must either complete totally (and return + // `Ok(())` or it must have no side-effects on storage and return `Err('Some reason')`. + // + // The first is relatively easy to audit for - just ensure all panickers are removed from + // logic that executes in production (which you do anyway, right?!). To ensure the second + // is followed, you should do all tests for validity at the top of your function. This + // is stuff like checking the sender (`origin`) or that state is such that the operation + // makes sense. + // + // Once you've determined that it's all good, then enact the operation and change storage. + // If you can't be certain that the operation will succeed without substantial computation + // then you have a classic blockchain attack scenario. The normal way of managing this is + // to attach a bond to the operation. As the first major alteration of storage, reserve + // some value from the sender's account (`Balances` module has a `reserve` function for + // exactly this scenario). This amount should be enough to cover any costs of the + // substantial execution in case it turns out that you can't proceed with the operation. + // + // If it eventually transpires that the operation is fine and, therefore, that the + // expense of the checks should be borne by the network, then you can refund the reserved + // deposit. If, however, the operation turns out to be invalid and the computation is + // wasted, then you can burn it or repatriate elsewhere. + // + // Security bonds ensure that attackers can't game it by ensuring that anyone interacting + // with the system either progresses it or pays for the trouble of faffing around with + // no progress. + // + // If you don't respect these rules, it is likely that your chain will be attackable. + fn accumulate_dummy(origin, increase_by: T::Balance) -> Result { + // This is a public call, so we ensure that the origin is some signed account. + let _sender = ensure_signed(origin)?; + + // Read the value of dummy from storage. + // let dummy = Self::dummy(); + // Will also work using the `::get` on the storage item type itself: + // let dummy = >::get(); + + // Calculate the new value. + // let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by); + + // Put the new value into storage. + // >::put(new_dummy); + // Will also work with a reference: + // >::put(&new_dummy); + + // Here's the new one of read and then modify the value. + >::mutate(|dummy| { + let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by); + *dummy = Some(new_dummy); + }); + + // Let's deposit an event to let the outside world know this happened. + Self::deposit_event(RawEvent::Dummy(increase_by)); + + // All good. + Ok(()) + } /// A privileged call; in this case it resets our dummy value to something new. - fn set_dummy(new_dummy: T::Balance) -> Result; + // Implementation of a privileged call. This doesn't have an `origin` parameter because + // it's not (directly) from an extrinsic, but rather the system as a whole has decided + // to execute it. Different runtimes have different reasons for allow privileged + // calls to be executed - we don't need to care why. Because it's privileged, we can + // assume it's a one-off operation and substantial processing/storage/memory can be used + // without worrying about gameability or attack scenarios. + fn set_dummy(new_value: T::Balance) -> Result { + // Put the new value into storage. + >::put(new_value); + + // All good. + Ok(()) + } // The signature could also look like: `fn on_finalise()` fn on_finalise(_n: T::BlockNumber) { @@ -171,79 +245,11 @@ decl_storage! { // The main implementation block for the module. Functions here fall into three broad // categories: -// - Implementations of dispatch functions. The dispatch code generated by the module macro -// expects each of its functions to be implemented. // - Public interface. These are functions that are `pub` and generally fall into inspector // functions that do not write to storage and operation functions that do. // - Private functions. These are your usual private utilities unavailable to other modules. impl Module { - // Implement Calls and add public immutables and private mutables. - - // Implement dispatched function `accumulate_dummy`. This just increases the value - // of `Dummy` by `increase_by`. - // - // Since this is a dispatched function there are two extremely important things to - // remember: - // - // - MUST NOT PANIC: Under no circumstances (save, perhaps, storage getting into an - // irreparably damaged state) must this function panic. - // - NO SIDE-EFFECTS ON ERROR: This function must either complete totally (and return - // `Ok(())` or it must have no side-effects on storage and return `Err('Some reason')`. - // - // The first is relatively easy to audit for - just ensure all panickers are removed from - // logic that executes in production (which you do anyway, right?!). To ensure the second - // is followed, you should do all tests for validity at the top of your function. This - // is stuff like checking the sender (`origin`) or that state is such that the operation - // makes sense. - // - // Once you've determined that it's all good, then enact the operation and change storage. - // If you can't be certain that the operation will succeed without substantial computation - // then you have a classic blockchain attack scenario. The normal way of managing this is - // to attach a bond to the operation. As the first major alteration of storage, reserve - // some value from the sender's account (`Balances` module has a `reserve` function for - // exactly this scenario). This amount should be enough to cover any costs of the - // substantial execution in case it turns out that you can't proceed with the operation. - // - // If it eventually transpires that the operation is fine and, therefore, that the - // expense of the checks should be borne by the network, then you can refund the reserved - // deposit. If, however, the operation turns out to be invalid and the computation is - // wasted, then you can burn it or repatriate elsewhere. - // - // Security bonds ensure that attackers can't game it by ensuring that anyone interacting - // with the system either progresses it or pays for the trouble of faffing around with - // no progress. - // - // If you don't respect these rules, it is likely that your chain will be attackable. - fn accumulate_dummy(origin: T::Origin, increase_by: T::Balance) -> Result { - // This is a public call, so we ensure that the origin is some signed account. - let _sender = ensure_signed(origin)?; - - // Read the value of dummy from storage. - // let dummy = Self::dummy(); - // Will also work using the `::get` on the storage item type itself: - // let dummy = >::get(); - - // Calculate the new value. - // let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by); - - // Put the new value into storage. - // >::put(new_dummy); - // Will also work with a reference: - // >::put(&new_dummy); - - // Here's the new one of read and then modify the value. - >::mutate(|dummy| { - let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by); - *dummy = Some(new_dummy); - }); - - // Let's deposit an event to let the outside world know this happened. - Self::deposit_event(RawEvent::Dummy(increase_by)); - - // All good. - Ok(()) - } - + // Add public immutables and private mutables. #[allow(dead_code)] fn accumulate_foo(origin: T::Origin, increase_by: T::Balance) -> Result { let _sender = ensure_signed(origin)?; @@ -253,20 +259,6 @@ impl Module { Ok(()) } - - // Implementation of a privileged call. This doesn't have an `origin` parameter because - // it's not (directly) from an extrinsic, but rather the system as a whole has decided - // to execute it. Different runtimes have different reasons for allow privileged - // calls to be executed - we don't need to care why. Because it's privileged, we can - // assume it's a one-off operation and substantial processing/storage/memory can be used - // without worrying about gameability or attack scenarios. - fn set_dummy(new_value: T::Balance) -> Result { - // Put the new value into storage. - >::put(new_value); - - // All good. - Ok(()) - } } #[cfg(test)] diff --git a/srml/session/src/lib.rs b/srml/session/src/lib.rs index f125528a43512..cc75c75bc52b6 100644 --- a/srml/session/src/lib.rs +++ b/srml/session/src/lib.rs @@ -68,10 +68,27 @@ pub trait Trait: timestamp::Trait { decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; - fn set_key(origin, key: T::SessionKey) -> Result; - fn set_length(new: ::Type) -> Result; - fn force_new_session(apply_rewards: bool) -> Result; + /// Sets the session key of `_validator` to `_key`. This doesn't take effect until the next + /// session. + fn set_key(origin, key: T::SessionKey) -> Result { + let who = ensure_signed(origin)?; + // set new value for next session + >::insert(who, key); + Ok(()) + } + + /// Set a new session length. Won't kick in until the next session change (at current length). + fn set_length(new: ::Type) -> Result { + >::put(new.into()); + Ok(()) + } + + /// Forces a new session. + fn force_new_session(apply_rewards: bool) -> Result { + Self::apply_force_new_session(apply_rewards) + } + fn on_finalise(n: T::BlockNumber) { Self::check_rotate_session(n); } @@ -122,28 +139,7 @@ impl Module { >::get().unwrap_or_else(T::BlockNumber::zero) } - /// Sets the session key of `_validator` to `_key`. This doesn't take effect until the next - /// session. - fn set_key(origin: T::Origin, key: T::SessionKey) -> Result { - let who = ensure_signed(origin)?; - // set new value for next session - >::insert(who, key); - Ok(()) - } - - /// Set a new session length. Won't kick in until the next session change (at current length). - fn set_length(new: ::Type) -> Result { - >::put(new.into()); - Ok(()) - } - - /// Forces a new session. - pub fn force_new_session(apply_rewards: bool) -> Result { - Self::apply_force_new_session(apply_rewards) - } - // INTERNAL API (available to other runtime modules) - /// Forces a new session, no origin. pub fn apply_force_new_session(apply_rewards: bool) -> Result { >::put(apply_rewards); diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 27080e2335b7d..680da340c54c5 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -106,17 +106,139 @@ decl_module! { #[cfg_attr(feature = "std", serde(bound(deserialize = "T::Balance: ::serde::de::DeserializeOwned")))] pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; - fn stake(origin) -> Result; - fn unstake(origin, intentions_index: Compact) -> Result; - fn nominate(origin, target: Address) -> Result; - fn unnominate(origin, target_index: Compact) -> Result; - fn register_preferences(origin, intentions_index: Compact, prefs: ValidatorPrefs) -> Result; - - fn set_sessions_per_era(new: ::Type) -> Result; - fn set_bonding_duration(new: ::Type) -> Result; - fn set_validator_count(new: Compact) -> Result; - fn force_new_era(apply_rewards: bool) -> Result; - fn set_offline_slash_grace(new: Compact) -> Result; + + /// Declare the desire to stake for the transactor. + /// + /// Effects will be felt at the beginning of the next era. + fn stake(origin) -> Result { + let who = ensure_signed(origin)?; + ensure!(Self::nominating(&who).is_none(), "Cannot stake if already nominating."); + let mut intentions = >::get(); + // can't be in the list twice. + ensure!(intentions.iter().find(|&t| t == &who).is_none(), "Cannot stake if already staked."); + + >::insert(&who, T::BlockNumber::max_value()); + intentions.push(who); + >::put(intentions); + Ok(()) + } + + /// Retract the desire to stake for the transactor. + /// + /// Effects will be felt at the beginning of the next era. + fn unstake(origin, intentions_index: Compact) -> Result { + let who = ensure_signed(origin)?; + let intentions_index: u32 = intentions_index.into(); + // unstake fails in degenerate case of having too few existing staked parties + if Self::intentions().len() <= Self::minimum_validator_count() as usize { + return Err("cannot unstake when there are too few staked participants") + } + Self::apply_unstake(&who, intentions_index as usize) + } + + fn nominate(origin, target: Address) -> Result { + let who = ensure_signed(origin)?; + let target = >::lookup(target)?; + + ensure!(Self::nominating(&who).is_none(), "Cannot nominate if already nominating."); + ensure!(Self::intentions().iter().find(|&t| t == &who).is_none(), "Cannot nominate if already staked."); + + // update nominators_for + let mut t = Self::nominators_for(&target); + t.push(who.clone()); + >::insert(&target, t); + + // update nominating + >::insert(&who, &target); + + // Update bondage + >::insert(&who, T::BlockNumber::max_value()); + + Ok(()) + } + + /// Will panic if called when source isn't currently nominating target. + /// Updates Nominating, NominatorsFor and NominationBalance. + fn unnominate(origin, target_index: Compact) -> Result { + let source = ensure_signed(origin)?; + let target_index: u32 = target_index.into(); + let target_index = target_index as usize; + + let target = >::get(&source).ok_or("Account must be nominating")?; + + let mut t = Self::nominators_for(&target); + if t.get(target_index) != Some(&source) { + return Err("Invalid target index") + } + + // Ok - all valid. + + // update nominators_for + t.swap_remove(target_index); + >::insert(&target, t); + + // update nominating + >::remove(&source); + + // update bondage + >::insert( + source, + >::block_number() + Self::bonding_duration() + ); + Ok(()) + } + + /// Set the given account's preference for slashing behaviour should they be a validator. + /// + /// An error (no-op) if `Self::intentions()[intentions_index] != origin`. + fn register_preferences( + origin, + intentions_index: Compact, + prefs: ValidatorPrefs + ) -> Result { + let who = ensure_signed(origin)?; + let intentions_index: u32 = intentions_index.into(); + + if Self::intentions().get(intentions_index as usize) != Some(&who) { + return Err("Invalid index") + } + + >::insert(who, prefs); + + Ok(()) + } + + /// Set the number of sessions in an era. + fn set_sessions_per_era(new: ::Type) -> Result { + >::put(new.into()); + Ok(()) + } + + /// The length of the bonding duration in eras. + fn set_bonding_duration(new: ::Type) -> Result { + >::put(new.into()); + Ok(()) + } + + /// The length of a staking era in sessions. + fn set_validator_count(new: Compact) -> Result { + let new: u32 = new.into(); + >::put(new); + Ok(()) + } + + /// Force there to be a new era. This also forces a new session immediately after. + /// `apply_rewards` should be true for validators to get the session reward. + fn force_new_era(apply_rewards: bool) -> Result { + Self::apply_force_new_era(apply_rewards) + } + + /// Set the offline slash grace period. + fn set_offline_slash_grace(new: Compact) -> Result { + let new: u32 = new.into(); + >::put(new); + Ok(()) + } } } @@ -190,6 +312,12 @@ decl_storage! { } impl Module { + // Just force_new_era without origin check. + fn apply_force_new_era(apply_rewards: bool) -> Result { + >::put(()); + >::apply_force_new_session(apply_rewards) + } + // PUBLIC IMMUTABLES /// The length of a staking era in blocks. @@ -220,147 +348,6 @@ impl Module { } } - // PUBLIC DISPATCH - - /// Declare the desire to stake for the transactor. - /// - /// Effects will be felt at the beginning of the next era. - fn stake(origin: T::Origin) -> Result { - let who = ensure_signed(origin)?; - ensure!(Self::nominating(&who).is_none(), "Cannot stake if already nominating."); - let mut intentions = >::get(); - // can't be in the list twice. - ensure!(intentions.iter().find(|&t| t == &who).is_none(), "Cannot stake if already staked."); - - >::insert(&who, T::BlockNumber::max_value()); - intentions.push(who); - >::put(intentions); - Ok(()) - } - - /// Retract the desire to stake for the transactor. - /// - /// Effects will be felt at the beginning of the next era. - fn unstake(origin: T::Origin, intentions_index: Compact) -> Result { - let who = ensure_signed(origin)?; - let intentions_index: u32 = intentions_index.into(); - // unstake fails in degenerate case of having too few existing staked parties - if Self::intentions().len() <= Self::minimum_validator_count() as usize { - return Err("cannot unstake when there are too few staked participants") - } - Self::apply_unstake(&who, intentions_index as usize) - } - - fn nominate(origin: T::Origin, target: Address) -> Result { - let who = ensure_signed(origin)?; - let target = >::lookup(target)?; - - ensure!(Self::nominating(&who).is_none(), "Cannot nominate if already nominating."); - ensure!(Self::intentions().iter().find(|&t| t == &who).is_none(), "Cannot nominate if already staked."); - - // update nominators_for - let mut t = Self::nominators_for(&target); - t.push(who.clone()); - >::insert(&target, t); - - // update nominating - >::insert(&who, &target); - - // Update bondage - >::insert(&who, T::BlockNumber::max_value()); - - Ok(()) - } - - /// Will panic if called when source isn't currently nominating target. - /// Updates Nominating, NominatorsFor and NominationBalance. - fn unnominate(origin: T::Origin, target_index: Compact) -> Result { - let source = ensure_signed(origin)?; - let target_index: u32 = target_index.into(); - let target_index = target_index as usize; - - let target = >::get(&source).ok_or("Account must be nominating")?; - - let mut t = Self::nominators_for(&target); - if t.get(target_index) != Some(&source) { - return Err("Invalid target index") - } - - // Ok - all valid. - - // update nominators_for - t.swap_remove(target_index); - >::insert(&target, t); - - // update nominating - >::remove(&source); - - // update bondage - >::insert(source, >::block_number() + Self::bonding_duration()); - Ok(()) - } - - /// Set the given account's preference for slashing behaviour should they be a validator. - /// - /// An error (no-op) if `Self::intentions()[intentions_index] != origin`. - fn register_preferences( - origin: T::Origin, - intentions_index: Compact, - prefs: ValidatorPrefs - ) -> Result { - let who = ensure_signed(origin)?; - let intentions_index: u32 = intentions_index.into(); - - if Self::intentions().get(intentions_index as usize) != Some(&who) { - return Err("Invalid index") - } - - >::insert(who, prefs); - - Ok(()) - } - - // PRIV DISPATCH - - /// Set the number of sessions in an era. - fn set_sessions_per_era(new: ::Type) -> Result { - >::put(new.into()); - Ok(()) - } - - /// The length of the bonding duration in eras. - fn set_bonding_duration(new: ::Type) -> Result { - >::put(new.into()); - Ok(()) - } - - /// The length of a staking era in sessions. - fn set_validator_count(new: Compact) -> Result { - let new: u32 = new.into(); - >::put(new); - Ok(()) - } - - /// Force there to be a new era. This also forces a new session immediately after. - /// `apply_rewards` should be true for validators to get the session reward. - fn force_new_era(apply_rewards: bool) -> Result { - Self::apply_force_new_era(apply_rewards) - } - - // Just force_new_era without origin check. - fn apply_force_new_era(apply_rewards: bool) -> Result { - >::put(()); - >::apply_force_new_session(apply_rewards) - } - - - /// Set the offline slash grace period. - fn set_offline_slash_grace(new: Compact) -> Result { - let new: u32 = new.into(); - >::put(new); - Ok(()) - } - // PUBLIC MUTABLES (DANGEROUS) /// Slash a given validator by a specific amount. Removes the slash from their balance by preference, diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index a19bb67cb3fc1..8a131336ae717 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -171,7 +171,7 @@ macro_rules! decl_module { { $( $on_finalise:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* - fn $fn_name:ident(origin $(, $param_name:ident : $param:ty)* ) -> $result:ty ; + $fn_vis:vis fn $fn_name:ident($origin:ident $(, $param_name:ident : $param:ty)* ) -> $result:ty { $( $impl:tt )* } $($rest:tt)* ) => { decl_module!(@normalize @@ -180,7 +180,11 @@ macro_rules! decl_module { for enum $call_type where origin: $origin_type, system = $system { $( $deposit_event )* } { $( $on_finalise )* } - [ $($t)* $(#[doc = $doc_attr])* fn $fn_name(origin $( , $param_name : $param )* ) -> $result; ] + [ + $($t)* + $(#[doc = $doc_attr])* + $fn_vis fn $fn_name($origin $( , $param_name : $param )* ) -> $result { $( $impl )* } + ] $($rest)* ); }; @@ -192,7 +196,7 @@ macro_rules! decl_module { { $( $on_finalise:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* - fn $fn_name:ident($( $param_name:ident : $param:ty),* ) -> $result:ty ; + $fn_vis:vis fn $fn_name:ident($( $param_name:ident : $param:ty),* ) -> $result:ty { $( $impl:tt )* } $($rest:tt)* ) => { decl_module!(@normalize @@ -201,7 +205,11 @@ macro_rules! decl_module { for enum $call_type where origin: $origin_type, system = $system { $( $deposit_event )* } { $( $on_finalise )* } - [ $($t)* $(#[doc = $doc_attr])* fn $fn_name(root $( , $param_name : $param )* ) -> $result; ] + [ + $($t)* + $(#[doc = $doc_attr])* + fn $fn_name(root $( , $param_name : $param )* ) -> $result { $( $impl )* } + ] $($rest)* ); }; @@ -224,12 +232,6 @@ macro_rules! decl_module { ); }; - (@call - origin - $mod_type:ident $trait_instance:ident $fn_name:ident $origin:ident $system:ident [ $( $param_name:ident),* ] - ) => { - <$mod_type<$trait_instance>>::$fn_name( $origin $(, $param_name )* ) - }; (@call root $mod_type:ident $trait_instance:ident $fn_name:ident $origin:ident $system:ident [ $( $param_name:ident),* ] @@ -239,6 +241,12 @@ macro_rules! decl_module { <$mod_type<$trait_instance>>::$fn_name( $( $param_name ),* ) } }; + (@call + $ingore:ident + $mod_type:ident $trait_instance:ident $fn_name:ident $origin:ident $system:ident [ $( $param_name:ident),* ] + ) => { + <$mod_type<$trait_instance>>::$fn_name( $origin $(, $param_name )* ) + }; // no `deposit_event` function wanted (@impl_deposit_event @@ -302,13 +310,40 @@ macro_rules! decl_module { for $module<$trait_instance> {} }; + (@impl_function + $module:ident<$trait_instance:ident: $trait_name:ident>; + $origin_ty:ty; + root; + $vis:vis fn $name:ident ( root $(, $param:ident : $param_ty:ty )* ) -> $result:ty { $( $impl:tt )* } + ) => { + impl<$trait_instance: $trait_name> $module<$trait_instance> { + $vis fn $name($( $param: $param_ty ),* ) -> $result { + $( $impl )* + } + } + }; + (@impl_function + $module:ident<$trait_instance:ident: $trait_name:ident>; + $origin_ty:ty; + $ignore:ident; + $vis:vis fn $name:ident ( $origin:ident $(, $param:ident : $param_ty:ty )* ) -> $result:ty { $( $impl:tt )* } + ) => { + impl<$trait_instance: $trait_name> $module<$trait_instance> { + $vis fn $name($origin: $origin_ty $(, $param: $param_ty )* ) -> $result { + $( $impl )* + } + } + }; + (@imp $(#[$attr:meta])* pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident> for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident { $( $(#[doc = $doc_attr:tt])* - fn $fn_name:ident($from:ident $( , $param_name:ident : $param:ty)*) -> $result:ty; + $fn_vis:vis fn $fn_name:ident( + $from:ident $( , $param_name:ident : $param:ty)* + ) -> $result:ty { $( $impl:tt )* } )* } { $( $deposit_event:tt )* } @@ -342,6 +377,16 @@ macro_rules! decl_module { $( $deposit_event )* } + $( + decl_module! { + @impl_function + $mod_type<$trait_instance: $trait_name>; + $origin_type; + $from; + $fn_vis fn $fn_name ($from $(, $param_name : $param )* ) -> $result { $( $impl )* } + } + )* + #[cfg(feature = "std")] $(#[$attr])* #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] @@ -449,7 +494,11 @@ macro_rules! decl_module { match self { $( $call_type::$fn_name( $( $param_name ),* ) => { - decl_module!(@call $from $mod_type $trait_instance $fn_name _origin $system [ $( $param_name ),* ]) + decl_module!( + @call + $from + $mod_type $trait_instance $fn_name _origin $system [ $( $param_name ),* ] + ) }, )* _ => { panic!("__PhantomItem should never be used.") }, @@ -786,11 +835,11 @@ mod tests { decl_module! { pub struct Module for enum Call where origin: T::Origin { /// Hi, this is a comment. - fn aux_0(origin) -> Result; - fn aux_1(origin, data: i32) -> Result; - fn aux_2(origin, data: i32, data2: String) -> Result; - fn aux_3() -> Result; - fn aux_4(data: i32) -> Result; + fn aux_0(_origin) -> Result { unreachable!() } + fn aux_1(_origin, _data: i32) -> Result { unreachable!() } + fn aux_2(_origin, _data: i32, _data2: String) -> Result { unreachable!() } + fn aux_3() -> Result { unreachable!() } + fn aux_4(_data: i32) -> Result { unreachable!() } } } @@ -812,7 +861,7 @@ mod tests { name: DecodeDifferent::Encode("aux_1"), arguments: DecodeDifferent::Encode(&[ FunctionArgumentMetadata { - name: DecodeDifferent::Encode("data"), + name: DecodeDifferent::Encode("_data"), ty: DecodeDifferent::Encode("i32"), } ]), @@ -823,11 +872,11 @@ mod tests { name: DecodeDifferent::Encode("aux_2"), arguments: DecodeDifferent::Encode(&[ FunctionArgumentMetadata { - name: DecodeDifferent::Encode("data"), + name: DecodeDifferent::Encode("_data"), ty: DecodeDifferent::Encode("i32"), }, FunctionArgumentMetadata { - name: DecodeDifferent::Encode("data2"), + name: DecodeDifferent::Encode("_data2"), ty: DecodeDifferent::Encode("String"), } ]), @@ -844,7 +893,7 @@ mod tests { name: DecodeDifferent::Encode("aux_4"), arguments: DecodeDifferent::Encode(&[ FunctionArgumentMetadata { - name: DecodeDifferent::Encode("data"), + name: DecodeDifferent::Encode("_data"), ty: DecodeDifferent::Encode("i32"), } ]), @@ -854,28 +903,6 @@ mod tests { }, }; - impl Module { - fn aux_0(_: T::Origin) -> Result { - unreachable!() - } - - fn aux_1(_: T::Origin, _: i32) -> Result { - unreachable!() - } - - fn aux_2(_: T::Origin, _: i32, _: String) -> Result { - unreachable!() - } - - fn aux_3() -> Result { - unreachable!() - } - - fn aux_4(_: i32) -> Result { - unreachable!() - } - } - struct TraitImpl {} impl Trait for TraitImpl { diff --git a/srml/support/src/metadata.rs b/srml/support/src/metadata.rs index 79df4815bb229..a64d36149de8f 100644 --- a/srml/support/src/metadata.rs +++ b/srml/support/src/metadata.rs @@ -162,13 +162,7 @@ mod tests { decl_module! { pub struct Module for enum Call where origin: T::Origin { - fn aux_0(origin) -> Result; - } - } - - impl Module { - fn aux_0(_: T::Origin) -> Result { - unreachable!() + fn aux_0(_origin) -> Result { unreachable!() } } } } diff --git a/srml/timestamp/src/lib.rs b/srml/timestamp/src/lib.rs index 6b42a8218a22e..1779807af137b 100644 --- a/srml/timestamp/src/lib.rs +++ b/srml/timestamp/src/lib.rs @@ -74,7 +74,33 @@ pub trait Trait: consensus::Trait + system::Trait { decl_module! { pub struct Module for enum Call where origin: T::Origin { - fn set(origin, now: ::Type) -> Result; + /// Set the current time. + /// + /// Extrinsic with this call should be placed at the specific position in the each block + /// (specified by the Trait::TIMESTAMP_SET_POSITION) typically at the start of the each block. + /// This call should be invoked exactly once per block. It will panic at the finalization phase, + /// if this call hasn't been invoked by that time. + /// + /// The timestamp should be greater than the previous one by the amount specified by `block_period`. + fn set(origin, now: ::Type) -> Result { + ensure_inherent(origin)?; + let now = now.into(); + + assert!(!::DidUpdate::exists(), "Timestamp must be updated only once in the block"); + assert!( + >::extrinsic_index() == Some(T::TIMESTAMP_SET_POSITION), + "Timestamp extrinsic must be at position {} in the block", + T::TIMESTAMP_SET_POSITION + ); + assert!( + Self::now().is_zero() || now >= Self::now() + Self::block_period(), + "Timestamp must increment by at least between sequential blocks" + ); + ::Now::put(now); + ::DidUpdate::put(true); + Ok(()) + } + fn on_finalise() { assert!(::DidUpdate::take(), "Timestamp must be updated once in the block"); } @@ -103,33 +129,6 @@ impl Module { Self::now() } - /// Set the current time. - /// - /// Extrinsic with this call should be placed at the specific position in the each block - /// (specified by the Trait::TIMESTAMP_SET_POSITION) typically at the start of the each block. - /// This call should be invoked exactly once per block. It will panic at the finalization phase, - /// if this call hasn't been invoked by that time. - /// - /// The timestamp should be greater than the previous one by the amount specified by `block_period`. - fn set(origin: T::Origin, now: ::Type) -> Result { - ensure_inherent(origin)?; - let now = now.into(); - - assert!(!::DidUpdate::exists(), "Timestamp must be updated only once in the block"); - assert!( - >::extrinsic_index() == Some(T::TIMESTAMP_SET_POSITION), - "Timestamp extrinsic must be at position {} in the block", - T::TIMESTAMP_SET_POSITION - ); - assert!( - Self::now().is_zero() || now >= Self::now() + Self::block_period(), - "Timestamp must increment by at least between sequential blocks" - ); - ::Now::put(now); - ::DidUpdate::put(true); - Ok(()) - } - /// Set the timestamp to something in particular. Only used for tests. #[cfg(feature = "std")] pub fn set_timestamp(now: T::Moment) { diff --git a/srml/treasury/src/lib.rs b/srml/treasury/src/lib.rs index ed2b7d6dfb924..7d314674bc392 100644 --- a/srml/treasury/src/lib.rs +++ b/srml/treasury/src/lib.rs @@ -74,23 +74,79 @@ decl_module! { // Simple declaration of the `Module` type. Lets the macro know what its working on. pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; - // Put forward a suggestion for spending. A deposit proportional to the value - // is reserved and slashed if the proposal is rejected. It is returned once the - // proposal is awarded. - fn propose_spend(origin, value: ::Type, beneficiary: Address) -> Result; + /// Put forward a suggestion for spending. A deposit proportional to the value + /// is reserved and slashed if the proposal is rejected. It is returned once the + /// proposal is awarded. + fn propose_spend( + origin, + value: ::Type, + beneficiary: Address + ) -> Result { + let proposer = ensure_signed(origin)?; + let beneficiary = >::lookup(beneficiary)?; + let value = value.into(); + + let bond = Self::calculate_bond(value); + >::reserve(&proposer, bond) + .map_err(|_| "Proposer's balance too low")?; + + let c = Self::proposal_count(); + >::put(c + 1); + >::insert(c, Proposal { proposer, value, beneficiary, bond }); + + Self::deposit_event(RawEvent::Proposed(c)); + + Ok(()) + } + + /// Set the balance of funds available to spend. + fn set_pot(new_pot: ::Type) -> Result { + // Put the new value into storage. + >::put(new_pot.into()); - // Set the balance of funds available to spend. - fn set_pot(new_pot: ::Type) -> Result; + // All good. + Ok(()) + } + + /// (Re-)configure this module. + fn configure( + proposal_bond: Permill, + proposal_bond_minimum: ::Type, + spend_period: ::Type, + burn: Permill + ) -> Result { + >::put(proposal_bond); + >::put(proposal_bond_minimum.into()); + >::put(spend_period.into()); + >::put(burn); + Ok(()) + } - // (Re-)configure this module. - fn configure(proposal_bond: Permill, proposal_bond_minimum: ::Type, spend_period: ::Type, burn: Permill) -> Result; + /// Reject a proposed spend. The original deposit will be slashed. + fn reject_proposal(origin, proposal_id: Compact) -> Result { + T::RejectOrigin::ensure_origin(origin)?; + let proposal_id: ProposalIndex = proposal_id.into(); + + let proposal = >::take(proposal_id).ok_or("No proposal at that index")?; + + let value = proposal.bond; + let _ = >::slash_reserved(&proposal.proposer, value); + + Ok(()) + } - // Reject a proposed spend. The original deposit will be slashed. - fn reject_proposal(origin, proposal_id: Compact) -> Result; + /// Approve a proposal. At a later time, the proposal will be allocated to the beneficiary + /// and the original deposit will be returned. + fn approve_proposal(origin, proposal_id: Compact) -> Result { + T::ApproveOrigin::ensure_origin(origin)?; + let proposal_id = proposal_id.into(); - // Approve a proposal. At a later time, the proposal will be allocated to the beneficiary - // and the original deposit will be returned. - fn approve_proposal(origin, proposal_id: Compact) -> Result; + ensure!(>::exists(proposal_id), "No proposal at that index"); + + >::mutate(|v| v.push(proposal_id)); + + Ok(()) + } fn on_finalise(n: T::BlockNumber) { // Check to see if we should spend some funds! @@ -161,69 +217,7 @@ decl_event!( ); impl Module { - // Implement Calls and add public immutables and private mutables. - - fn propose_spend(origin: T::Origin, value: ::Type, beneficiary: Address) -> Result { - let proposer = ensure_signed(origin)?; - let beneficiary = >::lookup(beneficiary)?; - let value = value.into(); - - let bond = Self::calculate_bond(value); - >::reserve(&proposer, bond) - .map_err(|_| "Proposer's balance too low")?; - - let c = Self::proposal_count(); - >::put(c + 1); - >::insert(c, Proposal { proposer, value, beneficiary, bond }); - - Self::deposit_event(RawEvent::Proposed(c)); - - Ok(()) - } - - fn reject_proposal(origin: T::Origin, proposal_id: Compact) -> Result { - T::RejectOrigin::ensure_origin(origin)?; - let proposal_id: ProposalIndex = proposal_id.into(); - - let proposal = >::take(proposal_id).ok_or("No proposal at that index")?; - - let value = proposal.bond; - let _ = >::slash_reserved(&proposal.proposer, value); - - Ok(()) - } - - fn approve_proposal(origin: T::Origin, proposal_id: Compact) -> Result { - T::ApproveOrigin::ensure_origin(origin)?; - let proposal_id = proposal_id.into(); - - ensure!(>::exists(proposal_id), "No proposal at that index"); - - >::mutate(|v| v.push(proposal_id)); - - Ok(()) - } - - fn set_pot(new_pot: ::Type) -> Result { - // Put the new value into storage. - >::put(new_pot.into()); - - // All good. - Ok(()) - } - - fn configure( - proposal_bond: Permill, - proposal_bond_minimum: ::Type, - spend_period: ::Type, - burn: Permill - ) -> Result { - >::put(proposal_bond); - >::put(proposal_bond_minimum.into()); - >::put(spend_period.into()); - >::put(burn); - Ok(()) - } + // Add public immutables and private mutables. /// The needed bond for a proposal whose spend is `value`. fn calculate_bond(value: T::Balance) -> T::Balance {