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

Asset Hubs: auto incremented asset id for trust backed assets #414

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Jul 31, 2024

Setup auto incremented asset id to 50_000_000 for trust backed assets.

Migration

Stakeholders: all clients providing asset creation functionality on Polkadot/Kusama Asset Hub

This change does not break the API but introduces a new constraint. It implements an auto-incremented ID strategy for Trust-Backed Assets (50 pallet instance indexes on both networks), starting at ID 50,000,000. Each new asset must be created with an ID that is one greater than the last asset created. The next ID can be fetched from the NextAssetId storage item of the assets pallet. An empty NextAssetId storage item indicates no constraint on the next asset ID and can serve as a feature flag for this release.

@@ -1080,6 +1059,11 @@ pub type Migrations = (
frame_support::migrations::RemovePallet<DmpQueueName, RocksDbWeight>,
pallet_collator_selection::migration::v2::MigrationToV2<Runtime>,
cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5<Runtime>,
pallet_assets::migration::next_asset_id::SetNextAssetId<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joepetrowski still needs to delete MAX_PRIME :P

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkchr
Copy link
Contributor

bkchr commented Aug 12, 2024

/merge

@bkchr
Copy link
Contributor

bkchr commented Aug 12, 2024

@anaelleltd this needs to be communicated to asset issuers.

@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 enabled auto-merge (squash) August 12, 2024 19:07
@fellowship-merge-bot
Copy link
Contributor

Failed to update PR ❌

There was an error while trying to keep this PR up-to-date

You may have conflicts ‼️ or may have to manually sync it with the target branch 👉❇️

More info in the logs 📋

@anaelleltd
Copy link
Collaborator

@anaelleltd this needs to be communicated to asset issuers.

As indicated in step 6 of the guidelines re: Runtime release process, the author of the PR should indicate:

  • what kind of changes the PR introduces (breaking? disruption? downtime?)
  • who will be affected by theses changes in practice (parachains? wallets? UIs? CEXes? DEXes?)
  • how the team/builders affected need to follow-up (recommended changes/updates to the code base)

cc @muharem

@fellowship-merge-bot
Copy link
Contributor

Failed to update PR ❌

There was an error while trying to keep this PR up-to-date

You may have conflicts ‼️ or may have to manually sync it with the target branch 👉❇️

More info in the logs 📋

@fellowship-merge-bot
Copy link
Contributor

Failed to update PR ❌

There was an error while trying to keep this PR up-to-date

You may have conflicts ‼️ or may have to manually sync it with the target branch 👉❇️

More info in the logs 📋

@fellowship-merge-bot fellowship-merge-bot bot merged commit c125f5c into polkadot-fellows:main Aug 13, 2024
46 checks passed
@muharem
Copy link
Contributor Author

muharem commented Aug 15, 2024

I have update a description with more information for clients.

@bkchr @joepetrowski should we include this into 1.3? If we are going to cut it form master, it is already included. We trying to understand with @anaelleltd if we should notify the clients already.

@joepetrowski
Copy link
Contributor

Yeah it will be included from master.

@kianenigma
Copy link
Contributor

The next ID can be fetched from the NextAssetId storage item of the assets pallet. An empty NextAssetId storage item indicates no constraint on the next asset ID and can serve as a feature flag for this release.

I am starting to move some reasonable items to being in #364.

Is this a reasonable one to also move over?

@muharem
Copy link
Contributor Author

muharem commented Aug 16, 2024

@kianenigma you mean as I dynamic parameter? I think no, we do not need it to be a dynamic paramater

@kianenigma
Copy link
Contributor

@anaelleltd this needs to be communicated to asset issuers.

As indicated in step 6 of the guidelines re: Runtime release process, the author of the PR should indicate:

  • what kind of changes the PR introduces (breaking? disruption? downtime?)
  • who will be affected by theses changes in practice (parachains? wallets? UIs? CEXes? DEXes?)
  • how the team/builders affected need to follow-up (recommended changes/updates to the code base)

cc @muharem

Let's make sure these steps that need to be done by the PR author are part of a PR template?

@kianenigma
Copy link
Contributor

@kianenigma you mean as I dynamic parameter? I think no, we do not need it to be a dynamic paramater

No, I mean a runtime API to get the next available AssetId, abstracting over where this storage live, and what its semantics are.

github-merge-queue bot pushed a commit to paritytech/polkadot-sdk that referenced this pull request Sep 16, 2024
Setup auto incremented asset id to `50_000_000` for trust backed assets.

In order to align with Polkadot/Kusama Asset Hub -
polkadot-fellows/runtimes#414
The next closes existing assets IDs in Rococo is `69_696_969`, in
Westend is `88_228_866`.

### Migration
**Stakeholders**: all clients providing asset creation functionality on
Westend/Rococo Asset Hub

This change does not break the API but introduces a new constraint. It
implements an auto-incremented ID strategy for Trust-Backed Assets (50
pallet instance indexes on both networks), starting at ID 50,000,000.
Each new asset must be created with an ID that is one greater than the
last asset created. The next ID can be fetched from the `NextAssetId`
storage item of the assets pallet. An empty `NextAssetId` storage item
indicates no constraint on the next asset ID and can serve as a feature
flag for this release.
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.

6 participants