Multivalue-opcodes
Before finishing the ergonomic part of Wide64 arithmetic in https://github.com/algorand/pyteal/pull/541, the basic opcodes should be implement first.
Goal
Here I'm trying to add addw, mulw, divw(exsited), divmodw. And maybe some other stack ops like dig n, swap, cover n, uncover n.
TBD
- I didn't work on the stack ops like
dig n,swap,cover n,uncover nmanipulate stack hence are not very useful in pyteal, where we don't usually interact the stack. - Structure & Naming: I created a new class
WideExpr(MultiValue)in new file 'wideexpr.py', and named the methods asAddW,MulW, etc. instead ofAddw, etc, since they are all multivariable functions (likeaddwis binary-multivalue), hence current system which categorizes functions based on their input var num (like 'binaryexpr', 'ternaryexpr', 'naryexpr') doesn't work well on them. The common property is that they all focus onWideInt, but this might breaks the current categorization to be not mutually exclusive and might leads to confusion.
Tasks
- [x] add
addw - [x] test
addwfor valid case - [x] test
addwwith invalid case, - [x] add Class WideExpr
- [x] add
mulw,expw,divmodwwith their tests - [ ] ~~add function overload for
addw(adding many 64bit uint)~~ - [ ] ~~test for addw overload for valid case and invalid case~~
- [ ] ~~add function overload for
mulw(multiplying many 64bit uint)~~
It seems there's no arithmetic test in tests so I skip them too. These parts I didn't finish are moved to PR#541 where I implement the ergo-features.
With some vision from https://github.com/algorand/pyteal/pull/541 , where I'm going to create a Wide64 type similar to MaybeValue in 'wide64.py', and for a w: Wide64, w.high() load the high word scratch, and w.low() the low.
With this in mind, I'm going to update the addw to return a more specific Wide64 instead of MultiValue. So these methods can be categorize to 'wideexpr.py'.
So I split them for now to a ' multiexpr.py' and rename it in the next branch.
For DivW, I added a AVM version test and rename, but didn't move it. I think it's nicely categorized as ternary expr, since it doesn't "produce WideInt".
Would you mind reviewing this PR as well? @michaeldiamant It precedes https://github.com/algorand/pyteal/pull/541.
@PabloLION Thanks for putting in some cycles here.
Due to other priorities, it's been some time since I last looked into the PR's topic. From my perspective, I see the ergonomics and API design as intertwined. Since pyteal makes API backwards compatibility guarantees, I'm hesitant to bring the PR in and consider ergonomics later.
I'm not looking to discourage you from continuing ahead. But, I want to be upfront that given the current priority set, we're unlikely to merge the PR in a near future.
@michaeldiamant Thanks for the reply.
I appreciate the backwards compatibility too. Otherwise I would've changed the second overload of Bytes in "bytes.py" and put the encoding as the second variable 😅. And here's actually a breaking change on the naming of Divw. Maybe we should keep the old one and redirect DivW to it.
Also, I'm glad that you noticed that PR https://github.com/algorand/pyteal/pull/544 cannot be finished before those "TBD"s are resolved. I'll try your recommendation on the pyteal-utils which I think solves my current problem. Thanks.