Add event field names to HRMP Event variants (#1695)

Update the HRMP pallet to use field names for Event variants to improve metadata for a better client experience. Event variants are now structs instead of unnamed tuples. Partially implements Substrate issue [9903](paritytech/substrate#9903) which doesn't appear to have been moved to the monorepo.
paritytech · Sep 28, 2023 · 4bc97e4 · 4bc97e4
1 parent 769bdd3
commit 4bc97e4
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 66 deletions.
diff --git a/...ins/integration-tests/e2e/collectives/collectives-polkadot/0_xcm/3_hrmp-open-channels.yml b/...ins/integration-tests/e2e/collectives/collectives-polkadot/0_xcm/3_hrmp-open-channels.yml
@@ -37,7 +37,12 @@ tests:
                   ]
                   events:
                     - name: hrmp.HrmpChannelForceOpened
-                      result: [*cp_id, *sp_id, *hrmp_proposed_max_capacity, *hrmp_proposed_max_message_size]
+                      result: {
+                        sender: *cp_id,
+                        recipient: *sp_id,
+                        proposed_max_capacity: *hrmp_proposed_max_capacity,
+                        proposed_max_message_size: *hrmp_proposed_max_message_size
+                      }
       - name: Force Open HRMP Channel From AssetHub Parachain → Collectives Parachain
         its:
           - name: Alice calls hrmp.forceOpenHrmpChannel
@@ -56,4 +61,9 @@ tests:
                   ]
                   events:
                     - name: hrmp.HrmpChannelForceOpened
-                      result: [*sp_id, *cp_id, *hrmp_proposed_max_capacity, *hrmp_proposed_max_message_size]
+                      result: {
+                        sender: *sp_id,
+                        recipient: *cp_id,
+                        proposed_max_capacity: *hrmp_proposed_max_capacity,
+                        proposed_max_message_size: *hrmp_proposed_max_message_size
+                      }
diff --git a/.../parachains/integration-tests/emulated/assets/asset-hub-kusama/src/tests/hrmp_channels.rs b/.../parachains/integration-tests/emulated/assets/asset-hub-kusama/src/tests/hrmp_channels.rs
@@ -79,9 +79,12 @@ fn open_hrmp_channel_between_paras_works() {
 				},
 				// Open channel requested from Para A to Para B
 				RuntimeEvent::Hrmp(
-					polkadot_runtime_parachains::hrmp::Event::OpenChannelRequested(
-						sender, recipient, max_capacity, max_message_size
-					)
+					polkadot_runtime_parachains::hrmp::Event::OpenChannelRequested {
+						sender,
+						recipient,
+						proposed_max_capacity: max_capacity,
+						proposed_max_message_size: max_message_size
+					}
 				) => {
 					sender: *sender == para_a_id.into(),
 					recipient: *recipient == para_b_id.into(),
@@ -133,9 +136,9 @@ fn open_hrmp_channel_between_paras_works() {
 				},
 				// Open channel accepted for Para A to Para B
 				RuntimeEvent::Hrmp(
-					polkadot_runtime_parachains::hrmp::Event::OpenChannelAccepted(
+					polkadot_runtime_parachains::hrmp::Event::OpenChannelAccepted {
 						sender, recipient
-					)
+					}
 				) => {
 					sender: *sender == para_a_id.into(),
 					recipient: *recipient == para_b_id.into(),
@@ -175,9 +178,12 @@ fn force_open_hrmp_channel_for_system_para_works() {
 			vec![
 				// HRMP channel forced opened
 				RuntimeEvent::Hrmp(
-					polkadot_runtime_parachains::hrmp::Event::HrmpChannelForceOpened(
-						sender, recipient, max_capacity, max_message_size
-					)
+					polkadot_runtime_parachains::hrmp::Event::HrmpChannelForceOpened{
+						sender,
+						recipient,
+						proposed_max_capacity: max_capacity,
+						proposed_max_message_size: max_message_size
+					}
 				) => {
 					sender: *sender == system_para_id.into(),
 					recipient: *recipient == para_a_id.into(),

diff --git a/...arachains/integration-tests/emulated/assets/asset-hub-polkadot/src/tests/hrmp_channels.rs b/...arachains/integration-tests/emulated/assets/asset-hub-polkadot/src/tests/hrmp_channels.rs
@@ -79,9 +79,9 @@ fn open_hrmp_channel_between_paras_works() {
 				},
 				// Open channel requested from Para A to Para B
 				RuntimeEvent::Hrmp(
-					polkadot_runtime_parachains::hrmp::Event::OpenChannelRequested(
-						sender, recipient, max_capacity, max_message_size
-					)
+					polkadot_runtime_parachains::hrmp::Event::OpenChannelRequested {
+						sender, recipient, proposed_max_capacity: max_capacity, proposed_max_message_size: max_message_size
+					}
 				) => {
 					sender: *sender == para_a_id.into(),
 					recipient: *recipient == para_b_id.into(),
@@ -133,9 +133,9 @@ fn open_hrmp_channel_between_paras_works() {
 				},
 				// Open channel accepted for Para A to Para B
 				RuntimeEvent::Hrmp(
-					polkadot_runtime_parachains::hrmp::Event::OpenChannelAccepted(
+					polkadot_runtime_parachains::hrmp::Event::OpenChannelAccepted {
 						sender, recipient
-					)
+					}
 				) => {
 					sender: *sender == para_a_id.into(),
 					recipient: *recipient == para_b_id.into(),
@@ -175,9 +175,9 @@ fn force_open_hrmp_channel_for_system_para_works() {
 			vec![
 				// HRMP channel forced opened
 				RuntimeEvent::Hrmp(
-					polkadot_runtime_parachains::hrmp::Event::HrmpChannelForceOpened(
-						sender, recipient, max_capacity, max_message_size
-					)
+					polkadot_runtime_parachains::hrmp::Event::HrmpChannelForceOpened{
+						sender, recipient, proposed_max_capacity: max_capacity, proposed_max_message_size: max_message_size
+					}
 				) => {
 					sender: *sender == system_para_id.into(),
 					recipient: *recipient == para_a_id.into(),

diff --git a/polkadot/runtime/parachains/src/hrmp.rs b/polkadot/runtime/parachains/src/hrmp.rs
@@ -278,24 +278,34 @@ pub mod pallet {
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
 		/// Open HRMP channel requested.
-		/// `[sender, recipient, proposed_max_capacity, proposed_max_message_size]`
-		OpenChannelRequested(ParaId, ParaId, u32, u32),
+		OpenChannelRequested {
+			sender: ParaId,
+			recipient: ParaId,
+			proposed_max_capacity: u32,
+			proposed_max_message_size: u32,
+		},
 		/// An HRMP channel request sent by the receiver was canceled by either party.
-		/// `[by_parachain, channel_id]`
-		OpenChannelCanceled(ParaId, HrmpChannelId),
-		/// Open HRMP channel accepted. `[sender, recipient]`
-		OpenChannelAccepted(ParaId, ParaId),
-		/// HRMP channel closed. `[by_parachain, channel_id]`
-		ChannelClosed(ParaId, HrmpChannelId),
+		OpenChannelCanceled { by_parachain: ParaId, channel_id: HrmpChannelId },
+		/// Open HRMP channel accepted.
+		OpenChannelAccepted { sender: ParaId, recipient: ParaId },
+		/// HRMP channel closed.
+		ChannelClosed { by_parachain: ParaId, channel_id: HrmpChannelId },
 		/// An HRMP channel was opened via Root origin.
-		/// `[sender, recipient, proposed_max_capacity, proposed_max_message_size]`
-		HrmpChannelForceOpened(ParaId, ParaId, u32, u32),
+		HrmpChannelForceOpened {
+			sender: ParaId,
+			recipient: ParaId,
+			proposed_max_capacity: u32,
+			proposed_max_message_size: u32,
+		},
 		/// An HRMP channel was opened between two system chains.
-		/// `[sender, recipient, proposed_max_capacity, proposed_max_message_size]`
-		HrmpSystemChannelOpened(ParaId, ParaId, u32, u32),
+		HrmpSystemChannelOpened {
+			sender: ParaId,
+			recipient: ParaId,
+			proposed_max_capacity: u32,
+			proposed_max_message_size: u32,
+		},
 		/// An HRMP channel's deposits were updated.
-		/// `[sender, recipient]`
-		OpenChannelDepositsUpdated(ParaId, ParaId),
+		OpenChannelDepositsUpdated { sender: ParaId, recipient: ParaId },
 	}
 
 	#[pallet::error]
@@ -499,12 +509,12 @@ pub mod pallet {
 				proposed_max_capacity,
 				proposed_max_message_size,
 			)?;
-			Self::deposit_event(Event::OpenChannelRequested(
-				origin,
+			Self::deposit_event(Event::OpenChannelRequested {
+				sender: origin,
 				recipient,
 				proposed_max_capacity,
 				proposed_max_message_size,
-			));
+			});
 			Ok(())
 		}
 
@@ -516,7 +526,7 @@ pub mod pallet {
 		pub fn hrmp_accept_open_channel(origin: OriginFor<T>, sender: ParaId) -> DispatchResult {
 			let origin = ensure_parachain(<T as Config>::RuntimeOrigin::from(origin))?;
 			Self::accept_open_channel(origin, sender)?;
-			Self::deposit_event(Event::OpenChannelAccepted(sender, origin));
+			Self::deposit_event(Event::OpenChannelAccepted { sender, recipient: origin });
 			Ok(())
 		}
 
@@ -532,7 +542,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			let origin = ensure_parachain(<T as Config>::RuntimeOrigin::from(origin))?;
 			Self::close_channel(origin, channel_id.clone())?;
-			Self::deposit_event(Event::ChannelClosed(origin, channel_id));
+			Self::deposit_event(Event::ChannelClosed { by_parachain: origin, channel_id });
 			Ok(())
 		}
 
@@ -611,7 +621,7 @@ pub mod pallet {
 				Error::<T>::WrongWitness
 			);
 			Self::cancel_open_request(origin, channel_id.clone())?;
-			Self::deposit_event(Event::OpenChannelCanceled(origin, channel_id));
+			Self::deposit_event(Event::OpenChannelCanceled { by_parachain: origin, channel_id });
 			Ok(())
 		}
 
@@ -651,12 +661,12 @@ pub mod pallet {
 			// that it will not require deposits from either member.
 			Self::init_open_channel(sender, recipient, max_capacity, max_message_size)?;
 			Self::accept_open_channel(recipient, sender)?;
-			Self::deposit_event(Event::HrmpChannelForceOpened(
+			Self::deposit_event(Event::HrmpChannelForceOpened {
 				sender,
 				recipient,
-				max_capacity,
-				max_message_size,
-			));
+				proposed_max_capacity: max_capacity,
+				proposed_max_message_size: max_message_size,
+			});
 
 			Ok(Some(<T as Config>::WeightInfo::force_open_hrmp_channel(cancel_request)).into())
 		}
@@ -695,12 +705,12 @@ pub mod pallet {
 			Self::init_open_channel(sender, recipient, max_capacity, max_message_size)?;
 			Self::accept_open_channel(recipient, sender)?;
 
-			Self::deposit_event(Event::HrmpSystemChannelOpened(
+			Self::deposit_event(Event::HrmpSystemChannelOpened {
 				sender,
 				recipient,
-				max_capacity,
-				max_message_size,
-			));
+				proposed_max_capacity: max_capacity,
+				proposed_max_message_size: max_message_size,
+			});
 
 			Ok(Pays::No.into())
 		}
@@ -796,7 +806,7 @@ pub mod pallet {
 				Ok(())
 			})?;
 
-			Self::deposit_event(Event::OpenChannelDepositsUpdated(sender, recipient));
+			Self::deposit_event(Event::OpenChannelDepositsUpdated { sender, recipient });
 
 			Ok(())
 		}

diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs
@@ -166,8 +166,13 @@ mod benchmarks {
 		_(sender_origin, recipient_id, capacity, message_size);
 
 		assert_last_event::<T>(
-			Event::<T>::OpenChannelRequested(sender_id, recipient_id, capacity, message_size)
-				.into(),
+			Event::<T>::OpenChannelRequested {
+				sender: sender_id,
+				recipient: recipient_id,
+				proposed_max_capacity: capacity,
+				proposed_max_message_size: message_size,
+			}
+			.into(),
 		);
 	}
 
@@ -179,7 +184,7 @@ mod benchmarks {
 		#[extrinsic_call]
 		_(recipient_origin, sender);
 
-		assert_last_event::<T>(Event::<T>::OpenChannelAccepted(sender, recipient).into());
+		assert_last_event::<T>(Event::<T>::OpenChannelAccepted { sender, recipient }.into());
 	}
 
 	#[benchmark]
@@ -191,7 +196,9 @@ mod benchmarks {
 		#[extrinsic_call]
 		_(sender_origin, channel_id.clone());
 
-		assert_last_event::<T>(Event::<T>::ChannelClosed(sender, channel_id).into());
+		assert_last_event::<T>(
+			Event::<T>::ChannelClosed { by_parachain: sender, channel_id }.into(),
+		);
 	}
 
 	// NOTE: a single parachain should have the maximum number of allowed ingress and egress
@@ -411,8 +418,13 @@ mod benchmarks {
 		_(frame_system::Origin::<T>::Root, sender_id, recipient_id, capacity, message_size);
 
 		assert_last_event::<T>(
-			Event::<T>::HrmpChannelForceOpened(sender_id, recipient_id, capacity, message_size)
-				.into(),
+			Event::<T>::HrmpChannelForceOpened {
+				sender: sender_id,
+				recipient: recipient_id,
+				proposed_max_capacity: capacity,
+				proposed_max_message_size: message_size,
+			}
+			.into(),
 		);
 	}
 
@@ -435,8 +447,13 @@ mod benchmarks {
 		_(frame_system::RawOrigin::Signed(caller), sender_id, recipient_id);
 
 		assert_last_event::<T>(
-			Event::<T>::HrmpSystemChannelOpened(sender_id, recipient_id, capacity, message_size)
-				.into(),
+			Event::<T>::HrmpSystemChannelOpened {
+				sender: sender_id,
+				recipient: recipient_id,
+				proposed_max_capacity: capacity,
+				proposed_max_message_size: message_size,
+			}
+			.into(),
 		);
 	}
 
@@ -478,7 +495,8 @@ mod benchmarks {
 		_(frame_system::RawOrigin::Signed(caller), sender_id, recipient_id);
 
 		assert_last_event::<T>(
-			Event::<T>::OpenChannelDepositsUpdated(sender_id, recipient_id).into(),
+			Event::<T>::OpenChannelDepositsUpdated { sender: sender_id, recipient: recipient_id }
+				.into(),
 		);
 		let channel = HrmpChannels::<T>::get(&channel_id).unwrap();
 		// Check that the deposit was updated in the channel state.