From 5682f9a273309c7160722a7937a75152a2793b23 Mon Sep 17 00:00:00 2001
From: Alistair Singh <alistair.singh7@gmail.com>
Date: Thu, 24 Oct 2024 14:27:01 +0200
Subject: [PATCH] Snowbridge: PNA Audit Better Documentation and minor
Refactorings (#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.
---
.../primitives/router/src/inbound/mod.rs | 2 ++
.../primitives/router/src/outbound/mod.rs | 26 ++++++++++++++-----
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/bridges/snowbridge/primitives/router/src/inbound/mod.rs b/bridges/snowbridge/primitives/router/src/inbound/mod.rs
index fbfc52d01c8..a9324ac4247 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),
}?;
diff --git a/bridges/snowbridge/primitives/router/src/outbound/mod.rs b/bridges/snowbridge/primitives/router/src/outbound/mod.rs
index efc1ef56f30..3b5dbdb77c8 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,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 {
--
GitLab