Reverse ENS resolution support in debugger and decoder

[haltman-at]

Addresses #3839. Breaking change to @truffle/codec.

This PR adds support for reverse ENS resolution in Truffle Codec, and in Truffle Decoder and Truffle Debugger that use it. This means that, if enabled, values of address or contract type that have an ENS name will have an ensName interpretation in their (newly-added) interpretations field, and values of external function type will have something similar.

(Apologies for this PR being a big ball of messy commits. You know how it goes with big PRs...)

In order to accomplish this, a third type of decoding request was added to Truffle Codec, the ENS reverse resolution request. This request will be made for all values of address, contract, or external function type, and it is up to the caller to handle them. That makes this a breaking change to @truffle/codec, as callers must deal with this new request type. They can respond with null if they don't support ENS resolution, but they must respond.

(Notionally, this could have been done in a nonbreaking way, by not making such requests unless an appropriate option was passed. I didn't see any particular reason to do this, however.)

So, what's new here?

Well, first off, value results (not errors) now include an interpretations field like decodings do. For most types this is at present always empty. However, for addresses and contracts there is now the ensName interpretation, and for external functions there's the contractEnsName interpretation. Like all interpretations, it's optional and will only be present when relevant. Note that the value of this interpretation is a StringValueInfo, rather than a string, in order to account for the possibility that the ENS name consists of malformed UTF-8 (even though in reality that won't come up in this implementation).

Now you may wonder, why is this in interpretations rather than value? And if this is in interpretations rather than value, shouldn't that also be true of a bunch of the other stuff currently in value? Well... yes! But it's not our intent to just go breaking everything, making breaking decoder or debugger changes (just codec), so basically stuff already in value is grandfathered in, and new stuff will generally go under interpretations.

You might also wonder: If we're doing this, shouldn't address values also get an interpretation with their contract type when available, effectively unifying address values with contract values (in information content if not in format)? Yes! We should also do that! But in order to reduce complication, that is being pushed off to a future PR.

Then, as mentioned, there's the new request type. I didn't want to go fundamentally changing how decoding requests work, so the response type here is still Uint8Array | null rather than anything more sensible. But you respond with a Uint8Array (in UTF-8) if there is an ENS name, and null if there isn't or ENS reverse resolution isn't turned on.

Note that it is up to the caller to perform a forward-resolution check! There is no separate request type for forward resolution (well, that exists as an encoding request type, but not decoding); the caller simply has to know that as part of reverse resolution you must confirm with a forward check. The debugger and decoder will both respond with null should the forward check fail.

Of course, in addition to the updates to the debugger and the decoder to handle this new request type -- which I'll get back to in a moment -- we've also got to update the inspectors (as well as abify.ts). So, ResultInspector will now print out an ENS names, colored light blue, rather than an address, when it's available. However in some contexts (such as the debugger) we don't want to hide the address, so, the various inspector constructors now take an optional options argument, which allows one to pass options such as noHideAddress (the only option at present). I believe @cds-amal intends to do more with these options in the future, however. :) (Specifically, in regard to #5860.)

(Thought: Should I have used subclasses instead of flags? Oh well.)

Note there are more places we could put reverse ENS resolution in the debugger -- for instance, in the affected instances display, or stacktraces, or etc. So the reverse ENS resolution support in the debugger is rater partial as it pertains to decoded values only. But for obvious reasons, those other places we could support it were considered out of scope for this PR.

Also, I left interpretations blank for all values created by wrap. I thought about including an ensName interpretation when the input was an ENS name, but 1. this is actually incompatible with the current architecture, and 2. it's not useful anyway. So I didn't do it.

With all that out of the way, let's discuss the changes to the decoder and the debugger! (Including the debugger CLI.)

Let's start with the decoder because it's simpler. The ProjectDecoder now gains an init method where ENS is set up, and the methods that create ProjectDecoders are updated to call it beofre returning the decoder to you. Also, decoding methods now handle "ens" requests using ENSJS, on top of which I've stuck a cache.

(I didn't bother with a separate cache for forward requests, I only put a combined one for reverse-requests-together-with-forward-requests, but I could add one for forward requests too?)

Note that reverse ENS resolution is turned on by default; if you want to turn it off, you have to do so manually by specifying provider: null in the ENS options. In the debugger CLI, I added two new command-line options; --no-ens to turn off ENS support, and --registry to specify a registry address other than the default.

Btw, you'll notice some code repeated here from the encoder related to determining the default registry address based on the network ID (and you'll also see this in debugger). ENSJS is supposed to do this automatically, but in actual fact, it does not, and so we have to work around this bug. This workaround didn't seem worth factoring, so it's just repeated several times. Sorry.

In the debugger, things are more complicated. The basic idea is the same as in the decoder, but the details are a bit messier. The debugger now has an ensRegistryAddress option, alongside the light mode option, to set the ENS registry address; you can also set it to null to turn off reverse ENS resolution.

Adding reverse ENS resolution meant adding not only a new adapter method, but also all the machinery in the web3 saga to interface with that method. I'll skip going into the details here. One thing worth noting though: You'll note that the web3 saga now ends with putting an action signalling that the web3 submodule is ready, which it didn't before. Turns out we had a synchronization bug! This ENS work exposed it and so I had to fix it.

Also, there was the question of where in the state to put the ENS cache (again, I did the cache same way as in the decoder, I didn't bother separately caching forward requests). After talking to @gnidan we decided that the appropriate location was a new submodule, ens. This submodule doesn't have a listener though. It only does things when data asks it to. So it's not adding a lot more complication.

Finally, let's talk about the tests, which I think I have to due to some annoying problems with them. Now, these are not the first tests that involve setting up ENS resolution -- encoder tests already did that. But I did it a different way for the decoder and debugger tests, and it seems to have caused a problem.

For the debugger tests, the tests would not pass unless Ganache was set to eager mode. (Thanks to @cds-amal for finding this.) So, to get this up, that's what I did. I hope that's OK. @cds-amal thinks there may be a Ganache issue? I didn't follow this. (Note that my manual tests pass without problem!)

For the decoder tests -- which I put in a separate folder, ens, separate from current, to avoid any interference -- not only did we have to set eager mode, but also, to prevent the tests from hanging after finishing, we had to stick a --exit on the Mocha invocation. (Again, thanks to @cds-amal for finding this.) That's a bit worrying, I have to admit; but I really want to get this PR up, so, whatever, that's what I'm going with. Hopefully we can figure out what's going on with that and find a better way?

One final minor note -- because we're now lossily converting a StringValueInfo to a string in more places, I factored out a function for doing that. It's also exported from Codec if consumers of the library want to use it!

Testing instructions

There are new automated tests in both the decoder and debugger packages (about which see above). But you'll likely want to test this manually too. Here's how I did it:

1. Go to ens-test in the solidity-test-cases repo.
2. Compile, migrate.
3. Open up the Truffle console, get the EnsTest instance, and call instance.registryAddress() to get the registry address.
4. Run instance.run().
5. Debug the resulting transaction, using --registry to set the registry address to the address you got above.
6. Step through, decode variables, and observe! :)

[gnidan]

Quick comment here before I get you a full review: MetaMask has raised the concern that @truffle/decoder is too big with all its dependencies (something like 15% of total MM bundle size), so we must not increase the number of dependencies at this time.

This PR might not add net new dependencies at all? We'll have to verify that. But if this does add stuff, then we'll need to rework this to use ethers.js directly and not add ens.js or anything else.

[gnidan]

Generally looks good!

Just a few naming requests, and a note that we can't merge this until we know what impact this has on the total install size for @truffle/decoder.

[gnidan]

Looks good @haltman-at!