Add GroupID global to TEAL#2649
Conversation
| int 32 | ||
| bzero | ||
| == | ||
| && |
There was a problem hiding this comment.
Ideally we should also have a test where GroupID is not empty...
There was a problem hiding this comment.
That should be doable with some hacking around in evalStateful_test.go, which builds up some groups with two txns in them.
|
That definitely looks like something missing that should be in there, but I kinda feel like this should be a |
|
It seems to me that |
|
It's absolutely debatable, since we've (mostly) used global to mean "Truly global - things about the blockchain" and txn to mean "About a single txn within the group". For me (and zach, it seems), the decider was GroupSize, which we did put under global. It seemed right to put GroupID and GroupSize in the same place. And yet, "grp" is indeed truly in the individual txns, so it seems like you should be able to get it from there. Honestly, it seems weird, but I could imagine allowing it from both. |
| CreatorAddress, globalV5TestProgram, | ||
| EvalStateful, CheckStateful, | ||
| }, | ||
| 6: {GroupID, globalV5TestProgram, Eval, Check}, |
There was a problem hiding this comment.
I'm not sure why does it work, this assumes assembler version 6 but the field defined as version 5 constant
{GroupID, StackBytes, modeAny, 5},
This means the test is never called. I'll submit a PR checking len(tests) <= AssemblerMaxVersion
algorandskiy
left a comment
There was a problem hiding this comment.
Requesting two changes:
- Add non-empty group id test
- Rebase on master -
TestGlobalwill fail because of non-existing version 6
|
Redone as #2838 to deal with conflicts. |
Summary
Adds the global field
GroupIDto retrieve the group of the transaction. This follows the pattern established forGroupSize.Test Plan
Added/ran unit tests.