Debugger/decoder support for custom errors; fix for assembly parameters in 0.8.4

[haltman-at]

Partially addresses #3996. (Also, note to whoever does the release on this, this is a minor version for @truffle/debugger, @truffle/codec, @truffle/decoder, and maybe @truffle/debug-utils.)

This PR does two things. Primarily, it adds support for custom errors in the debugger and the decoder (although for technical reasons it does not add proper support for them in stacktraces). In addition, this PR also fixes the debugger's tracking of assembly function parameters (including return parameters) on Solidity 0.8.4.

(...also, I fixed an unrelated circular dependency while I was at it.)

The latter change, while not being the main point, is simpler, so I'll discuss it first. (I made this change because I had to upgrade our debugger tests to Solidity 0.8.4, which broke our tests of assembly variables, so I had to fix that.)

Assembly variable tracking in Solidity 0.8.4

So, the debugger doesn't always allocate parameters for Yul functions on hitting a YulFunctionDefinition node, because Yul will actually hit these nodes sometimes even when you're not actually entering the function, so the debugger does an additional check to see if you're actually entering the function body. But Solidity 0.8.4 changed the behavior when there are return parameters. Previously, the last step on the function definition would be after all parameters (input and output) had been allocated, and then it'd enter the function. Now, the last step on the function definition would be after all input parameters are allocated, and then it steps onto the output parameters, one by one. (Which is how it already worked for Solidity!)

So, to handle 0.8.4 in a backwards compatible manner (and without explicit version checks), we now check if you're about to step either into the function body, or onto the return parameters. If the former, we use the old method. If the latter, we only allocate input parameters, not output parameters. Then, I added a separate YulTypedName case for allocating the output parameters (as this is the node type of the output parameters that it will step onto). The latter also acts as potential future-proofing for if they ever change how ordinary variable declarations are handled.

Custom errors

OK, so now for the main thing -- custom errors. There are several parts to this.

Decoding custom errors isn't actually that hard; after all, we already decode errors, just not custom ones. I didn't change this very much -- I could have changed it more to perhaps make it more efficient, but I figured it wasn't a big concern. Just, now when we attempt to decode return data, which might be an error, we also check for custom errors. Of course, those errors also have to get allocated at some point, and I'll get back to that later!

There is one truly new thing in error decoding though, and it's a change I've made to event decoding as well: The option to pass in an ID. Full-mode errors and events now get IDs, the same way that user-defined types and contracts do. This option is meant for use by the debugger -- it's not exposed in Truffle Decoder; IDs are supposed to be an internal matter, after all (I haven't even added them to the return format). The idea is that the debugger can pass in an ID to help disambiguate in the case of ambiguous events or errors. (The debugger doesn't currently decode events, but it will in the future, per #3824.) If any of the possible decodings match that ID, only that decoding will be returned. If none do, though, then they'll all be returned as normal, so that a bad ID (such as due to optimization, or due to the return data being anything other than a custom error!) doesn't cause a total decoding failure.

(For events, this is passed in as an option, whereas for errors, to keep compatibility, I added it as an additional positional argument; realistically you're not going to be passing this in unless you're also passing in a status, which is how the debugger does it, so I think this is fine.)

The fact that I didn't change much means there is some redundant selector checking, but, oh well. I think it's fine.

Note that the debugger performs the decoding in the data module, but it uses the stacktrace module to find the error ID. (Since it has to find where the error was originally thrown, not what contracts lower on the callstack might have forwarded it!) So this means that data now depends on stacktrace, which it didn't previously.

Also note that because right now decoding of this -- as well as setting up the allocations -- is done in the data submodule, that means that stacktrace can't decode custom errors. (Not sure how I'd format those, anyway. :P ) (And remember, stacktrace has to be able to run in light mode, with data turned off.) So instead it just assumes that any error it can't decode is a custom error and just reports "Custom error". 🤷 You want more information, you can use the debugger!

In addition to the actual decoding itself, of course, the debugger CLI and debug-utils now contain functions for formatting and printing these errors.

Allocation

So now we come to the hard part: Allocation. Both the debugger and decoder first have to obtain the needed information for allocation. Which of course they were mostly already doing, but there's one new thing -- error and event definitions now go in referenceDeclarations. Really we're only using this for errors at the moment, but I've included it for events as well because Solidity has been considering changes to the event ABI and AST that would make it useful in that case as well. (In the debugger case, this has resulted in some new actions and state being added to the data module.)

But, that's just setup for allocation. What about allocation itself?

As with other types of things, we split allocation into three levels: Allocating an individual error, collecting the error allocations for an individual contract, and collecting the error allocations for the entire project.

Allocating an individual error is pretty straightforward and largely reuses existing code via the allocateDataArguments function, so I don't have a lot to say about that.

The other two levels are where I perhaps took some shortcuts and should possibly redo things a little.

When it comes to collecting the error allocations for an individual contract, we encounter an oddity: Solidity has decided to include in the ABI not just the errors declared in a contract, but all the errors declared in there or that it could directly emit -- and contracts can emit errors declared anywhere, so these could come from any contract at all (or none, errors can be top-level!). However they've also added a usedErrors property to the contract node on the AST, giving the IDs of these.

So, what I did is the following. We try using usedErrors to allocate in full mode (for everything). If however we run into a problem in the initial stages, we use just the ABI and fall back to ABI mode.

I'm not entirely happy with this because this can mean falling back to ABI mode for all of a contract's errors, even if something goes wrong with just one of them. I may redo this. However redoing this will I think require searching every contract in the compilation to find the corresponding error node. I didn't do that because, well, I was wary of that, but, well, we actually already basically do that during allocation to find which contract a given error was defined in, so, um, yeah, maybe that's OK. (Or, maybe I should get rid of the search there too...) Anyway @gnidan I would like your opinion on whether I should redo this...

Then we come to collecting errors for the entire compilation, and here I took another shortcut. Since any contract can notionally throw any error (since a contract can call any contract, and the called contract can throw whatever, and the caller will then forward it), I just tossed all errors with the same selector into a big pile, more or less. That is to say, if for some reason an error is ambiguous, the decoder doesn't put first errors that could be thrown directly by the specific contract. Again, I'm kind of uneasy with this, and am wondering if I should change it. This would take a bit of work though.

Note that because of how Solidity is doing the error ABIs, we have this question of what counts as the same error, since an error declared in one place might show up in the ABIs of multiple contracts. My approach was as follows: I considered full-mode errors declared in different places to be different, but all ABI-mode errors with matching signature to be the same. If there are both full-mode and abi-mode allocations for the same signature, we toss out the abi-mode ones and keep just the full-mode ones. Seems sensible to me?

Anyway yeah that's it. @gnidan please let me know if I should redo the two things mentioned above.

[gnidan]

Couple variously small comments, but implementation-wise this seems alright.

As for your questions:

• I don't think it's a priority to spend the time fixing usedErrors now; we can fall back to ABI mode like that for now. Open an issue to address later?
• I do think it's worth redoing the other one, though; it seems important to provide good disambiguation ordering in the decoder.

[haltman-at]

OK, I've changed things to guarantee the ordering a bit better. Apologies for how awful the code is, but, uh, turns out this is not so easy... :-/

[gnidan]

aight let's do it