OpenST / mosaic-contracts

Mosaic-0: Gateways and anchors on top of Ethereum to scale DApps
https://discuss.openst.org/c/mosaic
MIT License
114 stars 31 forks source link

Generate proof data #743

Closed schemar closed 5 years ago

schemar commented 5 years ago

Fixes #718

schemar commented 5 years ago

Ready for review

schemar commented 5 years ago

Moved to bounce, will add more patterns.

schemar commented 5 years ago

Ready for review again.

schemar commented 5 years ago

Nice work 👍 Overall looks good 🐼 Question: Why values are stored as a buffer, not as a string like other files in test/data folder. Did you try to recreate the js buffer object from serialized data in generatedProofData.json?

@sarvesh-ost yes I convert them there:

https://github.com/OpenST/mosaic-contracts/pull/751/files#diff-45732ae0a73ed327c02d5c209394c4c8R27

Should I store them right away in hex instead?

0xsarvesh commented 5 years ago

Nice work 👍 Overall looks good 🐼 Question: Why values are stored as a buffer, not as a string like other files in test/data folder. Did you try to recreate the js buffer object from serialized data in generatedProofData.json?

@sarvesh-ost yes I convert them there:

https://github.com/OpenST/mosaic-contracts/pull/751/files#diff-45732ae0a73ed327c02d5c209394c4c8R27

Should I store them right away in hex instead?

I think yes, will reduce the overhead of reconverting to hex and this also makes it consistent to other JSON files in the same folder.

schemar commented 5 years ago

Nice work 👍 Overall looks good 🐼 Question: Why values are stored as a buffer, not as a string like other files in test/data folder. Did you try to recreate the js buffer object from serialized data in generatedProofData.json?

@sarvesh-ost yes I convert them there: https://github.com/OpenST/mosaic-contracts/pull/751/files#diff-45732ae0a73ed327c02d5c209394c4c8R27 Should I store them right away in hex instead?

I think yes, will reduce the overhead of reconverting to hex and this also makes it consistent to other JSON files in the same folder.

:+1: on it!

schemar commented 5 years ago

Ready for review again. /cc @sarvesh-ost