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

frame_support::Incrementable: shouldn't it return None if max is reached? #7845

Closed
clangenb opened this issue Mar 7, 2025 · 0 comments · Fixed by #7846
Closed

frame_support::Incrementable: shouldn't it return None if max is reached? #7845

clangenb opened this issue Mar 7, 2025 · 0 comments · Fixed by #7846

Comments

@clangenb
Copy link
Contributor

clangenb commented Mar 7, 2025

The documentation of incrementable says:

ReturnsSomewith the incremented value if it is possible, orNone if it is not.

as can be seen here.

However, instead of returning none if the current value is the maximum already, it returns the saturated value, this is counterintuitive for me. Additionally, it seems that this behavior is a bug in the AutoIncAssetId, which IMO should return an error if the maximum value for the asset it is reached, otherwise we will keep overwriting the asset with AssetId::MAX, when a new asset is created, see here.

This is the only usage (as far as I can tell) in the polkadot-sdk, so my suggestion is to change this to this line to simply use a checked operation instead of a saturating one.

github-merge-queue bot pushed a commit that referenced this issue Mar 10, 2025
Closes #7845

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Guillaume Thiolliere <[email protected]>
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 a pull request may close this issue.

1 participant