Snowbridge: PNA Audit Better Documentation and minor Refactorings (pa…

…ritytech#6216)

# Description

Snowbridge PNA has been audited. A number of issues where raised due to
not understanding the fee model for Polkadot Native Assets(PNA)
implementation. This PR addresses this by adding more comments and
better naming of private functions.

## Integration

None, documentation and private method name changes.

bridges/snowbridge/primitives/router/src/inbound/mod.rs

@@ -416,6 +416,8 @@ where
 			// Final destination is a 32-byte account on AssetHub
 			Destination::AccountId32 { id } =>
 				Ok(Location::new(0, [AccountId32 { network: None, id }])),
+			// Forwarding to a destination parachain is not allowed for PNA and is validated on the
+			// Ethereum side. https://github.com/Snowfork/snowbridge/blob/e87ddb2215b513455c844463a25323bb9c01ff36/contracts/src/Assets.sol#L216-L224
 			_ => Err(ConvertMessageError::InvalidDestination),
 		}?;
 

bridges/snowbridge/primitives/router/src/outbound/mod.rs

@@ -207,9 +207,9 @@ where
 
 	fn convert(&mut self) -> Result<(Command, [u8; 32]), XcmConverterError> {
 		let result = match self.peek() {
-			Ok(ReserveAssetDeposited { .. }) => self.send_native_tokens_message(),
+			Ok(ReserveAssetDeposited { .. }) => self.make_mint_foreign_token_command(),
 			// Get withdraw/deposit and make native tokens create message.
-			Ok(WithdrawAsset { .. }) => self.send_tokens_message(),
+			Ok(WithdrawAsset { .. }) => self.make_unlock_native_token_command(),
 			Err(e) => Err(e),
 			_ => return Err(XcmConverterError::UnexpectedInstruction),
 		}?;
@@ -222,7 +222,9 @@ where
 		Ok(result)
 	}
 
-	fn send_tokens_message(&mut self) -> Result<(Command, [u8; 32]), XcmConverterError> {
+	fn make_unlock_native_token_command(
+		&mut self,
+	) -> Result<(Command, [u8; 32]), XcmConverterError> {
 		use XcmConverterError::*;
 
 		// Get the reserve assets from WithdrawAsset.
@@ -271,7 +273,12 @@ where
 		ensure!(reserve_assets.len() == 1, TooManyAssets);
 		let reserve_asset = reserve_assets.get(0).ok_or(AssetResolutionFailed)?;
 
-		// If there was a fee specified verify it.
+		// Fees are collected on AH, up front and directly from the user, to cover the
+		// complete cost of the transfer. Any additional fees provided in the XCM program are
+		// refunded to the beneficiary. We only validate the fee here if its provided to make sure
+		// the XCM program is well formed. Another way to think about this from an XCM perspective
+		// would be that the user offered to pay X amount in fees, but we charge 0 of that X amount
+		// (no fee) and refund X to the user.
 		if let Some(fee_asset) = fee_asset {
 			// The fee asset must be the same as the reserve asset.
 			if fee_asset.id != reserve_asset.id || fee_asset.fun > reserve_asset.fun {
@@ -328,7 +335,9 @@ where
 	/// # BuyExecution
 	/// # DepositAsset
 	/// # SetTopic
-	fn send_native_tokens_message(&mut self) -> Result<(Command, [u8; 32]), XcmConverterError> {
+	fn make_mint_foreign_token_command(
+		&mut self,
+	) -> Result<(Command, [u8; 32]), XcmConverterError> {
 		use XcmConverterError::*;
 
 		// Get the reserve assets.
@@ -377,7 +386,12 @@ where
 		ensure!(reserve_assets.len() == 1, TooManyAssets);
 		let reserve_asset = reserve_assets.get(0).ok_or(AssetResolutionFailed)?;
 
-		// If there was a fee specified verify it.
+		// Fees are collected on AH, up front and directly from the user, to cover the
+		// complete cost of the transfer. Any additional fees provided in the XCM program are
+		// refunded to the beneficiary. We only validate the fee here if its provided to make sure
+		// the XCM program is well formed. Another way to think about this from an XCM perspective
+		// would be that the user offered to pay X amount in fees, but we charge 0 of that X amount
+		// (no fee) and refund X to the user.
 		if let Some(fee_asset) = fee_asset {
 			// The fee asset must be the same as the reserve asset.
 			if fee_asset.id != reserve_asset.id || fee_asset.fun > reserve_asset.fun {