Handle AccountIndices in transaction pool (#225)

* Merge remote-tracking branch 'origin/master' into gav-xts-dont-panic

* Update wasm.

* consensus, session and staking all panic-safe.

* Democracy doesn't panic in apply.

* Fix tests.

* Extra helper macro, council depanicked.

* Fix one test.

* Fix up all council tests. No panics!

* Council voting depanicked.

* Dispatch returns result.

* session & staking tests updated

* Fix democracy tests.

* Fix council tests.

* Fix up polkadot parachains in runtime

* Fix borked merge

* More Slicable support

Support general `Option` and array types.

* Basic storage types.

* Existential deposit for contract creation

* Basic implemnetation along with removals

* Fix tests.

* externalities builder fix.

* Tests.

* Fix up the runtime.

* Fix tests.

* Add generic `Address` type.

* Initial function integration of Address into Extrinsic.

* Fix build

* All tests compile.

* Fix (some) tests.

* Fix signing.

* Push error.

* transfer can accept Address

* Make Address generic over AccountIndex

* Fix test

* Make Council use Address for dispatch.

* Fix build

* Bend over backwards to support braindead derive.

* Repot some files.

* Fix tests.

* Fix grumbles

* Remove Default bound

* Fix build for new nightly.

* Make `apply_extrinsic` never panic, return useful Result.

* More merge hell

* Doesn't build, but might do soon

* Serde woes

* get substrate-runtime-staking compiling

* Polkadot builds again!

* Fix all build.

* Fix tests & binaries.

* Reserve some extra initial byte values of address for future format changes

* Make semantic of `ReservedBalance` clear.

* Fix panic handler.

* Integrate other balance transformations into the new model

Fix up staking tests.

* Fix runtime tests.

* Fix panic build.

* Tests for demonstrating interaction between balance types.

* Repot some runtime code

* Fix checkedblock in non-std builds

* Get rid of `DoLookup` phantom.

* Attempt to make transaction_pool work with lookups.

* Remove vscode settings

* New attempt at making transaction pool work.

* It builds again!

* --all builds

* Fix tests.

* New build.

* Test account nonce reset.

* polkadot transaction pool tests/framework.

* Address grumbles.

* Pool support non-verified transactions.

* Revert bad `map_or`

* Rebuild binaries, workaround.

* Avoid casting to usize early.

* Make verification use provided block_id.

* Fix tests.

* Alter tests to use retry.

* Fix tests & add call to re-verify.

* Semi-refactor.

* Integrate new queue with the rest of the code.

* Fix tests.

* Add reverify_transaction method.

* Use result.

substrate/substrate/client/src/client.rs

@@ -155,7 +155,7 @@ pub fn new_in_mem<E, Block, S>(
 	Client::new(backend, executor, genesis_storage)
 }
 
-impl<B, E, Block: BlockT> Client<B, E, Block> where
+impl<B, E, Block> Client<B, E, Block> where
 	B: backend::Backend<Block>,
 	E: CallExecutor<Block>,
 	Block: BlockT,

substrate/substrate/extrinsic-pool/src/api.rs

@@ -16,9 +16,7 @@
 
 //! External API for extrinsic pool.
 
-use std::fmt;
-use std::ops::Deref;
-use txpool::{self, VerifiedTransaction};
+use txpool;
 
 /// Extrinsic pool error.
 pub trait Error: ::std::error::Error + Send + Sized {
@@ -35,28 +33,10 @@ impl Error for txpool::Error {
 }
 
 /// Extrinsic pool.
-pub trait ExtrinsicPool<Ex, Hash>: Send + Sync + 'static {
+pub trait ExtrinsicPool<Ex, BlockId, Hash>: Send + Sync + 'static {
 	/// Error type
 	type Error: Error;
 
 	/// Submit a collection of extrinsics to the pool.
-	fn submit(&self, xt: Vec<Ex>) -> Result<Vec<Hash>, Self::Error>;
-}
-
-// Blanket implementation for anything that `Derefs` to the pool.
-impl<Ex, Hash, V, S, E, T> ExtrinsicPool<Ex, Hash> for T where
-	Hash: ::std::hash::Hash + Eq + Copy + fmt::Debug + fmt::LowerHex + Default,
-	T: Deref<Target=super::Pool<Ex, Hash, V, S, E>> + Send + Sync + 'static,
-	V: txpool::Verifier<Ex>,
-	S: txpool::Scoring<V::VerifiedTransaction>,
-	V::VerifiedTransaction: txpool::VerifiedTransaction<Hash=Hash>,
-	E: From<V::Error>,
-	E: From<txpool::Error>,
-	E: Error,
-{
-	type Error = E;
-
-	fn submit(&self, xt: Vec<Ex>) -> Result<Vec<Hash>, Self::Error> {
-		self.deref().submit(xt).map(|result| result.into_iter().map(|xt| *xt.hash()).collect())
-	}
+	fn submit(&self, block: BlockId, xt: Vec<Ex>) -> Result<Vec<Hash>, Self::Error>;
 }

substrate/substrate/extrinsic-pool/src/lib.rs

@@ -32,5 +32,6 @@ mod listener;
 mod pool;
 mod watcher;
 
+pub use self::listener::Listener;
 pub use self::pool::Pool;
 pub use self::watcher::Watcher;

substrate/substrate/extrinsic-pool/src/listener.rs

@@ -23,17 +23,22 @@ use txpool;
 
 use watcher;
 
+/// Extrinsic pool default listener.
 #[derive(Default)]
 pub struct Listener<H: ::std::hash::Hash + Eq> {
 	watchers: HashMap<H, watcher::Sender<H>>
 }
 
 impl<H: ::std::hash::Hash + Eq + Copy + fmt::Debug + fmt::LowerHex + Default> Listener<H> {
+	/// Creates a new watcher for given verified extrinsic.
+	///
+	/// The watcher can be used to subscribe to lifecycle events of that extrinsic.
 	pub fn create_watcher<T: txpool::VerifiedTransaction<Hash=H>>(&mut self, xt: Arc<T>) -> watcher::Watcher<H> {
 		let sender = self.watchers.entry(*xt.hash()).or_insert_with(watcher::Sender::default);
 		sender.new_watcher()
 	}
 
+	/// Notify the listeners about extrinsic broadcast.
 	pub fn broadcasted(&mut self, hash: &H, peers: Vec<String>) {
 		self.fire(hash, |watcher| watcher.broadcast(peers));
 	}

substrate/substrate/extrinsic-pool/src/pool.rs

@@ -29,41 +29,37 @@ use listener::Listener;
 use watcher::Watcher;
 
 /// Extrinsics pool.
-pub struct Pool<Ex, Hash, V, S, E> where
+pub struct Pool<Hash, VEx, S, E> where
 	Hash: ::std::hash::Hash + Eq + Copy + fmt::Debug + fmt::LowerHex,
-	V: txpool::Verifier<Ex>,
-	S: txpool::Scoring<V::VerifiedTransaction>,
+	S: txpool::Scoring<VEx>,
+	VEx: txpool::VerifiedTransaction<Hash=Hash>,
 {
 	_error: Mutex<PhantomData<E>>,
 	pool: RwLock<txpool::Pool<
-		V::VerifiedTransaction,
+		VEx,
 		S,
 		Listener<Hash>,
 	>>,
-	verifier: V,
-	import_notification_sinks: Mutex<Vec<mpsc::UnboundedSender<Weak<V::VerifiedTransaction>>>>,
+	import_notification_sinks: Mutex<Vec<mpsc::UnboundedSender<Weak<VEx>>>>,
 }
 
-impl<Ex, Hash, V, S, E> Pool<Ex, Hash, V, S, E> where
+impl<Hash, VEx, S, E> Pool<Hash, VEx, S, E> where
 	Hash: ::std::hash::Hash + Eq + Copy + fmt::Debug + fmt::LowerHex + Default,
-	V: txpool::Verifier<Ex>,
-	S: txpool::Scoring<V::VerifiedTransaction>,
-	V::VerifiedTransaction: txpool::VerifiedTransaction<Hash=Hash>,
-	E: From<V::Error>,
+	S: txpool::Scoring<VEx>,
+	VEx: txpool::VerifiedTransaction<Hash=Hash>,
 	E: From<txpool::Error>,
 {
 	/// Create a new transaction pool.
-	pub fn new(options: txpool::Options, verifier: V, scoring: S) -> Self {
+	pub fn new(options: txpool::Options, scoring: S) -> Self {
 		Pool {
 			_error: Default::default(),
 			pool: RwLock::new(txpool::Pool::new(Listener::default(), scoring, options)),
-			verifier,
 			import_notification_sinks: Default::default(),
 		}
 	}
 
 	/// Imports a pre-verified extrinsic to the pool.
-	pub fn import(&self, xt: V::VerifiedTransaction) -> Result<Arc<V::VerifiedTransaction>, E> {
+	pub fn import(&self, xt: VEx) -> Result<Arc<VEx>, E> {
 		let result = self.pool.write().import(xt)?;
 
 		let weak = Arc::downgrade(&result);
@@ -74,7 +70,7 @@ impl<Ex, Hash, V, S, E> Pool<Ex, Hash, V, S, E> where
 	}
 
 	/// Return an event stream of transactions imported to the pool.
-	pub fn import_notification_stream(&self) -> mpsc::UnboundedReceiver<Weak<V::VerifiedTransaction>> {
+	pub fn import_notification_stream(&self) -> mpsc::UnboundedReceiver<Weak<VEx>> {
 		let (sink, stream) = mpsc::unbounded();
 		self.import_notification_sinks.lock().push(sink);
 		stream
@@ -87,11 +83,15 @@ impl<Ex, Hash, V, S, E> Pool<Ex, Hash, V, S, E> where
 		}
 	}
 
-	/// Imports a bunch of extrinsics to the pool
-	pub fn submit(&self, xts: Vec<Ex>) -> Result<Vec<Arc<V::VerifiedTransaction>>, E> {
+	/// Imports a bunch of unverified extrinsics to the pool
+	pub fn submit<V, Ex, T>(&self, verifier: V, xts: T) -> Result<Vec<Arc<VEx>>, E> where
+		V: txpool::Verifier<Ex, VerifiedTransaction=VEx>,
+		E: From<V::Error>,
+		T: IntoIterator<Item=Ex>
+	{
 		xts
 			.into_iter()
-			.map(|xt| self.verifier.verify_transaction(xt))
+			.map(|xt| verifier.verify_transaction(xt))
 			.map(|xt| {
 				Ok(self.pool.write().import(xt?)?)
 			})
@@ -99,13 +99,16 @@ impl<Ex, Hash, V, S, E> Pool<Ex, Hash, V, S, E> where
 	}
 
 	/// Import a single extrinsic and starts to watch their progress in the pool.
-	pub fn submit_and_watch(&self, xt: Ex) -> Result<Watcher<Hash>, E> {
-		let xt = self.submit(vec![xt])?.pop().expect("One extrinsic passed; one result returned; qed");
+	pub fn submit_and_watch<V, Ex>(&self, verifier: V, xt: Ex) -> Result<Watcher<Hash>, E> where
+		V: txpool::Verifier<Ex, VerifiedTransaction=VEx>,
+		E: From<V::Error>,
+	{
+		let xt = self.submit(verifier, vec![xt])?.pop().expect("One extrinsic passed; one result returned; qed");
 		Ok(self.pool.write().listener_mut().create_watcher(xt))
 	}
 
 	/// Remove from the pool.
-	pub fn remove(&self, hashes: &[Hash], is_valid: bool) -> Vec<Option<Arc<V::VerifiedTransaction>>> {
+	pub fn remove(&self, hashes: &[Hash], is_valid: bool) -> Vec<Option<Arc<VEx>>> {
 		let mut pool = self.pool.write();
 		let mut results = Vec::with_capacity(hashes.len());
 		for hash in hashes {
@@ -115,24 +118,14 @@ impl<Ex, Hash, V, S, E> Pool<Ex, Hash, V, S, E> where
 	}
 
 	/// Cull transactions from the queue.
-	pub fn cull<R>(&self, senders: Option<&[<V::VerifiedTransaction as txpool::VerifiedTransaction>::Sender]>, ready: R) -> usize where
-		R: txpool::Ready<V::VerifiedTransaction>,
+	pub fn cull<R>(&self, senders: Option<&[<VEx as txpool::VerifiedTransaction>::Sender]>, ready: R) -> usize where
+		R: txpool::Ready<VEx>,
 	{
 		self.pool.write().cull(senders, ready)
 	}
 
-	/// Cull transactions from the queue and then compute the pending set.
-	pub fn cull_and_get_pending<R, F, T>(&self, ready: R, f: F) -> T where
-		R: txpool::Ready<V::VerifiedTransaction> + Clone,
-		F: FnOnce(txpool::PendingIterator<V::VerifiedTransaction, R, S, Listener<Hash>>) -> T,
-	{
-		let mut pool = self.pool.write();
-		pool.cull(None, ready.clone());
-		f(pool.pending(ready))
-	}
-
 	/// Get the full status of the queue (including readiness)
-	pub fn status<R: txpool::Ready<V::VerifiedTransaction>>(&self, ready: R) -> txpool::Status {
+	pub fn status<R: txpool::Ready<VEx>>(&self, ready: R) -> txpool::Status {
 		self.pool.read().status(ready)
 	}
 
@@ -140,4 +133,21 @@ impl<Ex, Hash, V, S, E> Pool<Ex, Hash, V, S, E> where
 	pub fn light_status(&self) -> txpool::LightStatus {
 		self.pool.read().light_status()
 	}
+
+	/// Removes all transactions from given sender
+	pub fn remove_sender(&self, sender: VEx::Sender) -> Vec<Arc<VEx>> {
+		let mut pool = self.pool.write();
+		let pending = pool.pending_from_sender(|_: &VEx| txpool::Readiness::Ready, &sender).collect();
+		// remove all transactions from this sender
+		pool.cull(Some(&[sender]), |_: &VEx| txpool::Readiness::Stale);
+		pending
+	}
+
+	/// Retrieve the pending set. Be careful to not leak the pool `ReadGuard` to prevent deadlocks.
+	pub fn pending<R, F, T>(&self, ready: R, f: F) -> T where
+		R: txpool::Ready<VEx>,
+		F: FnOnce(txpool::PendingIterator<VEx, R, S, Listener<Hash>>) -> T,
+	{
+		f(self.pool.read().pending(ready))
+	}
 }

substrate/substrate/rpc/src/author/mod.rs

@@ -17,8 +17,13 @@
 //! Substrate block-author/full-node API.
 
 use std::sync::Arc;
+
+use client::{self, Client};
 use extrinsic_pool::api::{Error, ExtrinsicPool};
 
+use runtime_primitives::{generic, traits::Block as BlockT};
+use state_machine;
+
 pub mod error;
 
 #[cfg(test)]
@@ -35,13 +40,34 @@ build_rpc_trait! {
 	}
 }
 
-impl<Ex, Hash, T> AuthorApi<Hash, Ex> for Arc<T> where
-	T: ExtrinsicPool<Ex, Hash>,
-	T::Error: 'static,
+/// Authoring API
+pub struct Author<B, E, Block: BlockT, P> {
+	/// Substrate client
+	client: Arc<Client<B, E, Block>>,
+	/// Extrinsic pool
+	pool: Arc<P>,
+}
+
+impl<B, E, Block: BlockT, P> Author<B, E, Block, P> {
+	/// Create new instance of Authoring API.
+	pub fn new(client: Arc<Client<B, E, Block>>, pool: Arc<P>) -> Self {
+		Author { client, pool }
+	}
+}
+
+
+impl<B, E, Block, P, Ex, Hash> AuthorApi<Hash, Ex> for Author<B, E, Block, P> where
+	B: client::backend::Backend<Block> + Send + Sync + 'static,
+	E: client::CallExecutor<Block> + Send + Sync + 'static,
+	Block: BlockT + 'static,
+	client::error::Error: From<<<B as client::backend::Backend<Block>>::State as state_machine::backend::Backend>::Error>,
+	P: ExtrinsicPool<Ex, generic::BlockId<Block>, Hash>,
+	P::Error: 'static,
 {
 	fn submit_extrinsic(&self, xt: Ex) -> Result<Hash> {
-		self
-			.submit(vec![xt])
+		let best_block_hash = self.client.info().unwrap().chain.best_hash;
+		self.pool
+			.submit(generic::BlockId::hash(best_block_hash), vec![xt])
 			.map(|mut res| res.pop().expect("One extrinsic passed; one result back; qed"))
 			.map_err(|e| e.into_pool_error()
 				.map(Into::into)

substrate/substrate/rpc/src/author/tests.rs

@@ -18,6 +18,7 @@ use super::*;
 
 use std::{fmt, sync::Arc};
 use extrinsic_pool::api;
+use test_client;
 use parking_lot::Mutex;
 
 type Extrinsic = u64;
@@ -40,11 +41,11 @@ impl fmt::Display for Error {
 	}
 }
 
-impl api::ExtrinsicPool<Extrinsic, Hash> for DummyTxPool {
+impl<BlockHash> api::ExtrinsicPool<Extrinsic, BlockHash, u64> for DummyTxPool {
 	type Error = Error;
 
 	/// Submit extrinsic for inclusion in block.
-	fn submit(&self, xt: Vec<Extrinsic>) -> ::std::result::Result<Vec<Hash>, Self::Error> {
+	fn submit(&self, _block: BlockHash, xt: Vec<Extrinsic>) -> ::std::result::Result<Vec<Hash>, Self::Error> {
 		let mut submitted = self.submitted.lock();
 		if submitted.len() < 1 {
 			let hashes = xt.iter().map(|_xt| 1).collect();
@@ -58,7 +59,10 @@ impl api::ExtrinsicPool<Extrinsic, Hash> for DummyTxPool {
 
 #[test]
 fn submit_transaction_should_not_cause_error() {
-	let p = Arc::new(DummyTxPool::default());
+	let p = Author {
+		client: Arc::new(test_client::new()),
+		pool: Arc::new(DummyTxPool::default()),
+	};
 
 	assert_matches!(
 		AuthorApi::submit_extrinsic(&p, 5),

substrate/substrate/runtime/primitives/src/generic.rs

@@ -115,7 +115,7 @@ where
 	}
 
 	fn check<ThisLookup>(self, lookup: ThisLookup) -> Result<Self::Checked, &'static str> where
-		ThisLookup: FnOnce(Address) -> Result<AccountId, &'static str> + Send + Sync,
+		ThisLookup: FnOnce(Address) -> Result<AccountId, &'static str>,
 	{
 		if !self.is_signed() {
 			Ok(CheckedExtrinsic(Extrinsic {

substrate/substrate/runtime/primitives/src/testing.rs

@@ -162,7 +162,7 @@ impl<Call: 'static + AuxDispatchable + Slicable + Sized + Send + Sync + Serializ
 	type Address = u64;
 	type AccountId = u64;
 	fn sender(&self) -> &u64 { &(self.0).0 }
-	fn check<ThisLookup: FnOnce(Self::Address) -> Result<Self::AccountId, &'static str> + Send + Sync>(self, _lookup: ThisLookup) -> Result<Self::Checked, &'static str> { Ok(self) }
+	fn check<ThisLookup: FnOnce(Self::Address) -> Result<Self::AccountId, &'static str>>(self, _lookup: ThisLookup) -> Result<Self::Checked, &'static str> { Ok(self) }
 }
 impl<Call: AuxDispatchable<Aux = u64> + Slicable + Sized + Send + Sync + Serialize + DeserializeOwned + Clone + Eq + Debug> Applyable for TestXt<Call> {
 	type AccountId = u64;

substrate/substrate/runtime/primitives/src/traits.rs

@@ -376,13 +376,13 @@ pub trait Checkable: Sized + Send + Sync {
 	type AccountId: Member + MaybeDisplay;
 	type Checked: Member;
 	fn sender(&self) -> &Self::Address;
-	fn check<ThisLookup: FnOnce(Self::Address) -> Result<Self::AccountId, &'static str> + Send + Sync>(self, lookup: ThisLookup) -> Result<Self::Checked, &'static str>;
+	fn check<ThisLookup: FnOnce(Self::Address) -> Result<Self::AccountId, &'static str>>(self, lookup: ThisLookup) -> Result<Self::Checked, &'static str>;
 }
 
 /// A "checkable" piece of information, used by the standard Substrate Executive in order to
 /// check the validity of a piece of extrinsic information, usually by verifying the signature.
 ///
-/// This does that checking without requiring a lookup argument. 
+/// This does that checking without requiring a lookup argument.
 pub trait BlindCheckable: Sized + Send + Sync {
 	type Address: Member + MaybeDisplay;
 	type Checked: Member;
@@ -395,7 +395,7 @@ impl<T: BlindCheckable> Checkable for T {
 	type AccountId = <Self as BlindCheckable>::Address;
 	type Checked = <Self as BlindCheckable>::Checked;
 	fn sender(&self) -> &Self::Address { BlindCheckable::sender(self) }
-	fn check<ThisLookup: FnOnce(Self::Address) -> Result<Self::AccountId, &'static str> + Send + Sync>(self, _: ThisLookup) -> Result<Self::Checked, &'static str> { BlindCheckable::check(self) }
+	fn check<ThisLookup: FnOnce(Self::Address) -> Result<Self::AccountId, &'static str>>(self, _: ThisLookup) -> Result<Self::Checked, &'static str> { BlindCheckable::check(self) }
 }
 
 /// An "executable" piece of information, used by the standard Substrate Executive in order to