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

feat: update templates and Dockerfile to support Zombienet #587

Merged
merged 4 commits into from
Nov 27, 2023
Merged

feat: update templates and Dockerfile to support Zombienet #587

merged 4 commits into from
Nov 27, 2023

Conversation

ntn-x2
Copy link
Member

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

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.

@ntn-x2 ntn-x2 self-assigned this Nov 23, 2023
@ntn-x2 ntn-x2 mentioned this pull request Nov 23, 2023
Merged
30 tasks
@ntn-x2 ntn-x2 force-pushed the aa/zombienet-support branch from c0c778e to 12f018c Compare November 24, 2023 09:40
Ad96el
Ad96el reviewed Nov 24, 2023
View reviewed changes
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. Nice changes. I didn't even know you could load the chain spec from a JSON file. I also appreciate the Dockerfile changes, making it possible to compile a specific binary seems reasonable. I don't see any issues there.

Could we perhaps avoid renaming the ChainSpec trait? I can imagine this being something to handle when updating Polkadot dependencies. I don't really see any benefits.

@ntn-x2 ntn-x2 merged commit 66b46be into aa/dip-comments Nov 27, 2023
1 check passed
@ntn-x2 ntn-x2 deleted the aa/zombienet-support branch November 27, 2023 09:31
ntn-x2 added a commit that referenced this pull request Nov 27, 2023
@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.
weichweich
weichweich reviewed Nov 28, 2023
View reviewed changes
Dockerfile
Comment on lines +9 to +13
ARG BINARY=kilt-parachain

COPY . .

RUN cargo build --locked --release --features $FEATURES
RUN cargo build --locked --release --features $FEATURES -p $BINARY
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI the reason to compile all binaries in the first stage was so that we only need to run the first stage once and only compile dependencies once. this would lead to a slightly longer first stage, but you only need to run it once.

Copy link
Member Author

Choose a reason for hiding this comment

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

Where do we make use of that information? I only see a second stage being used, and we don't, for instance, use the same first stage to also build the mashnet-node. Am I wrong?

Copy link
Contributor

Choose a reason for hiding this comment

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

We (at least try to) do that here: https://github.com/KILTprotocol/kilt-node/blob/develop/.maintain/build-image.sh
I would need to verify that this works. The last pipeline took ~80min

Copy link
Contributor

Choose a reason for hiding this comment

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

Also this PR breaks this script.

Copy link
Member Author

Choose a reason for hiding this comment

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

The first stage takes around 70 min anyway. But good to know. If that is the case, I will revert this commit before merging the DIP branch in develop.

@ntn-x2 ntn-x2 mentioned this pull request Dec 12, 2023
Merged
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 <48685760+Ad96el@users.noreply.github.com>
Co-authored-by: Chris Chinchilla <chris@chrischinchilla.com>
Co-authored-by: Albrecht <albrecht@kilt.io>
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 <48685760+Ad96el@users.noreply.github.com>
Co-authored-by: Chris Chinchilla <chris@chrischinchilla.com>
Co-authored-by: Albrecht <albrecht@kilt.io>
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 <48685760+Ad96el@users.noreply.github.com>
Co-authored-by: Chris Chinchilla <chris@chrischinchilla.com>
Co-authored-by: Albrecht <albrecht@kilt.io>
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 <48685760+Ad96el@users.noreply.github.com>
Co-authored-by: Chris Chinchilla <chris@chrischinchilla.com>
Co-authored-by: Albrecht <albrecht@kilt.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ad96el Ad96el Ad96el left review comments

@weichweich weichweich weichweich left review comments

@trusch trusch Awaiting requested review from trusch

Assignees

@ntn-x2 ntn-x2

Labels
🗺️ DIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ntn-x2 @weichweich @Ad96el