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

Utility for increasing budget during execution of PyTeal applications #274

Merged
merged 23 commits into from
Apr 26, 2022

Conversation

algoidurovic
Copy link
Contributor

@algoidurovic algoidurovic commented Apr 12, 2022

This PR implements a PyTeal utility for increasing the opcode budget during execution of an app call using inner app call transactions.

Two modes are provided:

  1. Explicit: the app to call for increasing budget is provided by the user.
  2. OnCall: the app to call is created during each request to increase the budget, then deleted afterwards.

Two budget increase methods are provided:

  1. ensure_budget(budget): increase the current opcode budget to at least the budget requested by the user.
  2. maximize_budget(fee): increase the current opcode budget as much as possible without spending more than the given fee. This can be used when the contract developer doesn't want to spend more than the fee provided to increase the budget without needing to do in depth opcode cost analysis on their program. If the fee isn't sufficient for executing the program then a failure may be desired. It can also allow the app caller to specify the maximum fee they want to spend for increasing opcode budget. This behavior can fairly easily be extended to support a global fee limit for budget increase requests. (Inspired by the algofi implementation of opup).

Testing
Unit tests for type checking enforcement and sandbox tests for e2e functionality. The PyTeal programs I used for testing in sandbox are located in examples/application/opup.py

pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
@algoidurovic algoidurovic changed the title WIP: Utility for increasing budget during execution of PyTeal applications Utility for increasing budget during execution of PyTeal applications Apr 14, 2022
@tzaffi
Copy link
Contributor

tzaffi commented Apr 14, 2022

Two budget increase methods are provided:

  1. ensure_budget(budget): increase the current opcode budget to at least the budget requested by the user.
  2. maximize_budget(fee): increase the current opcode budget as much as possible without spending more than the given fee.

I think method 1 is essential, but I'm not as sure about method 2. On the face of it, maximize_budget() gives you an unknown amount of extra budget, so you could run out unexpectedly or get much more than you really need. One could argue that method 1 suffers from the opposite problem - what if fees increase and the app can no longer afford to op-up? But that seems a little easier to ameliorate dynamically, if another method is added fee_for_budget(budget) which would let the user know how much they'll use up and adapt accordingly.

pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
@algoidurovic
Copy link
Contributor Author

Two budget increase methods are provided:

  1. ensure_budget(budget): increase the current opcode budget to at least the budget requested by the user.
  2. maximize_budget(fee): increase the current opcode budget as much as possible without spending more than the given fee.

I think method 1 is essential, but I'm not as sure about method 2. On the face of it, maximize_budget() gives you an unknown amount of extra budget, so you could run out unexpectedly or get much more than you really need. One could argue that method 1 suffers from the opposite problem - what if fees increase and the app can no longer afford to op-up? But that seems a little easier to ameliorate dynamically, if another method is added fee_for_budget(budget) which would let the user know how much they'll use up and adapt accordingly.

I updated the PR description to further elaborate on the use cases for maximize_budget(). I do think it's useful to have a fee_for_budget() method available as well.

pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
pyteal/ast/opup.py Outdated Show resolved Hide resolved
Copy link
Member

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Except for my one minor comment, this looks good to me, thank you for incorporating my feedback.

The one comment is about shrinking the on call program code. I edited the TEAL by hand, and even though it's a simple change I'd still recommend you briefly test it in sandbox again just to make sure there are no unintended consequences.

pyteal/ast/opup.py Outdated Show resolved Hide resolved
algoidurovic and others added 2 commits April 21, 2022 14:56
Co-authored-by: Jason Paulos <jasonpaulos@users.noreply.github.com>
docs/opup.rst Outdated Show resolved Hide resolved
docs/opup.rst Outdated Show resolved Hide resolved
docs/opup.rst Outdated Show resolved Hide resolved
docs/opup.rst Outdated Show resolved Hide resolved
algoidurovic and others added 4 commits April 22, 2022 11:35
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
Copy link
Contributor

@michaeldiamant michaeldiamant left a comment

Choose a reason for hiding this comment

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

@algoidurovic Thanks for your effort here - I'm excited to see the feature used by the community! ☕

I left an optional remark in #274 (comment). I'd appreciate considering it before merging though it need not hold us up.

Copy link
Member

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

The docs look great, I just have some suggestions, only one of which is important (the 700 vs ~700 comment).

docs/opup.rst Outdated Show resolved Hide resolved
docs/opup.rst Outdated Show resolved Hide resolved
docs/opup.rst Outdated Show resolved Hide resolved
docs/opup.rst Outdated Show resolved Hide resolved
docs/opup.rst Show resolved Hide resolved
docs/opup.rst Outdated Show resolved Hide resolved
docs/opup.rst Outdated Show resolved Hide resolved
Copy link
Member

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Looks great, I just have one comment that I missed before


from pyteal import *

from pyteal.ast.opup import OpUp, OpUpMode
Copy link
Member

Choose a reason for hiding this comment

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

This line shouldn't be necessary

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.

just one question about ensure_budget



ON_CALL_APP = Bytes("base16", "068101") # v6 teal program "int 1"
MIN_TXN_FEE = Int(1000)
Copy link
Contributor

Choose a reason for hiding this comment

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

minor proposal, do you think MIN_TXN_FEE should be available in a larger scope, say it lives in pyteal/config.py? I guess some folks would want this available not just in opup.

Copy link
Member

Choose a reason for hiding this comment

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

Global.min_txn_fee() is available to PyTeal, perhaps this code should use that instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

^ sounds pretty reasonable to me

pyteal/ast/opup.py Show resolved Hide resolved
@algoidurovic algoidurovic merged commit a4f42b6 into algorand:master Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants