-
Notifications
You must be signed in to change notification settings - Fork 132
Test identical functionality across versions #605
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
Conversation
* 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 first valid time factory and update min version * Include FirstValidTime in txn tests * Add transaction field docs
* 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, |
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.
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, |
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 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}, |
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.
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: ( |
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.
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, |
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.
Here are new "identities" tests in a action.
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.
See also lines:
- 462
- 467
- 559
- 609
- 615
- 782
- 793
- 904
- 910
- 960
- 966
- 1091
- 1102
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class _MatchMode(Generic[Output]): |
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.
FYI: #628 completely eliminates _MatchMode
|
|
||
| self.traces: list = [] | ||
|
|
||
| def add_trace(self, trace: Any) -> None: |
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.
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 |
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.
bad rebase causing this already existing change to appear as changing
michaeldiamant
left a comment
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.
@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. |
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
v0.7.0Shall we merge in the following first?Refactor graviton usage #628 into this PRGraviton #44