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

feat(p2p): verify data integrity #751

Merged
merged 2 commits into from
Dec 20, 2018
Merged

feat(p2p): verify data integrity #751

merged 2 commits into from
Dec 20, 2018

Conversation

moshababo
Copy link
Collaborator

Solves #157.

As discussed here, the packet body serialization for the hash function is done with JSON.stringify. Lack of consistency in the serialization was already found, so I used json-stable-stringify to fix it. Without it the tests would fail.

@moshababo moshababo requested a review from sangaman December 17, 2018 15:23
@ghost ghost assigned moshababo Dec 17, 2018
@ghost ghost added the in progress label Dec 17, 2018
}
}
}
public abstract serialize(): Uint8Array;

private hash(value: any): string {
return MD5(stringify(value)).toString(CryptoJS.enc.Base64);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be nice to have some sanity tests for this method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean to test against static values (input/output)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add some testing in the upcoming #756. Merging this now due to dependency in #756.

@moshababo moshababo merged commit 3ff06b1 into master Dec 20, 2018
@ghost ghost removed the in progress label Dec 20, 2018
@moshababo moshababo deleted the data_integrity branch December 20, 2018 10:22
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.

2 participants