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.

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

ABI Data Schema #584

Merged
merged 68 commits into from
Nov 14, 2022
Merged

ABI Data Schema #584

merged 68 commits into from
Nov 14, 2022

Conversation

ahangsu
Copy link
Contributor

@ahangsu ahangsu commented Oct 25, 2022

After discussion with @jasonpaulos in 2022/10/24, we decide to provide a set of data retrieval interface for ABI values, not just scratch slots. This will hopefully decrease (or eliminate) the ABI scratch slot use in subroutine.

Based on #562 for now. #562 is based on this one.

barnjamin and others added 23 commits June 24, 2022 10:05
* adding program page related ops
* 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

# Conflicts:
#	pyteal/ir/ops.py

* Disable flake8 errors on formatted lines

* Add past version failure check

* Remove unnecessary ignore Expr equality context
* Add JsonRef

* Use named class methods to specify value type

* Remove unnecessary ignore Expr equality context

* Fix docstring link
* Add Base64Decode

* Remove unnecessary ignore Expr equality context
* 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

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

* Tidy with `MultiValue`’s compile check
* Add sha3_256

* Add crypto docs
* Add first valid time factory and update min version

* Include FirstValidTime in txn tests

* Add transaction field docs
* 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>
* 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>
* 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>
@ahangsu ahangsu marked this pull request as draft October 25, 2022 21:04
@ahangsu ahangsu force-pushed the abi-value-data-schema branch 2 times, most recently from 2a688b2 to 65e4a95 Compare October 25, 2022 21:31
ahangsu and others added 3 commits November 7, 2022 15:07
* memory layout structure in frame var and frame related ops

* validate type

* minor, error msg

* testing for memory layout, need test frame var

* kwarg/posarg

* simpler condition in check frame mem layout args

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

* Better type check in `__getitem__` return var

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

* vanilla flavored class decl

* take away output index

* good testcase for succinct repr

* better assert for type checking

* better error checking in proto, inherit layouts from expr not seq

* tie for local type segment invalid none type

* take out confusing algorithm

* better tie error msg check

* super

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

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

The actual changes seem pretty straightforward, but I do have a few questions about specifics.

By the way, my preference is to first merge this into master, then merge #562 into master, if that makes sense. That way we can keep changesets small.

pyteal/ast/abstractvar.py Outdated Show resolved Hide resolved
pyteal/ast/abi/type.py Outdated Show resolved Hide resolved
pyteal/ast/abi/type.py Outdated Show resolved Hide resolved
pyteal/ast/subroutine.py Outdated Show resolved Hide resolved
pyteal/ast/subroutine.py Show resolved Hide resolved
)
if mem_layout:
if mem_layout.num_return_allocs > num_returns:
Copy link
Member

Choose a reason for hiding this comment

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

Why is it ok for these to be unequal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

num_return_allocs are applied for ABI return value, while num_returns in proto only matters about how many remaining things on the stack. So not all return on stack are ABI values, num_return_allocs <= num_returns.

Copy link
Member

Choose a reason for hiding this comment

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

I think based on previous discussion we concluded that ABI return values can be placed on the stack. So should the condition here be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we are fine with return value to be in local variables, then the condition should be fine without changing?

Copy link
Member

Choose a reason for hiding this comment

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

The right thing to do here depends on what happens in #562, so I'm fine with merging this as it is currently and then revisiting.

pyteal/ast/frame.py Outdated Show resolved Hide resolved
pyteal/ast/frame.py Show resolved Hide resolved
pyteal/ast/frame.py Show resolved Hide resolved
pyteal/ast/frame.py Outdated Show resolved Hide resolved
pyteal/ast/frame.py Outdated Show resolved Hide resolved
Copy link
Member

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for clarifying and taking suggestions

@ahangsu ahangsu merged commit 2d65bb6 into master Nov 14, 2022
@ahangsu ahangsu deleted the abi-value-data-schema branch November 14, 2022 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants