Issue 212 - refactor Checkable trait to be more generic

demo/runtime/src/lib.rs

@@ -89,6 +89,7 @@ impl system::Trait for Concrete {
 	type Hashing = BlakeTwo256;
 	type Digest = generic::Digest<Vec<u8>>;
 	type AccountId = AccountId;
+	type Address = Address;
 	type Header = generic::Header<BlockNumber, BlakeTwo256, Vec<u8>>;
 }
 

polkadot/runtime/src/lib.rs

@@ -92,6 +92,8 @@ pub type Address = staking::Address<Concrete>;
 pub type BlockId = generic::BlockId<Block>;
 /// Unchecked extrinsic type as expected by this runtime.
 pub type UncheckedExtrinsic = generic::UncheckedExtrinsic<Address, Index, Call, Signature>;
+/// Checked extrinsic type as expected by this runtime.
+pub type CheckedExtrinsic = generic::CheckedExtrinsic<AccountId, Index, Call>;
 /// Extrinsic type as expected by this runtime. This is not the type that is signed.
 pub type Extrinsic = generic::Extrinsic<Address, Index, Call>;
 /// Extrinsic type that is signed.
@@ -132,6 +134,7 @@ impl system::Trait for Concrete {
 	type Hashing = BlakeTwo256;
 	type Digest = generic::Digest<Log>;
 	type AccountId = AccountId;
+	type Address = Address;
 	type Header = Header;
 }
 /// System module for this concrete runtime.

polkadot/runtime/src/parachains.rs

@@ -254,6 +254,7 @@ mod tests {
 		type Hashing = BlakeTwo256;
 		type Digest = Digest;
 		type AccountId = u64;
+		type Address = u64;
 		type Header = Header;
 	}
 	impl session::Trait for Test {

polkadot/runtime/src/utils.rs

@@ -47,5 +47,5 @@ pub fn inherent_extrinsics(timestamp: ::primitives::Timestamp, parachain_heads:
 
 /// Checks an unchecked extrinsic for validity.
 pub fn check_extrinsic(xt: UncheckedExtrinsic) -> bool {
-	xt.check(Staking::lookup).is_ok()
+	xt.check_with(Staking::lookup).is_ok()
 }

polkadot/transaction-pool/src/lib.rs

@@ -48,15 +48,12 @@ use extrinsic_pool::{Pool, Listener, txpool::{self, Readiness, scoring::{Change,
 use extrinsic_pool::api::ExtrinsicPool;
 use polkadot_api::PolkadotApi;
 use primitives::{AccountId, BlockId, Hash, Index, UncheckedExtrinsic as FutureProofUncheckedExtrinsic};
-use runtime::{Address, UncheckedExtrinsic};
+use runtime::{Address, UncheckedExtrinsic, CheckedExtrinsic};
 use substrate_runtime_primitives::traits::{Bounded, Checkable, Hashing, BlakeTwo256};
 
 pub use extrinsic_pool::txpool::{Options, Status, LightStatus, VerifiedTransaction as VerifiedTransactionOps};
 pub use error::{Error, ErrorKind, Result};
 
-/// Type alias for convenience.
-pub type CheckedExtrinsic = <UncheckedExtrinsic as Checkable>::Checked;
-
 /// A verified transaction which should be includable and non-inherent.
 #[derive(Clone, Debug)]
 pub struct VerifiedTransaction {
@@ -281,11 +278,11 @@ impl<'a, A> txpool::Verifier<UncheckedExtrinsic> for Verifier<'a, A> where
 		}
 
 		let (encoded_size, hash) = uxt.using_encoded(|e| (e.len(), BlakeTwo256::hash(e)));
-		let inner = match uxt.clone().check(|a| self.lookup(a)) {
+		let inner = match uxt.clone().check_with(|a| self.lookup(a)) {
 			Ok(xt) => Some(xt),
 			// keep the transaction around in the future pool and attempt to promote it later.
-			Err(Self::NO_ACCOUNT) => None,
-			Err(e) => bail!(e),
+			Err((_, Self::NO_ACCOUNT)) => None,
+			Err((_, e)) => bail!(e),
 		};
 		let sender = inner.as_ref().map(|x| x.signed.clone());
 

substrate/runtime/council/src/lib.rs

@@ -549,7 +549,7 @@ impl<T: Trait> Module<T> {
 #[serde(rename_all = "camelCase")]
 #[serde(deny_unknown_fields)]
 pub struct GenesisConfig<T: Trait> {
-	// for the voting onto the  council
+	// for the voting onto the council
 	pub candidacy_bond: T::Balance,
 	pub voter_bond: T::Balance,
 	pub present_slash_per_voter: T::Balance,
@@ -647,6 +647,7 @@ mod tests {
 		type Hashing = BlakeTwo256;
 		type Digest = Digest;
 		type AccountId = u64;
+		type Address = u64;
 		type Header = Header;
 	}
 	impl session::Trait for Test {

substrate/runtime/democracy/src/lib.rs

@@ -389,6 +389,7 @@ mod tests {
 		type Hashing = BlakeTwo256;
 		type Digest = Digest;
 		type AccountId = u64;
+		type Address = u64;
 		type Header = Header;
 	}
 	impl session::Trait for Test {

substrate/runtime/executive/src/lib.rs

@@ -84,12 +84,12 @@ pub struct Executive<
 impl<
 	System: system::Trait,
 	Block: traits::Block<Header=System::Header, Hash=System::Hash>,
-	Lookup: AuxLookup<Source=<Block::Extrinsic as Checkable>::Address, Target=System::AccountId>,
+	Lookup: AuxLookup<Source=System::Address, Target=System::AccountId>,
 	Payment: MakePayment<System::AccountId>,
 	Finalisation: Executable,
 > Executive<System, Block, Lookup, Payment, Finalisation> where
-	Block::Extrinsic: Checkable<AccountId=System::AccountId> + Slicable,
-	<Block::Extrinsic as Checkable>::Checked: Applyable<Index=System::Index, AccountId=System::AccountId>
+	Block::Extrinsic: Checkable<fn(System::Address) -> Result<System::AccountId, &'static str>> + Slicable,
+	<Block::Extrinsic as Checkable<fn(System::Address) -> Result<System::AccountId, &'static str>>>::Checked: Applyable<Index=System::Index, AccountId=System::AccountId>
 {
 	/// Start the execution of a particular block.
 	pub fn initialise_block(header: &System::Header) {
@@ -172,7 +172,7 @@ impl<
 	/// 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) -> result::Result<internal::ApplyOutcome, internal::ApplyError> {
 		// Verify the signature is good.
-		let xt = uxt.check(Lookup::lookup).map_err(internal::ApplyError::BadSignature)?;
+		let xt = uxt.check_with(Lookup::lookup).map_err(|_| internal::ApplyError::BadSignature(""))?;
 
 		if xt.sender() != &Default::default() {
 			// check index
@@ -248,6 +248,7 @@ mod tests {
 		type Hashing = BlakeTwo256;
 		type Digest = Digest;
 		type AccountId = u64;
+		type Address = u64;
 		type Header = Header;
 	}
 	impl session::Trait for Test {

substrate/runtime/primitives/src/generic.rs

@@ -96,7 +96,7 @@ impl<Address, AccountId, Index, Call, Signature> UncheckedExtrinsic<Address, Ind
 	}
 }
 
-impl<Address, AccountId, Index, Call, Signature> traits::Checkable
+impl<Address, AccountId, Index, Call, Signature, ThisLookup> traits::Checkable<ThisLookup>
 	for UncheckedExtrinsic<Address, Index, Call, ::MaybeUnsigned<Signature>>
 where
 	Address: Member + Default + MaybeDisplay,
@@ -106,18 +106,12 @@ where
 	AccountId: Member + Default + MaybeDisplay,
 	::MaybeUnsigned<Signature>: Member,
 	Extrinsic<AccountId, Index, Call>: Slicable,
+	ThisLookup: FnOnce(Address) -> Result<AccountId, &'static str>,
 {
-	type Address = Address;
-	type AccountId = AccountId;
 	type Checked = CheckedExtrinsic<AccountId, Index, Call>;
+	type Error = &'static str;
 
-	fn sender(&self) -> &Address {
-		&self.extrinsic.signed
-	}
-
-	fn check<ThisLookup>(self, lookup: ThisLookup) -> Result<Self::Checked, &'static str> where
-		ThisLookup: FnOnce(Address) -> Result<AccountId, &'static str>,
-	{
+	fn check_with(self, lookup: ThisLookup) -> Result<Self::Checked, (Self, Self::Error)> {
 		if !self.is_signed() {
 			Ok(CheckedExtrinsic(Extrinsic {
 				signed: Default::default(),
@@ -127,14 +121,17 @@ where
 		} else {
 			let extrinsic: Extrinsic<AccountId, Index, Call>
 				= Extrinsic {
-					signed: lookup(self.extrinsic.signed)?,
-					index: self.extrinsic.index,
-					function: self.extrinsic.function,
+					signed: match lookup(self.extrinsic.signed.clone()) {
+						Ok(success) => success,
+						Err(error) => return Err((self, error)),
+					},
+					index: self.extrinsic.index.clone(),
+					function: self.extrinsic.function.clone(),
 				};
 			if ::verify_encoded_lazy(&self.signature, &extrinsic, &extrinsic.signed) {
 				Ok(CheckedExtrinsic(extrinsic))
 			} else {
-				Err("bad signature in extrinsic")
+				Err((self, "bad signature in extrinsic"))
 			}
 		}
 	}

substrate/runtime/primitives/src/testing.rs

@@ -157,12 +157,10 @@ impl<Call: AuxDispatchable + Slicable + Sized + Send + Sync + Serialize + Deseri
 		self.0.encode()
 	}
 }
-impl<Call: 'static + AuxDispatchable + Slicable + Sized + Send + Sync + Serialize + DeserializeOwned + Clone + Eq + Debug> Checkable for TestXt<Call> {
+impl<Call: 'static + AuxDispatchable + Slicable + Sized + Send + Sync + Serialize + DeserializeOwned + Clone + Eq + Debug, Ctx> Checkable<Ctx> for TestXt<Call> {
+	type Error = ();
 	type Checked = Self;
-	type Address = u64;
-	type AccountId = u64;
-	fn sender(&self) -> &u64 { &(self.0).0 }
-	fn check<ThisLookup: FnOnce(Self::Address) -> Result<Self::AccountId, &'static str>>(self, _lookup: ThisLookup) -> Result<Self::Checked, &'static str> { Ok(self) }
+	fn check_with(self, _: Ctx) -> Result<Self::Checked, (Self, ())> { Ok(self) }
 }
 impl<Call: AuxDispatchable<Aux = u64> + Slicable + Sized + Send + Sync + Serialize + DeserializeOwned + Clone + Eq + Debug> Applyable for TestXt<Call> {
 	type AccountId = u64;

substrate/runtime/primitives/src/traits.rs

@@ -371,31 +371,39 @@ pub type HashingFor<B> = <<B as Block>::Header as Header>::Hashing;
 
 /// 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.
-pub trait Checkable: Sized + Send + Sync {
-	type Address: Member + MaybeDisplay;
-	type AccountId: Member + MaybeDisplay;
-	type Checked: Member;
-	fn sender(&self) -> &Self::Address;
-	fn check<ThisLookup: FnOnce(Self::Address) -> Result<Self::AccountId, &'static str>>(self, lookup: ThisLookup) -> Result<Self::Checked, &'static str>;
+/// Implement for pieces of information that require some additional context `Ctx` in order to be
+/// checked.
+pub trait Checkable<Ctx>: Sized {
+	/// Returned if `check_with` succeeds.
+	type Checked;
+	/// Returned if `check_with` fails.
+	type Error;
+
+	/// Gives back ownership of unchanged `self` in case of an error.
+	fn check_with(self, ctx: Ctx) -> Result<Self::Checked, (Self, Self::Error)>;
 }
 
 /// 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.
-pub trait BlindCheckable: Sized + Send + Sync {
-	type Address: Member + MaybeDisplay;
-	type Checked: Member;
-	fn sender(&self) -> &Self::Address;
-	fn check(self) -> Result<Self::Checked, &'static str>;
+/// Implement for pieces of information that don't require additional context in order to be
+/// checked.
+pub trait BlindCheckable: Sized {
+	/// Returned if `check` succeeds.
+	type Checked;
+	/// Returned if `check` fails.
+	type Error;
+
+	/// Gives back ownership of unchanged `self` in case of an error.
+	fn check(self) -> Result<Self::Checked, (Self, Self::Error)>;
 }
 
-impl<T: BlindCheckable> Checkable for T {
-	type Address = <Self as BlindCheckable>::Address;
-	type AccountId = <Self as BlindCheckable>::Address;
+// Every `BlindCheckable` is also a `Checkable` for arbitrary context `Ctx`.
+impl<T: BlindCheckable, Ctx> Checkable<Ctx> for T {
 	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>>(self, _: ThisLookup) -> Result<Self::Checked, &'static str> { BlindCheckable::check(self) }
+	type Error = T::Error;
+	fn check_with(self, _: Ctx) -> Result<Self::Checked, (Self, Self::Error)> {
+		BlindCheckable::check(self)
+	}
 }
 
 /// An "executable" piece of information, used by the standard Substrate Executive in order to

substrate/runtime/session/src/lib.rs

@@ -283,6 +283,7 @@ mod tests {
 		type Hashing = BlakeTwo256;
 		type Digest = Digest;
 		type AccountId = u64;
+		type Address = u64;
 		type Header = Header;
 	}
 	impl timestamp::Trait for Test {

substrate/runtime/staking/src/mock.rs

@@ -43,6 +43,7 @@ impl system::Trait for Test {
 	type Hashing = ::primitives::traits::BlakeTwo256;
 	type Digest = Digest;
 	type AccountId = u64;
+	type Address = u64;
 	type Header = Header;
 }
 impl session::Trait for Test {

substrate/runtime/system/src/lib.rs

@@ -69,6 +69,7 @@ pub trait Trait: Eq + Clone {
 	type Hashing: Hashing<Output = Self::Hash>;
 	type Digest: Parameter + Member + Default + traits::Digest;
 	type AccountId: Parameter + Member + MaybeDisplay + Ord + Default;
+	type Address: Parameter + Member + MaybeDisplay;
 	type Header: Parameter + traits::Header<
 		Number = Self::BlockNumber,
 		Hashing = Self::Hashing,

substrate/runtime/timestamp/src/lib.rs

@@ -158,6 +158,7 @@ mod tests {
 		type Hashing = BlakeTwo256;
 		type Digest = Digest;
 		type AccountId = u64;
+		type Address = u64;
 		type Header = Header;
 	}
 	impl consensus::Trait for Test {

substrate/test-runtime/src/lib.rs

@@ -119,16 +119,13 @@ impl Slicable for Extrinsic {
 
 impl BlindCheckable for Extrinsic {
 	type Checked = Self;
-	type Address = AccountId;
+	type Error = &'static str;
 
-	fn sender(&self) -> &Self::Address {
-		&self.transfer.from
-	}
-	fn check(self) -> Result<Self, &'static str> {
+	fn check(self) -> Result<Self, (Self, Self::Error)> {
 		if ::runtime_primitives::verify_encoded_lazy(&self.signature, &self.transfer, &self.transfer.from) {
 			Ok(self)
 		} else {
-			Err("bad signature")
+			Err((self, "bad signature"))
 		}
 	}
 }