Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Redo output format with interfaces instead of classes (also some bugfixes) #2180

Merged
merged 8 commits into from
Jul 16, 2019

Conversation

haltman-at
Copy link
Contributor

This PR does two things.

Firstly, it redoes the decoder output format with interfaces instead of classes. The format is still the same; just now it's with interfaces and functions instead of classes and method.

The reason it originally used classes was to get util.inspect.custom to work. However now instead we handle this problem but having a single ResultInspector class which contains a single Result; instead of inspecting Results, when we want to print them we first wrap them in a ResultInspector.

Similarly, the nativize method is now a function. The toSoliditySha3Input method has been kind of split into two different functions. There's now a mapping key encoder in addition to the ABI encoder; that's what we now use for mapping keys instead of toSoliditySha3Input. However, key.ts also contains a debugging function returning info for printing mapping keys. This is basically like the old toSoliditySha3Input, except it's freed from the constraints of having to actually work as input to that function. :) (So don't use it for that, as it won't work for that anymore!)

Secondly, this PR contains two bug fixes regarding compiler versions, one major and one minor. (Don't worry, these both only exist on next, not develop.)

The first bug fix is that, in order to determine what compiler a given user-defined type was compiled with, we would... do a complicated thing that was wrong, and involved looking it up in the context; this would run into a problem when the type was defined in an interface or abstract contract, as those don't get contexts. Now we do it the right way but just looking up the source ID in data.info.scopes and then getting the compiler from solidity.info.sources.

The second (really minor) bug fix is that I altered the compiler version check so it won't mistake prerelease versions of 0.5.0 as not being 0.5.x.

This commit fixes a bug where the debugger couldn't find
the compiler for interfaces or abstract contracts.

It also fixes an oversight where prerelease versions of 0.5.0
wouldn't be counted as 0.5.0.
@haltman-at haltman-at requested a review from gnidan July 12, 2019 04:56
@coveralls
Copy link

coveralls commented Jul 12, 2019

Coverage Status

Coverage remained the same at 70.543% when pulling 63d732f on declassify into f68962d on wire-decoder.

@haltman-at
Copy link
Contributor Author

haltman-at commented Jul 12, 2019

OK, some last-minute additions:

  1. Hell with it, I added the tuple type. Note that use of this type is really not supported yet, but I added it.

  2. I added optional type hints to the uint, address, and tuple types. Note that we don't actually use these yet. Note that I didn't bother with very strong typing for these, because, well, they're just hints.

  3. I added decodingMode to the decoding objects, which can be "full" or "abi". Right now it's always "full", of course.

  4. Unrelated nonfunctional change: I rewrote the allocation-attempt loop in decodeEvent to use a labeled continue rather than having a giant try block like I had earlier. I figure try blocks should be kept to just the part you expect to throw the error, if possible.

(If you really don't like the labeled continue we could have the try block encompass the whole inner for loop. But I had it way too big before, I had it around the entire body of the outer for loop for some reason...)

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Definitely nice to see the direction this is going.

I spared you architecture thoughts, but the proliferation of long switch statements is something we're going to have to deal with in the future.

packages/truffle-codec-utils/src/conversion.ts Outdated Show resolved Hide resolved
packages/truffle-codec-utils/src/conversion.ts Outdated Show resolved Hide resolved
packages/truffle-codec/lib/decode/abi.ts Show resolved Hide resolved
packages/truffle-codec/lib/decode/special.ts Show resolved Hide resolved
packages/truffle-codec/lib/decode/value.ts Show resolved Hide resolved
packages/truffle-codec/lib/interface/decoding.ts Outdated Show resolved Hide resolved
@haltman-at
Copy link
Contributor Author

OK, made the following changes (these are unrelated to Nick's review above):

  1. I added another strict-mode error, this time for an overlarge pointer in the ABI. In this case the potential problem isn't a DOS -- it's just that we're going to convert this pointer to a number, and we don't want to crash when that happens! (OK, I guess we wouldn't actually crash due to the try block, but still, I'd prefer to throw our own structured errors when the error is one we expect. :) )

(Mind you, we'll still crash if this happens outside of strict mode. But the question of how to handle this sort of thing outside of strict mode is, well, a question for another time.)

  1. ResultInspector and nativize now handle malformed strings by using node's builtin UTF-8 conversion (via Buffer) which uses the replacement character (U+FFFD) for malformed UTF-8 rather than throwing an exception. Mind you, we don't use this in the decoder itself, because there we don't want to lose information.

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@haltman-at haltman-at merged commit 93cb78b into wire-decoder Jul 16, 2019
@haltman-at haltman-at deleted the declassify branch July 16, 2019 00:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants