Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Max asset ID restriction for new trusted assets #346

Merged

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Jun 7, 2024

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..

muharem and others added 2 commits June 7, 2024 16:31
@github-actions github-actions bot requested a review from bkchr June 7, 2024 14:32
Copy link

github-actions bot commented Jun 7, 2024

Review required! Latest push from author must always be reviewed

CHANGELOG.md Outdated Show resolved Hide resolved
@xlc
Copy link
Contributor

xlc commented Jun 8, 2024

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
Copy link
Contributor

bkchr commented Jun 8, 2024

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

@joepetrowski
Copy link
Contributor

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
Copy link
Contributor

/merge

@fellowship-merge-bot fellowship-merge-bot bot enabled auto-merge (squash) June 13, 2024 10:42
@fellowship-merge-bot
Copy link
Contributor

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

@fellowship-merge-bot fellowship-merge-bot bot merged commit c05dbd1 into polkadot-fellows:main Jun 13, 2024
37 checks passed
@joepetrowski joepetrowski mentioned this pull request Jun 13, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants