diff --git a/demo/cli/src/lib.rs b/demo/cli/src/lib.rs index 6a2730280db63..d0704438b7a5c 100644 --- a/demo/cli/src/lib.rs +++ b/demo/cli/src/lib.rs @@ -115,7 +115,8 @@ pub fn run(args: I) -> error::Result<()> where staking: Some(StakingConfig { current_era: 0, intentions: vec![], - transaction_fee: 100, + transaction_base_fee: 100, + transaction_byte_fee: 1, balances: vec![(god_key.clone(), 1u64 << 63)].into_iter().collect(), validator_count: 12, sessions_per_era: 24, // 24 hours per era. diff --git a/demo/executor/src/lib.rs b/demo/executor/src/lib.rs index 3d0aa193e40eb..85bc248e2ae3d 100644 --- a/demo/executor/src/lib.rs +++ b/demo/executor/src/lib.rs @@ -82,8 +82,9 @@ mod tests { #[test] fn panic_execution_with_foreign_code_gives_error() { let mut t: TestExternalities = map![ - twox_128(&>::key_for(*Alice)).to_vec() => vec![68u8, 0, 0, 0, 0, 0, 0, 0], - twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(&>::key_for(*Alice)).to_vec() => vec![69u8, 0, 0, 0, 0, 0, 0, 0], + twox_128(>::key()).to_vec() => vec![70u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], twox_128(&>::key_for(0)).to_vec() => vec![0u8; 32] ]; @@ -96,8 +97,9 @@ mod tests { #[test] fn panic_execution_with_native_equivalent_code_gives_error() { let mut t: TestExternalities = map![ - twox_128(&>::key_for(*Alice)).to_vec() => vec![68u8, 0, 0, 0, 0, 0, 0, 0], - twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(&>::key_for(*Alice)).to_vec() => vec![69u8, 0, 0, 0, 0, 0, 0, 0], + twox_128(>::key()).to_vec() => vec![70u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], twox_128(&>::key_for(0)).to_vec() => vec![0u8; 32] ]; @@ -111,7 +113,8 @@ mod tests { fn successful_execution_with_native_equivalent_code_gives_ok() { let mut t: TestExternalities = map![ twox_128(&>::key_for(*Alice)).to_vec() => vec![111u8, 0, 0, 0, 0, 0, 0, 0], - twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], twox_128(&>::key_for(0)).to_vec() => vec![0u8; 32] ]; @@ -130,7 +133,8 @@ mod tests { fn successful_execution_with_foreign_code_gives_ok() { let mut t: TestExternalities = map![ twox_128(&>::key_for(*Alice)).to_vec() => vec![111u8, 0, 0, 0, 0, 0, 0, 0], - twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], twox_128(&>::key_for(0)).to_vec() => vec![0u8; 32] ]; @@ -162,7 +166,8 @@ mod tests { intentions: vec![Alice.into(), Bob.into(), Charlie.into()], validator_count: 3, bonding_duration: 0, - transaction_fee: 1, + transaction_base_fee: 1, + transaction_byte_fee: 0, }), democracy: Some(Default::default()), council: Some(Default::default()), @@ -197,7 +202,7 @@ mod tests { construct_block( 1, [69u8; 32].into(), - hex!("a63d59c6a7347cd7a1dc1ec139723b531f0ac450e39b1c532d5ca69ff74ad811").into(), + hex!("76b0393b4958d3cb98bb51d9f4edb316af48485142b8721e94c3b52c75ec3243").into(), vec![Extrinsic { signed: Alice.into(), index: 0, @@ -210,7 +215,7 @@ mod tests { construct_block( 2, block1().1, - hex!("1c3623b2e3f7e43752debb9015bace4f6931593579b5af34457b931315f5e2ab").into(), + hex!("8ae9828a5988459d35fb428086170dead660176ee0766e89bc1a4b48153d4e88").into(), vec![ Extrinsic { signed: Bob.into(), @@ -267,8 +272,9 @@ mod tests { #[test] fn panic_execution_gives_error() { let mut t: TestExternalities = map![ - twox_128(&>::key_for(*Alice)).to_vec() => vec![68u8, 0, 0, 0, 0, 0, 0, 0], - twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(&>::key_for(*Alice)).to_vec() => vec![69u8, 0, 0, 0, 0, 0, 0, 0], + twox_128(>::key()).to_vec() => vec![70u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], twox_128(&>::key_for(0)).to_vec() => vec![0u8; 32] ]; @@ -283,7 +289,8 @@ mod tests { fn successful_execution_gives_ok() { let mut t: TestExternalities = map![ twox_128(&>::key_for(*Alice)).to_vec() => vec![111u8, 0, 0, 0, 0, 0, 0, 0], - twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], + twox_128(>::key()).to_vec() => vec![0u8; 8], twox_128(&>::key_for(0)).to_vec() => vec![0u8; 32] ]; diff --git a/demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.compact.wasm b/demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.compact.wasm index 358a69ea49e1e..312bc37e0ab13 100644 Binary files a/demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.compact.wasm and b/demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.compact.wasm differ diff --git a/demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.wasm b/demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.wasm index 53036e4f65706..9ddaf31b9ffa0 100755 Binary files a/demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.wasm and b/demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.wasm differ diff --git a/polkadot/cli/src/cli.yml b/polkadot/cli/src/cli.yml index 81ea904959b09..3f37d79239510 100644 --- a/polkadot/cli/src/cli.yml +++ b/polkadot/cli/src/cli.yml @@ -65,6 +65,6 @@ args: - chain: long: chain value_name: CHAIN_SPEC - help: Specify the chain specification (one of dev, local or poc-1) + help: Specify the chain specification (one of dev, local or poc-2) takes_value: true subcommands: diff --git a/polkadot/cli/src/lib.rs b/polkadot/cli/src/lib.rs index d26ebf75568fc..90b0655cd48e7 100644 --- a/polkadot/cli/src/lib.rs +++ b/polkadot/cli/src/lib.rs @@ -110,7 +110,7 @@ impl substrate_rpc::system::SystemApi for Configuration { Ok(match self.0.chain_spec { ChainSpec::Development => "dev", ChainSpec::LocalTestnet => "local", - ChainSpec::PoC1Testnet => "poc-1", + ChainSpec::PoC2Testnet => "poc-2", }.into()) } } @@ -174,14 +174,14 @@ pub fn run(args: I) -> error::Result<()> where match matches.value_of("chain") { Some("dev") => config.chain_spec = ChainSpec::Development, Some("local") => config.chain_spec = ChainSpec::LocalTestnet, - Some("poc-1") => config.chain_spec = ChainSpec::PoC1Testnet, + Some("poc-2") => config.chain_spec = ChainSpec::PoC2Testnet, None => (), Some(unknown) => panic!("Invalid chain name: {}", unknown), } info!("Chain specification: {}", match config.chain_spec { ChainSpec::Development => "Development", ChainSpec::LocalTestnet => "Local Testnet", - ChainSpec::PoC1Testnet => "PoC-1 Testnet", + ChainSpec::PoC2Testnet => "PoC-2 Testnet", }); config.roles = role; diff --git a/polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.compact.wasm b/polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.compact.wasm index 35f1cfd18b2c5..e340c3df12554 100644 Binary files a/polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.compact.wasm and b/polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.compact.wasm differ diff --git a/polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.wasm b/polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.wasm index 7bc43f97d5520..e081e7ca2b368 100755 Binary files a/polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.wasm and b/polkadot/runtime/wasm/target/wasm32-unknown-unknown/release/polkadot_runtime.wasm differ diff --git a/polkadot/service/src/config.rs b/polkadot/service/src/config.rs index 16b39631b9d9a..c6edbe141cfb0 100644 --- a/polkadot/service/src/config.rs +++ b/polkadot/service/src/config.rs @@ -28,8 +28,8 @@ pub enum ChainSpec { Development, /// Whatever the current runtime is, with simple Alice/Bob auths. LocalTestnet, - /// The PoC-1 testnet. - PoC1Testnet, + /// The PoC-2 testnet. + PoC2Testnet, } /// Service configuration. diff --git a/polkadot/service/src/lib.rs b/polkadot/service/src/lib.rs index 5113193a36905..436479d0a5f2c 100644 --- a/polkadot/service/src/lib.rs +++ b/polkadot/service/src/lib.rs @@ -141,7 +141,7 @@ pub struct ChainConfig { boot_nodes: Vec, } -fn poc_1_testnet_config() -> ChainConfig { +fn poc_2_testnet_config() -> ChainConfig { let initial_authorities = vec![ hex!["82c39b31a2b79a90f8e66e7a77fdb85a4ed5517f2ae39f6a80565e8ecae85cf5"].into(), hex!["4de37a07567ebcbf8c64568428a835269a566723687058e017b6d69db00a77e7"].into(), @@ -164,7 +164,8 @@ fn poc_1_testnet_config() -> ChainConfig { staking: Some(StakingConfig { current_era: 0, intentions: initial_authorities.clone(), - transaction_fee: 100, + transaction_base_fee: 100, + transaction_byte_fee: 1, balances: endowed_accounts.iter().map(|&k|(k, 1u128 << 60)).collect(), validator_count: 12, sessions_per_era: 24, // 24 hours per era. @@ -222,7 +223,8 @@ fn testnet_config(initial_authorities: Vec) -> ChainConfig { staking: Some(StakingConfig { current_era: 0, intentions: initial_authorities.clone(), - transaction_fee: 1, + transaction_base_fee: 1, + transaction_byte_fee: 0, balances: endowed_accounts.iter().map(|&k|(k, (1u128 << 60))).collect(), validator_count: 2, sessions_per_era: 5, @@ -364,7 +366,7 @@ impl Service let ChainConfig { genesis_config, boot_nodes } = match config.chain_spec { ChainSpec::Development => development_config(), ChainSpec::LocalTestnet => local_testnet_config(), - ChainSpec::PoC1Testnet => poc_1_testnet_config(), + ChainSpec::PoC2Testnet => poc_2_testnet_config(), }; config.network.boot_nodes.extend(boot_nodes); diff --git a/substrate/executor/wasm/target/wasm32-unknown-unknown/release/runtime_test.compact.wasm b/substrate/executor/wasm/target/wasm32-unknown-unknown/release/runtime_test.compact.wasm index f9e5d9423d112..f0cada56e2063 100644 Binary files a/substrate/executor/wasm/target/wasm32-unknown-unknown/release/runtime_test.compact.wasm and b/substrate/executor/wasm/target/wasm32-unknown-unknown/release/runtime_test.compact.wasm differ diff --git a/substrate/executor/wasm/target/wasm32-unknown-unknown/release/runtime_test.wasm b/substrate/executor/wasm/target/wasm32-unknown-unknown/release/runtime_test.wasm index 6b9fd2905255a..4f22f717428b6 100755 Binary files a/substrate/executor/wasm/target/wasm32-unknown-unknown/release/runtime_test.wasm and b/substrate/executor/wasm/target/wasm32-unknown-unknown/release/runtime_test.wasm differ diff --git a/substrate/runtime-support/src/lib.rs b/substrate/runtime-support/src/lib.rs index 5af5ae18c5989..514869a1b2798 100644 --- a/substrate/runtime-support/src/lib.rs +++ b/substrate/runtime-support/src/lib.rs @@ -44,3 +44,62 @@ mod hashable; pub use self::storage::{StorageVec, StorageList, StorageValue, StorageMap}; 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; + }} +} + +#[macro_export] +macro_rules! ensure { + ( $x:expr, $y:expr ) => {{ + 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} + } + } +} + +#[macro_export] +macro_rules! ensure_unwrap_err { + ($x:expr, $y: expr) => { + if let Err(v) = $x { + v + } else { + fail!{$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()); + } +} diff --git a/substrate/runtime/council/src/lib.rs b/substrate/runtime/council/src/lib.rs index 78fc0e39eb54c..0a7531e076894 100644 --- a/substrate/runtime/council/src/lib.rs +++ b/substrate/runtime/council/src/lib.rs @@ -224,8 +224,8 @@ 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) { - assert!(!Self::presentation_active()); - assert_eq!(index, Self::vote_index()); + ensure!(!Self::presentation_active()); + ensure!(index == Self::vote_index()); if !>::exists(aux.ref_into()) { // not yet a voter - deduct bond. >::reserve_balance(aux.ref_into(), Self::voting_bond()); @@ -245,16 +245,16 @@ impl Module { /// /// 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) { - assert!(!Self::presentation_active(), "cannot reap during presentation period"); - assert!(Self::voter_last_active(aux.ref_into()).is_some(), "reaper must be a voter"); - let last_active = Self::voter_last_active(&who).expect("target for inactivity cleanup must be active"); - assert!(assumed_vote_index == Self::vote_index(), "vote index not current"); - assert!(last_active < assumed_vote_index - Self::inactivity_grace_period(), "cannot reap during grace perid"); + 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"); + 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 signed_index = signed_index as usize; let who_index = who_index as usize; - assert!(signed_index < voters.len() && &voters[signed_index] == aux.ref_into(), "bad reporter index"); - assert!(who_index < voters.len() && voters[who_index] == who, "bad target index"); + ensure!(signed_index < voters.len() && &voters[signed_index] == aux.ref_into(), "bad reporter index"); + ensure!(who_index < voters.len() && voters[who_index] == who, "bad target index"); // will definitely kill one of signed or who now. @@ -263,8 +263,8 @@ impl Module { .any(|(&appr, addr)| appr && *addr != T::AccountId::default() && - Self::candidate_reg_info(addr) - .expect("all items in candidates list are registered").0 <= last_active); + 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 { aux.ref_into() }, @@ -280,12 +280,12 @@ impl Module { /// Remove a voter. All votes are cancelled and the voter deposit is returned. fn retract_voter(aux: &T::PublicAux, index: u32) { - assert!(!Self::presentation_active(), "cannot retract when presenting"); - assert!(>::exists(aux.ref_into()), "cannot retract non-voter"); + 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; - assert!(index < voters.len(), "retraction index invalid"); - assert!(&voters[index] == aux.ref_into(), "retraction index mismatch"); + 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()); } @@ -294,18 +294,19 @@ impl Module { /// /// Account must have enough transferrable funds in it to pay the bond. fn submit_candidacy(aux: &T::PublicAux, slot: u32) { - assert!(!Self::is_a_candidate(aux.ref_into()), "duplicate candidate submission"); - assert!(>::deduct_unbonded(aux.ref_into(), Self::candidacy_bond()), "candidate has not enough funds"); + 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(); - assert!( + ensure!( (slot == count && count == candidates.len()) || (slot < candidates.len() && candidates[slot] == T::AccountId::default()), "invalid candidate slot" ); + >::deduct_unbonded(aux.ref_into(), Self::candidacy_bond()); let mut candidates = candidates; if slot == candidates.len() { candidates.push(aux.ref_into().clone()); @@ -321,23 +322,22 @@ impl Module { /// 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) { - assert_eq!(index, Self::vote_index(), "index not current"); - let (_, _, expiring) = Self::next_finalise() - .expect("cannot present outside of presentation period"); + ensure!(index == Self::vote_index(), "index not current"); + let (_, _, expiring) = ensure_unwrap!(Self::next_finalise(), "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()); - assert!(>::can_slash(aux.ref_into(), bad_presentation_punishment), "presenter must have sufficient slashable funds"); + ensure!(>::can_slash(aux.ref_into(), bad_presentation_punishment), "presenter must have sufficient slashable funds"); - let mut leaderboard = Self::leaderboard().expect("leaderboard must exist while present phase active"); - assert!(total > leaderboard[0].0, "candidate not worthy of leaderboard"); + let mut leaderboard = ensure_unwrap!(Self::leaderboard(), "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) { - assert!(p < expiring.len(), "candidate must not form a duplicated member if elected"); + 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).expect("presented candidate must be current"); + ensure_unwrap!(Self::candidate_reg_info(&candidate), "presented candidate must be current"); let actual_total = voters.iter() .zip(stakes.iter()) .filter_map(|(voter, stake)| @@ -440,8 +440,8 @@ impl Module { /// Clears all presented candidates, returning the bond of the elected ones. fn finalise_tally() { >::kill(); - let (_, coming, expiring): (T::BlockNumber, u32, Vec) = >::take() - .expect("finalise can only be called after a tally is started."); + let (_, coming, expiring): (T::BlockNumber, u32, Vec) = + ensure_unwrap!(>::take(), "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(); @@ -476,7 +476,7 @@ impl Module { .rev() .take_while(|&(b, _)| !b.is_zero()) .skip(coming as usize) - .map(|(_, a)| { let i = Self::candidate_reg_info(&a).expect("runner up must be registered").1; (a, i) }); + .filter_map(|(_, a)| Self::candidate_reg_info(&a).map(|i| (a, i.1))); let mut count = 0u32; for (address, slot) in runners_up { new_candidates[slot as usize] = address; @@ -626,7 +626,8 @@ mod tests { intentions: vec![], validator_count: 2, bonding_duration: 0, - transaction_fee: 0, + transaction_base_fee: 0, + transaction_byte_fee: 0, }.build_externalities()); t.extend(democracy::GenesisConfig::{ launch_period: 1, @@ -760,55 +761,50 @@ mod tests { } #[test] - #[should_panic(expected = "invalid candidate slot")] - fn candidate_submission_not_using_free_slot_should_panic() { - let mut t = new_test_ext_with_candidate_holes(); - - with_externalities(&mut t, || { + fn candidate_submission_not_using_free_slot_should_not_work() { + with_externalities(&mut new_test_ext_with_candidate_holes(), || { System::set_block_number(1); - Council::submit_candidacy(&4, 3); + assert_noop!{Council::submit_candidacy(&4, 3)}; // gives "invalid candidate slot" }); } #[test] - #[should_panic(expected = "invalid candidate slot")] - fn bad_candidate_slot_submission_should_panic() { + fn bad_candidate_slot_submission_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); assert_eq!(Council::candidates(), Vec::::new()); - Council::submit_candidacy(&1, 1); + assert_noop!{Council::submit_candidacy(&1, 1)}; // gives "invalid candidate slot" }); } #[test] - #[should_panic(expected = "invalid candidate slot")] - fn non_free_candidate_slot_submission_should_panic() { + fn non_free_candidate_slot_submission_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); assert_eq!(Council::candidates(), Vec::::new()); Council::submit_candidacy(&1, 0); - Council::submit_candidacy(&2, 0); + assert_eq!(Council::candidates(), vec![1]); + assert_noop!{Council::submit_candidacy(&2, 0)}; // gives "invalid candidate slot" }); } #[test] - #[should_panic(expected = "duplicate candidate submission")] - fn dupe_candidate_submission_should_panic() { + fn dupe_candidate_submission_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); assert_eq!(Council::candidates(), Vec::::new()); Council::submit_candidacy(&1, 0); - Council::submit_candidacy(&1, 1); + assert_eq!(Council::candidates(), vec![1]); + assert_noop!{Council::submit_candidacy(&1, 1)}; // gives "duplicate candidate submission" }); } #[test] - #[should_panic(expected = "candidate has not enough funds")] - fn poor_candidate_submission_should_panic() { + fn poor_candidate_submission_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(1); assert_eq!(Council::candidates(), Vec::::new()); - Council::submit_candidacy(&7, 0); + assert_noop!{Council::submit_candidacy(&7, 0)}; // gives "candidate has not enough funds" }); } @@ -906,36 +902,34 @@ mod tests { } #[test] - #[should_panic(expected = "retraction index mismatch")] - fn invalid_retraction_index_should_panic() { + 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); - Council::retract_voter(&1, 1); + assert_eq!(Council::voters(), vec![1, 2]); + assert_noop!{Council::retract_voter(&1, 1)}; // gives "retraction index mismatch" }); } #[test] - #[should_panic(expected = "retraction index invalid")] - fn overflow_retraction_index_should_panic() { + 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); - Council::retract_voter(&1, 1); + assert_noop!{Council::retract_voter(&1, 1)}; // gives "retraction index invalid" }); } #[test] - #[should_panic(expected = "cannot retract non-voter")] - fn non_voter_retraction_should_panic() { + 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); - Council::retract_voter(&2, 0); + assert_noop!{Council::retract_voter(&2, 0)}; // gives "cannot retract non-voter" }); } @@ -1028,8 +1022,7 @@ mod tests { } #[test] - #[should_panic(expected = "candidate must not form a duplicated member if elected")] - fn presenting_for_double_election_should_panic() { + 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); @@ -1046,7 +1039,7 @@ mod tests { Council::end_block(System::block_number()); System::set_block_number(10); - Council::present_winner(&4, 2, 11, 1); + assert_noop!{Council::present_winner(&4, 2, 11, 1)}; // gives "candidate must not form a duplicated member if elected" }); } @@ -1088,8 +1081,7 @@ mod tests { } #[test] - #[should_panic(expected = "bad reporter index")] - fn retracting_inactive_voter_with_bad_reporter_index_should_panic() { + 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); @@ -1109,17 +1101,16 @@ mod tests { Council::present_winner(&4, 5, 38, 1); Council::end_block(System::block_number()); - 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" }); } #[test] - #[should_panic(expected = "bad target index")] - fn retracting_inactive_voter_with_bad_target_index_should_panic() { + 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); @@ -1139,11 +1130,11 @@ mod tests { Council::present_winner(&4, 5, 38, 1); Council::end_block(System::block_number()); - 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" }); } @@ -1190,8 +1181,7 @@ mod tests { } #[test] - #[should_panic(expected = "reaper must be a voter")] - fn attempting_to_retract_inactive_voter_by_nonvoter_should_panic() { + 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); @@ -1211,17 +1201,16 @@ mod tests { Council::present_winner(&4, 5, 41, 1); Council::end_block(System::block_number()); - 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" }); } #[test] - #[should_panic(expected = "candidate not worthy of leaderboard")] - fn presenting_loser_should_panic() { + fn presenting_loser_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); Council::submit_candidacy(&1, 0); @@ -1241,7 +1230,7 @@ mod tests { Council::present_winner(&4, 3, 21, 0); Council::present_winner(&4, 4, 31, 0); Council::present_winner(&4, 5, 41, 0); - Council::present_winner(&4, 2, 11, 0); + assert_noop!{Council::present_winner(&4, 2, 11, 0)}; // gives "candidate not worthy of leaderboard" }); } @@ -1278,18 +1267,16 @@ mod tests { } #[test] - #[should_panic(expected = "cannot present outside of presentation period")] - fn present_panics_outside_of_presentation_period() { + fn present_outside_of_presentation_period_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); assert!(!Council::presentation_active()); - Council::present_winner(&5, 5, 1, 0); + assert_noop!{Council::present_winner(&5, 5, 1, 0)}; // gives "cannot present outside of presentation period" }); } #[test] - #[should_panic(expected = "index not current")] - fn present_panics_with_invalid_vote_index() { + 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); @@ -1299,13 +1286,12 @@ mod tests { Council::end_block(System::block_number()); System::set_block_number(6); - Council::present_winner(&4, 2, 11, 1); + assert_noop!{Council::present_winner(&4, 2, 11, 1)}; // gives "index not current" }); } #[test] - #[should_panic(expected = "presenter must have sufficient slashable funds")] - fn present_panics_when_presenter_is_poor() { + fn present_when_presenter_is_poor_should_not_work() { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); assert!(!Council::presentation_active()); @@ -1318,7 +1304,7 @@ mod tests { System::set_block_number(6); assert_eq!(Staking::balance(&1), 1); - Council::present_winner(&1, 1, 30, 0); + assert_noop!{Council::present_winner(&1, 1, 30, 0)}; // gives "presenter must have sufficient slashable funds" }); } diff --git a/substrate/runtime/council/src/voting.rs b/substrate/runtime/council/src/voting.rs index 8c2362ed76353..dedb3a23427ee 100644 --- a/substrate/runtime/council/src/voting.rs +++ b/substrate/runtime/council/src/voting.rs @@ -75,12 +75,12 @@ impl Module { // Dispatch fn propose(aux: &T::PublicAux, proposal: Box) { let expiry = >::block_number() + Self::voting_period(); - assert!(Self::will_still_be_councillor_at(aux.ref_into(), expiry)); + ensure!(Self::will_still_be_councillor_at(aux.ref_into(), expiry)); let proposal_hash = T::Hashing::hash_of(&proposal); - assert!(!>::exists(proposal_hash), "No duplicate proposals allowed"); - assert!(!Self::is_vetoed(&proposal_hash)); + ensure!(!>::exists(proposal_hash), "No duplicate proposals allowed"); + ensure!(!Self::is_vetoed(&proposal_hash)); let mut proposals = Self::proposals(); proposals.push((expiry, proposal_hash)); @@ -102,14 +102,14 @@ impl Module { } fn veto(aux: &T::PublicAux, proposal_hash: T::Hash) { - assert!(Self::is_councillor(aux.ref_into()), "only councillors may veto council proposals"); - assert!(>::exists(&proposal_hash), "proposal must exist to be vetoed"); + 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 = existing_vetoers.binary_search(aux.ref_into()) - .expect_err("a councillor may not veto a proposal twice"); + let insert_position = ensure_unwrap_err!(existing_vetoers.binary_search(aux.ref_into()), + "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); @@ -163,8 +163,7 @@ impl Module { Some(&(expiry, hash)) if expiry == n => { // yes this is horrible, but fixing it will need substantial work in storage. Self::set_proposals(&proposals[1..].to_vec()); - let proposal = >::take(hash).expect("all queued proposal hashes must have associated proposals"); - Some((proposal, hash)) + >::take(hash).map(|p| (p, hash)) /* defensive only: all queued proposal hashes must have associated proposals*/ } _ => None, } @@ -311,8 +310,7 @@ mod tests { } #[test] - #[should_panic] - fn double_veto_should_panic() { + fn double_veto_should_not_work() { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); @@ -322,13 +320,12 @@ mod tests { System::set_block_number(3); CouncilVoting::propose(&1, Box::new(proposal.clone())); - CouncilVoting::veto(&2, hash); + assert_noop!{CouncilVoting::veto(&2, hash)}; }); } #[test] - #[should_panic] - fn retry_in_cooloff_should_panic() { + fn retry_in_cooloff_should_not_work() { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); @@ -337,7 +334,7 @@ mod tests { CouncilVoting::veto(&2, hash); System::set_block_number(2); - CouncilVoting::propose(&1, Box::new(proposal.clone())); + assert_noop!{CouncilVoting::propose(&1, Box::new(proposal.clone()))}; }); } @@ -447,12 +444,11 @@ mod tests { } #[test] - #[should_panic] - fn propose_by_public_should_panic() { + fn propose_by_public_should_not_work() { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = bonding_duration_proposal(42); - CouncilVoting::propose(&4, Box::new(proposal)); + assert_noop!{CouncilVoting::propose(&4, Box::new(proposal))}; }); } } diff --git a/substrate/runtime/democracy/src/lib.rs b/substrate/runtime/democracy/src/lib.rs index 86851d6e7ab4f..ffae2f92bcffc 100644 --- a/substrate/runtime/democracy/src/lib.rs +++ b/substrate/runtime/democracy/src/lib.rs @@ -102,7 +102,7 @@ impl Module { // exposed immutables. - /// Get the amount locked in support of `proposal`; false if proposal isn't a valid proposal + /// Get the amount locked in support of `proposal`; `None` if proposal isn't a valid proposal /// index. pub fn locked_for(proposal: PropIndex) -> Option { Self::deposit_of(proposal).map(|(d, l)| d * T::Balance::sa(l.len())) @@ -135,7 +135,7 @@ impl Module { /// Get the voters for the current proposal. pub fn tally(ref_index: ReferendumIndex) -> (T::Balance, T::Balance) { Self::voters_for(ref_index).iter() - .map(|a| (>::balance(a), Self::vote_of((ref_index, a.clone())).expect("all items come from `voters`; for an item to be in `voters` there must be a vote registered; qed"))) + .map(|a| (>::balance(a), Self::vote_of((ref_index, a.clone())).unwrap_or(false)/*defensive only: all items come from `voters`; for an item to be in `voters` there must be a vote registered; qed*/)) .map(|(bal, vote)| if vote { (bal, Zero::zero()) } else { (Zero::zero(), bal) }) .fold((Zero::zero(), Zero::zero()), |(a, b), (c, d)| (a + c, b + d)) } @@ -144,8 +144,8 @@ impl Module { /// Propose a sensitive action to be taken. fn propose(aux: &T::PublicAux, proposal: Box, value: T::Balance) { - assert!(value >= Self::minimum_deposit()); - assert!(>::deduct_unbonded(aux.ref_into(), value)); + ensure!(value >= Self::minimum_deposit()); + ensure!(>::deduct_unbonded(aux.ref_into(), value)); let index = Self::public_prop_count(); >::put(index + 1); @@ -158,21 +158,23 @@ impl Module { /// Propose a sensitive action to be taken. fn second(aux: &T::PublicAux, proposal: PropIndex) { - let mut deposit = Self::deposit_of(proposal).expect("can only second an existing proposal"); - assert!(>::deduct_unbonded(aux.ref_into(), deposit.0)); - - deposit.1.push(aux.ref_into().clone()); - >::insert(proposal, deposit); + 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"); + } } /// 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) { - panic!("vote given for invalid referendum.") + fail!("vote given for invalid referendum.") } if >::balance(aux.ref_into()).is_zero() { - panic!("transactor must have balance to signal approval."); + fail!("transactor must have balance to signal approval."); } if !>::exists(&(ref_index, aux.ref_into().clone())) { let mut voters = Self::voters_for(ref_index); @@ -184,7 +186,7 @@ impl Module { /// Start a referendum. fn start_referendum(proposal: Box, vote_threshold: VoteThreshold) { - Self::inject_referendum(>::block_number() + Self::voting_period(), *proposal, vote_threshold); + let _ = Self::inject_referendum(>::block_number() + Self::voting_period(), *proposal, vote_threshold); } /// Remove a referendum. @@ -196,7 +198,7 @@ impl Module { /// Start a referendum. Can be called directly by the council. pub fn internal_start_referendum(proposal: T::Proposal, vote_threshold: VoteThreshold) { - >::inject_referendum(>::block_number() + >::voting_period(), proposal, vote_threshold); + let _ = >::inject_referendum(>::block_number() + >::voting_period(), proposal, vote_threshold); } /// Remove a referendum. Can be called directly by the council. @@ -211,15 +213,15 @@ impl Module { end: T::BlockNumber, proposal: T::Proposal, vote_threshold: VoteThreshold - ) -> ReferendumIndex { + ) -> 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) { - panic!("Cannot inject a referendum that ends earlier than preceeding referendum"); + Err("Cannot inject a referendum that ends earlier than preceeding referendum")? } >::put(ref_index + 1); >::insert(ref_index, (end, proposal, vote_threshold)); - ref_index + Ok(ref_index) } /// Remove all info on a referendum. @@ -232,23 +234,25 @@ impl Module { } /// Current era is ending; we should finish up any proposals. - fn end_block(now: T::BlockNumber) { + fn end_block(now: T::BlockNumber) -> Result<(), &'static str> { // pick out another public referendum if it's time. if (now % Self::launch_period()).is_zero() { let mut public_props = Self::public_props(); if let Some((winner_index, _)) = public_props.iter() .enumerate() - .max_by_key(|x| Self::locked_for((x.1).0).expect("All current public proposals have an amount locked")) + .max_by_key(|x| Self::locked_for((x.1).0).unwrap_or_else(Zero::zero)/*defensive only: All current public proposals have an amount locked*/) { let (prop_index, proposal, _) = public_props.swap_remove(winner_index); - let (deposit, depositors): (T::Balance, Vec) = - >::take(prop_index).expect("depositors always exist for current proposals"); - // refund depositors - for d in &depositors { - >::refund(d, deposit); + if let Some((deposit, depositors)) = >::take(prop_index) {//: (T::Balance, Vec) = + // refund depositors + for d in &depositors { + >::refund(d, deposit); + } + >::put(public_props); + Self::inject_referendum(now + Self::voting_period(), proposal, VoteThreshold::SuperMajorityApprove)?; + } else { + Err("depositors always exist for current proposals")? } - >::put(public_props); - Self::inject_referendum(now + Self::voting_period(), proposal, VoteThreshold::SuperMajorityApprove); } } @@ -262,12 +266,15 @@ impl Module { } >::put(index + 1); } + Ok(()) } } impl Executable for Module { fn execute() { - Self::end_block(>::block_number()); + if let Err(e) = Self::end_block(>::block_number()) { + runtime_io::print(e); + } } } @@ -388,7 +395,8 @@ mod tests { intentions: vec![], validator_count: 2, bonding_duration: 3, - transaction_fee: 0, + transaction_base_fee: 0, + transaction_byte_fee: 0, }.build_externalities()); t.extend(GenesisConfig::{ launch_period: 1, @@ -437,7 +445,7 @@ mod tests { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); propose_sessions_per_era(1, 2, 1); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); System::set_block_number(2); let r = 0; @@ -448,7 +456,7 @@ mod tests { assert_eq!(Democracy::vote_of((r, 1)), Some(true)); assert_eq!(Democracy::tally(r), (10, 0)); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::era_length(), 2); @@ -479,7 +487,7 @@ mod tests { Democracy::second(&5, 0); Democracy::second(&5, 0); Democracy::second(&5, 0); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); assert_eq!(Staking::balance(&1), 10); assert_eq!(Staking::balance(&2), 20); assert_eq!(Staking::balance(&5), 50); @@ -487,35 +495,35 @@ mod tests { } #[test] - #[should_panic] fn proposal_with_deposit_below_minimum_should_panic() { 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); }); } #[test] - #[should_panic] fn poor_proposer_should_panic() { 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); }); } #[test] - #[should_panic] fn poor_seconder_should_panic() { 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)); }); } fn propose_bonding_duration(who: u64, value: u64, locked: u64) { - Democracy::propose(&who, Box::new(Proposal::Staking(staking::PrivCall::set_bonding_duration(value))), locked); + Democracy::propose(&who, Box::new(Proposal::Staking(staking::PrivCall::set_bonding_duration(value))), locked); } #[test] @@ -525,23 +533,23 @@ mod tests { propose_bonding_duration(1, 2, 2); propose_bonding_duration(1, 4, 4); propose_bonding_duration(1, 3, 3); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); System::set_block_number(1); Democracy::vote(&1, 0, true); - Democracy::end_block(System::block_number()); + 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); - Democracy::end_block(System::block_number()); + 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); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::bonding_duration(), 2); }); @@ -555,14 +563,14 @@ mod tests { fn simple_passing_should_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove); + let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); Democracy::vote(&1, r, true); assert_eq!(Democracy::voters_for(r), vec![1]); assert_eq!(Democracy::vote_of((r, 1)), Some(true)); assert_eq!(Democracy::tally(r), (10, 0)); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::era_length(), 2); @@ -573,11 +581,11 @@ mod tests { fn cancel_referendum_should_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove); + let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); Democracy::vote(&1, r, true); Democracy::cancel_referendum(r); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::era_length(), 1); @@ -588,14 +596,14 @@ mod tests { fn simple_failing_should_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove); + let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove).unwrap(); Democracy::vote(&1, r, false); assert_eq!(Democracy::voters_for(r), vec![1]); assert_eq!(Democracy::vote_of((r, 1)), Some(false)); assert_eq!(Democracy::tally(r), (0, 10)); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::era_length(), 1); @@ -606,7 +614,7 @@ mod tests { fn controversial_voting_should_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove); + 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); @@ -616,7 +624,7 @@ mod tests { assert_eq!(Democracy::tally(r), (110, 100)); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::era_length(), 2); @@ -627,13 +635,13 @@ mod tests { fn controversial_low_turnout_voting_should_work() { with_externalities(&mut new_test_ext(), || { System::set_block_number(1); - let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove); + 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_eq!(Democracy::tally(r), (60, 50)); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::era_length(), 1); @@ -647,14 +655,14 @@ mod tests { assert_eq!(Staking::total_stake(), 210); System::set_block_number(1); - let r = Democracy::inject_referendum(1, sessions_per_era_proposal(2), VoteThreshold::SuperMajorityApprove); + 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_eq!(Democracy::tally(r), (100, 50)); - Democracy::end_block(System::block_number()); + assert_eq!(Democracy::end_block(System::block_number()), Ok(())); Staking::check_new_era(); assert_eq!(Staking::era_length(), 2); diff --git a/substrate/runtime/executive/src/lib.rs b/substrate/runtime/executive/src/lib.rs index 35982f754b5d3..38c6cbf10a30d 100644 --- a/substrate/runtime/executive/src/lib.rs +++ b/substrate/runtime/executive/src/lib.rs @@ -97,7 +97,7 @@ impl< // execute transactions let (header, extrinsics) = block.deconstruct(); - extrinsics.into_iter().for_each(Self::apply_extrinsic_inner); + extrinsics.into_iter().for_each(Self::apply_extrinsic_no_note); // post-transactional book-keeping. Finalisation::execute(); @@ -116,33 +116,42 @@ impl< >::finalise() } - /// Apply outside of the block execution function. + /// Apply extrinsic outside of the block execution function. /// This doesn't attempt to validate anything regarding the block, but it builds a list of uxt /// hashes. pub fn apply_extrinsic(uxt: Block::Extrinsic) { - >::note_extrinsic(uxt.encode()); - Self::apply_extrinsic_inner(uxt); + let encoded = uxt.encode(); + let encoded_len = encoded.len(); + >::note_extrinsic(encoded); + Self::apply_extrinsic_no_note_with_len(uxt, encoded_len); } - /// Apply outside of the block execution function. - /// This doesn't attempt to validate anything regarding the block. - fn apply_extrinsic_inner(uxt: Block::Extrinsic) { + /// Apply an extrinsic inside the block execution function. + fn apply_extrinsic_no_note(uxt: Block::Extrinsic) { + let l = uxt.encode().len(); + Self::apply_extrinsic_no_note_with_len(uxt, l); + } + + /// Actually apply an extrinsic given its `encoded_len`; this doesn't note its hash. + fn apply_extrinsic_no_note_with_len(uxt: Block::Extrinsic, encoded_len: usize) { // Verify the signature is good. let xt = match uxt.check() { Ok(xt) => xt, - Err(_) => panic!("All transactions should be properly signed"), + Err(_) => panic!("All extrinsics should be properly signed"), }; if xt.sender() != &Default::default() { // check index let expected_index = >::account_index(xt.sender()); - assert!(xt.index() == &expected_index, "All transactions should have the correct nonce"); + assert!(xt.index() == &expected_index, "All extrinsics should have the correct nonce"); + + // pay any fees. + assert!(Payment::make_payment(xt.sender(), encoded_len), "All extrinsics should have sender able to pay their fees"); + + // AUDIT: Under no circumstances may this function panic from here onwards. // increment nonce in storage >::inc_account_index(xt.sender()); - - // pay any fees. - Payment::make_payment(xt.sender()); } // decode parameters and dispatch @@ -213,7 +222,8 @@ mod tests { intentions: vec![], validator_count: 0, bonding_duration: 0, - transaction_fee: 10, + transaction_base_fee: 10, + transaction_byte_fee: 0, }.build_externalities()); let xt = primitives::testing::TestXt((1, 0, Call::transfer(2, 69))); with_externalities(&mut t, || { @@ -239,7 +249,7 @@ mod tests { header: Header { parent_hash: [69u8; 32].into(), number: 1, - state_root: hex!("aa0cff04242e55fc780861b890aa8deba555f6ed95bd8fa575dfd80864f3b93e").into(), + state_root: hex!("1d43ef0fcabb78d925093fe22e50cc9ca5d182d189a3407c778e5fca714177dd").into(), extrinsics_root: hex!("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421").into(), digest: Digest { logs: vec![], }, }, @@ -273,7 +283,7 @@ mod tests { header: Header { parent_hash: [69u8; 32].into(), number: 1, - state_root: hex!("aa0cff04242e55fc780861b890aa8deba555f6ed95bd8fa575dfd80864f3b93e").into(), + state_root: hex!("1d43ef0fcabb78d925093fe22e50cc9ca5d182d189a3407c778e5fca714177dd").into(), extrinsics_root: [0u8; 32].into(), digest: Digest { logs: vec![], }, }, diff --git a/substrate/runtime/primitives/src/traits.rs b/substrate/runtime/primitives/src/traits.rs index 4b7530e6db70c..24cfa73ffced4 100644 --- a/substrate/runtime/primitives/src/traits.rs +++ b/substrate/runtime/primitives/src/traits.rs @@ -35,12 +35,13 @@ pub trait Verify { /// Simple payment making trait, operating on a single generic `AccountId` type. pub trait MakePayment { - /// Make some sort of payment concerning `who`. - fn make_payment(who: &AccountId); + /// Make some sort of payment concerning `who` for an extrinsic (transaction) of encoded length + /// `encoded_len` bytes. Return true iff the payment was successful. + fn make_payment(who: &AccountId, encoded_len: usize) -> bool; } impl MakePayment for () { - fn make_payment(_: &T) {} + fn make_payment(_: &T, _: usize) -> bool { true } } /// Extensible conversion trait. Generic over both source and destination types. diff --git a/substrate/runtime/staking/src/contract.rs b/substrate/runtime/staking/src/contract.rs index f3ac2816a63c4..4f03333e84756 100644 --- a/substrate/runtime/staking/src/contract.rs +++ b/substrate/runtime/staking/src/contract.rs @@ -205,6 +205,7 @@ pub(crate) fn execute<'a, 'b: 'a, T: Trait>( let account = e.account().clone(); if let Some(commit_state) = Module::::effect_transfer(&account, &transfer_to, value, e.account_db()) + .map_err(|_| sandbox::Error::Execution)? { e.account_db_mut().merge(commit_state); } @@ -231,6 +232,7 @@ pub(crate) fn execute<'a, 'b: 'a, T: Trait>( let account = e.account().clone(); if let Some(commit_state) = Module::::effect_create(&account, &code, value, e.account_db()) + .map_err(|_| sandbox::Error::Execution)? { e.account_db_mut().merge(commit_state); } diff --git a/substrate/runtime/staking/src/lib.rs b/substrate/runtime/staking/src/lib.rs index b1a9bae49fa6f..763c4fe2e7696 100644 --- a/substrate/runtime/staking/src/lib.rs +++ b/substrate/runtime/staking/src/lib.rs @@ -52,7 +52,7 @@ use rstd::cell::RefCell; use rstd::collections::btree_map::{BTreeMap, Entry}; use codec::Slicable; use runtime_support::{StorageValue, StorageMap, Parameter}; -use primitives::traits::{Zero, One, Bounded, RefInto, SimpleArithmetic, Executable, MakePayment}; +use primitives::traits::{Zero, One, Bounded, RefInto, SimpleArithmetic, Executable, MakePayment, As}; mod contract; #[cfg(test)] @@ -122,8 +122,10 @@ decl_storage! { pub SessionsPerEra get(sessions_per_era): b"sta:spe" => required T::BlockNumber; // The total amount of stake on the system. pub TotalStake get(total_stake): b"sta:tot" => required T::Balance; - // The fee to be paid for making a transaction. - pub TransactionFee get(transaction_fee): b"sta:fee" => required T::Balance; + // The fee to be paid for making a transaction; the base. + pub TransactionBaseFee get(transaction_base_fee): b"sta:basefee" => required T::Balance; + // The fee to be paid for making a transaction; the per-byte portion. + pub TransactionByteFee get(transaction_byte_fee): b"sta:bytefee" => required T::Balance; // The current era index. pub CurrentEra get(current_era): b"sta:era" => required T::BlockNumber; @@ -165,12 +167,22 @@ impl Module { Self::free_balance(who) + Self::reserved_balance(who) } - /// Some result as `slash(who, value)` (but without the side-effects) asuming there are no + /// Some result as `slash(who, value)` (but without the side-effects) assuming there are no /// balance changes in the meantime. pub fn can_slash(who: &T::AccountId, value: T::Balance) -> bool { Self::balance(who) >= value } + /// Same result as `deduct_unbonded(who, value)` (but without the side-effects) assuming there + /// are no balance changes in the meantime. + pub fn can_deduct_unbonded(who: &T::AccountId, value: T::Balance) -> bool { + if let LockStatus::Liquid = Self::unlock_block(who) { + Self::free_balance(who) >= value + } else { + false + } + } + /// The block at which the `who`'s funds become entirely liquid. pub fn unlock_block(who: &T::AccountId) -> LockStatus { match Self::bondage(who) { @@ -183,7 +195,7 @@ impl Module { /// Create a smart-contract account. pub fn create(aux: &T::PublicAux, code: &[u8], value: T::Balance) { // commit anything that made it this far to storage - if let Some(commit) = Self::effect_create(aux.ref_into(), code, value, &DirectAccountDb) { + if let Ok(Some(commit)) = Self::effect_create(aux.ref_into(), code, value, &DirectAccountDb) { >::merge(&mut DirectAccountDb, commit); } } @@ -194,7 +206,7 @@ impl Module { /// TODO: probably want to state gas-limit and gas-price. fn transfer(aux: &T::PublicAux, dest: T::AccountId, value: T::Balance) { // commit anything that made it this far to storage - if let Some(commit) = Self::effect_transfer(aux.ref_into(), &dest, value, &DirectAccountDb) { + if let Ok(Some(commit)) = Self::effect_transfer(aux.ref_into(), &dest, value, &DirectAccountDb) { >::merge(&mut DirectAccountDb, commit); } } @@ -205,7 +217,7 @@ impl Module { fn stake(aux: &T::PublicAux) { let mut intentions = >::get(); // can't be in the list twice. - assert!(intentions.iter().find(|&t| t == aux.ref_into()).is_none(), "Cannot stake if already staked."); + 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()); @@ -218,11 +230,11 @@ impl Module { 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 { - panic!("Cannot unstake if not already staked."); + fail!("Cannot unstake if not already staked."); } - >::put(intentions); - >::insert(aux.ref_into(), Self::current_era() + Self::bonding_duration()); } // PRIV DISPATCH @@ -280,11 +292,14 @@ impl Module { } /// Moves `value` from balance to reserved balance. - pub fn reserve_balance(who: &T::AccountId, value: T::Balance) { + pub fn reserve_balance(who: &T::AccountId, value: T::Balance) -> bool { let b = Self::free_balance(who); - assert!(b >= value); + if b < value { + return false; + } >::insert(who, b - value); >::insert(who, Self::reserved_balance(who) + value); + true } /// Moves `value` from reserved balance to balance. @@ -536,26 +551,28 @@ impl Module { code: &[u8], value: T::Balance, account_db: &DB, - ) -> Option> { + ) -> Result>, &'static str> { let from_balance = account_db.get_balance(transactor); // TODO: a fee. - assert!(from_balance >= value); + if from_balance < value { + return Err("balance too low to send value"); + } let dest = T::DetermineContractAddress::contract_address_for(code, transactor); // early-out if degenerate. if &dest == transactor { - return None; + return Ok(None); } let mut local = BTreeMap::new(); // two inserts are safe - assert!(&dest != transactor); + // note that we now know that `&dest != transactor` due to early-out before. local.insert(dest, ChangeEntry { balance: Some(value), code: Some(code.to_vec()), storage: Default::default() }); local.insert(transactor.clone(), ChangeEntry::balance_changed(from_balance - value)); - Some(local) + Ok(Some(local)) } fn effect_transfer>( @@ -563,13 +580,19 @@ impl Module { dest: &T::AccountId, value: T::Balance, account_db: &DB, - ) -> Option> { + ) -> Result>, &'static str> { let from_balance = account_db.get_balance(transactor); - assert!(from_balance >= value); + if from_balance < value { + return Err("balance too low to send value"); + } let to_balance = account_db.get_balance(dest); - assert!(>::get(transactor) <= >::get(dest)); - assert!(to_balance + value > to_balance); // no overflow + if >::get(transactor) > >::get(dest) { + return Err("bondage too high to send value"); + } + if to_balance + value <= to_balance { + return Err("destination balance too high to receive value"); + } // TODO: a fee, based upon gaslimit/gasprice. let gas_limit = 100_000; @@ -594,20 +617,23 @@ impl Module { contract::execute(&dest_code, dest, &mut overlay, gas_limit).is_ok() }; - if should_commit { + Ok(if should_commit { Some(overlay.into_state()) } else { None - } + }) } } impl MakePayment for Module { - fn make_payment(transactor: &T::AccountId) { + fn make_payment(transactor: &T::AccountId, encoded_len: usize) -> bool { let b = Self::free_balance(transactor); - let transaction_fee = Self::transaction_fee(); - assert!(b >= transaction_fee, "attempt to transact without enough funds to pay fee"); + let transaction_fee = Self::transaction_base_fee() + Self::transaction_byte_fee() * >::sa(encoded_len); + if b < transaction_fee { + return false; + } >::insert(transactor, b - transaction_fee); + true } } @@ -628,7 +654,8 @@ pub struct GenesisConfig { pub intentions: Vec, pub validator_count: u64, pub bonding_duration: T::BlockNumber, - pub transaction_fee: T::Balance, + pub transaction_base_fee: T::Balance, + pub transaction_byte_fee: T::Balance, } #[cfg(any(feature = "std", test))] @@ -642,7 +669,8 @@ impl GenesisConfig where T::AccountId: From { intentions: vec![T::AccountId::from(1), T::AccountId::from(2), T::AccountId::from(3)], validator_count: 3, bonding_duration: T::BlockNumber::sa(0), - transaction_fee: T::Balance::sa(0), + transaction_base_fee: T::Balance::sa(0), + transaction_byte_fee: T::Balance::sa(0), } } @@ -663,7 +691,8 @@ impl GenesisConfig where T::AccountId: From { intentions: vec![T::AccountId::from(1), T::AccountId::from(2), T::AccountId::from(3)], validator_count: 3, bonding_duration: T::BlockNumber::sa(0), - transaction_fee: T::Balance::sa(1), + transaction_base_fee: T::Balance::sa(1), + transaction_byte_fee: T::Balance::sa(0), } } } @@ -679,7 +708,8 @@ impl Default for GenesisConfig { intentions: vec![], validator_count: 0, bonding_duration: T::BlockNumber::sa(1000), - transaction_fee: T::Balance::sa(0), + transaction_base_fee: T::Balance::sa(0), + transaction_byte_fee: T::Balance::sa(0), } } } @@ -697,7 +727,8 @@ impl primitives::BuildExternalities for GenesisConfig { twox_128(>::key()).to_vec() => self.sessions_per_era.encode(), twox_128(>::key()).to_vec() => self.validator_count.encode(), twox_128(>::key()).to_vec() => self.bonding_duration.encode(), - twox_128(>::key()).to_vec() => self.transaction_fee.encode(), + twox_128(>::key()).to_vec() => self.transaction_base_fee.encode(), + twox_128(>::key()).to_vec() => self.transaction_byte_fee.encode(), twox_128(>::key()).to_vec() => self.current_era.encode(), twox_128(>::key()).to_vec() => total_stake.encode() ]; @@ -854,12 +885,12 @@ mod tests { } #[test] - #[should_panic] fn staking_balance_transfer_when_bonded_panics() { 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); }); } @@ -880,13 +911,13 @@ mod tests { }); } - #[test] #[should_panic] fn staking_balance_transfer_when_reserved_panics() { 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); }); } diff --git a/substrate/runtime/staking/src/mock.rs b/substrate/runtime/staking/src/mock.rs index 5769409519e53..7a805a6c02a72 100644 --- a/substrate/runtime/staking/src/mock.rs +++ b/substrate/runtime/staking/src/mock.rs @@ -68,7 +68,8 @@ pub fn new_test_ext(session_length: u64, sessions_per_era: u64, current_era: u64 intentions: vec![], validator_count: 2, bonding_duration: 3, - transaction_fee: 0, + transaction_base_fee: 0, + transaction_byte_fee: 0, }.build_externalities()); t } diff --git a/substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.compact.wasm b/substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.compact.wasm index c182bcf42f3e9..6a6e29748ce2a 100644 Binary files a/substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.compact.wasm and b/substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.compact.wasm differ diff --git a/substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.wasm b/substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.wasm index 7ef117e9c5db8..b09eb0853583e 100755 Binary files a/substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.wasm and b/substrate/test-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.wasm differ