Fixed call encoding in signature digest (paritytech#699)

* fixed call encoding in signature digets * udpated test
serban300 · Apr 10, 2024 · d6e1698 · d6e1698
1 parent b906a10
commit d6e1698
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 18 deletions.
diff --git a/bridges/bin/runtime-common/src/messages.rs b/bridges/bin/runtime-common/src/messages.rs
@@ -27,7 +27,7 @@ use bp_message_lane::{
 	InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData,
 };
 use bp_runtime::InstanceId;
-use codec::{Compact, Decode, Encode};
+use codec::{Decode, Encode};
 use frame_support::{traits::Instance, weights::Weight, RuntimeDebug};
 use hash_db::Hasher;
 use pallet_substrate_bridge::StorageProofChecker;
@@ -352,9 +352,7 @@ pub mod target {
 
 	impl<B: MessageBridge> From<FromBridgedChainEncodedMessageCall<B>> for Result<CallOf<ThisChain<B>>, ()> {
 		fn from(encoded_call: FromBridgedChainEncodedMessageCall<B>) -> Self {
-			let mut input = &encoded_call.encoded_call[..];
-			let _skipped_length = Compact::<u32>::decode(&mut input).map_err(drop)?;
-			CallOf::<ThisChain<B>>::decode(&mut input).map_err(drop)
+			CallOf::<ThisChain<B>>::decode(&mut &encoded_call.encoded_call[..]).map_err(drop)
 		}
 	}
 
@@ -797,6 +795,7 @@ mod tests {
 				},
 			}
 		);
+		assert_eq!(Ok(ThisChainCall::Transfer), message_on_this_chain.call.into());
 	}
 
 	const TEST_LANE_ID: &LaneId = b"test";

diff --git a/bridges/modules/call-dispatch/src/lib.rs b/bridges/modules/call-dispatch/src/lib.rs
@@ -145,9 +145,6 @@ pub trait Config<I = DefaultInstance>: frame_system::Config {
 	/// that all other stuff (like `spec_version`) is ok. If we would try to decode
 	/// `Call` which has been encoded using previous `spec_version`, then we might end
 	/// up with decoding error, instead of `MessageVersionSpecMismatch`.
-	///
-	/// The `Encode` implementation should match `Encode` implementation of the actual
-	/// `Call`, that (may) have been used to produce signature for `CallOrigin::TargetAccount`.
 	type EncodedCall: Decode + Encode + Into<Result<<Self as Config<I>>::Call, ()>>;
 	/// A type which can be turned into an AccountId from a 256-bit hash.
 	///
@@ -231,6 +228,16 @@ impl<T: Config<I>, I: Instance> MessageDispatch<T::MessageId> for Module<T, I> {
 			return;
 		}
 
+		// now that we have spec version checked, let's decode the call
+		let call = match message.call.into() {
+			Ok(call) => call,
+			Err(_) => {
+				frame_support::debug::trace!("Failed to decode Call from message {:?}/{:?}", bridge, id,);
+				Self::deposit_event(RawEvent::MessageCallDecodeFailed(bridge, id));
+				return;
+			}
+		};
+
 		// prepare dispatch origin
 		let origin_account = match message.origin {
 			CallOrigin::SourceRoot => {
@@ -240,7 +247,7 @@ impl<T: Config<I>, I: Instance> MessageDispatch<T::MessageId> for Module<T, I> {
 				target_id
 			}
 			CallOrigin::TargetAccount(source_account_id, target_public, target_signature) => {
-				let digest = account_ownership_digest(&message.call, source_account_id, message.spec_version, bridge);
+				let digest = account_ownership_digest(&call, source_account_id, message.spec_version, bridge);
 
 				let target_account = target_public.into_account();
 				if !target_signature.verify(&digest[..], &target_account) {
@@ -266,16 +273,6 @@ impl<T: Config<I>, I: Instance> MessageDispatch<T::MessageId> for Module<T, I> {
 			}
 		};
 
-		// now that we have everything checked, let's decode the call
-		let call = match message.call.into() {
-			Ok(call) => call,
-			Err(_) => {
-				frame_support::debug::trace!("Failed to decode Call from message {:?}/{:?}", bridge, id,);
-				Self::deposit_event(RawEvent::MessageCallDecodeFailed(bridge, id));
-				return;
-			}
-		};
-
 		// filter the call
 		if !T::CallFilter::filter(&call) {
 			frame_support::debug::trace!(