Skip to content

Conversation

@ntn-x2
Copy link
Contributor

@ntn-x2 ntn-x2 commented Feb 23, 2024

Partially fixes https://github.com/KILTprotocol/ticket/issues/2562.

Adds unit tests for the verifier components (components tested are shown in the checklist below).
It also splits up the proof verification logic into multiple files, split by when those checks are performed. Check the crates/kilt-dip-primitives/src/merkle/v0/ folder for more details. It addresses an open comment in the DIP refactoring PR: #602 (comment).

Elements to add tests for

@ntn-x2 ntn-x2 self-assigned this Feb 23, 2024
@ntn-x2 ntn-x2 marked this pull request as ready for review March 1, 2024 10:10
@ntn-x2 ntn-x2 requested a review from Ad96el March 1, 2024 10:10
@ntn-x2
Copy link
Contributor Author

ntn-x2 commented Mar 1, 2024

@Ad96el please ignore the CI error, it is a consequence of the shitty benchmarking logic, which I plan to get rid of in the upcoming PR.

Base automatically changed from aa/dip-unit-tests to develop March 1, 2024 13:41
Copy link
Contributor

@Ad96el Ad96el left a comment

Choose a reason for hiding this comment

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

LGTM overall. Nice job 👏

Some minor comments

@ntn-x2 ntn-x2 force-pushed the aa/dip-consumer-tests branch from 3cd2292 to 740c79d Compare March 6, 2024 12:16
@Ad96el
Copy link
Contributor

Ad96el commented Mar 7, 2024

Last picky comment: In the crates/kilt-dip-primitives, there is the module merkle. This somehow triggers me. I am always thinking about the politician Angela Merkel 😆 . Wouldn't a better name be merkle_proof or merkle_tree? You can ignore this comment if you think the naming is good.

Copy link
Contributor

@Ad96el Ad96el left a comment

Choose a reason for hiding this comment

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

🥳

@ntn-x2 ntn-x2 enabled auto-merge (squash) March 7, 2024 12:29
@ntn-x2 ntn-x2 disabled auto-merge March 7, 2024 12:33
@ntn-x2 ntn-x2 added the ✋on hold status: on hold label Mar 7, 2024
ntn-x2 added 2 commits March 28, 2024 08:32
Fixes https://github.com/KILTprotocol/ticket/issues/3104, based on top
of #611.

It fixes the logic for the `dip-consumer` pallet, by delegating the
generation of a proof worst case to the proof verifier, which must make
sure the proof is indeed the one that requires the most weight to
verify, and that it verifies successfully. This also means that each
consumer runtime is responsible to implement this method, as there
cannot be a "universal" worst proof, as that depends on the use case.
The pallet benchmarking logic is now generic enough to make this
possible and flexible ✨✨✨
@ntn-x2 ntn-x2 removed the ✋on hold status: on hold label Mar 28, 2024
@ntn-x2 ntn-x2 enabled auto-merge (squash) March 28, 2024 07:46
@ntn-x2 ntn-x2 merged commit 5cf55eb into develop Mar 28, 2024
@ntn-x2 ntn-x2 deleted the aa/dip-consumer-tests branch March 28, 2024 08:19
Ad96el pushed a commit that referenced this pull request Apr 2, 2024
Partially fixes https://github.com/KILTprotocol/ticket/issues/2562.

Adds unit tests for the verifier components (components tested are shown
in the checklist below).
It also splits up the proof verification logic into multiple files,
split by when those checks are performed. Check the
`crates/kilt-dip-primitives/src/merkle/v0/` folder for more details. It
addresses an open comment in the DIP refactoring PR:
#602 (comment).

## Elements to add tests for

- [x] `pallet-dip-consumer`
- [x] `pallet-relay-store`
- [x] `ProofVerifier`
- [x] Merge #612
- [x] Merge #613
ntn-x2 added a commit that referenced this pull request Apr 4, 2024
Partially fixes https://github.com/KILTprotocol/ticket/issues/2562.

Adds unit tests for the verifier components (components tested are shown
in the checklist below).
It also splits up the proof verification logic into multiple files,
split by when those checks are performed. Check the
`crates/kilt-dip-primitives/src/merkle/v0/` folder for more details. It
addresses an open comment in the DIP refactoring PR:
#602 (comment).

## Elements to add tests for

- [x] `pallet-dip-consumer`
- [x] `pallet-relay-store`
- [x] `ProofVerifier`
- [x] Merge #612
- [x] Merge #613
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants