-
Notifications
You must be signed in to change notification settings - Fork 162
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
Solidity pack Array support #50
base: master
Are you sure you want to change the base?
Conversation
Solidity pack Array support
Hum, I am quite confused by the automatics tests. They fail during the npm install, right? |
Any feedback, please? |
Hmm, this library hasn't been touched in a while and I'm not sure if anyone feels actively responsible right now. I'm not really that much into Solidity/API stuff and not feeling competent enough in this area to make a review. Maybe ask @axic, since he is doing both |
No problem, I didn't want to be that pushing. |
@vrolland thanks for the PR! Can you please explain the feature? |
Hello axic ! commit : c5d4746 - add toSolidityBytes32 commit 5acbaf1 - Solidity pack Array support |
Oh I see, thanks, it makes sense. I'd prefer to have the two changes separate and lets tackle the support for array first here. I think instead of code duplication you could recursively call Would you be interested updating the PR accordingly? |
Sure, I can make two different PR.
Actually, this is not possible for most of the types. The array packing behavior is different than the regular packing. All the variables will be put in a format bytes32 (in arrays the data are not really packed). What do you think is the best? |
Why would it be different? This is the documentation we have for this format: http://solidity.readthedocs.io/en/develop/abi-spec.html#non-standard-packed-mode Do you have some test cases which triggers this in Solidity? |
Yes, I actually already use this code in a project. Solidity code : JS code : But, I may did somehting wrong. |
Hi, |
Guys, I added a commit to allow array with a dynamic size |
@axic Also can't judge on the validity of this PR here, needs some further discussion/clarification. |
Solidity pack Array support