-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fix a bug in pubsub message ID hash function #791
Conversation
The prior implementation I believe intended to include additional fields into account when calculating the message ID from data. But was only returning hash of data for both manifest and GPBFT messages. Fix this by writing the data to the hasher, _then_ computing the hash with no suffix.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #791 +/- ##
==========================================
- Coverage 69.64% 69.38% -0.27%
==========================================
Files 77 77
Lines 7900 7904 +4
==========================================
- Hits 5502 5484 -18
- Misses 1949 1962 +13
- Partials 449 458 +9
|
Does that mean our message IDs contained all of the data? |
Great catch!! |
No no. It contained only the hash of all of data. Previously we called |
One note is that this will break the message propagation. |
So if we want to merge this it's best to fix the tag for the container used by manifest server and observer so that they do not pull the latest 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's... wow.
We can merge this once this pr is merged: https://github.com/filecoin-project/f3-passive-testing/pull/142 |
The prior implementation I believe intended to include additional fields into account when calculating the message ID from data. But was only returning hash of data for both manifest and GPBFT messages.
Fix this by writing the data to the hasher, then computing the hash with no suffix.