algochoi/pyteal_audit_thoughts.md

## pyteal_audit_thoughts.md

      
    Raw
  

              pyteal_audit_thoughts.md
            
          
    Writing PyTeal Contracts for audits

Some thoughts to keep in mind when developing PyTeal smart contracts. Hopefully, these points will make it easier for reviewers and auditors to read and understand the contract logic.
Using pragma to enforce a PyTeal compiler version

Since PyTeal is still under active development, there may be bugs fixes or patches in future versions. Although specifying the exact compiler version is likely the best practice, allowing minor version bumps through the caret (^) seems acceptable too.
from pyteal import *
# Enforce a minimum PyTeal compiler version of 0.18.1, 
# and accept any minor version bumps such as 0.18.2 and above, 
# but not any major version bumps such as 0.19.0. 
pragma(compiler_version="^0.18.1") 

# Your smart contract here...

TODO: Upgrading contracts?
Using the docstring for a router ABI method

The PyTeal compiler can parse docstrings on an ABI method and populate common fields such as the description, parameter/argument name, and returns for the generated ABI JSON.
@router.method
def fruits(apple: abi.Asset, banana: abi.Asset, *, output: abi.String) -> Expr:
    """Given some fruits, return a fruit basket.

    Args:
        apple: This is an apple.
        banana: This is a banana.

    Returns:
        A fruit basket.
    """
    # Implementation here.

Generated JSON for the fruits method:
{
    "name": "fruits",
    "args": [
        {
            "type": "asset",
            "name": "apple",
            "desc": "This is an apple."
        },
        {
            "type": "asset",
            "name": "banana",
            "desc": "This is a banana."
        }
    ],
    "returns": {
        "type": "string",
        "desc": "A fruit basket."
    },
    "desc": "Given some fruits, return a fruit basket."
}

Code Style

Since PyTeal is written in Python, it's recommended to follow PEP 8 style conventions.
There are open-sourced formatters and linters such as Flake8 and black that will help do this.
Consider the Application Transaction On-Complete matrix

An appl call transaction can have six outcomes after it is executed. A no-op transaction can also be responsible for creating the app, if its appl ID is 0. Think about what should and should not be allowed for the transaction lifecycle.
e.g. Just for demonstration purposes:


no-op (create)
no-op (call)
opt-in
close-out
clear-state
update
delete


router (main)
✅
❌
❌
❌
✅
❌
❌


deploy
✅
❌
❌
❌
❌
❌
❌


deposit
✅
✅
✅
❌
❌
❌
❌


withdraw
❌
✅
❌
✅
❌
❌
❌


Checking Transaction types in ABI arguments

If a particular transaction type is being passed in as an ABI argument, then it seems prudent to check the type of that transaction in the method.
e.g.
@router.method
def deposit(
    deposit_axfer: abi.AssetTransferTransaction
) -> Expr:
        # Check previous transaction is of type axfer
        Assert(deposit_axfer.get().type_enum() == TxnType.AssetTransfer),

This could also apply to any ABI argument, as type checks are quite lax in Python in general.
Decimals, fractions, and division

The AVM does not support fractional or floating point numbers. Hence, operations that require decimals or fractional values should be converted to a whole number. For instance, if one is dealing with US dollar amounts, it feels prudent for any calculations involving this amount to be multiplied by 100, e.g. $3.51 -> 351).
Similarly, simple division (/) in TEAL will truncate fractional amounts. This can be particularly problematic when trying to get a fractional or percentage value (e.g. 3 / 100 to get 3%). Fractional values should be kept whole, by keeping track of the numerator and denomiator, if possible.
There is a community library that can deal with fixed point arithmetic: https://github.com/barnjamin/pyteal-utils/blob/fp-class/pytealutils/math/fixed_point.py
There are a couple of common strategies for dividing:

If you are dividing by something that may produce a fractional value, and do not care about precision, round in favor of the contract.

e.g. contract A determines that it should pay out “4.21” units to Charlie => round down to 4
e.g. contract A determines that it should receive "12.88" units from Sally => round up to 13


If you are dividing something and require precision, do this off-chain. You could store the resulting numerator and denominator in global state and let some off-chain process do the actual division. e.g. If you need two-thirds of a percent in the contract (0.00666...), store 2 and 300 for the numerator and denominator separately in state, read this, and then carry out the calculation 2/300 off chain.

Misc thoughts


It seems pretty common for smart contract devs to provide a PyTeal contract and an annotated TEAL contract with comments. However, it's hard to check if the annotated TEAL contract was really produced from the PyTeal program. It would be nice if the PyTeal generated a hash of the generated TEAL to cross-check contracts faster.
Naming arguments, especially when accessing global/local state (e.g. specifying keyword arguments account=..., key=..., value=... or using newline/indentation to deliniate key-value pairs), would help make contract comprehension easier.
	no-op (create)	no-op (call)	opt-in	close-out	clear-state	update	delete
router (main)	✅	❌	❌	❌	✅	❌	❌
deploy	✅	❌	❌	❌	❌	❌	❌
deposit	✅	✅	✅	❌	❌	❌	❌
withdraw	❌	✅	❌	✅	❌	❌	❌