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

test: DIP consumer tests #611

Merged
merged 40 commits into from
Mar 28, 2024
Merged

test: DIP consumer tests #611

merged 40 commits into from
Mar 28, 2024

Conversation

ntn-x2
Copy link
Member

@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
Member 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
Member

@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

@Ad96el
Copy link
Member

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
Member

@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
Fixes KILTprotocol/ticket#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
2 checks passed
@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 KILTprotocol/ticket#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 KILTprotocol/ticket#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.

2 participants