This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add
ExistenceRequirement
toCurrency
trait #4000Add
ExistenceRequirement
toCurrency
trait #4000Changes from 6 commits
eebdb06
412ff66
e659ee6
caa9476
91e73bd
e95212d
9b49f3f
e14169d
ac0baa2
8b1559f
1b62003
596c8b2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not in
impl Currency
?substrate/srml/balances/src/lib.rs
Line 789 in 91e73bd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to hassle
Currency
implementors with writing an implementation oftransfer_inner
/transfer_some
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could change transfer signature of Currency trait to:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should try to avoid breaking anything if we can. See @xlc's first comment in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Xlc comment is about not changing the Call. We can still have transfer and transfer_keep_alive in Call and have
transfer(...., liveness: ExistenceRequirement)
in Currency trait.I feel generalizing the trait here doesn't harm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends if anyone else has implemented anything on it - third parties might have done. That said, if it is to be changed, now is the time to do it.