Decouple randomness-collective-flip (#3792)

* Abstract Randomness trait

* bump version

* fix doc test

* simpify code a bit

* Apply suggestions from code review

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* fix tests

substrate/node-template/runtime/src/lib.rs

@@ -33,7 +33,7 @@ pub use sr_primitives::BuildStorage;
 pub use timestamp::Call as TimestampCall;
 pub use balances::Call as BalancesCall;
 pub use sr_primitives::{Permill, Perbill};
-pub use support::{StorageValue, construct_runtime, parameter_types};
+pub use support::{StorageValue, construct_runtime, parameter_types, traits::Randomness};
 
 /// An index to a block.
 pub type BlockNumber = u32;

substrate/node/runtime/src/lib.rs

@@ -22,7 +22,7 @@
 
 use rstd::prelude::*;
 use support::{
-	construct_runtime, parameter_types, traits::{SplitTwoWays, Currency}
+	construct_runtime, parameter_types, traits::{SplitTwoWays, Currency, Randomness}
 };
 use primitives::u32_trait::{_1, _2, _3, _4};
 use node_primitives::{
@@ -85,7 +85,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
 	spec_version: 174,
-	impl_version: 174,
+	impl_version: 175,
 	apis: RUNTIME_API_VERSIONS,
 };
 
@@ -398,6 +398,7 @@ parameter_types! {
 impl contracts::Trait for Runtime {
 	type Currency = Balances;
 	type Time = Timestamp;
+	type Randomness = RandomnessCollectiveFlip;
 	type Call = Call;
 	type Event = Event;
 	type DetermineContractAddress = contracts::SimpleAddressDeterminator<Runtime>;

substrate/srml/contracts/Cargo.toml

@@ -17,8 +17,6 @@ rstd = { package = "sr-std", path = "../../core/sr-std", default-features = fals
 sandbox = { package = "sr-sandbox", path = "../../core/sr-sandbox", default-features = false }
 support = { package = "srml-support", path = "../support", default-features = false }
 system = { package = "srml-system", path = "../system", default-features = false }
-randomness-collective-flip = { package = "srml-randomness-collective-flip", path = "../randomness-collective-flip", default-features = false }
-timestamp = { package = "srml-timestamp", path = "../timestamp", default-features = false }
 
 [dev-dependencies]
 wabt = "0.9.2"
@@ -27,6 +25,7 @@ hex-literal = "0.2.1"
 balances = { package = "srml-balances", path = "../balances" }
 hex = "0.3.2"
 timestamp = { package = "srml-timestamp", path = "../timestamp" }
+randomness-collective-flip = { package = "srml-randomness-collective-flip", path = "../randomness-collective-flip" }
 
 [features]
 default = ["std"]
@@ -35,7 +34,6 @@ std = [
 	"codec/std",
 	"primitives/std",
 	"sr-primitives/std",
-	"randomness-collective-flip/std",
 	"runtime-io/std",
 	"rstd/std",
 	"sandbox/std",

substrate/srml/contracts/src/exec.rs

@@ -22,7 +22,7 @@ use crate::rent;
 
 use rstd::prelude::*;
 use sr_primitives::traits::{Bounded, CheckedAdd, CheckedSub, Zero};
-use support::traits::{WithdrawReason, Currency, Time};
+use support::traits::{WithdrawReason, Currency, Time, Randomness};
 
 pub type AccountIdOf<T> = <T as system::Trait>::AccountId;
 pub type CallOf<T> = <T as Trait>::Call;
@@ -753,7 +753,7 @@ where
 	}
 
 	fn random(&self, subject: &[u8]) -> SeedOf<T> {
-		randomness_collective_flip::Module::<T>::random(subject)
+		T::Randomness::random(subject)
 	}
 
 	fn now(&self) -> &MomentOf<T> {

substrate/srml/contracts/src/lib.rs

@@ -122,9 +122,9 @@ use sr_primitives::{
 use support::dispatch::{Result, Dispatchable};
 use support::{
 	Parameter, decl_module, decl_event, decl_storage, storage::child,
-	parameter_types,
+	parameter_types, IsSubType
 };
-use support::{traits::{OnFreeBalanceZero, OnUnbalanced, Currency, Get, Time}, IsSubType};
+use support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency, Get, Time, Randomness};
 use system::{ensure_signed, RawOrigin, ensure_root};
 use primitives::storage::well_known_keys::CHILD_STORAGE_KEY_PREFIX;
 
@@ -335,6 +335,7 @@ parameter_types! {
 pub trait Trait: system::Trait {
 	type Currency: Currency<Self::AccountId>;
 	type Time: Time;
+	type Randomness: Randomness<Self::Hash>;
 
 	/// The outer call dispatch type.
 	type Call: Parameter + Dispatchable<Origin=<Self as system::Trait>::Origin> + IsSubType<Module<Self>, Self>;

substrate/srml/contracts/src/tests.rs

@@ -159,6 +159,7 @@ parameter_types! {
 impl Trait for Test {
 	type Currency = Balances;
 	type Time = Timestamp;
+	type Randomness = Randomness;
 	type Call = Call;
 	type DetermineContractAddress = DummyContractAddressFor;
 	type Event = MetaEvent;
@@ -187,6 +188,7 @@ type Balances = balances::Module<Test>;
 type Timestamp = timestamp::Module<Test>;
 type Contract = Module<Test>;
 type System = system::Module<Test>;
+type Randomness = randomness_collective_flip::Module<Test>;
 
 pub struct DummyContractAddressFor;
 impl ContractAddressFor<H256, u64> for DummyContractAddressFor {

substrate/srml/randomness-collective-flip/src/lib.rs

@@ -35,7 +35,7 @@
 //! ### Example - Get random seed for the current block
 //!
 //! ```
-//! use support::{decl_module, dispatch::Result};
+//! use support::{decl_module, dispatch::Result, traits::Randomness};
 //!
 //! pub trait Trait: system::Trait {}
 //!
@@ -54,7 +54,7 @@
 
 use rstd::{prelude::*, convert::TryInto};
 use sr_primitives::traits::Hash;
-use support::{decl_module, decl_storage};
+use support::{decl_module, decl_storage, traits::Randomness};
 use safe_mix::TripletMix;
 use codec::Encode;
 use system::Trait;
@@ -91,16 +91,7 @@ decl_storage! {
 	}
 }
 
-impl<T: Trait> Module<T> {
+impl<T: Trait> Randomness<T::Hash> for Module<T> {
-	/// Get the basic random seed.
-	///
-	/// In general you won't want to use this, but rather `Self::random` which allows you to give a
-	/// subject for the random result and whose value will be independently low-influence random
-	/// from any other such seeds.
-	pub fn random_seed() -> T::Hash {
-		Self::random(&[][..])
-	}
-
 	/// Get a low-influence "random" value.
 	///
 	/// Being a deterministic block chain, real randomness is difficult to come by. This gives you
@@ -138,7 +129,7 @@ impl<T: Trait> Module<T> {
 	/// WARNING: Hashing the result of this function will remove any low-influence properties it has
 	/// and mean that all bits of the resulting value are entirely manipulatable by the author of
 	/// the parent block, who can determine the value of `parent_hash`.
-	pub fn random(subject: &[u8]) -> T::Hash {
+	fn random(subject: &[u8]) -> T::Hash {
 		let block_number = <system::Module<T>>::block_number();
 		let index = block_number_to_index::<T>(block_number);
 
@@ -166,7 +157,7 @@ mod tests {
 		Perbill, traits::{BlakeTwo256, OnInitialize, Header as _, IdentityLookup}, testing::Header,
 		set_and_run_with_externalities,
 	};
-	use support::{impl_outer_origin, parameter_types};
+	use support::{impl_outer_origin, parameter_types, traits::Randomness};
 
 	#[derive(Clone, PartialEq, Eq)]
 	pub struct Test;
@@ -202,7 +193,7 @@ mod tests {
 	}
 
 	type System = system::Module<Test>;
-	type Randomness = Module<Test>;
+	type CollectiveFlip = Module<Test>;
 
 	fn new_test_ext() -> runtime_io::TestExternalities {
 		let t = system::GenesisConfig::default().build_storage::<Test>().unwrap();
@@ -221,7 +212,7 @@ mod tests {
 
 		for i in 1 .. (blocks + 1) {
 			System::initialize(&i, &parent_hash, &Default::default(), &Default::default());
-			Randomness::on_initialize(i);
+			CollectiveFlip::on_initialize(i);
 
 			let header = System::finalize();
 			parent_hash = header.hash();
@@ -236,7 +227,7 @@ mod tests {
 
 			setup_blocks(38);
 
-			let random_material = Randomness::random_material();
+			let random_material = CollectiveFlip::random_material();
 
 			assert_eq!(random_material.len(), 38);
 			assert_eq!(random_material[0], genesis_hash);
@@ -250,7 +241,7 @@ mod tests {
 
 			setup_blocks(81);
 
-			let random_material = Randomness::random_material();
+			let random_material = CollectiveFlip::random_material();
 
 			assert_eq!(random_material.len(), 81);
 			assert_ne!(random_material[0], random_material[1]);
@@ -265,7 +256,7 @@ mod tests {
 
 			setup_blocks(162);
 
-			let random_material = Randomness::random_material();
+			let random_material = CollectiveFlip::random_material();
 
 			assert_eq!(random_material.len(), 81);
 			assert_ne!(random_material[0], random_material[1]);
@@ -279,13 +270,13 @@ mod tests {
 			setup_blocks(162);
 
 			assert_eq!(System::block_number(), 162);
-			assert_eq!(Randomness::random_seed(), Randomness::random_seed());
+			assert_eq!(CollectiveFlip::random_seed(), CollectiveFlip::random_seed());
-			assert_ne!(Randomness::random(b"random_1"), Randomness::random(b"random_2"));
+			assert_ne!(CollectiveFlip::random(b"random_1"), CollectiveFlip::random(b"random_2"));
 
-			let random = Randomness::random_seed();
+			let random = CollectiveFlip::random_seed();
 
 			assert_ne!(random, H256::zero());
-			assert!(!Randomness::random_material().contains(&random));
+			assert!(!CollectiveFlip::random_material().contains(&random));
 		});
 	}
 }

substrate/srml/support/src/traits.rs

@@ -717,3 +717,23 @@ pub trait InitializeMembers<AccountId> {
 impl<T> InitializeMembers<T> for () {
 	fn initialize_members(_: &[T]) {}
 }
+
+// A trait that is able to provide randomness.
+pub trait Randomness<Output> {
+	/// Get a "random" value
+	///
+	/// Being a deterministic blockchain, real randomness is difficult to come by. This gives you
+	/// something that approximates it. `subject` is a context identifier and allows you to get a
+	/// different result to other callers of this function; use it like
+	/// `random(&b"my context"[..])`.
+	fn random(subject: &[u8]) -> Output;
+
+	/// Get the basic random seed.
+	///
+	/// In general you won't want to use this, but rather `Self::random` which allows you to give a
+	/// subject for the random result and whose value will be independently low-influence random
+	/// from any other such seeds.
+	fn random_seed() -> Output {
+		Self::random(&[][..])
+	}
+}