Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

AccountTouch trait: deposit_required accepts asset id #14147

Merged
merged 1 commit into from
May 15, 2023
Merged

AccountTouch trait: deposit_required accepts asset id #14147

merged 1 commit into from
May 15, 2023

Conversation

muharem
Copy link
Contributor

@muharem muharem commented May 15, 2023

deposit_required accepts an asset id to determine a required deposit.

@muharem muharem added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit F3-breaks_API This PR changes public API; next release should be major. labels May 15, 2023
@muharem muharem mentioned this pull request May 15, 2023
Merged
@muharem muharem requested a review from gilescope May 15, 2023 14:35
@muharem muharem marked this pull request as ready for review May 15, 2023 14:39
@muharem muharem requested a review from a team May 15, 2023 14:39
gilescope
gilescope approved these changes May 15, 2023
View reviewed changes
Copy link
Contributor

@gilescope gilescope left a comment

Choose a reason for hiding this comment

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

Thank you. This gives the implementer enough information to potentially change the deposit dependent on the information contained within the asset id.

@muharem
Copy link
Contributor Author

muharem commented May 15, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit e4b08db into master May 15, 2023
@paritytech-processbot paritytech-processbot bot deleted the muharem-asset-account-touch-fix branch May 15, 2023 16:08
@gilescope
Copy link
Contributor

We can take off the brakes API label as the api was only merged in a week ago, and no new polkadot branches or monthly branches have been created so that api has not yet been released so this is not a breaking change in and of itself.

EgorPopelyaev pushed a commit that referenced this pull request May 16, 2023
@muharem @EgorPopelyaev
AccountTouch: deposit_required requires asset id (#14147)
d5a6524
@muharem muharem removed the F3-breaks_API This PR changes public API; next release should be major. label May 16, 2023
@muharem
Copy link
Contributor Author

muharem commented May 16, 2023

We can take off the brakes API label as the api was only merged in a week ago, and no new polkadot branches or monthly branches have been created so that api has not yet been released so this is not a breaking change in and of itself.

removed

@kiltbot kiltbot mentioned this pull request Jun 21, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
@jasl jasl mentioned this pull request Jun 30, 2023
Merged
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@muharem @nathanwhit
AccountTouch: deposit_required requires asset id (paritytech#14147)
cca606f
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gilescope gilescope gilescope approved these changes

@jsidorenko jsidorenko jsidorenko approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@muharem @gilescope @jsidorenko