Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

chore: update Deterministic message hashing algorithm. #632

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

ABresting
Copy link
Contributor

As discussed,
Updating the Deterministic message hashing computation, adding the sender timestamp in computation. It will make the message hash more unique.

@@ -161,8 +164,9 @@ pubsub_topic = "/waku/2/default-waku/proto" (0x2f77616b752f322f64656661756c742d7
message.payload = 0x010203045445535405060708
message.content_topic = "/waku/2/default-content/proto" (0x2f77616b752f322f64656661756c742d636f6e74656e742f70726f746f)
message.meta = <not-present>
Copy link
Member

Choose a reason for hiding this comment

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

Since the timestamp is still optional (IIUC), should we have an example without timestamp similar to this example with empty meta?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so basically timestamp if not explicitly provided is filled anyway by our fakeWakuMessage proc. Are you saying to write a test case where we overwrite the filled timestamp as empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@richard-ramos
Copy link
Member

The change looks good to me, I'm just wondering if this could this cause any backward compatibility issues? The reason I ask is because the versions of status desktop/mobile running are heterogeneous so there will be clients using older version of the hash algorithm

@ABresting
Copy link
Contributor Author

The change looks good to me, I'm just wondering if this could this cause any backward compatibility issues? The reason I ask is because the versions of status desktop/mobile running are heterogeneous so there will be clients using older version of the hash algorithm

Currently, this function is not used by anything in old devices/client since this function will be used by the new DB column hash, the sync protocol will only be able to sync with new/updated devices?

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

Approved, but with comment below to make field consistent.

content/docs/rfcs/14/README.md Outdated Show resolved Hide resolved
@ABresting ABresting merged commit cfaef76 into master Nov 15, 2023
@ABresting ABresting deleted the update-deterministic-hash-algorithm branch November 15, 2023 09:34
jimstir pushed a commit to jimstir/rfc that referenced this pull request Dec 6, 2023
* chores: update Deterministic message hashing algorithm.

* type fix in commented code
jimstir pushed a commit to jimstir/rfc that referenced this pull request Dec 6, 2023
* chores: update Deterministic message hashing algorithm.

* type fix in commented code
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants