Test Boxes field in application calls static array

[algochoi]

This PR provides SDK tests for application transactions with the new boxes field:

• Added encoding tests for boxes in application transactions

SDK implementation:

• Python: Box support for Application calls and Atomic Transaction Composer py-algorand-sdk#338

TODO:

• Add integration tests for box references in application calls (Will be handled in a later PR after box read API is implemented)
• Add tests for Atomic Transaction Composer (Is this necessary?)

Tracks #175

[algochoi]

Currently points to the fork, but should revert this back before merging to master.

[tzaffi]

• Add tests for Atomic Transaction Composer (Is this necessary?)

I think this would be nice to have if not essential. Currently the step variants of "add a method call with the signing account" do not claim the ability to add reference arg arrays to the method call (i.e. accounts, foreign_apps, foreign_assets, and boxes). This seems like an important feature to test, but since the existing tests are so intricate, a smallish new scenario focusing on this aspect might make the most sense.

[tzaffi]

Left a couple of comments/questions.

It seems that this is a "placeholder" PR which isn't concretely asserting anything about how boxes are handled. Is this true, and if so, are we waiting on algod code in the go-algorand boxes branch in order to understand the box API, and only then can we spell out precisely what the test will be?

[algochoi]

Appreciate the quick feedback:

I think this would be nice to have if not essential. Currently the step variants of "add a method call with the signing account" do not claim the ability to add reference arg arrays to the method call (i.e. accounts, foreign_apps, foreign_assets, and boxes). This seems like an important feature to test, but since the existing tests are so intricate, a smallish new scenario focusing on this aspect might make the most sense.

I think that makes sense like an integration test for the boxes field.

It seems that this is a "placeholder" PR which isn't concretely asserting anything about how boxes are handled. Is this true, and if so, are we waiting on algod code in the go-algorand boxes branch in order to understand the box API, and only then can we spell out precisely what the test will be?

Yes, the next part will be to implement the algod APIs to read from boxes. Then we can assert round-trip results (create, write, read from boxes) in the integration tests with some TEAL code, probably in a new PR to the feature/box-storage branch.

[tzaffi]

I believe this is probably approvable but must confess to my not understanding exactly what is going on. Could you explain how the test is actually doing anything? Specifically, in features/unit/transactions.feature I see that one can allow boxes to be added to an ApplicationCallTxn. The hash of the txn should then reflect this new information, and this should create a totally new "golden". Is this understanding correct and are the golden's reflecting the expected transaction hash which includes boxes?

[algochoi]

The hash of the txn should then reflect this new information, and this should create a totally new "golden". Is this understanding correct and are the golden's reflecting the expected transaction hash which includes boxes?

That's correct - it's checking if the boxes are encoded in the appl static box arrays. You can also visually check by taking the b64 encoded golden msgpack format and decoding it back to JSON to reverse engineer the new fields in the transaction.

[tzaffi]

LGTM

[jasonpaulos]

The encoding tests look good, I just left a comment about another case I'd like to see. I'll approve since this is going into a feature branch, so not everything needs to be complete yet