Decode tuples per abi

[pacrob]

What was wrong?

Named objects can already be passed to contracts where tuples are expected as input, but there is no way to decode outputs as named objects despite having the names available in the ABI.

Original PR #1353
Related to Issue #1267

How was it fixed?

A decode_tuples option is added when instantiating a Contract. When set, tuples returned from a contract function call will be a new NamedTuple of the name ABIDecodedNamedTuple. NamedTuples are compatible wherever regular tuples are used, but fields can be accessed with tuple_name.key syntax. I set the NamedTuple option rename=True to handle restricted field names, rather than add a custom renaming scheme as in the original PR. More info on NamedTuples.

Contract event data was previously returned as an AttributeDict with any nested info as unnamed tuples. It is now AttributeDicts all the way down.

Todo:

• Add entry to the release notes

Cute Animal Picture

[kclowes]

I looked through this on a pretty high level, and it's looking good to me. I left some nits, and then a few thoughts I had are:

• Do we need to pass decode_tuples to ContractEvents as well?
• Can we also add a test to the contract caller tests to make sure that's working okay?
• I think I'm okay with the rename option, since if you are opting in to this functionality, you should have some idea of how namedtuples work, but I'm curious to get feedback from @fselmo here too. I think originally we had talked about returning a regular tuple as a fallback if there were fields that aren't allowed as namedtuple field names.
• It might be nice to give banteg some credit here, either by cherry-picking a PR or two, or maybe by tagging him in the notes when you merge or something.

[fselmo]

I did a first pass. This looks great. I don't think I like the internal renaming of keywords to _0, _1, _2... etc. I wonder if we can come up with a better solution there? 🤔

If we kept track of them somehow and remapped them to from_, class_, etc... would that be better? I haven't looked but is that an option in namedtuple?

If this seems like the best approach I think I'm OK with it since this is just an enhancement / nice-to-have flag. But ideally, and for a better user experience, I would like to know what those fields were called by having the name from the key in one way or another.

Thoughts?

[fselmo]

* I think I'm okay with the `rename` option, since if you are opting in to this functionality, you should have some idea of how `namedtuples` work, but I'm curious to get feedback from @fselmo here too. I think originally we had talked about returning a regular tuple as a fallback if there were fields that aren't allowed as namedtuple field names.

Yeah I had some thoughts on this here. Not sure what the best approach is but I'd like to have a better UX than just _0, _1, etc... worst case, I don't think I'm against just inherent namedtuple handling.

* It might be nice to give banteg some credit here, either by cherry-picking a PR or two, or maybe by tagging him in the notes when you merge or something.

Agreed!

[pacrob]

I chose to go with namedtuple's rename=True for predictability. They have a set of rules for renaming that we can point to and say "this is how it behaves". If a user has previous experience with namedtuples, they would be expecting this renaming pattern. If we roll our own renaming scheme, we need to explain it in detail and still run the risk of missing edge cases.

Regarding returning just a plain tuple instead: I feel it would be more confusing, having set decode_tuples=True, to just get a plain tuple back with no explanation. If the user sees a ABIDecodedNamedTuple with renamed fields, they at least know they set the flag correctly and can do more research.

[fselmo]

I answered a bit on the typing. Let me know if I missed something there but that all does work locally for me. I think this all looks good to me since the tests are passing but I'd like a bit more time to look carefully at it. I can make another pass Monday 👀.

[fselmo]

This is looking good. I had some final thoughts I wanted to discuss before approving. I think the best approach would be a user choice approach where in all cases the ABIDeocdedNamedTuple is an opted-for choice. Curious on your input here and from @kclowes

---

edit: You may ignore any mentions above, this lgtm 🎉

[fselmo]

Ignore my previous ping, this lgtm in current state... 🎉 I'll resolve those conversations