-
Notifications
You must be signed in to change notification settings - Fork 646
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
fix(Clarity2):Factor out 'stx-transfer-memo' into a separate function #2732
Conversation
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.
LGTM! Just commented about a small test addition.
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.
thanks for review!
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 didn't see any gating for stx-transfer-memo?
. I think you'll want to add that in first before merging -- stx-transfer-memo?
is not valid in Clarity 1.
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.
@jcnelson thanks for the comments.
I rebased against 'next' so now this PR has version guarding.
Refactored tests in src/vm/analysis/arithmetic_checker/tests.rs
test the version guarding.
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.
LGTM now. Thanks @gregorycoppola!
Thanks for the reviews everyone. |
Fixes #2615
Motivation
Currently, in the next branch, it is possible to supply a memo to
stx-transfer?
. However, it has been decided that it might be better to force the user to use a different function,stx-transfer-memo?
if they are going to add a memo. The reason for this is to allow better type checking -- the user will either have to add a memo or not, depending on which version of the function they use, and they won't be able to forget to put a memo if they are using the "memo" version.We also version-guard this function, so that one has to be using Stacks 2.1 in order to use it.
Testing
Tested using unit tests. The following tests have been modified: