Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Frame Ops to avm8 branch PR #585

Merged
merged 9 commits into from
Nov 2, 2022
Merged

Frame Ops to avm8 branch PR #585

merged 9 commits into from
Nov 2, 2022

Conversation

ahangsu
Copy link
Contributor

@ahangsu ahangsu commented Oct 26, 2022

Introduction to frame pointer ops in avm8 PR

@ahangsu
Copy link
Contributor Author

ahangsu commented Oct 26, 2022 •
edited
Loading

A few things to debate about:

  • on using frame pointer in subroutine, we only used proto/frame_bury/frame_dig/Dupn, while bury/popn are not used.
    I am happy to hide/delete bury/popn expressions, and revisit until we need them.
  • proto can only be used at the beginning of subroutine, and I don't feel good about exposing it. Shall we remove it in export list, namely __init__.py?

tzaffi
tzaffi reviewed Nov 1, 2022
View reviewed changes
Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

I still have more suggestions, but I'm having cookie-related-issues (?) seeing what my suggestions were so am going to add a preliminary review now

tzaffi
tzaffi reviewed Nov 1, 2022
View reviewed changes
pyteal/ast/frame.py Outdated Show resolved Hide resolved
tzaffi
tzaffi reviewed Nov 1, 2022
View reviewed changes
Copy link
Contributor

@tzaffi tzaffi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Functionally everything looks great. I'm just quibbling over the namings. That's part of what makes computer science so challenging and fun.

pyteal/ast/frame.py Outdated Show resolved Hide resolved
pyteal/ast/frame.py Outdated Show resolved Hide resolved
pyteal/ast/frame.py Outdated Show resolved Hide resolved
@tzaffi
Copy link
Contributor

tzaffi commented Nov 2, 2022

A few things to debate about:

  • on using frame pointer in subroutine, we only used proto/frame_bury/frame_dig/Dupn, while bury/popn are not used.
    I am happy to hide/delete bury/popn expressions, and revisit until we need them.
  • proto can only be used at the beginning of subroutine, and I don't feel good about exposing it. Shall we remove it in export list, namely __init__.py?

Thanks for deleting bury. You are mentioning that popn isn't used either. Therefore, I recommend removing it as well, unless there is a clear use case (nothing POPS OUT to me 🤣 )

@ahangsu
Copy link
Contributor Author

ahangsu commented Nov 2, 2022

You are mentioning that popn isn't used either. Therefore, I recommend removing it as well, unless there is a clear use case (nothing POPS OUT to me 🤣 )

pop popn

tzaffi
tzaffi approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

LGTM

@ahangsu
Copy link
Contributor Author

ahangsu commented Nov 2, 2022 •
edited
Loading

reminder: CHANGELOG.md not really, we are not exposing it, so I don't want to expose it to CHANGELOG.md

jasonpaulos
jasonpaulos reviewed Nov 2, 2022
View reviewed changes
pyteal/__init__.py Outdated Show resolved Hide resolved
@ahangsu ahangsu merged commit dd687c7 into avm8 Nov 2, 2022
@ahangsu ahangsu deleted the frame-ops branch November 2, 2022 21:24
ahangsu added a commit that referenced this pull request Nov 3, 2022
@michaeldiamant @barnjamin
@jdtzmn @ahangsu @tzaffi @jasonpaulos
AVM 8 (#439)
bdcdd5c 
* upping max teal version

* adding program page related ops (#412)

* adding program page related ops

* Add Replace (#413)

* Add Replace

* Remove replace auto-import

* Use scripts/generate_init.py

* Add more tests to replace, substring, and extract (#1)

Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>

* Add Block (#415)

* Add Block

# Conflicts:
#	pyteal/ir/ops.py

* Disable flake8 errors on formatted lines

* Add past version failure check

* Remove unnecessary ignore Expr equality context

* Add JsonRef (#417)

* Add JsonRef

* Use named class methods to specify value type

* Remove unnecessary ignore Expr equality context

* Fix docstring link

* Add Base64Decode (#418)

* Add Base64Decode

* Remove unnecessary ignore Expr equality context

* Support Secp256r1 curve (#423)

* Support Secp256r1 curve

* Fix type errors in ecdsa tests

* Fix typo

* Test Secp256k1 curve against TEAL 5 instead

* Add compile check to `MultiValue` class

* Use `MultiValue` compile checks instead of inheritance

* Add VrfVerify (#419)

* Add VrfVerify

# Conflicts:
#	pyteal/ast/__init__.py
#	pyteal/ir/ops.py

* Tidy with `MultiValue`’s compile check

* Add `Sha3_256` (#425)

* Add sha3_256

* Add crypto docs

* Support `FirstValidTime` transaction field (#424)

* Add first valid time factory and update min version

* Include FirstValidTime in txn tests

* Add transaction field docs

* Add `Ed25519Verify_Bare` (#426)

* Add ed25519verify_bare

* Fix typos in Ed25519 docstrings (#2)

* Add crypto doc for Ed25519Verify_Bare

Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>

* AVM Boxes Ops in Pyteal (#438)

* add box ops

* full support on ops

* first set of test, add versioning in multi

* remove some seemingly not necessary code?

* update testcase

* check invalid arguments

* finish testcase

* move stuffs to app

* version check trick

* verifyTealVersion apply

* error message

* update docs structures

* period

* update doc

* update doc

* update doc

* per pr review on implementation

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* hex box size goes wild

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* warning about MBR

* wording

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* emphasize

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Update docs/state.rst

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* polishing

* remove redundant box_put doc segment

* per zeph pr review

* use note and warning

* per zeph's pr review

* Update docs/state.rst

Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>

* creating boxes

* Update docs/state.rst

Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>

* per pr review

* table for state types

Co-authored-by: Ben Guidarelli <ben.guidarelli@gmail.com>
Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>

* Merge Teal7 to AVM8, and consolidate Teal to AVM versioning  (#470)

* swapping base64 modes to match the rest (#446)

* Merge master into teal7 (#450)

* AVM 7:  Address integration branch feedback (#452)

* Add Execute Method (#444)

* adding execute method to allow omission of begin/submit for common use case

* exec docstring

* update testcase

Co-authored-by: Hang Su <hang.su@algorand.com>

* Merge branch 'master' into teal7 (#463)

* fix misspelling of uint (#431)

* fix misspelling of uint

* Clarify minimum Python version management docs (#435)

* Foreign prefix on App and Asset arrays (#440)

* replacing foreignapps with applications

* fix assets as well

* Add Execute Method (#444)

* adding execute method to allow omission of begin/submit for common use case

* exec docstring

* update testcase

Co-authored-by: Hang Su <hang.su@algorand.com>

Co-authored-by: Ben Guidarelli <ben.guidarelli@gmail.com>
Co-authored-by: Hang Su <hang.su@algorand.com>

* Consolidate TEAL and AVM versioning (#441)

* fix misspelling of uint (#431)

* fix misspelling of uint

* Clarify minimum Python version management docs (#435)

* Convert TEAL version references to program version by hand

* Replace `teal#Options` with `avm#Options`

* Deprecate `*_TEAL_VERSION` in favor of `*_PROGRAM_VERSION`

* Fix docs typo

Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>

* Minor `versions.rst` changes

* Fix `verifyTealVersion` in new opcode files

* Fix linter errors

* Fix language discrepencies introduced by the merge

* Remove incorrect avm replacement

* Fix inconsistent language introduced by merge

Co-authored-by: Ben Guidarelli <ben.guidarelli@gmail.com>
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>

* max program version

Co-authored-by: Ben Guidarelli <ben.guidarelli@gmail.com>
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
Co-authored-by: Jacob Daitzman <jdtzmn@gmail.com>

* change according to https://github.com/algorand/go-algorand/pull/4323/files (#488)

* Changes to avm8 docs (#546)

* Support new AVM 8 account parameters (#555)

* CHANGELOG.md

* Frame Ops to `avm8` branch PR (#585)

* add frame ops to avm8 branch

* specify FRAME_POINTER_VERSION in frame-op branch

* per review comments

* per review comments

* per review comments, depth -> frame_depth

* take bury out

* pop popn

* unexport use of frame ops

* hide FRAME_POINTER_VERISON

* CHANGELOG 0.20.0

Co-authored-by: Ben Guidarelli <ben.guidarelli@gmail.com>
Co-authored-by: Jacob Daitzman <jdtzmn@gmail.com>
Co-authored-by: Hang Su <hang.su@algorand.com>
Co-authored-by: Hang Su <87964331+ahangsu@users.noreply.github.com>
Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>
Co-authored-by: Jason Paulos <jasonpaulos@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonpaulos jasonpaulos jasonpaulos approved these changes

@tzaffi tzaffi tzaffi approved these changes

@michaeldiamant michaeldiamant Awaiting requested review from michaeldiamant

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ahangsu @tzaffi @jasonpaulos