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

add Txn field Nonparticipation #106

Merged
merged 4 commits into from
Sep 7, 2021
Merged

add Txn field Nonparticipation #106

merged 4 commits into from
Sep 7, 2021

Conversation

shiqizng
Copy link
Contributor

@shiqizng shiqizng commented Sep 1, 2021

No description provided.

@shiqizng shiqizng requested a review from jasonpaulos September 1, 2021 19:41
Copy link
Contributor

@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.

I just created #107 to merge the teal5 branch to master. Since that updates the compiler to work with v5, that will have to be merged before this.

@@ -85,6 +85,7 @@ class TxnField(Enum):
local_num_uints = (54, "LocalNumUint", TealType.uint64, 3)
local_num_byte_slices = (55, "LocalNumByteSlice", TealType.uint64, 3)
extra_program_pages = (56, "ExtraProgramPages", TealType.uint64, 4)
nonparticipation = (57, "Nonparticipation", TealType.uint64, 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately this isn't documented anywhere, but the last element of this tuple is the minimum TEAL version required to access that field, so it should be 5.

@@ -675,6 +675,19 @@ def test_gtxn_id_dynamic():
assert actual == expected


def test_gtxn_nonparticipation():
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also add a test called test_gtxn_nonparticipation_dynamic which tests the same thing as this, but uses a PyTeal expression (e.g. Int(0)) as the txn index? There are a bunch of tests like that in this file too.

Additionally, once you change the minimum TEAL version to 5, can you add a check to verify compiling with an older version of TEAL raises an error? See test_txn_extra_program_pages for an example

Copy link
Contributor

@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 good!

@shiqizng shiqizng merged commit 1157c57 into master Sep 7, 2021
@jasonpaulos jasonpaulos deleted the txnfield branch September 27, 2021 18:53
@jasonpaulos jasonpaulos mentioned this pull request Sep 27, 2021
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.

2 participants