Refactor Dry Run Execution #44
Conversation
| @@ -260,1287 +274,1269 @@ def final_as_row(self) -> dict: | |||
| } | |||
|
|
|||
|
|
|||
| class DryRunEncoder: | |||
| """Encoding utilities for dry run executions and results""" | |||
| class DryRunInspector: | |||
There was a problem hiding this comment.
moving class up which causes a big diff
There was a problem hiding this comment.
@tzaffi For future PRs - As a reviewer, I prefer to isolate source code moves, particularly when the PR otherwise makes extensive changes. As presented, it complicates the review process because I feel I can't rely on the diff.
| Local Delta: | ||
| [] | ||
| =============== | ||
| TXN AS ROW: {' Run': 0, ' budget_added': None, ' budget_consumed': None, ' cost': None, ' last_log': '`None', ' final_message': 'PASS', ' Status': 'PASS', 'steps': 10, ' top_of_stack': 4, 'max_stack_height': 2, 's@000': 2, 'Arg_00': 2} |
There was a problem hiding this comment.
Real Diffs: budget_added + budget_consumed (None because this is a logic sig)
graviton/blackbox.py
Outdated
| TODO: eliminate type-switch on inputs using | ||
| functools: singledispatch + partial |
There was a problem hiding this comment.
@tzaffi Sanity-checking PR scope: Is the highlighted TODO intended to be addressed in-scope to the PR?
Based on the related issue, it seems like yes. Though it might be the case that you're looking to hold off due to the size of changes already involved.
There was a problem hiding this comment.
This is more of "a wish" than a todo. After writing this I decided internally not to pursue for now. But I forgot to remove the TODO. I'll remove it
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
Note for reviewers.
The diff in
graviton_blackbox.pyis difficult to read through. Instead of looking at the diff, I encourage looking at the actual code. Additionally, this is a testing library, and therefore (IMO) we can gain most of the confidence by looking at the resulting tests both here and in the companion pyteal PR.Issues Addressed
report()#38DryRunExecutor#40Summary of changes
graviton/blackbox.py- this one's hard to grok because I reordered the classes. In addition:class DryRunTransactionParamsfor better grouping of some variablesclass DryRunExecutor:dryrun_*methods and consolidated functionality as follows:DryRunExecutorobject and callrun()on it. Convenience methodsrun_one()andrun_sequence()are also provided, and these have better type guarantees.*_test.pyandquadratic_factoring_game.ipynb- Migrating all the tests to userun_one()andrun_sequence()methods (except for the usage ofAbiContractExecutor.dry_run_on_sequence()which remains unaffected)TODO's