Merge pull request paritytech#218 from subspace/extrinsics-shuffling

Implement the extrinsics shuffling

crates/cirrus-node-primitives/src/lib.rs

@@ -26,7 +26,7 @@ use sp_core::bytes;
 use sp_executor::{ExecutionReceipt, OpaqueBundle};
 use sp_runtime::traits::Hash as HashT;
 use std::pin::Pin;
-use subspace_core_primitives::Tag;
+use subspace_core_primitives::{Randomness, Tag};
 use subspace_runtime_primitives::{BlockNumber, Hash};
 
 /// Data required to produce bundles on executor node.
@@ -163,7 +163,11 @@ pub type BundlerFn = Box<
 ///
 /// Returns an optional [`ProcessorResult`].
 pub type ProcessorFn = Box<
-    dyn Fn(Hash, Vec<OpaqueBundle>) -> Pin<Box<dyn Future<Output = Option<ProcessorResult>> + Send>>
+    dyn Fn(
+            Hash,
+            Vec<OpaqueBundle>,
+            Randomness,
+        ) -> Pin<Box<dyn Future<Output = Option<ProcessorResult>> + Send>>
         + Send
         + Sync,
 >;

crates/sp-executor/Cargo.toml

@@ -19,6 +19,7 @@ sp-core = { version = "4.1.0-dev", default-features = false, git = "https://gith
 sp-runtime = { version = "4.0.0", default-features = false, git = "https://github.com/paritytech/substrate", rev = "5bd5b842d4ea520d281b1398e1f54907c9862fcd" }
 sp-std = { version = "4.0.0-dev", default-features = false, git = "https://github.com/paritytech/substrate", rev = "5bd5b842d4ea520d281b1398e1f54907c9862fcd" }
 sp-trie = { version = "4.0.0", default-features = false, git = "https://github.com/paritytech/substrate", rev = "5bd5b842d4ea520d281b1398e1f54907c9862fcd" }
+subspace-core-primitives = { version = "0.1.0", default-features = false, path = "../subspace-core-primitives" }
 
 [features]
 default = ["std"]
@@ -30,4 +31,5 @@ std = [
 	"sp-runtime/std",
 	"sp-std/std",
 	"sp-trie/std",
+	"subspace-core-primitives/std",
 ]

crates/sp-executor/src/lib.rs

@@ -24,6 +24,7 @@ use sp_runtime::traits::{BlakeTwo256, Block as BlockT, Hash as HashT, Header as
 use sp_runtime::{OpaqueExtrinsic, RuntimeDebug};
 use sp_std::vec::Vec;
 use sp_trie::StorageProof;
+use subspace_core_primitives::Randomness;
 
 /// Header of transaction bundle.
 #[derive(Decode, Encode, TypeInfo, PartialEq, Eq, Clone, RuntimeDebug)]
@@ -140,6 +141,9 @@ sp_api::decl_runtime_apis! {
         /// Extract the bundles from extrinsics in a block.
         fn extract_bundles(extrinsics: Vec<OpaqueExtrinsic>) -> Vec<OpaqueBundle>;
 
+        /// Generates a randomness seed for extrinsics shuffling.
+        fn extrinsics_shuffling_seed(header: Block::Header) -> Randomness;
+
         /// Returns the block hash given the block number.
         fn head_hash(
             number: <<Block as BlockT>::Header as HeaderT>::Number,

crates/subspace-runtime/src/lib.rs

@@ -37,15 +37,16 @@ use frame_system::limits::{BlockLength, BlockWeights};
 use frame_system::EnsureNever;
 use pallet_balances::NegativeImbalance;
 use sp_api::impl_runtime_apis;
+use sp_consensus_subspace::digests::CompatibleDigestItem;
 use sp_consensus_subspace::{
     Epoch, EquivocationProof, FarmerPublicKey, Salts, SubspaceEpochConfiguration,
     SubspaceGenesisConfiguration,
 };
 use sp_core::{crypto::KeyTypeId, OpaqueMetadata};
 use sp_executor::{FraudProof, OpaqueBundle};
 use sp_runtime::traits::{
-    AccountIdLookup, BlakeTwo256, Block as BlockT, DispatchInfoOf, Header as HeaderT,
-    PostDispatchInfoOf, Zero,
+    AccountIdLookup, BlakeTwo256, Block as BlockT, DispatchInfoOf, Hash as HashT,
+    Header as HeaderT, PostDispatchInfoOf, Zero,
 };
 use sp_runtime::transaction_validity::{
     InvalidTransaction, TransactionSource, TransactionValidity, TransactionValidityError,
@@ -57,7 +58,7 @@ use sp_std::prelude::*;
 use sp_version::NativeVersion;
 use sp_version::RuntimeVersion;
 use subspace_core_primitives::objects::{BlockObject, BlockObjectMapping};
-use subspace_core_primitives::{RootBlock, Sha256Hash, PIECE_SIZE};
+use subspace_core_primitives::{Randomness, RootBlock, Sha256Hash, PIECE_SIZE};
 pub use subspace_runtime_primitives::{
     opaque, AccountId, Balance, BlockNumber, Hash, Index, Moment, Signature, CONFIRMATION_DEPTH_K,
     MIN_REPLICATION_FACTOR, RECORDED_HISTORY_SEGMENT_SIZE, RECORD_SIZE,
@@ -746,6 +747,28 @@ fn extract_bundles(extrinsics: Vec<OpaqueExtrinsic>) -> Vec<OpaqueBundle> {
         .collect()
 }
 
+fn extrinsics_shuffling_seed<Block: BlockT>(header: Block::Header) -> Randomness {
+    if header.number().is_zero() {
+        Randomness::default()
+    } else {
+        let mut pre_digest: Option<_> = None;
+        for log in header.digest().logs() {
+            match (
+                log.as_subspace_pre_digest::<FarmerPublicKey>(),
+                pre_digest.is_some(),
+            ) {
+                (Some(_), true) => panic!("Multiple Subspace pre-runtime digests in a header"),
+                (None, _) => {}
+                (s, false) => pre_digest = s,
+            }
+        }
+
+        let pre_digest = pre_digest.expect("Header must contain one pre-runtime digest; qed");
+
+        BlakeTwo256::hash_of(&pre_digest.solution.signature).into()
+    }
+}
+
 impl_runtime_apis! {
     impl sp_api::Core<Block> for Runtime {
         fn version() -> RuntimeVersion {
@@ -905,6 +928,10 @@ impl_runtime_apis! {
             extract_bundles(extrinsics)
         }
 
+        fn extrinsics_shuffling_seed(header: <Block as BlockT>::Header) -> Randomness {
+            extrinsics_shuffling_seed::<Block>(header)
+        }
+
         fn head_hash(number: <<Block as BlockT>::Header as HeaderT>::Number) -> Option<<Block as BlockT>::Hash> {
             Executor::head_hash(number)
         }

cumulus/client/cirrus-executor/Cargo.toml

@@ -22,6 +22,8 @@ cumulus-client-consensus-common = { path = "../consensus/common" }
 codec = { package = "parity-scale-codec", version = "2.3.0", features = [ "derive" ] }
 futures = { version = "0.3.17", features = ["compat"] }
 futures-timer = "3.0.1"
+rand = "0.8.4"
+rand_chacha = "0.3.1"
 parking_lot = "0.11.2"
 tracing = "0.1.25"
 
@@ -31,4 +33,8 @@ polkadot-node-subsystem = { path = "../../../polkadot/node/subsystem" }
 cirrus-node-primitives = { path = "../../../crates/cirrus-node-primitives" }
 cirrus-primitives = { path = "../../primitives" }
 sp-executor = { path = "../../../crates/sp-executor" }
+subspace-core-primitives = { path = "../../../crates/subspace-core-primitives" }
 subspace-runtime-primitives = { path = "../../../crates/subspace-runtime-primitives" }
+
+[dev-dependencies]
+sp-keyring = { git = "https://github.com/paritytech/substrate", rev = "5bd5b842d4ea520d281b1398e1f54907c9862fcd" }

cumulus/client/cirrus-executor/src/lib.rs

@@ -43,6 +43,7 @@ use cirrus_node_primitives::{
 };
 use cirrus_primitives::{AccountId, SecondaryApi};
 use sp_executor::{Bundle, BundleHeader, ExecutionReceipt, FraudProof, OpaqueBundle};
+use subspace_core_primitives::Randomness;
 use subspace_runtime_primitives::Hash as PHash;
 
 use codec::{Decode, Encode};
@@ -327,8 +328,9 @@ where
 		self,
 		primary_hash: PHash,
 		bundles: Vec<OpaqueBundle>,
+		shuffling_seed: Randomness,
 	) -> Option<ProcessorResult> {
-		self.process_bundles_impl(primary_hash, bundles).await
+		self.process_bundles_impl(primary_hash, bundles, shuffling_seed).await
 	}
 }
 
@@ -397,11 +399,11 @@ pub async fn start_executor<Block, RA, BS, Spawner, Client, TransactionPool>(
 
 			bundler.produce_bundle(slot_info).instrument(bundler_span_clone.clone()).boxed()
 		}),
-		processor: Box::new(move |primary_hash, bundles| {
+		processor: Box::new(move |primary_hash, bundles, shuffling_seed| {
 			let processor = executor.clone();
 
 			processor
-				.process_bundles(primary_hash, bundles)
+				.process_bundles(primary_hash, bundles, shuffling_seed)
 				.instrument(span.clone())
 				.boxed()
 		}),

cumulus/client/cirrus-executor/src/processor.rs

@@ -1,15 +1,65 @@
+use rand::{seq::SliceRandom, SeedableRng};
+use rand_chacha::ChaCha8Rng;
 use sc_client_api::BlockBackend;
 use sp_api::ProvideRuntimeApi;
 use sp_runtime::{generic::BlockId, traits::Block as BlockT};
+use std::{
+	collections::{BTreeMap, VecDeque},
+	fmt::Debug,
+};
 
 use cirrus_node_primitives::ProcessorResult;
 use cirrus_primitives::{AccountId, SecondaryApi};
 use sp_executor::{ExecutionReceipt, OpaqueBundle};
+use subspace_core_primitives::Randomness;
 use subspace_runtime_primitives::Hash as PHash;
 
 use super::{Executor, LOG_TARGET};
 use codec::{Decode, Encode};
 
+/// Shuffles the extrinsics in a deterministic way.
+///
+/// The extrinsics are grouped by the signer. The extrinsics without a signer, i.e., unsigned
+/// extrinsics, are considered as a special group. The items in different groups are cross shuffled,
+/// while the order of items inside the same group is still maintained.
+fn shuffle_extrinsics<Extrinsic: Debug>(
+	extrinsics: Vec<(Option<AccountId>, Extrinsic)>,
+	shuffling_seed: Randomness,
+) -> Vec<Extrinsic> {
+	let mut rng = ChaCha8Rng::from_seed(shuffling_seed);
+
+	let mut positions = extrinsics
+		.iter()
+		.map(|(maybe_signer, _)| maybe_signer)
+		.cloned()
+		.collect::<Vec<_>>();
+
+	// Shuffles the positions using Fisher–Yates algorithm.
+	positions.shuffle(&mut rng);
+
+	let mut grouped_extrinsics: BTreeMap<Option<AccountId>, VecDeque<_>> =
+		extrinsics.into_iter().fold(BTreeMap::new(), |mut groups, (maybe_signer, tx)| {
+			groups.entry(maybe_signer).or_insert_with(VecDeque::new).push_back(tx);
+			groups
+		});
+
+	// The relative ordering for the items in the same group does not change.
+	let shuffled_extrinsics = positions
+		.into_iter()
+		.map(|maybe_signer| {
+			grouped_extrinsics
+				.get_mut(&maybe_signer)
+				.expect("Extrinsics are grouped correctly; qed")
+				.pop_front()
+				.expect("Extrinsic definitely exists as it's correctly grouped above; qed")
+		})
+		.collect::<Vec<_>>();
+
+	tracing::trace!(target: LOG_TARGET, ?shuffled_extrinsics, "Shuffled extrinsics");
+
+	shuffled_extrinsics
+}
+
 impl<Block, BS, RA, Client, TransactionPool> Executor<Block, BS, RA, Client, TransactionPool>
 where
 	Block: BlockT,
@@ -24,11 +74,8 @@ where
 		self,
 		primary_hash: PHash,
 		bundles: Vec<OpaqueBundle>,
+		shuffling_seed: Randomness,
 	) -> Option<ProcessorResult> {
-		// TODO:
-		// 1. [x] convert the bundles to a full tx list
-		// 2. [x] duplicate the full tx list
-		// 3. shuffle the full tx list by sender account
 		let mut extrinsics = bundles
 			.into_iter()
 			.flat_map(|bundle| {
@@ -65,6 +112,8 @@ where
 		});
 		drop(seen);
 
+		tracing::trace!(target: LOG_TARGET, ?extrinsics, "Origin deduplicated extrinsics");
+
 		let block_number = self.client.info().best_number;
 		let extrinsics: Vec<_> = match self
 			.runtime_api
@@ -82,7 +131,8 @@ where
 			},
 		};
 
-		let _final_extrinsics = Self::shuffle_extrinsics(extrinsics);
+		let _final_extrinsics =
+			shuffle_extrinsics::<<Block as BlockT>::Extrinsic>(extrinsics, shuffling_seed);
 
 		// TODO: now we have the final transaction list:
 		// - apply each tx one by one.
@@ -110,13 +160,36 @@ where
 			None
 		}
 	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use sp_keyring::sr25519::Keyring;
+	use sp_runtime::traits::{BlakeTwo256, Hash as HashT};
+
+	#[test]
+	fn shuffle_extrinsics_should_work() {
+		let alice = Keyring::Alice.to_account_id();
+		let bob = Keyring::Bob.to_account_id();
+		let charlie = Keyring::Charlie.to_account_id();
+
+		let extrinsics = vec![
+			(Some(alice.clone()), 10),
+			(None, 100),
+			(Some(bob.clone()), 1),
+			(Some(bob), 2),
+			(Some(charlie.clone()), 30),
+			(Some(alice.clone()), 11),
+			(Some(charlie), 31),
+			(None, 101),
+			(None, 102),
+			(Some(alice), 12),
+		];
+
+		let dummy_seed = BlakeTwo256::hash_of(&[1u8; 64]).into();
+		let shuffled_extrinsics = shuffle_extrinsics(extrinsics, dummy_seed);
 
-	// TODO:
-	// 1. determine the randomness generator
-	// 2. Fisher-Yates
-	fn shuffle_extrinsics(
-		_extrinsics: Vec<(Option<AccountId>, <Block as BlockT>::Extrinsic)>,
-	) -> Vec<<Block as BlockT>::Extrinsic> {
-		todo!()
+		assert_eq!(shuffled_extrinsics, vec![100, 30, 10, 1, 11, 101, 31, 12, 102, 2]);
 	}
 }

cumulus/parachain-template/node/src/command.rs

@@ -336,8 +336,8 @@ impl CliConfiguration<Self> for RelayChainCli {
 		_logger_hook: F,
 		_config: &sc_service::Configuration,
 	) -> Result<()>
-		where
-			F: FnOnce(&mut sc_cli::LoggerBuilder, &sc_service::Configuration),
+	where
+		F: FnOnce(&mut sc_cli::LoggerBuilder, &sc_service::Configuration),
 	{
 		unreachable!("PolkadotCli is never initialized; qed");
 	}

cumulus/parachain-template/runtime/src/lib.rs

@@ -24,14 +24,13 @@ use sp_version::RuntimeVersion;
 pub use cirrus_primitives::{AccountId, Address, Balance, BlockNumber, Hash, Index, Signature};
 use frame_support::{
 	construct_runtime, parameter_types,
-	traits::Everything,
+	traits::{ConstU16, ConstU32, Everything},
 	weights::{
 		constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight, WEIGHT_PER_SECOND},
 		DispatchClass, Weight, WeightToFeeCoefficient, WeightToFeeCoefficients,
 		WeightToFeePolynomial,
 	},
 };
-use frame_support::traits::{ConstU16, ConstU32};
 use frame_system::limits::{BlockLength, BlockWeights};
 pub use sp_runtime::{MultiAddress, Perbill, Permill};