Move from promise-based decoder that makes web3 calls to generator-based decoder that makes requests to caller

[haltman-at]

This PR (the idea for which is due to @gnidan) restructures the decoder so that, instead of being based on async functions and promises, it is based on generators and iterators; and instead of the decoding functions having web3 and contractAddress passed in as arguments, and the decoder making web3 calls, it makes requests to the caller that the caller must respond to.

Don't worry -- if you're using the contract decoder, you shouldn't notice any changes! (Aside from two minor enhancements which I'll get back to later; skip to the list of minor changes at the end if you want to read about those.) If you're using the forEvmState interface, however, this is a breaking change. Fortunately, I don't think anyone is using that interface except for the debugger, and the upsides of the change are definitely worth it.

So how does this new interface work? As mentioned, decode (for which forEvmState remains just a wrapper) is now a generator, meaning it returns an iterator (call it decoder). To use it, call decoder.next() to get a result; if this is the final result, i.e. if result.done is true, then result.value holds the decoded value. If it's not the final result, i.e. if result.done is false, then result.value holds a request object; one must formulate a response to this request and call decoder.next(response), repeating until one gets a final result.

Currently, the only requests the decoder makes are for storage. These requests have the form {type: "storage", slot}, where slot is a BN. (Note there is no address field; this wouldn't fit into the current structure -- for now supplying an address is left to the caller; however, I might add an address field in the future once such a thing makes sense. Similarly there's no block field, and I have no plans to change that; that wouldn't make sense for the debugger's use case, anyway.)

The contract decoder responds to storage requests by looking up the storage slot using web3. (The address is the address of the contract being decoded; as for what block to use, see the part at the end about contract decoder enhancements regarding that.) The debugger, by contrast, responds to all storage requests with zeroes. This is because the debugger passes in all the nonzero storage it knows upfront; any storage it doesn't know, it presumes to be zero.

(I briefly considered moving to a structure where only definition and pointer would be passed in as arguments, with the decoder making requests for all other information it needed, rather than having info passed in upfront; but this would be a bunch of extra work for no upside that I could see other than a dubious sort of consistency. So instead I'm going the opposite route -- I'm going to continue have as much as possible passed in upfront and will only use requests where necessary.)

So, as mentioned, right now we're only using requests for storage (and only for those slots whose value wasn't passed in upfront). But the motivating reason for this change, the thing that necessitates it, is that soon we're going to need to make requests for code, too, and these will require looking up that code on the blockchain even when called from the debugger, and (for reasons not worth getting into here) it's not possible for the debugger to pass in a web3 object like the contract decoder previously did.

But this change has a number of other upsides, besides just allowing code requests! For instance, it saves us from having to pass in some sort of web3 options object, which we don't currently do but I was fearing would be necessary. Now, instead of passing in an options object, to dictate what web3 calls the decoder should make, the caller just decides what calls it wants to make to respond to requests. Also, this new structure should allow the decoder to be used to implement a major part of reconstruct mode, once we start on that.

So, that's how to use the new interface to the decoder. But what's changed internally? Surprisingly little! Parts that used Promise.all for parallelism have been changed to be serial instead, and of course the storage read functions have been changed to make requests when necessary, but a large portion of the change was effected with a simple :%s/await/yield*/. It turns out that the way that yield* works basically fits exactly with what we want, so functions can simply call each other with yield*.

You see, in a generator, the expression yield* it delegates to the iterator it, causing the generator to yield in sequence the values coming from it... except for the final, finishing value from it. That one, instead of being yielded by the generator, becomes the value of the expression yield* it. (If you wanted to yield it, you could write yield yield* it, but we don't want to do that.) This means that we can have our generator functions simply use return when we want them to return a value to their caller like usual, and use yield when we want them to issue a request, pause the decoder's execution, and wait for a response. It's really that simple!

Note, by the way, that many of the read functions were actually not async before, but were simple pure functions; only the ones that needed to be async were async. I've kept that in this PR -- the read functions that were previously pure functions, are still pure functions; only the async ones have been replaced by generators.

There is one ugly thing about this restructuring, though, and that's the TypeScript typings. You see, TypeScript has no way to know about the structure mentioned above, where real values are returned and requests are yielded. This means that the decoding functions have to include Uint8Array as a possible return/yield type, even though they will never return or yield this, because TypeScript cannot discern this; it can't tell that the read functions only return Uint8Arrays and never yield them. Until this proposed change makes it into TypeScript, we're just stuck with this ugliness. Note that due to our general abuse of any, this mostly isn't visible at the moment, but once we have the new decoder output format and things get more structured, it'll become noticeable.

So that's what's different in the decoder! What about the debugger? Well, the selectors data.views.decoder and data.current.identifiers.decoded are gone; with the new structure, it soon won't be possible to do decoding in a selector. (As of this PR, it's still possible because, as mentioned above, the only responses being passed in are zeroes, not information from web3; but that won't be the case for long.) In their place is a new data saga, decode.

The decode saga takes a definition and pointer as arguments and decodes them based on the current state; the main data saga now just calls this with a yield* rather than using a yield call to decode. Note that the part of the state that keeps track of whether the decoder is ready, and the actions that manage it, have been removed, as they're no longer necessary.

What about the interface to the decoder? Well, session.variable now just runs the decode saga on the specified variable, and session.variables now just runs it on all variables (as determined by data.current.identifiers.refs). As for session.decodeReady, it's been retained for compatibility, but it now simply always returns (a promise that resolves to) true. (Although, users should probably not have been using decodeReady directly, but, just in case they were, it's been retained.)

But wait! Session methods can't run sagas, you say! Ah, but they can! It turns out that the redux-saga API includes middleware.run, which allows for running any specified saga. However, in order to do this, we need a copy of the saga middleware object. So, over in store, the configureStore function has been modified so that, instead of just returning the store, it returns an object containing both the store and the saga middleware; and the Session constructor now saves the latter in this._sagaMiddleware in addition to saving the store. I then added an async method session._runSaga, which uses the redux-saga API to run a specified saga (with specified arguments) and return a promise that resolves when the saga is done running. (The name starts with an underscore to indicate that this is not for external use.) With this, it's now possible for Session methods to run sagas, as session.variable and session.variables now do!

Finally, a few other minor changes:

1. The contract decoder has been enhanced by finally having the state and variable functions actually respect the block argument if it's passed in, instead of ignoring it. This determines the block that will be used for all web3 requests.
2. Also, in the return value of state, since it already included both the variables and the balance, I decided to toss in the nonce too, because why not. I stopped short of also including the code, though.
3. The original decoding tests had a decode function in their helpers, that did some logic to convert the decoder output to more easily-usable JS objects; I've replaced this logic with a call to DebugUtils.nativize, since for some reason this logic wasn't working properly in some cases. (The first use of truffle-debug-utils in truffle-debugger, I believe, even if it's only in the tests.)
4. I uninstalled lodash.range from truffle-decoder since (as of this PR) it no longer uses it.

And that's it! With this, the preparatory work is out of the way, and we're ready to decode external function pointers.

[coveralls]

Coverage increased (+0.006%) to 69.637% when pulling 65967b4 on star-decode into 9eb3a53 on develop.

[gnidan]

😂

freakin' cool!

[haltman-at]

Note: I've updated the PR writeup to reflect the change from requesting to type, and to reflect the fact that actually there is a TypeScript PR in the works that will make the typing here better.