Skip to content

Conversation

@tzaffi
Copy link
Contributor

@tzaffi tzaffi commented Nov 15, 2022

Summary of changes

Use Graviton's new capability (Graviton #36) for asserting that two separate teal programs behave exactly the same relative a set of invariants to test v8 versions (with frame pointers) vs. v6 (without).

Graviton Dependency

barnjamin and others added 30 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>
Co-authored-by: Hang Su <87964331+ahangsu@users.noreply.github.com>
abi_argument_types=self.abi_argument_types(),
abi_return_type=self.abi_return_type(),
abi_method_signature=self.abi_method_signature(),
omit_method_selector=True,
Copy link
Contributor Author

@tzaffi tzaffi Dec 27, 2022

Choose a reason for hiding this comment

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

The parameters abi_argument_types and abi_return_type are removed from graviton's DryRunExecutor in favor of abi_method_signature

case_name = subr.name()
print(f"stable TEAL generation test for {case_name} in mode {mode}")

# HANG NOTE: I prefer not to modify this test, for it is skipped now on thread-unsafe behavior,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if this test now passes under frame pointers? But I haven't yet tried.

# since only a single input, just assert a constant in each case
"assertions": {
DRProp.cost: 11,
DRProp.cost: lambda _, actual: actual in {11, 12},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change (and other similar) is to accommodate running both v6 and v8 (frame pointers) which result in slight differences.

oldfac: {
"inputs": [(i,) for i in range(25)],
"assertions": {
DRProp.cost: lambda args, actual: (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In several cases, I completely removed assertions. I'm confident that we're good-to-go though because of the new "identities" tests.

Invariant.full_validation(
APP_IDENTICAL_PREDICATES,
inspectors=inspectors6,
identities=inspectors8,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here are new "identities" tests in a action.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See also lines:

  • 462
  • 467
  • 559
  • 609
  • 615
  • 782
  • 793
  • 904
  • 910
  • 960
  • 966
  • 1091
  • 1102



@dataclass(frozen=True)
class _MatchMode(Generic[Output]):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI: #628 completely eliminates _MatchMode


self.traces: list = []

def add_trace(self, trace: Any) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is removed in #628

:language: python

As shown in this exmaple, the logic of smart contract is expressed using PyTeal expressions constructed in Python. PyTeal overloads Python's arithmetic operators
As shown in this example, the logic of smart contract is expressed using PyTeal expressions constructed in Python. PyTeal overloads Python's arithmetic operators
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bad rebase causing this already existing change to appear as changing

Copy link
Contributor

@michaeldiamant michaeldiamant left a comment

Choose a reason for hiding this comment

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

@tzaffi Looks ready to me - I primarily focused my review on tests/integration/graviton_test.py. The remaining enabled assertions seem reasonable.

I can imagine wanting to split assertions by compiled version because there are significant differences between <= v7 and >= v8. However, it's not worth holding the PR.

@tzaffi
Copy link
Contributor Author

tzaffi commented Jan 11, 2023

@tzaffi Looks ready to me - I primarily focused my review on tests/integration/graviton_test.py. The remaining enabled assertions seem reasonable.

I can imagine wanting to split assertions by compiled version because there are significant differences between <= v7 and >= v8. However, it's not worth holding the PR.

I'm going to merge but I do want to follow up and understand this concern a bit better and possibly create a new issue.

@tzaffi tzaffi merged commit dc6466a into master Jan 11, 2023
@tzaffi tzaffi deleted the graviton-identical-subroutines branch January 11, 2023 16:04
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.

8 participants