Add decodeArgs() method for Event

[devictr]

Hey @esaulpaugh,

Here's my implementation of the decode function for Events.
I still need to add a few more tests for edge cases, but I wanted your opinion before doing so.
In a few words:

• Indexed parameters are passed as a topics array, that we can easily decode using the usual decoding functions. However, dynamic types are not decodable as only a special hash of their encoding is stored, so we decode that instead.
• Non-indexed parameters are all bundled in a data byte array, which we decode in one go. I'm not a fan of the data naming, but that's the terminology used in the Solidity docs, so I figured it's better to stay consistent.
• We reconstruct and return a Tuple that contains all those parameters in the right order

This split between topics and data as input parameters is similar to what is done in ethereumj or to how things are presented in tools like etherscan.

Would that implementation work for you?
Feel free to also let me know about any code style changes you would like me to make, in order to match the project's choices in that regard.

[esaulpaugh]

looks good so far. Other than more tests, all I can think to add right now is private static final ArrayType<ByteType, byte[]> BYTES_32 = TypeFactory.create("bytes32");

[esaulpaugh]

I would probably also eliminate decodedTopicsTuple and just use decodedTopics[topicIndex++]

[devictr]

@esaulpaugh Fixed your comments and added more tests

[esaulpaugh]

Okay, thanks. I will look at it.

[esaulpaugh]

I'm thinking that if it's going to be doing decoding, Event should derive indexed and non-indexed TupleTypes only once, when constructed, and reuse them. Right? Unless there's a compelling reason not to, I think Event could just keep references to three TupleTypes: inputs, indexed, and nonIndexed.

And non-anonymous Events would calculate topics[0] once during construction and use it to verify the topics arrays it's decoding with Arrays.equals? new Keccak(256).digest(Strings.decode(getCanonicalSignature(), Strings.ASCII))

What do you think?

[devictr]

That sounds great! Adding those things now

[devictr]

@esaulpaugh Here you go! I opted to throw a RuntimeException when the signature hashes don't match for non-anonymous events, but I'm thinking we should probably create our own exception for that. What do you think? Maybe there's another standard exception we could use

[devictr]

I also refactored my code a bit and extracted some parts into more methods

[esaulpaugh]

I usually just throw IllegalArgumentException for anything attributable to the caller.

[devictr]

Fixed! If this looks good to you and you decide to merge it, would you mind cutting a release for it? That way I can merge some internal PR I have open as well 🙂
I'd also be happy to contribute more. If you open issues for things you need help with, I'd love to take a look when I have time

[esaulpaugh]

LGTM. I will be releasing version 6.0.0 with this included soon.

Thanks. After the release I will go through the abi-spec some more and see if there is anything else I'm missing.

[devictr]

Great! Thank you