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.
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
method
pseudo-op support for ABI methods #153method
pseudo-op support for ABI methods #153Changes from 7 commits
a363756
4286afc
f5467e1
a111aa0
f54013e
091dafe
003771c
45e6414
e743eba
4b0397d
9c1825a
20505c9
9f7ec76
9539089
3327bd5
c027311
cbbf13e
7856987
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.
I'm less sure about the changes to this file, since not every subroutine will represent a method.
What if you changed
methodSignature
to justname
? I.e. if you provide thename
arg, then it would override whatever the function's__name__
is.@barnjamin thoughts on this?
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 had thought we'd be able to infer the method signature from the method definition, including name and types it accepts. That would mean defining the ABI types with accessors.
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 started something here https://github.com/barnjamin/pyteal-utils/blob/main/pytealutils/abi/types.py but maybe this kind of thing belongs in PyTEAL proper.
We should definitely have some representation of them so decoding from the over-the-wire format is painless
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 ended up just adding the ABI types to utils for now using the py-sdk to test encoding/decoding: https://github.com/algorand/pyteal-utils/blob/a5f35c9e217b2c6e259b8a725bb988ec631e6fc8/pytealutils/abi/test_fixed_point.py
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.
Just changed
methodSignature
name tonameStr
, and override function's__name__
on initialization, but I still feel this needs to be polished as an implementation, and should be modified after ABI in PyTeal is introduced.I would like to keep this thread open for more discussion on the design of subroutine (in combination with ABI methods).