Skip to content

Commit

Permalink
Better Router Mapping (#689)
Browse files Browse the repository at this point in the history 
* 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 <tzaffi@users.noreply.github.com>

* renaming i to something meaningful

* build program API change

* remove old comments

* or none

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

* 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 <87964331+ahangsu@users.noreply.github.com>

* 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 <87964331+ahangsu@users.noreply.github.com>

* 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 <tzaffi@users.noreply.github.com>

* update comments

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

* default optimize object

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

* update comments and docs

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

* update comments and docs

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

* update router comments

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

* default `wrap_to_name`

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

* update comments

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

* update comments

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

* update comments

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

* update comments

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

* update comments

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

* update comments

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

* update comments

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

* update comments and docs

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

* update comments and docs

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

* update comments and docs

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

* update comments and docs

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

* update comments and docs

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

* update comments and docs

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

* update comments and docs

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

* 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 <hang.su@algorand.com>
Co-authored-by: Hang Su <87964331+ahangsu@users.noreply.github.com>
  • Loading branch information
@tzaffi @ahangsu
3 people committed Mar 17, 2023
1 parent 402c6ed commit 871e580
Show file tree
Hide file tree
Showing 21 changed files with 1,859 additions and 1,136 deletions.
3 changes: 1 addition & 2 deletions .flake8
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ per-file-ignores =
pyteal/ir/tealconditionalblock.py: F821
pyteal/ir/tealsimpleblock.py: F821
pyteal/stack_frame.py: F821
tests/teal/rps_helpers/program.py: F401, F403, F405
tests/teal/rps.py: F403, F405, I252
tests/teal/rps.py: F401, F403, F405, I252
tests/unit/module_test.py: F401, F403

# from flake8-tidy-imports
Expand Down
3 changes: 1 addition & 2 deletions pyteal/ast/abi/method_return.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ def __init__(self, arg: BaseType):
if not isinstance(arg, BaseType):
raise TealInputError(f"Expecting an ABI type argument but get {arg}")
self.arg = arg
self._sframes_container: Expr | None = None

def __teal__(self, options: "CompileOptions") -> Tuple[TealBlock, TealSimpleBlock]:
if options.version < Op.log.min_version:
Expand All @@ -30,7 +29,7 @@ def __teal__(self, options: "CompileOptions") -> Tuple[TealBlock, TealSimpleBloc
start, end = Log(Concat(Bytes(RETURN_HASH_PREFIX), self.arg.encode())).__teal__(
options
)
NatalStackFrame.reframe_ops_in_blocks(self._sframes_container or self, start)
NatalStackFrame.reframe_ops_in_blocks(self, start)
return start, end

def __str__(self) -> str:
Expand Down
197 changes: 141 additions & 56 deletions pyteal/ast/router.py
View file

Large diffs are not rendered by default.

36 changes: 24 additions & 12 deletions pyteal/ast/router_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,17 @@ def test_bare_call_config_clear_state_failure():
)


def equal_ocas(oca1, oca2):
assert oca1.call_config == oca2.call_config
if oca1.action is None:
assert oca2.action is None
else:
assert isinstance(oca1.action, pt.Int)
assert isinstance(oca2.action, pt.Int)

return True


def test_BareCallActions_asdict():
no_action = pt.OnCompleteAction()
del_action = pt.OnCompleteAction(action=pt.Int(1), call_config=pt.CallConfig.ALL)
Expand All @@ -392,16 +403,7 @@ def test_BareCallActions_asdict():
close_out=close_action,
)

bcad = bca.asdict()
assert set(bcad.keys()) == {
"clear_state",
"close_out",
"delete_application",
"no_op",
"opt_in",
"update_application",
}
assert bcad == {
expected = {
"clear_state": no_action,
"close_out": close_action,
"delete_application": del_action,
Expand All @@ -410,6 +412,13 @@ def test_BareCallActions_asdict():
"update_application": no_action,
}

bcad = bca.asdict()
assert set(bcad.keys()) == set(expected.keys())

for k, oca1 in bcad.items():
oca2 = expected[k]
equal_ocas(oca1, oca2)


def test_BareCallActions_aslist():
no_action = pt.OnCompleteAction()
Expand All @@ -423,15 +432,18 @@ def test_BareCallActions_aslist():
opt_in=optin_action,
)

bcal = bca.aslist()
assert bcal == [
expected = [
no_action,
no_action,
no_action,
no_action,
optin_action,
update_action,
]
bcal = bca.aslist()
assert len(expected) == len(bcal)

assert all([equal_ocas(expected[i], actual) for i, actual in enumerate(bcal)])


def test_BareCallActions_get_method_config():
Expand Down
6 changes: 3 additions & 3 deletions pyteal/ast/subroutine.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1030,9 +1030,7 @@ def __proto(subroutine: SubroutineDefinition) -> Proto:
else int(subroutine.return_type != TealType.none)
)

proto = Proto(subroutine.argument_count(), num_stack_outputs, mem_layout=layout)
NatalStackFrame.mark_asts_as_compiler_gen(proto)
return proto
return Proto(subroutine.argument_count(), num_stack_outputs, mem_layout=layout)

def evaluate(self, subroutine: SubroutineDefinition) -> SubroutineDeclaration:
proto = self.__proto(subroutine)
Expand Down Expand Up @@ -1111,6 +1109,8 @@ def evaluate(self, subroutine: SubroutineDefinition) -> SubroutineDeclaration:
for var, index in arg_var_n_frame_index_pairs[::-1]
]

# don't reframe the subroutine body
subroutine.stack_frames.reframe(*body_ops)
body_ops.append(subroutine_body)
sd = SubroutineDeclaration(subroutine, Seq(body_ops), deferred_expr)
sd.trace = subroutine_body.trace
Expand Down
2 changes: 1 addition & 1 deletion pyteal/compiler/compiler.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ def compile(
teal_filename (optional): The filename to use in the sourcemap. Defaults to `None`.
pcs_in_sourcemap (optional): When `True`, the compiler will include the program counter in
relevant sourcemap artifacts. This requires an `AlgodClient` (see next param). Defaults to `False`.
algod_client (optional): An `AlgodClient` to use to fetch the program counter. Defaults to `None`.
algod_client (optional): An `AlgodClient` to use to fetch program counters. Defaults to `None`.
When `pcs_in_sourcemap` is `True` and `algod_client` is not provided, the compiler will
assume that an Algorand Sandbox algod client is running on the default port (4001) and -if
this is not the case- will raise an exception.
Expand Down
23 changes: 8 additions & 15 deletions pyteal/compiler/sourcemap.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class R3SourceMap:
This class is renames mjpieters' SourceMap
(https://gist.github.com/mjpieters/86b0d152bb51d5f5979346d11005588b#file-sourcemap-py-L62)
and tweaks it a bit, adding the following functionality:
* adds fields `file_liens`, `source_files`, `entries`
* adds fields `file_lines`, `source_files`, `entries`
* __post_init__ (new) - runs a sanity check validation on the ordering of provided entries
* __repr__ - printing out "R3SourceMap(...)" instead of "MJPSourceMap(...)"
* from_json - accepting new params `sources_override`, `sources_content_override`, `target`, `add_right_bounds`
Expand Down Expand Up @@ -588,17 +588,18 @@ class _PyTealSourceMapper:
for each component in components:
# Deduce the "best" frame for the component's PyTeal source (@ Expr creation)
# [1-7] Inside NatalStackFrame.__init__()
# [1-8] Inside NatalStackFrame.__init__()
1. call inspect.stack() to generate a full stack trace
2. filter out "py crud" frames whose filename starts with "<" and don't have a code_context
3. start searching at frame index i = 2 (right before Expr's NatalStackFrame was constructed)
4. fast forward through PyTeal frames until the first non PyTeal frame is found
5. in the case this was in import statement, back up until a known compiler generated line is discovered
6. keep the last frame in the list
7. convert this into a list[StackFrame] of size 1
5. back up looking for a compiler gateway and so signal that the expression was generated by pyteal itself
6. in the case this was in import statement, back up until a known compiler generated line is discovered
7. keep the last frame in the list
8. convert this into a list[StackFrame] of size 1
# [8] Inside _PyTealSourceMapper.build() @ source-map creation:
8. self._best_frames[i] = the singleton StackFrame in 7 converted to PyTealFrame # i == component's index of the component
# [9] Inside _PyTealSourceMapper.build() @ source-map creation:
9. self._best_frames[i] = the singleton StackFrame in 7 converted to PyTealFrame # i == component's index of the component
PASS II. Attempt to fill any "gaps" by inferring from adjacent BFC's
This logic is contained in _PyTealSourceMapper.infer():
Expand All @@ -622,9 +623,6 @@ class _PyTealSourceMapper:
if its reason is one of PT_GENERATED.{TYPE_ENUM_ONCOMPLETE, TYPE_ENUM_TXN, BRANCH_LABEL, BRANCH}:
# these occur when one of the # T2PT* comments is encountered
replace this pyteal_frame by next_pyteal_frame
if its reason is PT_GENERATED.FLAGGED_BY_DEV:
# this occurs when NatalStackFrame._compiler_gen is set True
replace this pyteal_frame by prev_pyteal_frame
if ONLY prev_frame exists:
replace this pyteal_frame by prev_pyteal_frame
Expand Down Expand Up @@ -968,11 +966,6 @@ def infer_source(i: int) -> PyTealFrame | None:
PyTealFrameStatus.PATCHED_BY_NEXT_OVERRIDE_PREV
)

if reason == PT_GENERATED.FLAGGED_BY_DEV:
return prev_frame.clone(
PyTealFrameStatus.PATCHED_BY_PREV_OVERRIDE_NEXT
)

# NO-OP otherwise:
return None

Expand Down

0 comments on commit 871e580

Please sign in to comment.