Add unit tests for hash matching for unchecked extrinsic (#636)

* add comments and extra test

* add unittest

* add tets

* fix comments

primitives/Cargo.toml

@@ -39,6 +39,7 @@ pallet-staking = { optional = true, git = "https://github.com/paritytech/substra
 [dev-dependencies]
 node-template-runtime = { git = "https://github.com/paritytech/substrate.git", branch = "master" }
 sp-keyring = { git = "https://github.com/paritytech/substrate.git", branch = "master" }
+pallet-transaction-payment = { git = "https://github.com/paritytech/substrate.git", branch = "master" }
 
 [features]
 default = ["std"]

primitives/src/extrinsics/extrinsic_params.rs

@@ -192,6 +192,12 @@ where
 	}
 }
 
+/// A payload that has been signed for an unchecked extrinsics.
+///
+/// Note that the payload that we sign to produce unchecked extrinsic signature
+/// is going to be different than the `SignaturePayload` - so the thing the extrinsic
+/// actually contains.
+// https://github.com/paritytech/substrate/blob/1612e39131e3fe57ba4c78447fb1cbf7c4f8830e/primitives/runtime/src/generic/unchecked_extrinsic.rs#L192-L197
 #[derive(Decode, Encode, Clone, Eq, PartialEq, Debug)]
 pub struct SignedPayload<Call, SignedExtra, AdditionalSigned>(
 	(Call, SignedExtra, AdditionalSigned),
@@ -203,6 +209,7 @@ where
 	SignedExtra: Encode,
 	AdditionalSigned: Encode,
 {
+	/// Create new `SignedPayload` from raw components.
 	pub fn from_raw(call: Call, extra: SignedExtra, additional_signed: AdditionalSigned) -> Self {
 		Self((call, extra, additional_signed))
 	}
@@ -280,3 +287,15 @@ impl From<AssetTip<u128>> for u128 {
 		tip.tip
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use sp_core::hashing::blake2_256;
+
+	#[test]
+	fn encode_blake2_256_works_as_expected() {
+		let bytes = "afaefafe1204udanfai9lfadmlk9aömlsa".as_bytes();
+		assert_eq!(&blake2_256(bytes)[..], &BlakeTwo256::hash(bytes)[..]);
+	}
+}

primitives/src/extrinsics/mod.rs

@@ -17,6 +17,7 @@
 
 //! Primitives for substrate extrinsics.
 
+use crate::OpaqueExtrinsic;
 use alloc::{format, vec::Vec};
 use codec::{Decode, Encode, Error, Input};
 use core::fmt;
@@ -39,11 +40,15 @@ pub mod signer;
 const V4: u8 = 4;
 
 /// Mirrors the currently used Extrinsic format (V4) from substrate. Has less traits and methods though.
-/// The SingedExtra used does not need to implement SingedExtension here.
+/// The SignedExtra used does not need to implement SignedExtension here.
 // see https://github.com/paritytech/substrate/blob/7d233c2446b5a60662400a0a4bcfb78bb3b79ff7/primitives/runtime/src/generic/unchecked_extrinsic.rs
 #[derive(Clone, Eq, PartialEq)]
 pub struct UncheckedExtrinsicV4<Address, Call, Signature, SignedExtra> {
+	/// The signature, address, number of extrinsics have come before from
+	/// the same signer and an era describing the longevity of this transaction,
+	/// if this is a signed extrinsic.
 	pub signature: Option<(Address, Signature, SignedExtra)>,
+	/// The function that should be called.
 	pub function: Call,
 }
 
@@ -109,6 +114,7 @@ where
 	}
 }
 
+// https://github.com/paritytech/substrate/blob/1612e39131e3fe57ba4c78447fb1cbf7c4f8830e/primitives/runtime/src/generic/unchecked_extrinsic.rs#L289C5-L320
 impl<Address, Call, Signature, SignedExtra> Encode
 	for UncheckedExtrinsicV4<Address, Call, Signature, SignedExtra>
 where
@@ -133,6 +139,7 @@ where
 	}
 }
 
+// https://github.com/paritytech/substrate/blob/1612e39131e3fe57ba4c78447fb1cbf7c4f8830e/primitives/runtime/src/generic/unchecked_extrinsic.rs#L250-L287
 impl<Address, Call, Signature, SignedExtra> Decode
 	for UncheckedExtrinsicV4<Address, Call, Signature, SignedExtra>
 where
@@ -179,6 +186,7 @@ where
 	}
 }
 
+// https://github.com/paritytech/substrate/blob/1612e39131e3fe57ba4c78447fb1cbf7c4f8830e/primitives/runtime/src/generic/unchecked_extrinsic.rs#L346-L357
 impl<'a, Address, Call, Signature, SignedExtra> serde::Deserialize<'a>
 	for UncheckedExtrinsicV4<Address, Call, Signature, SignedExtra>
 where
@@ -197,6 +205,23 @@ where
 	}
 }
 
+// https://github.com/paritytech/substrate/blob/1612e39131e3fe57ba4c78447fb1cbf7c4f8830e/primitives/runtime/src/generic/unchecked_extrinsic.rs#L376-L390
+impl<Address, Call, Signature, Extra> From<UncheckedExtrinsicV4<Address, Call, Signature, Extra>>
+	for OpaqueExtrinsic
+where
+	Address: Encode,
+	Signature: Encode,
+	Call: Encode,
+	Extra: Encode,
+{
+	fn from(extrinsic: UncheckedExtrinsicV4<Address, Call, Signature, Extra>) -> Self {
+		Self::from_bytes(extrinsic.encode().as_slice()).expect(
+			"both OpaqueExtrinsic and UncheckedExtrinsic have encoding that is compatible with \
+				raw Vec<u8> encoding; qed",
+		)
+	}
+}
+
 /// Same function as in primitives::generic. Needed to be copied as it is private there.
 fn encode_with_vec_prefix<T: Encode, F: Fn(&mut Vec<u8>)>(encoder: F) -> Vec<u8> {
 	let size = core::mem::size_of::<T>();
@@ -225,9 +250,23 @@ mod tests {
 	use super::*;
 	use crate::AssetRuntimeConfig;
 	use extrinsic_params::{GenericAdditionalParams, GenericExtrinsicParams, PlainTip};
-	use node_template_runtime::{BalancesCall, RuntimeCall, SignedExtra};
+	use frame_system::{
+		CheckEra, CheckGenesis, CheckNonZeroSender, CheckNonce, CheckSpecVersion, CheckTxVersion,
+		CheckWeight,
+	};
+	use node_template_runtime::{
+		BalancesCall, Runtime, RuntimeCall, SignedExtra, SystemCall, UncheckedExtrinsic, VERSION,
+	};
+	use pallet_transaction_payment::ChargeTransactionPayment;
 	use sp_core::{crypto::Ss58Codec, Pair, H256 as Hash};
-	use sp_runtime::{generic::Era, testing::sr25519, AccountId32, MultiAddress, MultiSignature};
+	use sp_keyring::AccountKeyring;
+	use sp_runtime::{
+		generic::Era, testing::sr25519, traits::Hash as HashTrait, AccountId32, MultiAddress,
+		MultiSignature,
+	};
+
+	type PlainTipExtrinsicParams =
+		GenericExtrinsicParams<crate::DefaultRuntimeConfig, PlainTip<u128>>;
 
 	#[test]
 	fn encode_decode_roundtrip_works() {
@@ -285,4 +324,121 @@ mod tests {
 		let call = extrinsic.function;
 		assert_eq!(call, call1);
 	}
+
+	#[test]
+	fn xt_hash_matches_substrate_impl() {
+		// Define extrinsic params.
+		let alice = MultiAddress::Id(AccountKeyring::Alice.to_account_id());
+		let bob = MultiAddress::Id(AccountKeyring::Bob.to_account_id());
+		let call =
+			RuntimeCall::Balances(BalancesCall::transfer_allow_death { dest: bob, value: 42 });
+
+		let msg = &b"test-message"[..];
+		let (pair, _) = sr25519::Pair::generate();
+		let signature = MultiSignature::from(pair.sign(msg));
+
+		let era = Era::Immortal;
+		let nonce = 10;
+		let fee = 100;
+
+		// Create Substrate extrinsic.
+		let substrate_signed_extra: SignedExtra = (
+			CheckNonZeroSender::<Runtime>::new(),
+			CheckSpecVersion::<Runtime>::new(),
+			CheckTxVersion::<Runtime>::new(),
+			CheckGenesis::<Runtime>::new(),
+			CheckEra::<Runtime>::from(era),
+			CheckNonce::<Runtime>::from(nonce),
+			CheckWeight::<Runtime>::new(),
+			ChargeTransactionPayment::<Runtime>::from(fee),
+		);
+
+		let substrate_extrinsic = UncheckedExtrinsic::new_signed(
+			call.clone(),
+			alice.clone(),
+			signature.clone(),
+			substrate_signed_extra,
+		);
+
+		// Create api-client extrinsic.
+		let additional_tx_params = GenericAdditionalParams::<PlainTip<u128>, Hash>::new()
+			.era(era, Hash::zero())
+			.tip(fee);
+		let extrinsic_params = PlainTipExtrinsicParams::new(
+			VERSION.spec_version,
+			VERSION.transaction_version,
+			nonce,
+			Hash::zero(),
+			additional_tx_params,
+		);
+		let api_client_extrinsic = UncheckedExtrinsicV4::new_signed(
+			call,
+			alice,
+			signature,
+			extrinsic_params.signed_extra(),
+		);
+
+		assert_eq!(
+			<Runtime as frame_system::Config>::Hashing::hash_of(&substrate_extrinsic),
+			<Runtime as frame_system::Config>::Hashing::hash_of(&api_client_extrinsic)
+		)
+	}
+
+	#[test]
+	fn xt_hash_matches_substrate_impl_large_xt() {
+		// Define xt parameters,
+		let alice = MultiAddress::Id(AccountKeyring::Alice.to_account_id());
+		let code: Vec<u8> = include_bytes!("test-runtime.compact.wasm").to_vec();
+		let call = RuntimeCall::System(SystemCall::set_code { code });
+
+		let msg = &b"test-message"[..];
+		let (pair, _) = sr25519::Pair::generate();
+		let signature = MultiSignature::from(pair.sign(msg));
+		let era = Era::Immortal;
+		let nonce = 10;
+		let fee = 100;
+
+		// Create Substrate extrinsic.
+		let substrate_signed_extra: SignedExtra = (
+			CheckNonZeroSender::<Runtime>::new(),
+			CheckSpecVersion::<Runtime>::new(),
+			CheckTxVersion::<Runtime>::new(),
+			CheckGenesis::<Runtime>::new(),
+			CheckEra::<Runtime>::from(era),
+			CheckNonce::<Runtime>::from(nonce),
+			CheckWeight::<Runtime>::new(),
+			ChargeTransactionPayment::<Runtime>::from(fee),
+		);
+		let substrate_extrinsic = UncheckedExtrinsic::new_signed(
+			call.clone(),
+			alice.clone(),
+			signature.clone(),
+			substrate_signed_extra,
+		);
+
+		// Define api-client extrinsic.
+		let additional_tx_params = GenericAdditionalParams::<PlainTip<u128>, Hash>::new()
+			.era(era, Hash::zero())
+			.tip(fee);
+
+		let extrinsic_params = PlainTipExtrinsicParams::new(
+			VERSION.spec_version,
+			VERSION.transaction_version,
+			nonce,
+			Hash::zero(),
+			additional_tx_params,
+		);
+
+		let api_client_extrinsic = UncheckedExtrinsicV4::new_signed(
+			call,
+			alice,
+			signature,
+			extrinsic_params.signed_extra(),
+		);
+
+		assert_eq!(
+			<Runtime as frame_system::Config>::Hashing::hash_of(&substrate_extrinsic),
+			<Runtime as frame_system::Config>::Hashing::hash_of(&api_client_extrinsic)
+		)
+	}
 }