ABI Reference Types

[barnjamin]

Adding reference types to ABI module so we can easily use them in Router.

Targeting abi-router since I'm using it for testing so branched off abi-router

Needs tests but want to make sure this is a reasonable way to attempt this

[ahangsu]

I guess this should comes to feature/abi first, not abi-router, since router work is kind of parallel to reference type?

nvm... just saw the description

[ahangsu]

took a quick pass and have some negligible comments, and the only unsettled question is set, I think.

[algoidurovic]

Looks good overall, just a few points I'd like you to address.

[ahangsu]

one thing to notice, Zeph merged in #322 to feature/abi today, you might want to update too...

[tzaffi]

one thing to notice, Zeph merged in #322 to feature/abi today, you might want to update too...

RIght. In particular, it introduced a new member annotation_type that should also be implemented in this PR.

[tzaffi]

Also, not to hold up the PR, but if simple test cases could be added here, that would be great. I believe that the companion abi_roundtrip.py class should be able to handle these new types (because their TypeSpecs subclass the UintTypeSpec) but I'm not 100% sure.

[barnjamin]

I think to actually fix tests we need to have a change to the py sdk to support reference types https://github.com/algorand/py-algorand-sdk/blob/master/algosdk/abi/base_type.py#L59

[tzaffi]

I think to actually fix tests we need to have a change to the py sdk to support reference types https://github.com/algorand/py-algorand-sdk/blob/master/algosdk/abi/base_type.py#L59

That's a good finding. Interestingly, we do have a related test case for handling reference types in method signatures, so I am a bit puzzled by how we pass that test without explicitly handling reference types in base_type.py#L59.

@algochoi - it's been a while since you worked on this but maybe you have an insight?

[ahangsu]

^ we should correspondingly check all other SDK's about unified behavior of type from string function...

[tzaffi]

@jasonpaulos has shed some light in a separate thread. This is related to a relatively recent change to arc-4

[algochoi]

I think to actually fix tests we need to have a change to the py sdk to support reference types https://github.com/algorand/py-algorand-sdk/blob/master/algosdk/abi/base_type.py#L59

That's a good finding. Interestingly, we do have a related test case for handling reference types in method signatures, so I am a bit puzzled by how we pass that test without explicitly handling reference types in base_type.py#L59.

@algochoi - it's been a while since you worked on this but maybe you have an insight?

The transaction references are handled in the atomic_transaction_composer.py and is only used when the method signature is parsed. +1 to this was a relatively recent thing we added.

[barnjamin]

Completely cheating right now by just skipping round trip tests for the Reference Types. Should we wait to merge this until those are available in the SDK?

[tzaffi]

Is there a reason that this is basing off of abi-router instead of feature/abi ?

[ahangsu]

Is there a reason that this is basing off of abi-router instead of feature/abi ?

I think Ben is testing his code against my router implementation, this PR going into feature/abi makes more sense to me.

[barnjamin]

superseded by #361