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

AVM: Add NumBoxes as a txn field #5820

Closed
wants to merge 1 commit into from

Conversation

nullun
Copy link
Contributor

@nullun nullun commented Nov 7, 2023

Summary

Adds the ability for programs to know how many boxes have been included with the call. For deterministic box names (e.g. "box0", "box1", "box2") a loop can be used to iterate through them, but we must know how many have been supplied.

Test Plan

I've only tested this locally on a private network. I didn't know if I should add a new test to eval_test.go or fit it into box_test.go

@nullun nullun changed the title New opcode txn NumBoxes AVM: Add NumBoxes as a txn field Nov 7, 2023
@jannotti
Copy link
Contributor

Can you elaborate more on the need for this? It's relatively simple, of course, but I worry that it leads people in the wrong direction. We did not give txn the ability to access box names because the box refs might be in different transactions of the group - we wanted to encourage contracts that explain what they need, but don't look to see if it's supplied. So, in this case, the caller would just supply a number as on of the ApplicationArgs. The contract would "trust" the number and loop. If it hit a box that was available, it would panic.

@nullun
Copy link
Contributor Author

nullun commented Nov 21, 2023

That makes sense. I think I was originally writing some "cleanup" code in a smart contract, where I wanted to delete an unknown number of boxes between 1 - 8 which were supplied in the call, and I immediately wondered if there was a NumBoxes, much like all the other NumAppArgs, NumApps, etc.

I think I got more excited about trying to add the field to go-algorand rather than think how else to do it, and you're right that supplying a number as an application argument would be more suitable here. I realise now that my implementation would only return the number of boxes within that transaction, so any shared resources would throw it off. It could have access to more than 8 boxes, but NumBoxes would only ever return up to 8.

I'll keep thinking if there's a valid reason, but otherwise happy to close this one.

@nullun nullun closed this Jan 5, 2024
@nullun nullun deleted the opcode-numboxes branch January 5, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants