essential-eth Decode uint128 & uint64

Adding Decoding for uint128 and uint64.

Dec 06 '22 11:12 KeKs0r

Hi @KeKs0r , thanks for the PR! Can you provide a test which failed before but succeeds now?

Dec 08 '22 01:12 dawsbot

Checking the other tests, those are quite e2e-ish. So directly against chain. And none of the currently included ABIs have those fields. Should I just add a new ABI that I know uses those fields?

Dec 08 '22 02:12 KeKs0r

Checking the other tests, those are quite e2e-ish. So directly against chain. And none of the currently included ABIs have those fields. Should I just add a new ABI that I know uses those fields?

Sure, you're right that I did not design the tests well previously because they are very e2e. If you can rip out and make a focused test with an ABI that includes uint128 & uint64 that would be epic!

Dec 08 '22 07:12 dawsbot

Hey @KeKs0r I forgot to follow-up here. Was there anything more you needed to get unblocked writing those tests?

Thanks for the PR 🙏

Jan 21 '23 06:01 dawsbot

Checking the other tests, those are quite e2e-ish. So directly against chain. And none of the currently included ABIs have those fields. Should I just add a new ABI that I know uses those fields?

You're spot on, these tests I've written are not using proper mocking nor proper separation of concerns. I'm doing at-least one step to improve this now with https://github.com/dawsbot/essential-eth/issues/201

Mar 14 '23 05:03 dawsbot

I think we're almost ready to solve this.

We can make a test which validates we match encodeFunctionData from ethers: https://docs.ethers.org/v5/api/utils/abi/interface/#Interface--encoding

We currently have this logic all crammed into the other functions but this task should also separate it properly

Apr 24 '23 11:04 dawsbot