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.

Sign up for GitHub

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

Jump to bottom

chore: add DIP-related comments #584

Merged
merged 18 commits into from
Dec 6, 2023
Merged

chore: add DIP-related comments #584

merged 18 commits into from
Dec 6, 2023

Conversation

ntn-x2
Copy link
Member

@ntn-x2 ntn-x2 commented Nov 20, 2023 •
edited
Loading

@ntn-x2 ntn-x2 self-assigned this Nov 20, 2023
@ntn-x2 ntn-x2 marked this pull request as ready for review November 21, 2023 10:20
@ntn-x2 ntn-x2 mentioned this pull request Nov 23, 2023
Merged
30 tasks
@ntn-x2
Documentation for provider pallet
ee92d76
@ntn-x2
Rust docs for traits
cf9a2e4
@ntn-x2
Add comments for dip-consumer
e87a820
@ntn-x2
Add comments to pallet-relay-store pallet
5911596
@ntn-x2
Add docs for the pallet-deposit-storage pallet
eaed208
@ntn-x2
wip kilt-dip-support docs
fa1906b
@ntn-x2
Minor docs fixes after fix rebasing
2d29592
@ntn-x2
Add rustdoc for DIP runtime-common
1ac0524
@ntn-x2
Wip docs for kilt-dip-support crate
82e275b
@ntn-x2
Comments complete and few renaming
cb38ef8
@ntn-x2
Template runtimes comments
ececfff
@ntn-x2
Finalize comments
1763f29
@ntn-x2
Remove unnecessary clippy ignores
f113e45
@ntn-x2
feat: update templates and Dockerfile to support Zombienet (#587)
f4e8bbf 
Zombienet uses the empty chainspec for a parachain, and that was not
supported by the node templates. Also, the Dockerfile assumes the
`kilt-parachain` is being run, but this is different now, and could also
be different in the future, so the binary target should be parametrized:
anyway, it is not necessary to compile the whole workspace if only a
specific binary is required.
ChrisChinchilla
ChrisChinchilla requested changes Nov 30, 2023
View reviewed changes
Copy link
Collaborator

@ChrisChinchilla ChrisChinchilla left a comment

Choose a reason for hiding this comment

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

A bunch of small tweaks for grammar polish, but overall good!

crates/kilt-dip-support/src/lib.rs Outdated Show resolved Hide resolved
crates/kilt-dip-support/src/lib.rs Outdated Show resolved Hide resolved
crates/kilt-dip-support/src/merkle.rs Outdated Show resolved Hide resolved
crates/kilt-dip-support/src/merkle.rs Outdated Show resolved Hide resolved
crates/kilt-dip-support/src/merkle.rs
/// verification.
/// The Merkle proof is assumed to have been generated using one of the
/// versioned identity commitment generators, as shown in the [KILT runtime
/// definitions](../../../runtimes/common/src/dip/README.md).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// definitions](../../../runtimes/common/src/dip/README.md).
/// definitions](https://github.com/KILTprotocol/kilt-node/blob/f4e8bbfe2a0fb6d5be8fe2172ee8eb868e72716a/runtimes/common/src/dip/README.md).

Maybe more stable so we can use the same link here and when rendered, but we will need to update when it's merged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ntn-x2 I went to check how these links looked rendered and I can't seem to find the text anywhere. Do you know where it is rendered? Or if maybe it's being skipped?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if this is more stable. It points to a specific branch, which can easily get outdated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ntn-x2 As said, we should update it to master/main when it is stable, but more important was my comment about the fact that these don't een render anyway… Any thoughts?

pallets/pallet-dip-provider/README.md Show resolved Hide resolved
runtimes/common/src/dip/README.md Outdated Show resolved Hide resolved
runtimes/common/src/dip/README.md Outdated
The V0 of the KILT DIP Provider specification defines the following components:

* **Identity details**: What are the pieces of a KILT identity that can be used for cross-chain transactions. V0 defines them to include the following information:
* All `DidKey`s stored under the subject's DID Document. For more details about how these keys are defined, please check the [KILT DID pallet](../../../../pallets/did).
Copy link
Collaborator

Choose a reason for hiding this comment

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

And same here, we should probably replace all these with hard URLs when we have them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will keep this (and the others) open as a reminder.

runtimes/common/src/dip/README.md Outdated Show resolved Hide resolved
runtimes/common/src/dip/README.md Outdated Show resolved Hide resolved
ChrisChinchilla
ChrisChinchilla requested changes Nov 30, 2023
View reviewed changes
Copy link
Collaborator

@ChrisChinchilla ChrisChinchilla left a comment

Choose a reason for hiding this comment

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

Addressing "bumps"

crates/kilt-dip-support/src/export/child.rs Outdated Show resolved Hide resolved
crates/kilt-dip-support/src/export/sibling.rs Outdated Show resolved Hide resolved
@ChrisChinchilla
Copy link
Collaborator

@ntn-x2 OK, I'm done, so take a run at the comments :)

@ntn-x2 ntn-x2 merged commit 8de6e8b into aa/dip Dec 6, 2023
1 of 2 checks passed
@ntn-x2 ntn-x2 deleted the aa/dip-comments branch December 6, 2023 11:20
ntn-x2 added a commit that referenced this pull request Dec 14, 2023
@ntn-x2 @Ad96el
@ChrisChinchilla @weichweich
feat: DIP v1 features (#494)
f9f4c3e 
Feature branch for everything DIP. It will collect other PRs until we
are happy with the features, and will add the DIP to some of our
runtimes and merge this into `develop`.

## WIP Checklist for the open tasks for v1

- [x] Basic structure ->
#489
- [x] Merkleization of DID Documents ->
#492
- [x] `RuntimeCall` verification logic ->
#502
- [x] DID signature verification ->
#516
- [x] Add support for linked accounts and web3name ->
#525
- [x] Configurable origin for `commit_identity` ->
#526
- [x] Proper fee management ->
#528
- [x] Update to Polkadot 0.9.43 ->
c18a6ce
- [x] Replace XCM with state proofs ->
#543
- [x] Add support for relaychain consumer ->
#553 (part of
#543)
- [x] Proper error handling ->
#572
- [x] Add support for versioning ->
#573
- [x] Take deposits for identity commitments ->
#574
- [x] Expose common definitions usable by consumers ->
#577
- [x] Change ensure_signed! to configurable origin also for the
`dispatch_as` function ->
#577
- [x] Proper benchmarking and weights ->
#585
- [x] Comments and docs ->
#584
- [x] Revert Dockerfile changes in
#587
- [x] [OPTIONAL] Add support for Zombienet ->
#587
- [x] [OPTIONAL] Add chain spec loading from file for template runtimes
-> #587
- [x] Big, final review ->
#494 (review)
- [x] Improvements n.1 PR ->
#591
- [x] Improvements n.2 PR ->
#592
- [x] Add to Peregrine runtime ->
#594
- [ ] Deploy on Peregrine
- [ ] Unit tests
- [ ] Add to Spiritnet runtime
- [ ] Deploy on Spiritnet
- [ ] [OPTIONAL] Move DIP-related stuff into its own repo

---------

Co-authored-by: Adel Golghalyani <[email protected]>
Co-authored-by: Chris Chinchilla <[email protected]>
Co-authored-by: Albrecht <[email protected]>
webguru9178 pushed a commit to webguru9178/kilt-node that referenced this pull request Jan 8, 2024
@ntn-x2 @Ad96el
@ChrisChinchilla @weichweich
feat: DIP v1 features (#494)
5755c98 
Feature branch for everything DIP. It will collect other PRs until we
are happy with the features, and will add the DIP to some of our
runtimes and merge this into `develop`.

## WIP Checklist for the open tasks for v1

- [x] Basic structure ->
KILTprotocol/kilt-node#489
- [x] Merkleization of DID Documents ->
KILTprotocol/kilt-node#492
- [x] `RuntimeCall` verification logic ->
KILTprotocol/kilt-node#502
- [x] DID signature verification ->
KILTprotocol/kilt-node#516
- [x] Add support for linked accounts and web3name ->
KILTprotocol/kilt-node#525
- [x] Configurable origin for `commit_identity` ->
KILTprotocol/kilt-node#526
- [x] Proper fee management ->
KILTprotocol/kilt-node#528
- [x] Update to Polkadot 0.9.43 ->
KILTprotocol/kilt-node@c18a6ce
- [x] Replace XCM with state proofs ->
KILTprotocol/kilt-node#543
- [x] Add support for relaychain consumer ->
KILTprotocol/kilt-node#553 (part of
KILTprotocol/kilt-node#543)
- [x] Proper error handling ->
KILTprotocol/kilt-node#572
- [x] Add support for versioning ->
KILTprotocol/kilt-node#573
- [x] Take deposits for identity commitments ->
KILTprotocol/kilt-node#574
- [x] Expose common definitions usable by consumers ->
KILTprotocol/kilt-node#577
- [x] Change ensure_signed! to configurable origin also for the
`dispatch_as` function ->
KILTprotocol/kilt-node#577
- [x] Proper benchmarking and weights ->
KILTprotocol/kilt-node#585
- [x] Comments and docs ->
KILTprotocol/kilt-node#584
- [x] Revert Dockerfile changes in
KILTprotocol/kilt-node#587
- [x] [OPTIONAL] Add support for Zombienet ->
KILTprotocol/kilt-node#587
- [x] [OPTIONAL] Add chain spec loading from file for template runtimes
-> KILTprotocol/kilt-node#587
- [x] Big, final review ->
KILTprotocol/kilt-node#494 (review)
- [x] Improvements n.1 PR ->
KILTprotocol/kilt-node#591
- [x] Improvements n.2 PR ->
KILTprotocol/kilt-node#592
- [x] Add to Peregrine runtime ->
KILTprotocol/kilt-node#594
- [ ] Deploy on Peregrine
- [ ] Unit tests
- [ ] Add to Spiritnet runtime
- [ ] Deploy on Spiritnet
- [ ] [OPTIONAL] Move DIP-related stuff into its own repo

---------

Co-authored-by: Adel Golghalyani <[email protected]>
Co-authored-by: Chris Chinchilla <[email protected]>
Co-authored-by: Albrecht <[email protected]>
Ad96el added a commit that referenced this pull request Feb 7, 2024
@ntn-x2 @Ad96el
@ChrisChinchilla @weichweich
feat: DIP v1 features (#494)
69033e0 
Feature branch for everything DIP. It will collect other PRs until we
are happy with the features, and will add the DIP to some of our
runtimes and merge this into `develop`.

## WIP Checklist for the open tasks for v1

- [x] Basic structure ->
#489
- [x] Merkleization of DID Documents ->
#492
- [x] `RuntimeCall` verification logic ->
#502
- [x] DID signature verification ->
#516
- [x] Add support for linked accounts and web3name ->
#525
- [x] Configurable origin for `commit_identity` ->
#526
- [x] Proper fee management ->
#528
- [x] Update to Polkadot 0.9.43 ->
c18a6ce
- [x] Replace XCM with state proofs ->
#543
- [x] Add support for relaychain consumer ->
#553 (part of
#543)
- [x] Proper error handling ->
#572
- [x] Add support for versioning ->
#573
- [x] Take deposits for identity commitments ->
#574
- [x] Expose common definitions usable by consumers ->
#577
- [x] Change ensure_signed! to configurable origin also for the
`dispatch_as` function ->
#577
- [x] Proper benchmarking and weights ->
#585
- [x] Comments and docs ->
#584
- [x] Revert Dockerfile changes in
#587
- [x] [OPTIONAL] Add support for Zombienet ->
#587
- [x] [OPTIONAL] Add chain spec loading from file for template runtimes
-> #587
- [x] Big, final review ->
#494 (review)
- [x] Improvements n.1 PR ->
#591
- [x] Improvements n.2 PR ->
#592
- [x] Add to Peregrine runtime ->
#594
- [ ] Deploy on Peregrine
- [ ] Unit tests
- [ ] Add to Spiritnet runtime
- [ ] Deploy on Spiritnet
- [ ] [OPTIONAL] Move DIP-related stuff into its own repo

---------

Co-authored-by: Adel Golghalyani <[email protected]>
Co-authored-by: Chris Chinchilla <[email protected]>
Co-authored-by: Albrecht <[email protected]>
Ad96el added a commit that referenced this pull request Apr 2, 2024
@ntn-x2 @Ad96el
@ChrisChinchilla @weichweich
feat: DIP v1 features (#494)
80c6ff7 
Feature branch for everything DIP. It will collect other PRs until we
are happy with the features, and will add the DIP to some of our
runtimes and merge this into `develop`.

## WIP Checklist for the open tasks for v1

- [x] Basic structure ->
#489
- [x] Merkleization of DID Documents ->
#492
- [x] `RuntimeCall` verification logic ->
#502
- [x] DID signature verification ->
#516
- [x] Add support for linked accounts and web3name ->
#525
- [x] Configurable origin for `commit_identity` ->
#526
- [x] Proper fee management ->
#528
- [x] Update to Polkadot 0.9.43 ->
c18a6ce
- [x] Replace XCM with state proofs ->
#543
- [x] Add support for relaychain consumer ->
#553 (part of
#543)
- [x] Proper error handling ->
#572
- [x] Add support for versioning ->
#573
- [x] Take deposits for identity commitments ->
#574
- [x] Expose common definitions usable by consumers ->
#577
- [x] Change ensure_signed! to configurable origin also for the
`dispatch_as` function ->
#577
- [x] Proper benchmarking and weights ->
#585
- [x] Comments and docs ->
#584
- [x] Revert Dockerfile changes in
#587
- [x] [OPTIONAL] Add support for Zombienet ->
#587
- [x] [OPTIONAL] Add chain spec loading from file for template runtimes
-> #587
- [x] Big, final review ->
#494 (review)
- [x] Improvements n.1 PR ->
#591
- [x] Improvements n.2 PR ->
#592
- [x] Add to Peregrine runtime ->
#594
- [ ] Deploy on Peregrine
- [ ] Unit tests
- [ ] Add to Spiritnet runtime
- [ ] Deploy on Spiritnet
- [ ] [OPTIONAL] Move DIP-related stuff into its own repo

---------

Co-authored-by: Adel Golghalyani <[email protected]>
Co-authored-by: Chris Chinchilla <[email protected]>
Co-authored-by: Albrecht <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisChinchilla ChrisChinchilla ChrisChinchilla requested changes

@trusch trusch Awaiting requested review from trusch

@weichweich weichweich Awaiting requested review from weichweich

@Ad96el Ad96el Awaiting requested review from Ad96el

Assignees

@ntn-x2 ntn-x2

Labels
🗺️ DIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ntn-x2 @ChrisChinchilla