[types] limit nesting of types via state

[wrwg]

Motivation

This is an alternative version of what #528 and the previous PR for limitation of nesting of type tags provides.

This approach installs custom serializers at the recursion points of the TypeTag enum. Those serializer use a thread local to track the nesting of the type tag, both for serialization and deserialization.

The usage of a thread local is safe and a common technique to provide extra state for implementations which miss it (e.g. serde). A given thread has a single execution stack and this will always count the depth counter consistently up and down. Because Rust has no exceptions are other exit points, this cannot be corrupted. Possibly, thread locals may cause inefficiency. However, we expect TypeTags to be small (a few constructors, limited nestings).

The technique used for type tags can be generalized for any other recursive type if needed. Eventually, we hope serde will support per-type nesting, and possibly we should talk with the author if we can add it ourselves.

(Write your motivation for proposed changes here.)

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

This uses and passes the tests from #528.

[dariorussi]

I'll just leave a note:
this is pretty cool and reminds me of back in the days when we had a custom function to serialize/deserialize Move Values and it was few lines of code and easy to understand. The visitor pattern of serde is pretty difficult to follow.
Later there was this push to make everything "pure serde" and that conversation/train was impossible to stop.

The use of thread local storage is difficult to evaluate for me. And of course I wish (as I am sure we all do) that it was easy to pass a custom value on the stack, basically what the visitor pattern allows but without the insanity of the visitor pattern. Anyway, enough of wishful thinking...

With nesting being a global property, a TLS seems fine, not sure that will always be true and that is one place where it could be difficult.
With leaf types (as TypeTag or Move Values) that is also fine, I think.

[runtian-zhou]

Do we need to land this change on aptos branch? Also will this be a substitute for #525 and #526 on aptos branch?

[sblackshear]

This solution seems reasonable to me. Couple of questions about the motivation: I'm assuming that this exists to prevent stack overflows when serializing or deserializing TypeTag's. Is the concern about:

1. Converting SignatureToken's in the bytecode format into TypeTag's (e.g., to write a value into global storage)
2. Accepting user-specified TypeTag's as tx inputs (e.g., entry fun pay<T>(amount: u64))
3. Accepting user-specified TypeTags in read API's (e.g., to simulate the result of a function call)

? (I'm guessing all of the above, but wanted to know which I am wrong about + which surfaces I might be overlooking). Also, if it's just (1) (or if we want to prevent runtime failures in the case of (1)), we could think about enforcing this in the file format or verifier.

[wrwg]

This solution seems reasonable to me. Couple of questions about the motivation: I'm assuming that this exists to prevent stack overflows when serializing or deserializing TypeTag's. Is the concern about:

1. Converting SignatureToken's in the bytecode format into TypeTag's (e.g., to write a value into global storage)
2. Accepting user-specified TypeTag's as tx inputs (e.g., entry fun pay<T>(amount: u64))
3. Accepting user-specified TypeTags in read API's (e.g., to simulate the result of a function call)

? (I'm guessing all of the above, but wanted to know which I am wrong about + which surfaces I might be overlooking). Also, if it's just (1) (or if we want to prevent runtime failures in the case of (1)), we could think about enforcing this in the file format or verifier.

This is not just about stackoverflows. We can discuss offline. Closing this one, as we have landed this now (with some mods) in aptos branch. Will send another cherrypick PR later.