feat: messageHash attribute added in SQLite + testcase

[ABresting]

Description

messageHash attributed added to SQLite, which aims to remove the id attribute in upcoming PRs. An associated test case also added.

migration scripts will be changed after the deletion of id attribute in upcoming PRs.

Changes

• messageHash attribute added
• added test case to test the messageHash attribute

Issue

#2112

[github-actions]

This PR may contain changes to database schema of one of the drivers.

If you are introducing any changes to the schema, make sure the upgrade from the latest release to this change passes without any errors/issues.

Please make sure the label release-notes is added to make sure upgrade instructions properly highlight this change.

[github-actions]

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2142

Built from 2efab14

[richard-ramos]

LGTM

[Ivansete-status]

Thanks for the PR! It is getting a look shape. However, I have the impression that we need to properly implement the computeDeterministicMessageHash proc, as I mentioned in a comment.

[Ivansete-status]

Maybe there is something that I'm missing but messageHash looks the same as id.

I think we need to create a new computeDeterministicMessageHash proc in waku_archive.nim where we create the following attribute: https://rfc.vac.dev/spec/14/#deterministic-message-hashing

[ABresting]

well, as per the initial approach, we aim to remove the id attribute and replace it with messageHash, as not only nomenclature is better but the way digest/hash is computed it makes sense.

the computation logic behind messageHash follows the RFC. The digest logic is implemented in the previous PR for the same base issue #2112

[Ivansete-status]

You are absolutely right that we have it implemented. I got confused because we have another computeDigest definition, which is different. We need to revisit it and check if we can only use one.

nwaku/waku/waku_store/common.nim

Line 30 in 817a7b2

proc computeDigest*(msg: WakuMessage): MessageDigest =

[Ivansete-status]

@jm-clius - something that I realized while reviewing this PR is that the messageHash won't be unique and therefore we need to also consider the storedAt attribute as part of the PK, as @ABresting is properly applying now.
wdyt?

[ABresting]

@jm-clius - something that I realized while reviewing this PR is that the messageHash won't be unique and therefore we need to also consider the storedAt attribute as part of the PK, as @ABresting is properly applying now. wdyt?

Well, the problem with storedAt is that it is very local since the storedAt time is the local node time, i.e. for every node this time attribute can be different. The aim of messageHash is to make sure we have a unique message identifier across the decentralized network which then be used in the new sync protocol for efficient synchronization.