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

SDKs: Replace logicsig langspec checks with self-contained checks #213

Closed
michaeldiamant opened this issue Jul 26, 2022 · 6 comments
Closed
Assignees

Comments

@michaeldiamant
Copy link
Contributor

michaeldiamant commented Jul 26, 2022

Problem

After each go-algorand AVM release, all SDKs require updated langspec.jsons + new tests as per https://algorand.atlassian.net/wiki/spaces/SCY/pages/2346844170/Updating+SDK+Langspecs. To minimize work needed per AVM release, the story requests replacing the existing assertions with equivalents (or near equivalents) that do not rely on a langspec.json artifact.

Example existing implementation from py-algorand-sdk: https://github.com/algorand/py-algorand-sdk/blob/f0509ba40f8ca02c02cc254ea80e5bdbfa2aa9a8/algosdk/logic.py#L45. Each SDK provides analogous checks.

The replacement checks must confirm the provided program:

  • Is not base64 encoded. Since the input is bytes, a common class of mistake is to supply a base64 encoded input. Other plausible mistakes are to send TEAL source, or empty bytes.
  • Contains TEAL bytecode. While the new check won't inspect particular opcodes, it feels prudent to confirm the input resembles TEAL.

As part of the investigation, consider paring down langspec.json to remove unnecessary fields (e.g. budget). We may ultimately create a separate story, but ought to consider the topic while in the neighborhood.

Solution

Dependencies

Urgency

@ahangsu
Copy link
Contributor

ahangsu commented Aug 18, 2022

The first round investigation result follows:

For Go/JS/Py/Java-SDK: we need to deprecate methods in Logic:

  • readProgram
  • checkProgram,

deprecate global variables under Logic:

  • spec/langspec
  • opcodes
  • logicVersion
  • evalMaxVersion.

Related methods using readProgram: LogicsigSignature constructor and LogicsigSignature.verify.
Related methods using checkProgram: only in template, and template is already marked deprecated.

Solution: implement another sanityCheckProgram method in Logic to substitute readProgram, and replace readProgram in 2 uses under LogicsigSignature.

@ahangsu
Copy link
Contributor

ahangsu commented Aug 18, 2022

Possible deprecation schemes cross SDKs:

@ahangsu
Copy link
Contributor

ahangsu commented Aug 18, 2022

Another sub-conclusion from deprecating langspec.json: The readProgram and checkProgram relies on knowledge of opcode size, i.e., how many bytes opcode segment totally occupy.

Without such knowledge, one cannot start meaningful test over byte string like disassemble-alike test, other than checking byte code complying to a certain TEAL version. Some other heuristic need to be figured out, without a list of ops with knowledge of size.

@ahangsu
Copy link
Contributor

ahangsu commented Aug 18, 2022

JJ made some suggestions:

  1. check that first byte is between 0-7 (roughly. goes up with AVM version)
  2. check that it doesn’t look b64 encoded, or an encoded algorand address
    a. but both of these are unneeded if you do 1
  3. check that it doesn’t contain all ascii printable characters (so it’s not teal source)
    a. this will matter when v10, since programs can start with newline, which is 10.

@michaeldiamant
Copy link
Contributor Author

Informational: I opened #223 to track following up in a later iteration to remove deprecated fields, methods, classes, and langspec.json resource file.

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

2 participants