-
Notifications
You must be signed in to change notification settings - Fork 1
Polkadot assets on ethereum #128
Changes from all commits
6019c8d
c613fc6
9672b52
c017bfe
3dc37ea
1f00024
a20a8b7
41104d5
a8b006b
b8a75d1
fec5a63
a683680
3621de4
f7ed096
36bb656
cf091f6
4921280
10c5e10
587fb6e
f56567d
c792204
a81b87f
ce91a8c
f3c9d11
41aa563
d6c6330
faf8f73
7001866
1edad16
933130a
8ace79e
40cc470
2d8f3b1
5a4f3af
50983ed
5ad44df
8a79d89
0251493
fa70f7b
3d0fae3
25c819a
3cb2f05
3a94733
9d900b2
33ea85e
d64e783
79fb3b5
8609eed
3f1ded3
0e06cae
4a04f1f
0d3988c
f1155b1
a44db43
0f0fd8e
f5c6b92
1095bff
21fa444
2675241
42933da
760d697
a550c42
736fe90
d1b8d16
897fdad
e733321
31c75e9
009402e
949fb45
b38b20c
0b2f782
62eef65
39a84b9
5ae8b0b
6ca963d
79e62fa
8a4f582
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,13 +63,16 @@ use frame_system::pallet_prelude::*; | |
use snowbridge_core::{ | ||
meth, | ||
outbound::{Command, Initializer, Message, OperatingMode, SendError, SendMessage}, | ||
sibling_sovereign_account, AgentId, Channel, ChannelId, ParaId, | ||
PricingParameters as PricingParametersRecord, PRIMARY_GOVERNANCE_CHANNEL, | ||
sibling_sovereign_account, AgentId, AssetMetadata, Channel, ChannelId, ParaId, | ||
PricingParameters as PricingParametersRecord, TokenId, TokenIdOf, PRIMARY_GOVERNANCE_CHANNEL, | ||
SECONDARY_GOVERNANCE_CHANNEL, | ||
}; | ||
use sp_core::{RuntimeDebug, H160, H256}; | ||
use sp_io::hashing::blake2_256; | ||
use sp_runtime::{traits::BadOrigin, DispatchError, SaturatedConversion}; | ||
use sp_runtime::{ | ||
traits::{BadOrigin, MaybeEquivalence}, | ||
DispatchError, SaturatedConversion, | ||
}; | ||
use sp_std::prelude::*; | ||
use xcm::prelude::*; | ||
use xcm_executor::traits::ConvertLocation; | ||
|
@@ -99,7 +102,7 @@ where | |
} | ||
|
||
/// Hash the location to produce an agent id | ||
fn agent_id_of<T: Config>(location: &Location) -> Result<H256, DispatchError> { | ||
pub fn agent_id_of<T: Config>(location: &Location) -> Result<H256, DispatchError> { | ||
T::AgentIdOf::convert_location(location).ok_or(Error::<T>::LocationConversionFailed.into()) | ||
} | ||
|
||
|
@@ -211,6 +214,11 @@ pub mod pallet { | |
PricingParametersChanged { | ||
params: PricingParametersOf<T>, | ||
}, | ||
/// Register token | ||
RegisterToken { | ||
asset_id: VersionedLocation, | ||
token_id: H256, | ||
}, | ||
} | ||
|
||
#[pallet::error] | ||
|
@@ -226,6 +234,7 @@ pub mod pallet { | |
InvalidTokenTransferFees, | ||
InvalidPricingParameters, | ||
InvalidUpgradeParameters, | ||
TokenExists, | ||
} | ||
|
||
/// The set of registered agents | ||
|
@@ -243,6 +252,15 @@ pub mod pallet { | |
pub type PricingParameters<T: Config> = | ||
StorageValue<_, PricingParametersOf<T>, ValueQuery, T::DefaultPricingParameters>; | ||
|
||
#[pallet::storage] | ||
#[pallet::getter(fn tokens)] | ||
pub type Tokens<T: Config> = StorageMap<_, Twox64Concat, TokenId, Location, OptionQuery>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. Location version is implicitly v4. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change was rolled back. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alistair-singh We should not use VersionedLocation for fields in storage. There is other commentary in the PR about this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is stopping someone from rolling over xcm versions prelude without us knowing and breaking the lookup of Tokens? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With paritytech@5ef32c8 will just not compile. |
||
|
||
#[pallet::storage] | ||
#[pallet::getter(fn location_tokens)] | ||
pub type LocationToToken<T: Config> = | ||
StorageMap<_, Twox64Concat, Location, TokenId, OptionQuery>; | ||
alistair-singh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
#[pallet::genesis_config] | ||
#[derive(frame_support::DefaultNoBound)] | ||
pub struct GenesisConfig<T: Config> { | ||
|
@@ -574,6 +592,29 @@ pub mod pallet { | |
}); | ||
Ok(()) | ||
} | ||
|
||
/// Sends a message to the Gateway contract to register a new | ||
/// token that represents `asset`. | ||
/// | ||
/// - `origin`: Must be `MultiLocation` from sibling parachain | ||
#[pallet::call_index(10)] | ||
#[pallet::weight(T::WeightInfo::register_token())] | ||
pub fn register_token( | ||
origin: OriginFor<T>, | ||
location: Box<VersionedLocation>, | ||
metadata: AssetMetadata, | ||
) -> DispatchResult { | ||
let who = ensure_signed(origin)?; | ||
alistair-singh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
let asset_loc: Location = | ||
(*location).try_into().map_err(|_| Error::<T>::UnsupportedLocationVersion)?; | ||
|
||
let pays_fee = PaysFee::<T>::Yes(who); | ||
|
||
Self::do_register_token(asset_loc, metadata, pays_fee)?; | ||
|
||
Ok(()) | ||
} | ||
} | ||
|
||
impl<T: Config> Pallet<T> { | ||
|
@@ -663,6 +704,30 @@ pub mod pallet { | |
let secondary_exists = Channels::<T>::contains_key(SECONDARY_GOVERNANCE_CHANNEL); | ||
primary_exists && secondary_exists | ||
} | ||
|
||
pub(crate) fn do_register_token( | ||
asset_loc: Location, | ||
metadata: AssetMetadata, | ||
pays_fee: PaysFee<T>, | ||
) -> Result<(), DispatchError> { | ||
// Record the token id or fail if it has already been created | ||
let token_id = TokenIdOf::convert_location(&asset_loc) | ||
.ok_or(Error::<T>::LocationConversionFailed)?; | ||
Tokens::<T>::insert(token_id, asset_loc.clone()); | ||
LocationToToken::<T>::insert(asset_loc.clone(), token_id); | ||
|
||
let command = Command::RegisterForeignToken { | ||
token_id, | ||
name: metadata.name.into_inner(), | ||
symbol: metadata.symbol.into_inner(), | ||
decimals: metadata.decimals, | ||
}; | ||
Self::send(SECONDARY_GOVERNANCE_CHANNEL, command, pays_fee)?; | ||
|
||
Self::deposit_event(Event::<T>::RegisterToken { asset_id: asset_loc.into(), token_id }); | ||
|
||
Ok(()) | ||
} | ||
} | ||
|
||
impl<T: Config> StaticLookup for Pallet<T> { | ||
|
@@ -684,4 +749,13 @@ pub mod pallet { | |
PricingParameters::<T>::get() | ||
} | ||
} | ||
|
||
impl<T: Config> MaybeEquivalence<TokenId, Location> for Pallet<T> { | ||
fn convert(id: &TokenId) -> Option<Location> { | ||
Tokens::<T>::get(id) | ||
} | ||
fn convert_back(loc: &Location) -> Option<TokenId> { | ||
LocationToToken::<T>::get(loc) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like we accepting this in Location relative to the Ethereum address.
I think we should accept an asset relative to bridge hub as input, and then re-achoring it onto the bridge location.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is the Location relative to the Ethereum after reanchored on AH in
polkadot-sdk/polkadot/xcm/pallet-xcm/src/lib.rs
Line 1944 in 9368712
On BH outbound router we check the asset location registered as expected in
polkadot-sdk/bridges/snowbridge/primitives/router/src/outbound/mod.rs
Lines 386 to 390 in b19c1e1
I prefer to register PNA with the reanchored location directly without adding reanchor logic in outbound router every time when receiving the xcm from AH.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am suggesting doing the re-anchoring once inside the registration extrinsic.
All other XCM related functions, when taking locations as input, are relative to the current chain. See pallet-xcm for example. So its not obvious to users that this is the required way to call it.
The other thing is that Locations are not distinct:
(2, [GlobalConsensus(Westend)])
and(1, [Here])
are the same asset and if re-anchoring from bridge hub will both collapse down to the unique(1, [GlobalConsensus(Westend)])
. So they get the same id.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's true. But it's a bit different here actually the asset location is reanchored in pallet-xcm relative to Ethereum in the view of AH. Then routed to BH with
SovereignPaidRemoteExporter
without reanchoring.TBH I don't think the reanchor in pallet-xcm is nessessary at all for our use case. But it's another story.
So I'd prefer to make things easy for now(Permissionless register token is not even enabled yet) and improve it later in another PR.
@alistair-singh WDYT?