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

Graviton 4 abi router #634

Merged
merged 268 commits into from
Feb 9, 2023
Merged

Graviton 4 abi router #634

merged 268 commits into from
Feb 9, 2023

Conversation

tzaffi
Copy link
Contributor

@tzaffi tzaffi commented Jan 17, 2023 •
edited
Loading

Graviton sibling PR:

algorand/graviton#49

Issues Addressed

#394

TODOS

  • incorporate the “negative” tests. I.e., instead of giving the router expected inputs and verifying that it doesn’t fail, give it bad inputs and verify that it does fail. This is the analog of this graviton test
  • Have a test that asserts that router v6 and router v8 behave exactly the same way. This will be done by using the identities parameter of Invariant.full_validation()
  • Handle / adapt / test the recenlty modified clear state router functionality
  • Figure out what to do with sim_*.teal fixtures - ALL DELETED
  • How about adding a non-trivial clear program example?
  • Has graviton v0.9.0 been released? YES

ahangsu and others added 30 commits August 15, 2022 15:56
@ahangsu
Merge branch 'master' into avm8
2c2b7b2
@ahangsu
Merge branch 'master' into avm8
61af648
@ahangsu
Merge branch 'master' into avm8
5282726
@jasonpaulos
Changes to avm8 docs (#546)
76010b2
@ahangsu
Merge branch 'master' into avm8
d767c66
@jdtzmn
Support new AVM 8 account parameters (#555)
cc49a55
@ahangsu
add frame ops to avm8 branch
1d7cd6a
@ahangsu
specify FRAME_POINTER_VERSION in frame-op branch
49d169e
@ahangsu
just the work of abi data schema
02f3775
@ahangsu
squashed
e8c5d2c
@ahangsu
change in round trip test, stop something blocking me
a044f80
@ahangsu
update frame dig and proto use
6739c00
@ahangsu
fix var n loaded
ce9aec8
@ahangsu
documentation
620c61e
@ahangsu
narrower name, module name
87d97b1
@ahangsu
Merge branch 'abi-value-data-schema' into feature/fp-subroutine
80a40d8
@ahangsu
reformat evaluator code, less indentation
208be80
@ahangsu
minor, found a bug on output setting in subroutine in frame pointer c…
d3fd16d 
…ompile
@ahangsu
one small step in testing, a big step for frame pointer!
2b4bb17
@ahangsu
work in black box testing
411f007
@ahangsu
0xBAD hack, need to revisit for ugly pass by for type system
a70c125
@ahangsu
move spot reserving in proto
1e12ca0
@ahangsu
reformat code a bit on black box example part
a5cd38a
@ahangsu
start changing abi roundtrip tesal file name
7db5dfa
@ahangsu
roundtrip test
e1dbcd7
@ahangsu
comments
ac98d94
@ahangsu
still wristing with docker, revert space reserving stuffs
cf0c331
@ahangsu
minor, pure compilation more flexible
769419b
@ahangsu
roundtrip dryrun flexibility
d77b442
@ahangsu
conditional factorial done, need int65 and complex130
20c0ef8
ahangsu
ahangsu reviewed Feb 7, 2023
View reviewed changes
tests/blackbox.py Outdated Show resolved Hide resolved
ahangsu
ahangsu reviewed Feb 7, 2023
View reviewed changes
tests/blackbox.py Outdated Show resolved Hide resolved
ahangsu
ahangsu reviewed Feb 7, 2023
View reviewed changes
tests/blackbox.py Outdated Show resolved Hide resolved
Zeph Grunschlag added 2 commits February 7, 2023 12:17
ahangsu
ahangsu reviewed Feb 7, 2023
View reviewed changes
tests/blackbox.py Outdated Show resolved Hide resolved
tests/blackbox.py Outdated Show resolved Hide resolved
pyteal/ast/router.py Outdated Show resolved Hide resolved
tests/blackbox.py Outdated Show resolved Hide resolved
tests/blackbox.py Outdated Show resolved Hide resolved
tests/blackbox.py Outdated Show resolved Hide resolved
tests/blackbox.py Outdated Show resolved Hide resolved
ahangsu
ahangsu reviewed Feb 7, 2023
View reviewed changes
tests/blackbox.py Outdated Show resolved Hide resolved
ahangsu
ahangsu reviewed Feb 7, 2023
View reviewed changes
Copy link
Contributor

@ahangsu ahangsu left a comment

Choose a reason for hiding this comment

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

I am still walking back and forth on blackbox.py for a while, but completing it should be soon.

tests/blackbox.py Show resolved Hide resolved
@tzaffi
Copy link
Contributor Author

tzaffi commented Feb 7, 2023 via email

ahangsu
ahangsu reviewed Feb 8, 2023
View reviewed changes
Copy link
Contributor

@ahangsu ahangsu 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.

I roughly finished my pass on blackbox.py, starting pass on abi_router_test.py.

tests/blackbox.py Outdated Show resolved Hide resolved
tests/blackbox.py Outdated Show resolved Hide resolved
tests/blackbox.py Show resolved Hide resolved
tests/blackbox.py Outdated Show resolved Hide resolved
tzaffi
tzaffi commented Feb 8, 2023
View reviewed changes
tests/blackbox.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Show resolved Hide resolved
tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
ahangsu
ahangsu reviewed Feb 8, 2023
View reviewed changes
Copy link
Contributor

@ahangsu ahangsu left a comment

Choose a reason for hiding this comment

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

negative testcases looks good, so I am almost leaning towards approving it.

tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Show resolved Hide resolved
pyteal/compiler/compiler_test.py Show resolved Hide resolved
pyteal/compiler/compiler_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Outdated Show resolved Hide resolved
tests/integration/abi_router_test.py Show resolved Hide resolved
ahangsu
ahangsu reviewed Feb 9, 2023
View reviewed changes
Copy link
Contributor

@ahangsu ahangsu left a comment

Choose a reason for hiding this comment

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

Let's take in as is, and hopefully we have some bandwidth for polishing it. But impressive work on getting router tested in pyteal not graviton!

@tzaffi tzaffi merged commit e153eae into feature/fp-router Feb 9, 2023
@tzaffi tzaffi deleted the graviton-4-abi-router branch February 9, 2023 18:56
@tzaffi tzaffi mentioned this pull request Feb 9, 2023
Closed
ahangsu added a commit that referenced this pull request Feb 23, 2023
@ahangsu @tzaffi
Frame Pointer usage in router (#600)
134f434 
* select related files

* or none...

* changelog

* compiler test

* const

* adding comments

* minor

* Update pyteal/ast/router.py

Co-authored-by: Zeph Grunschlag <[email protected]>

* renaming i to something meaningful

* build program API change

* remove old comments

* or none

Co-authored-by: Zeph Grunschlag <[email protected]>

* taking with minor mods

* renaming and comments

* function bundle things

* type casting

* test _failing_ on master

* wip

* unit test done

* stateful bug

* lint

* is this the bug that's been bugging me for almost a year?

* Update pyteal/ast/router.py

* does finally finally fix the bug?

* lint

* i think this works...

* 🤞

* lint and re-sort imports

* revert

* Update pyteal/ast/router_test.py

* oops - remove integration tests that snuck into unit tests

* hide build program method in router

* fix doc error

* Zeph's sick move

* Refactoring non-idempotent fix with contextmanager a bit (#649)

* minor, refactoring a bit

* renaming

* remove comments

* add 3rd case to test_build_program_clear_state_invalid_config

* Update pyteal/ast/router.py

Co-authored-by: Hang Su <[email protected]>

* minor refactoring

* bad merge

* minor, index_start_from

* documentation

* doc for de abify subroutine frame pointer

* doc for de abify subroutine vanilla

* Small tweaks of Router.bare_calls (#659)

* Graviton 4 abi router (#634)

* Tightening up types in 2 subroutine.py invokations (#672)

* Update pyteal/ast/router.py

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* default optimize object

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update router comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* default `wrap_to_name`

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* minor

* better yield pattern in context manager

Co-authored-by: Zeph Grunschlag <[email protected]>

* Merge master fp router (#678)

---------

Co-authored-by: Zeph Grunschlag <[email protected]>
Co-authored-by: Zeph Grunschlag <[email protected]>
bbroder-algo pushed a commit that referenced this pull request Mar 17, 2023
@tzaffi @ahangsu
Better Router Mapping (#689)
871e580 
* dupn 1 -> dup (and misc)

* const

* adding comments

* fix off-by-one-error

* passing CI 🤞

* line number assertion off-by-1

* @pytest.mark.serial

* minor

* Update pyteal/ast/router.py

Co-authored-by: Zeph Grunschlag <[email protected]>

* renaming i to something meaningful

* build program API change

* remove old comments

* or none

Co-authored-by: Zeph Grunschlag <[email protected]>

* taking with minor mods

* renaming and comments

* function bundle things

* type casting

* test _failing_ on master

* wip

* unit test done

* stateful bug

* lint

* is this the bug that's been bugging me for almost a year?

* Update pyteal/ast/router.py

* does finally finally fix the bug?

* lint

* i think this works...

* 🤞

* lint and re-sort imports

* revert

* Update pyteal/ast/router_test.py

* oops - remove integration tests that snuck into unit tests

* revive skipped tests in tests/unit/sourcemap_monkey_unit_test.py

* manual merge

* Update .gitignore

* reorder

* wip

* small improvments e.g. better PC column output

* fine grained control over sync/asyn of unit and integration tests

* finally all merged

* threading the synchronous tesst needle

* don't need notebooks

* remove mistakenly checked in files

* Update .flake8

* sort lines

* hide build program method in router

* fix doc error

* Zeph's sick move

* enforce "_root_expr" for members and "root_expr" everywhere else

* Refactoring non-idempotent fix with contextmanager a bit (#649)

* minor, refactoring a bit

* renaming

* bring in `test_constructs()` passing in `source-mapper-that-works`

* back to square 1.5

* actions -> as_list

* deleting obsolete test

* mostly just formatting

* remove comments

* add 3rd case to test_build_program_clear_state_invalid_config

* Update pyteal/ast/router.py

Co-authored-by: Hang Su <[email protected]>

* minor refactoring

* small changes to monkey unit test

* passing tests hopefully (at least on 3.11)

* Update pyteal/ast/router.py

Co-authored-by: Hang Su <[email protected]>

* Apply suggestions from code review

* should pass the docs build -even if haven't properly updated them

* Update pyteal/stack_frame_test.py

* unskip the weak idempotence reliant tests

* begone `source_inference` and `hybrid_source` params

* begone `source_inference` and `hybrid_source` params

* refactor part 1) CompileResults with safe data

* refactor part 2) RouterResults with safe data

* move SourceMapDisabledError up into errors.py

* side-effectless infer method

* small cleanup

* unify source mapper outputs in `PyTealSourceMap` type

* make `Compilation.compile()` kw-only

* small cleanup/fixes

* remove comment

* comment StackFrames + reorder params + reformat

* more comments + remove unused method + reduce the chance of a false positive in _frame_info_is_right_before_core

* remove StackFrames' `keep_one_frame_only` param and privatize the `_keep_all` param

* StackFrames ought not be subscriptable

* remove unused StackFrames.frame_infos method

* Privatizing StackFrames._frames

* StackFrames becomes NatalStackFrame

* pass the unit test

* commentary

* remove unused const _PYTEAL_FRAME

* R3SourceMap commentary

* fixture for source map integration tests

* non-controversial changes

* rename to what the test expects

* commentary

* Simplify NatalStackFrame's constructor while maintaining green tets

* hard code the logic and set last_drop_idx=1, but introduce unit test_frame_info_is_right_before_core_last_drop_idx to guard against regressions

* fix unit test now that _keep_all init param is gone

* commentary

* commentary

* make sourcemap-coverage

* EOL

* BareCallActions.asdict|list

* algokit "ruff"s up sourcemap.py

* bad merge

* removing ignoreExprEquality context

* minor, index_start_from

* remove dupe test_bare_call_actions_asdict

* documentation

* doc for de abify subroutine frame pointer

* doc for de abify subroutine frame pointer

* doc for de abify subroutine vanilla

* explanatory coment

* lint

* unshadow `str`

* Small tweaks of Router.bare_calls (#659)

* commentary

* first stab at issue #658

* _root_expr members renamed to more accurate _sframes_container

* remove obsolete comment

* Graviton 4 abi router (#634)

* try nightly

* typo

* try gin

* and agin

* tg

* default `annotate_teal=False` in `compile()`

* just look for startswith("def") instead of decorators AST property

* fix and test hybrid_w_offset

* mokey patched tests should be run serially

* temp todos

* fix MethodReturn

* no more BRUTE_FORCE_TERRIBLE_SKIP + improved interpretation of loading on stack before subroutine

* pass the tests

* remove unneeded duplicates pyteal printouts in annotate()

* Update pyteal/ast/router_test.py

* better import grouping

* working on nightly in separate branch

* Update pyteal/stack_frame.py

* comment per CR suggestion

* remove a TODO

* per CR suggestion, add tests that assert no regressions against algobank fixtures

* RPS fixture test to assert no regressions

* unit tests with RPS; next up, integration tests

* default Compilation.compile() to not calc sourcemap

* rps integration test

* router improvements + break out slow test

* lint

* register the custom pytest mark "slow"

* bugfix, but don't fix the test cases just yet...

* Tightening up types in 2 subroutine.py invokations (#672)

* Update pyteal/ast/router.py

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* default optimize object

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update router comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* default `wrap_to_name`

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* update comments and docs

Co-authored-by: Zeph Grunschlag <[email protected]>

* minor

* 80% of the way towards complete router mapping

* pass the tests

* 85% of the way

* 90%

* 92%

* passing tests locally

* display only the mypy ERRORS, not the warnings

* ci friendly grep

* revert grep

* wip

* passing lint

* sort imports

* get closer to passing unit tests

* pass more tests

* fix test_r3sourcemap

* improved RPS

* skip flaky sub-test

* begone Method._sframes_container

* lint

* shd pass unit tests in CI

* more tests skipped due to unstable scratch slots

* don't need root_expr in Break()

* don't need root_expr in Continue or TealSimpleBlock

* EnumInt.clone wasn't needed

* revert

* cut NatalStackFrame._compiler_gen_DEPRECATED and follow through

* improve test legibility

* improve test legibility

* remove unused pathway for detecting import

* lint

* unused var

* simplify with instance method

* clean up

* compress and comment

* apply review suggestions and fix a couple more typos

* per CR nudge - clean up _walk_asts logic

* Update pyteal/ast/router.py

* Update pyteal/stack_frame.py

* Update pyteal/stack_frame.py

* revert _debug_asts()

* unify RPS so easier to run directly

* Update .flake8

---------

Co-authored-by: Hang Su <[email protected]>
Co-authored-by: Hang Su <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahangsu ahangsu ahangsu approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@tzaffi @ahangsu @jasonpaulos @jdtzmn @michaeldiamant @lvorithm