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.

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

Refactor DryRunExecutor #40

Closed
13 of 14 tasks
tzaffi opened this issue Dec 7, 2022 · 1 comment
Closed
13 of 14 tasks

Refactor DryRunExecutor #40

tzaffi opened this issue Dec 7, 2022 · 1 comment
Assignees

Comments

@tzaffi
Copy link
Contributor

tzaffi commented Dec 7, 2022

Problems Summary

Too many executing methods

See below for a list of DryRunExecutor methods that allow executing dryruns. It is cumbersome and confusing to have so many methods that basically do the same thing. Ideally, it would be nice to have a single method with the following API:

class DryRunExecutor:
    @singledispatch
    @classmethod
    def execute(
            cls: self,
            algod: AlgodClient,
            program: str,
            input: Any,
            *, 
            mode: ExecutionMode = ExecutionMode.Application,
            abi_method_signature: Optional[str] = None,
            ... several more params such as sender, sp, is_app_create, accounts ...
        ):
            raise NotImplementedError("Implement execute method")

This makes use of functools singledispatch decorator

Inconsistent abi-type handling

Currently, graviton's dry run execution are inconsistent in how abi-information is handled. Some allow abi_argument_types and abi_return_types to be provided while some also allow abi_method_signature to be provided. The last parameter includes all the information that abi_argument_types and abi_return_types provides and therefore it is confusing to provide all the parameters.

After an investigation into the usage of dry run execution in PyTeal, it became apparent that in all relevant situations, an ABIReturnSubroutine object is available for inspection, and it comes with a method method_signature() which could be used to populate an abi_method_signature parameter for dry run execution.

Action Items

Streamline the dry run execution API by refactoring the dry run executing methods as follows:

  • remove parameter abi_argument_types
  • remove parameter abi_return_type
  • add parameter abi_method_signature
  • make all the current dry run executing methods private these have been deleted
  • unify into a single execute() function using @singledispatch and @process.register to dispatch via input's type (Further investigation is required here. It may not be possible given the nesting of possible inputs (list[tuple] ... etc .... We may also prefer a multipledispatch approach taking into account the mode parameter's type. It may also be infeasible to dispatch directly and therefore we might need to match directly and delegate.) This didn't totally work out. However, we've unified everything into a run() method, with 2 companion convenience methods run_one() and run_sequence()
  • prepare a PyTeal PR which adapts to this new API (Refactor graviton usage pyteal#628)
  • refactor supress_abi (not done)

List of dry run executing methods for refactoring

  • DryRunExecutor.execute_one_dryrun()
  • DryRunExecutor.dryrun_logicsig()
  • DryRunExecutor.dryrun_app()
  • DryRunExecutor.dryrun_logicsig_on_sequence()
  • DryRunExecutor.dryrun_multiapps_on_sequence()
  • DryRunExecutor.dryrun_app_pair_on_sequence()
  • ABIContractExecutor.dryrun_app_on_sequence()
@tzaffi
Copy link
Contributor Author

tzaffi commented Jan 19, 2023

resolving thanks to #45

@tzaffi tzaffi closed this as completed Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants