Add integrity check for signed extensions (paritytech#1780)

* Add integrity check for signed extensions * Remove unneeded type specification
serban300 · Jan 18, 2023 · fb3c5ef · fb3c5ef
1 parent 3959628
commit fb3c5ef
Show file tree

Hide file tree

Showing 14 changed files with 65 additions and 19 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/rialto-parachain/runtime/Cargo.toml b/bin/rialto-parachain/runtime/Cargo.toml
@@ -75,6 +75,9 @@ xcm-builder = { git = "https://github.com/paritytech/polkadot", branch = "master
 xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }
 pallet-xcm = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }
 
+[dev-dependencies]
+bridge-runtime-common = { path = "../../runtime-common", features = ["integrity-test"] }
+
 [features]
 default = ['std']
 runtime-benchmarks = [

diff --git a/bin/rialto-parachain/runtime/src/lib.rs b/bin/rialto-parachain/runtime/src/lib.rs
@@ -848,8 +848,11 @@ mod tests {
 		LaneId, MessageKey,
 	};
 	use bp_runtime::messages::MessageDispatchResult;
-	use bridge_runtime_common::messages::target::FromBridgedChainMessageDispatch;
+	use bridge_runtime_common::{
+		integrity::check_additional_signed, messages::target::FromBridgedChainMessageDispatch,
+	};
 	use codec::Encode;
+	use sp_runtime::generic::Era;
 
 	fn new_test_ext() -> sp_io::TestExternalities {
 		sp_io::TestExternalities::new(
@@ -909,4 +912,25 @@ mod tests {
 			);
 		})
 	}
+
+	#[test]
+	fn ensure_signed_extension_definition_is_correct() {
+		let payload: SignedExtra = (
+			frame_system::CheckNonZeroSender::new(),
+			frame_system::CheckSpecVersion::new(),
+			frame_system::CheckTxVersion::new(),
+			frame_system::CheckGenesis::new(),
+			frame_system::CheckEra::from(Era::Immortal),
+			frame_system::CheckNonce::from(10),
+			frame_system::CheckWeight::new(),
+			pallet_transaction_payment::ChargeTransactionPayment::from(10),
+		);
+		let indirect_payload = bp_rialto_parachain::SignedExtension::new(
+			((), (), (), (), Era::Immortal, 10.into(), (), 10.into()),
+			None,
+		);
+		assert_eq!(payload.encode(), indirect_payload.encode());
+
+		check_additional_signed::<SignedExtra, bp_rialto_parachain::SignedExtension>();
+	}
 }
diff --git a/bin/runtime-common/src/integrity.rs b/bin/runtime-common/src/integrity.rs
@@ -26,6 +26,7 @@ use bp_runtime::{Chain, ChainId};
 use codec::Encode;
 use frame_support::{storage::generator::StorageValue, traits::Get};
 use frame_system::limits;
+use sp_runtime::traits::SignedExtension;
 
 /// Macro that ensures that the runtime configuration and chain primitives crate are sharing
 /// the same types (index, block number, hash, hasher, account id and header).
@@ -319,3 +320,15 @@ pub fn check_message_lane_weights<C: Chain, T: frame_system::Config>(
 		this_chain_max_unconfirmed_messages,
 	);
 }
+
+/// Check that the `AdditionalSigned` type of a wrapped runtime is the same as the one of the
+/// corresponding actual runtime.
+///
+/// This method doesn't perform any `assert`. If the condition is not true it will generate a
+/// compile-time error.
+pub fn check_additional_signed<SignedExt, IndirectSignedExt: SignedExtension>()
+where
+	SignedExt: SignedExtension,
+	IndirectSignedExt: SignedExtension<AdditionalSigned = SignedExt::AdditionalSigned>,
+{
+}
diff --git a/primitives/chain-bridge-hub-cumulus/src/lib.rs b/primitives/chain-bridge-hub-cumulus/src/lib.rs
@@ -18,9 +18,9 @@
 
 use bp_messages::*;
 pub use bp_polkadot_core::{
-	AccountId, AccountInfoStorageMapKeyProvider, AccountPublic, Balance, BlockNumber,
-	BridgeSignedExtension, Hash, Hasher, Hashing, Header, Index, Nonce, Perbill,
-	PolkadotSignedExtension, Signature, SignedBlock, UncheckedExtrinsic, TX_EXTRA_BYTES,
+	AccountId, AccountInfoStorageMapKeyProvider, AccountPublic, Balance, BlockNumber, Hash, Hasher,
+	Hashing, Header, Index, Nonce, Perbill, PolkadotSignedExtension, Signature, SignedBlock,
+	UncheckedExtrinsic, TX_EXTRA_BYTES,
 };
 use frame_support::{
 	dispatch::DispatchClass,
@@ -86,6 +86,8 @@ pub type AccountSigner = MultiSigner;
 /// The address format for describing accounts.
 pub type Address = MultiAddress<AccountId, ()>;
 
+pub use bp_polkadot_core::BridgeSignedExtension as SignedExtension;
+
 // Note about selecting values of two following constants:
 //
 // Normal transactions have limit of 75% of 1/2 second weight for Cumulus parachains. Let's keep

diff --git a/primitives/chain-rialto-parachain/Cargo.toml b/primitives/chain-rialto-parachain/Cargo.toml
@@ -11,6 +11,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 # Bridge Dependencies
 
 bp-messages = { path = "../messages", default-features = false }
+bp-polkadot-core = { path = "../polkadot-core", default-features = false }
 bp-runtime = { path = "../runtime", default-features = false }
 
 # Substrate Based Dependencies

diff --git a/primitives/chain-rialto-parachain/src/lib.rs b/primitives/chain-rialto-parachain/src/lib.rs
@@ -133,6 +133,8 @@ impl Parachain for RialtoParachain {
 	const PARACHAIN_ID: u32 = RIALTO_PARACHAIN_ID;
 }
 
+pub use bp_polkadot_core::DefaultSignedExtension as SignedExtension;
+
 frame_support::parameter_types! {
 	pub BlockLength: limits::BlockLength =
 		limits::BlockLength::max_with_normal_ratio(5 * 1024 * 1024, NORMAL_DISPATCH_RATIO);

diff --git a/primitives/polkadot-core/src/lib.rs b/primitives/polkadot-core/src/lib.rs
@@ -266,7 +266,7 @@ impl PolkadotSignedExtension for DefaultSignedExtension {
 				(),              // Check weight
 				tip.into(),      // transaction payment / tip (compact encoding)
 			),
-			(
+			Some((
 				(),
 				spec_version,
 				transaction_version,
@@ -275,7 +275,7 @@ impl PolkadotSignedExtension for DefaultSignedExtension {
 				(),
 				(),
 				(),
-			),
+			)),
 		)
 	}
 
@@ -326,7 +326,7 @@ impl PolkadotSignedExtension for BridgeSignedExtension {
 				tip.into(),      // transaction payment / tip (compact encoding)
 				(),              // bridge reject obsolete headers and msgs
 			),
-			(
+			Some((
 				(),
 				spec_version,
 				transaction_version,
@@ -336,7 +336,7 @@ impl PolkadotSignedExtension for BridgeSignedExtension {
 				(),
 				(),
 				(),
-			),
+			)),
 		)
 	}
 

diff --git a/primitives/runtime/src/extensions.rs b/primitives/runtime/src/extensions.rs
@@ -96,8 +96,8 @@ pub struct GenericSignedExtension<S: SignedExtensionSchema> {
 }
 
 impl<S: SignedExtensionSchema> GenericSignedExtension<S> {
-	pub fn new(payload: S::Payload, additional_signed: S::AdditionalSigned) -> Self {
-		Self { payload, additional_signed: Some(additional_signed) }
+	pub fn new(payload: S::Payload, additional_signed: Option<S::AdditionalSigned>) -> Self {
+		Self { payload, additional_signed }
 	}
 }
 

diff --git a/relays/client-bridge-hub-rococo/src/lib.rs b/relays/client-bridge-hub-rococo/src/lib.rs
@@ -66,7 +66,7 @@ impl ChainWithTransactions for BridgeHubRococo {
 	) -> Result<Self::SignedTransaction, SubstrateError> {
 		let raw_payload = SignedPayload::new(
 			unsigned.call,
-			bp_bridge_hub_rococo::BridgeSignedExtension::from_params(
+			bp_bridge_hub_rococo::SignedExtension::from_params(
 				param.spec_version,
 				param.transaction_version,
 				unsigned.era,

diff --git a/relays/client-bridge-hub-rococo/src/runtime_wrapper.rs b/relays/client-bridge-hub-rococo/src/runtime_wrapper.rs
@@ -21,14 +21,14 @@
 use codec::{Decode, Encode};
 use scale_info::TypeInfo;
 
-use bp_bridge_hub_rococo::BridgeSignedExtension;
+use bp_bridge_hub_rococo::SignedExtension;
 pub use bp_header_chain::BridgeGrandpaCallOf;
 pub use bp_parachains::BridgeParachainCall;
 pub use bridge_runtime_common::messages::BridgeMessagesCallOf;
 pub use relay_substrate_client::calls::SystemCall;
 
 /// Unchecked BridgeHubRococo extrinsic.
-pub type UncheckedExtrinsic = bp_bridge_hub_rococo::UncheckedExtrinsic<Call, BridgeSignedExtension>;
+pub type UncheckedExtrinsic = bp_bridge_hub_rococo::UncheckedExtrinsic<Call, SignedExtension>;
 
 // The indirect pallet call used to sync `Wococo` GRANDPA finality to `BHRococo`.
 pub type BridgeWococoGrandpaCall = BridgeGrandpaCallOf<bp_wococo::Wococo>;

diff --git a/relays/client-bridge-hub-wococo/src/lib.rs b/relays/client-bridge-hub-wococo/src/lib.rs
@@ -66,7 +66,7 @@ impl ChainWithTransactions for BridgeHubWococo {
 	) -> Result<Self::SignedTransaction, SubstrateError> {
 		let raw_payload = SignedPayload::new(
 			unsigned.call,
-			bp_bridge_hub_wococo::BridgeSignedExtension::from_params(
+			bp_bridge_hub_wococo::SignedExtension::from_params(
 				param.spec_version,
 				param.transaction_version,
 				unsigned.era,

diff --git a/relays/client-bridge-hub-wococo/src/runtime_wrapper.rs b/relays/client-bridge-hub-wococo/src/runtime_wrapper.rs
@@ -19,14 +19,14 @@
 use codec::{Decode, Encode};
 use scale_info::TypeInfo;
 
-use bp_bridge_hub_wococo::BridgeSignedExtension;
+use bp_bridge_hub_wococo::SignedExtension;
 pub use bp_header_chain::BridgeGrandpaCallOf;
 pub use bp_parachains::BridgeParachainCall;
 pub use bridge_runtime_common::messages::BridgeMessagesCallOf;
 pub use relay_substrate_client::calls::SystemCall;
 
 /// Unchecked BridgeHubWococo extrinsic.
-pub type UncheckedExtrinsic = bp_bridge_hub_wococo::UncheckedExtrinsic<Call, BridgeSignedExtension>;
+pub type UncheckedExtrinsic = bp_bridge_hub_wococo::UncheckedExtrinsic<Call, SignedExtension>;
 
 // The indirect pallet call used to sync `Rococo` GRANDPA finality to `BHWococo`.
 pub type BridgeRococoGrandpaCall = BridgeGrandpaCallOf<bp_rococo::Rococo>;

diff --git a/relays/client-rialto-parachain/src/lib.rs b/relays/client-rialto-parachain/src/lib.rs
@@ -19,7 +19,7 @@
 pub mod runtime_wrapper;
 
 use bp_messages::MessageNonce;
-use bp_polkadot_core::{DefaultSignedExtension, PolkadotSignedExtension};
+use bp_polkadot_core::PolkadotSignedExtension;
 use codec::Encode;
 use relay_substrate_client::{
 	Chain, ChainWithBalances, ChainWithMessages, ChainWithTransactions, Error as SubstrateError,
@@ -79,15 +79,15 @@ impl ChainWithMessages for RialtoParachain {
 impl ChainWithTransactions for RialtoParachain {
 	type AccountKeyPair = sp_core::sr25519::Pair;
 	type SignedTransaction =
-		bp_polkadot_core::UncheckedExtrinsic<Self::Call, DefaultSignedExtension>;
+		bp_polkadot_core::UncheckedExtrinsic<Self::Call, bp_rialto_parachain::SignedExtension>;
 
 	fn sign_transaction(
 		param: SignParam<Self>,
 		unsigned: UnsignedTransaction<Self>,
 	) -> Result<Self::SignedTransaction, SubstrateError> {
 		let raw_payload = SignedPayload::new(
 			unsigned.call,
-			bp_polkadot_core::DefaultSignedExtension::from_params(
+			bp_rialto_parachain::SignedExtension::from_params(
 				param.spec_version,
 				param.transaction_version,
 				unsigned.era,