polkadot-fellows / runtimes

The various runtimes which make up the core subsystems of networks for which the Fellowship is represented.
GNU General Public License v3.0
125 stars 72 forks source link

Max asset ID restriction for new trusted assets #346

Closed muharem closed 2 weeks ago

muharem commented 3 weeks ago

Set the maximum asset ID for the creation of trusted assets.

This change will enable us to migrate the trusted assets instance to start auto-incrementing IDs from 50,000,000 with the release following the next one..

github-actions[bot] commented 3 weeks ago

Review required! Latest push from author must always be reviewed

xlc commented 3 weeks ago

It is probably too late to make such refactor but the asset id type should really be an enum

enum AssetId {
  Trusted(u32),
   Untrusted(u32),
   Foreign(Location),
}
bkchr commented 3 weeks ago

What is the difference between a trusted and untrusted asset? And what would be a remote asset? Token by another chain?

joepetrowski commented 3 weeks ago

It is probably too late to make such refactor but the asset id type should really be an enum

Yeah it probably would have been better than separate pallet instances for each type. But talk about a big migration. We just didn't know that far ahead when we made the first instance with type u32, and had to do some refactoring to the Assets pallet to support Location at all. Anyway, major breaking change plus migration but if the current structure becomes a problem then yeah we'd have to consider it.

What is the difference between a trusted and untrusted asset? And what would be a remote asset?

Not sure what Bryan meant or if it was just an example, but we have TrustBacked for assets like USDT that anyone can register, because the user "trusts" that they are backed by some off-chain claim. Foreign assets are assets backed by other locations, which could be protocols, e.g. the asset corresponding to (1, Parachain(9000)).

joepetrowski commented 2 weeks ago

/merge

fellowship-merge-bot[bot] commented 2 weeks ago

Enabled auto-merge in Pull Request

Available commands - `/merge`: Enables auto-merge for Pull Request - `/merge cancel`: Cancels auto-merge for Pull Request - `/merge help`: Shows this menu For more information see the [documentation](https://github.com/paritytech/auto-merge-bot)