Solidity pack Array support

[vrolland]

Solidity pack Array support

[vrolland]

Hum, I am quite confused by the automatics tests. They fail during the npm install, right?

[vrolland]

Any feedback, please?

[holgerd77]

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 ethereumjs-vm and Solidity work, but it will probably take a few days, please be a bit patient.

[vrolland]

No problem, I didn't want to be that pushing.
I am very impressed by your job guys, I just want to help.
Cheers!

[axic]

@vrolland thanks for the PR! Can you please explain the feature?

[vrolland]

Hello axic !

commit : c5d4746 - add toSolidityBytes32
=> add the function toSolidityBytes32() which mime the casting from many types (bytes*, bool, address, int*, uint*) to bytes32. Usefull to pass generic parameters to a solidity function in bytes32 and then parse it back to a specif type in the contract.

commit 5acbaf1 - Solidity pack Array support
=> add to the existing function solidityPack(), the support of arrays (bytes*[], bool[], address[], int*[], uint*[]). Usefull to mime array packing (e.g: to mime a solidity function like keccak256() with arrays)

[axic]

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 solidityPack inside the array case.

Would you be interested updating the PR accordingly?

[vrolland]

I'd prefer to have the two changes separate and lets tackle the support for array first here.

Sure, I can make two different PR.

I think instead of code duplication you could recursively call solidityPack inside the array case.

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).
I may be able to use "toSolidityBytes32" in solidityPack for arrays, but in this case, I would make only one PR.

What do you think is the best?

[axic]

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?

[vrolland]

Yes, I actually already use this code in a project.

Solidity code :
https://github.com/RequestNetwork/Request_SmartContracts/blob/master/contracts/synchrone/RequestEthereum.sol line 540 (see function getRequestHash() with a bytes[9] in parameters)

JS code :
https://github.com/RequestNetwork/Request_SmartContracts/blob/master/test/synchrone/ethereum/requestEthereumBroadcastSignedRequestAsPayer.js line 49 (the function hashRequest() mime the solidity getRequestHash())
As you can see in the same file (line 10) I use a modified version of "ethereumjs-abi" with the commits I did here.

But, I may did somehting wrong.

[vrolland]

Hi,
Anything I can do to make it happens?

[vrolland]

Guys, I added a commit to allow array with a dynamic size

[holgerd77]

@axic Also can't judge on the validity of this PR here, needs some further discussion/clarification.