From 4891f6ba6ae8c9068c0eabd55bc48d98fb03d802 Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Wed, 16 Oct 2024 13:03:41 +0200 Subject: [PATCH 1/4] better documenation about fee handling --- .../primitives/router/src/outbound/mod.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/bridges/snowbridge/primitives/router/src/outbound/mod.rs b/bridges/snowbridge/primitives/router/src/outbound/mod.rs index efc1ef56f304..ff2fa8ab01c7 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/outbound/mod.rs @@ -271,7 +271,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. + // Enough Fees are collected on Asset Hub, 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 beneficairy. 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 { @@ -377,7 +382,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. + // Enough Fees are collected on Asset Hub, 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 beneficairy. 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 { From 6a3a426d26d7c811778cd4466ab19e7252390d3d Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Sun, 20 Oct 2024 12:25:36 +0200 Subject: [PATCH 2/4] PR feedback --- .../primitives/router/src/outbound/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bridges/snowbridge/primitives/router/src/outbound/mod.rs b/bridges/snowbridge/primitives/router/src/outbound/mod.rs index ff2fa8ab01c7..08d61f86eded 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/mod.rs +++ b/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,7 @@ 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,9 +271,9 @@ where ensure!(reserve_assets.len() == 1, TooManyAssets); let reserve_asset = reserve_assets.get(0).ok_or(AssetResolutionFailed)?; - // Enough Fees are collected on Asset Hub, up front and directly from the user, to cover the + // 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 beneficairy. We only validate the fee here if its provided to make sure + // 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. @@ -333,7 +333,7 @@ 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. @@ -382,9 +382,9 @@ where ensure!(reserve_assets.len() == 1, TooManyAssets); let reserve_asset = reserve_assets.get(0).ok_or(AssetResolutionFailed)?; - // Enough Fees are collected on Asset Hub, up front and directly from the user, to cover the + // 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 beneficairy. We only validate the fee here if its provided to make sure + // 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. From 871d3016e29e785e62fce971c44a42042a833408 Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Tue, 22 Oct 2024 00:02:16 +0200 Subject: [PATCH 3/4] more documentation --- bridges/snowbridge/primitives/router/src/inbound/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bridges/snowbridge/primitives/router/src/inbound/mod.rs b/bridges/snowbridge/primitives/router/src/inbound/mod.rs index fbfc52d01c83..a9324ac42470 100644 --- a/bridges/snowbridge/primitives/router/src/inbound/mod.rs +++ b/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), }?; From f93a541dcdb448edd23ef0964895154f8ac2e5a4 Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Thu, 24 Oct 2024 13:29:14 +0200 Subject: [PATCH 4/4] cargo fmt --- bridges/snowbridge/primitives/router/src/outbound/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bridges/snowbridge/primitives/router/src/outbound/mod.rs b/bridges/snowbridge/primitives/router/src/outbound/mod.rs index 08d61f86eded..3b5dbdb77c89 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/outbound/mod.rs @@ -222,7 +222,9 @@ where Ok(result) } - fn make_unlock_native_token_command(&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. @@ -333,7 +335,9 @@ where /// # BuyExecution /// # DepositAsset /// # SetTopic - fn make_mint_foreign_token_command(&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.