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.

Sign up for GitHub

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

Jump to bottom

Generate proof data #743

Merged
merged 10 commits into from
May 22, 2019
Merged

Generate proof data #743

merged 10 commits into from
May 22, 2019

Conversation

schemar
Copy link
Contributor

@schemar schemar commented May 16, 2019

Fixes #718

@schemar
Copy link
Contributor Author

schemar commented May 17, 2019

Ready for review

benjaminbollen
benjaminbollen reviewed May 17, 2019
View reviewed changes
Copy link
Contributor

@benjaminbollen benjaminbollen left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented.

@schemar
Copy link
Contributor Author

schemar commented May 20, 2019

Moved to bounce, will add more patterns.

@schemar
Copy link
Contributor Author

schemar commented May 20, 2019

Ready for review again.

@0xsarvesh 0xsarvesh self-assigned this May 21, 2019
0xsarvesh
0xsarvesh reviewed May 21, 2019
View reviewed changes
Copy link
Contributor

@0xsarvesh 0xsarvesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@0xsarvesh 0xsarvesh removed their assignment May 21, 2019
@schemar
Copy link
Contributor Author

schemar commented May 21, 2019

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
Copy link
Contributor

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
Copy link
Contributor Author

schemar commented May 22, 2019

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.

👍 on it!

@schemar
Copy link
Contributor Author

schemar commented May 22, 2019

Ready for review again.
/cc @sarvesh-ost

@schemar schemar mentioned this pull request May 22, 2019
Merged
0xsarvesh
0xsarvesh approved these changes May 22, 2019
View reviewed changes
Copy link
Contributor

@0xsarvesh 0xsarvesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐇

@0xsarvesh 0xsarvesh merged commit 784fa38 into OpenST:develop May 22, 2019
@schemar schemar mentioned this pull request May 22, 2019
Closed
@schemar schemar deleted the generate_proof_data branch May 22, 2019 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjaminbollen benjaminbollen benjaminbollen left review comments

@0xsarvesh 0xsarvesh 0xsarvesh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Generate extensive data set using MPP fuzzer for unit test
3 participants
@schemar @0xsarvesh @benjaminbollen