Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix short address decoding #111

Closed
wants to merge 1 commit into from
Closed

Conversation

k06a
Copy link

@k06a k06a commented Oct 23, 2018

Example addresses:

0x0014F55A50b281EFD12294f0Cda821Bd8171e920
0x0000000000000000000000000000000000000000

Example addresses:
```
0x0014F55A50b281EFD12294f0Cda821Bd8171e920
0x0000000000000000000000000000000000000000
```
@Shirikatsu
Copy link
Contributor

Thanks for your pull request @k06a!

I'm unsure in which scenario an address would not be 20 bytes in length. What did you have in mind for this change?

@k06a
Copy link
Author

k06a commented Oct 24, 2018

@Shirikatsu at some moment of time client encoder may encode addresses as numbers and this may lead to zero bytes prefix compression for some addresses, containing first zero bytes. Address encoded this way will be absolutely legal, I just want to ensure smart contract will support this case.

@maxrobot
Copy link
Contributor

@k06a this change is probably better directed at the project https://github.com/androlo/standard-contracts/blob/master/contracts/src/codec/RLP.sol

Thank you for you contribution nonetheless.

Have you managed to use Ion in any meaningful way, I would be excited to hear your feedback?

@k06a
Copy link
Author

k06a commented Oct 24, 2018

This change was proposed to both original libraries containing Solidity RLP decoders:
hamdiallam/Solidity-RLP#2 (already merged)
androlo/standard-contracts#9 (not yet merged)

We in multitoken.io (multitoken.com) are working on decentralized bridges between blockchains and watching over related projects. Nice framework!

@Shirikatsu
Copy link
Contributor

Thanks @k06a!

The code looks good however I'm currently investigating where (and why) some of our tests are failing and it's currently boiling down to some error in this function from a brief look.

@Shirikatsu Shirikatsu closed this Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants