-
Notifications
You must be signed in to change notification settings - Fork 118
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
ARC-57: Application Action Routing #264
Open
joe-p
wants to merge
7
commits into
algorandfoundation:main
Choose a base branch
from
joe-p:action_routing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2c94180
draft: app action routing
joe-p d7a6556
security section
joe-p cdfbdb7
use ! instead of > for ApplicationID
joe-p a33f374
use pushint; update rationale, motiviation, and client parsing
joe-p 7028e6b
fix typo
joe-p 4f0e858
Assign number + fix format
SudoWeezy 36798c8
use pushints
joe-p File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
--- | ||
arc: 57 | ||
title: Application Action Routing | ||
description: Standardized handling of OnCompletion and ApplicationID actions | ||
author: Joe Polny (@joe-p) | ||
discussions-to: https://github.com/algorandfoundation/ARCs/issues/264 | ||
status: Draft | ||
type: Standards Track | ||
category: ARC | ||
created: 2024-01-02 | ||
--- | ||
|
||
## Abstract | ||
This ARC provides a standard way to handle an application call depending on the combination of the `OnCompletion` and `ApplicationID`. | ||
|
||
## Motivation | ||
App mutability is an important factor in a rational actors decision making when it comes to interacting with an app and sending funds to a contract account. If a contract is mutable, it is possible that funds could be stolen or lost after the initial program is changed or deleted. Currently, there is no easy way for an end-user to determine the mutability of a contract. It is possible to use static analysis on the decompiled TEAL, but this is not easily accessible and unrealistic for wallets and explorers to implement for every program. | ||
|
||
Additionally, there is no standard way for developers to route calls based on their OnCompletion. This can make it hard to determine what is allowed for a specific code path since a check for OnCompletion can be anywhere in the program. The current common practice is to include OnCompletion checks within the methods label/subroutine, but this can lead to a lot of extra bytes in the program. | ||
|
||
## Specification | ||
|
||
### TEAL | ||
The TEAL for an application **MUST** start with the following lines: | ||
|
||
``` | ||
txn ApplicationID | ||
! | ||
pushints 6 | ||
* | ||
txn OnCompletion | ||
+ | ||
switch call_NoOp call_OptIn call_CloseOut NOT_IMPLEMENTED call_UpdateApplication call_DeleteApplication create_NoOp create_OptIn NOT_IMPLEMENTED NOT_IMPLEMENTED NOT_IMPLEMENTED create_DeleteApplication | ||
NOT_IMPLEMENTED: | ||
err | ||
``` | ||
|
||
For every action (combination of `ApplicationID === 0` and `OnCompletion`) there is a label that corresponds to that action. If that action is not implemented in the contract, its label should be replaced by a label that simply contains `err`. In the above TEAL, this is the `NOT_IMPLEMENTED` label. | ||
|
||
The label containing the `err` opcode **MUST** come right after the `switch` opcode. | ||
|
||
The specific label names in the pre-compiled TEAL **MAY** be different, but it is **RECOMMENDED** to use the label names here for consistency across tooling. | ||
|
||
The `switch` statement in a contract **SHOULD** only include labels until the last supported action is reached. For example, if an application only supports `NoOp` creations, the switch can simply be `switch create_NoOp`. All other actions will simply go to the next opcode, which **MUST** be `err` as specified above. | ||
|
||
### Client Parsing | ||
|
||
A client **MUST** parse the decompiled TEAL to ensure that the firt nine lines match the lines specified in the TEAL specification. Prior to parsing, `inctblock` or `bytecblock` opcodes **MUST** be removed from the TEAL because they may come before the opcode in this ARC, but don't affect functionality. | ||
|
||
To support future changes where comments may get added to the decompiled TEAL, clients **MUST** only check the beginning of lines to ensure the opcode and its argument(s) match the specification. | ||
|
||
The client **MUST** follow the label for a specific action. If that label does not exist in the `switch` opcode OR if it leads to the `err` label, then that action is known to not be supported by the TEAL. | ||
|
||
## Rationale | ||
Standardizing the initial opcodes of the TEAL program makes it easy for anyone to parse the TEAL, follow a label for a specific action, and determine whether that action is supported or not. This makes it possible for tools in the ecosystem, such as explorers and wallets, to tell end-users whether apps they are interacting with are mutable or not. | ||
|
||
An alternative approach is to include asserts at the beginning of the program for any OnCompletion that is not supported by the contract, but this leads to a larger program size and potentially opcode cost. This approach also still makes it hard to determine what OnCompletions *are* supported for individual methods. | ||
|
||
## Backwards Compatibility | ||
N/A | ||
|
||
## Test Cases | ||
N/A | ||
|
||
## Reference Implementation | ||
TODO: Client parsing example | ||
|
||
## Security Considerations | ||
If a client improperly implements parsing, they may mislead users about the mutability of the contract. It is imperative that `intcblock` and `bytecblock` are handled properly and the following opcodes are exactly as specificed in this ARC. | ||
|
||
## Copyright | ||
Copyright and related rights waived via <a href="https://creativecommons.org/publicdomain/zero/1.0/">CCO</a>. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure I see the point of this ARC as even an immutable contract could steal your funds if you don't understand all the code. It might just provide a false sense of security.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no way to have 100% security guarantees. A bug in algod could result in loss of funds, but that doesn't mean we should discount all other security considerations.
The goal of this ARC is to make it easier to understand critical parts of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but what about existing immutable contracts that don't have this code pattern and can't be updated to use it. They miss out on this 'trusted' badge.
At first glance it also doesn't look like this pattern will work with the existing beaker template variables for TMPL_UPDATABLE and TMPL_DELETABLE. Obviously there are alternative solutions to this - perhaps this ARC should describe them?
I think ultimately this would be better resolved at the AVM level with a new opcode/pragma comment at the top of the program.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Support the idea of dealing with this at avm level. I have seen chat recently about legal issues of mutable contracts, so the requirement seems to be one of some consequence.