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

feat: add meaningful errors #572

Merged
merged 12 commits into from
Oct 31, 2023
Merged

feat: add meaningful errors #572

merged 12 commits into from
Oct 31, 2023

Conversation

ntn-x2
Copy link
Member

@ntn-x2 ntn-x2 commented Oct 20, 2023

Fixes https://github.com/KILTprotocol/ticket/issues/2552, by both removing all occurrences of .expect() and .unwrap() and replacing them with Internal errors, and by introducing error enums. The logic is that for both the provider and the consumer, a generic error is bubbled up from the runtime, and because of the constraints imposed by Substrate, the error reason is encoded as a u16, where each "class" of error is namespaced with a 255 value increment. A snippet of this is shown below:

fn from(
    value: DipSiblingProviderStateProofVerifierError<
        ParachainHeadMerkleProofVerificationError,
        IdentityCommitmentMerkleProofVerificationError,
        DipProofVerificationError,
        DidSignatureVerificationError
    >,
) -> Self {
    match value {
        DipSiblingProviderStateProofVerifierError::ParachainHeadMerkleProofVerificationError(error) => {
            error.into()
        }
        DipSiblingProviderStateProofVerifierError::IdentityCommitmentMerkleProofVerificationError(error) => {
            u8::MAX as u16 + error.into() as u16
        }
        DipSiblingProviderStateProofVerifierError::DipProofVerificationError(error) => {
            u8::MAX as u16 * 2 + error.into() as u16
        }
        DipSiblingProviderStateProofVerifierError::DidSignatureVerificationError(error) => {
            u8::MAX as u16 * 3 + error.into() as u16
        }
    }
}

The specific reason for the error can then be retrieved and decoded directly from the Polkadot Apps interface, as the example below shows, where the first byte 01 indicates the enum variant index, the second byte is the varint encoding of the SCALE-encoded u16, and the third and forth bytes e000 represent the u16 error reason. This can be decoded as decimal, where each value has a one-to-one link to a specific error from the runtime.

The pallet consumer expects any other implementation of the DIP consumer protocol to expose the proof verification error reason as a u16, but of course developers are free to not give any meaning to it and treat everything as a generic InvalidProof error.

This was referenced Oct 31, 2023
@ntn-x2
Copy link
Member Author

ntn-x2 commented Oct 31, 2023

Merging without reviews. It will be reviewed later on from the aa/dip branch directly.

@ntn-x2 ntn-x2 merged commit 85019bb into aa/dip Oct 31, 2023
@ntn-x2 ntn-x2 deleted the aa/error-handling branch October 31, 2023 13:41
ntn-x2 added a commit that referenced this pull request Oct 31, 2023
Fixes KILTprotocol/ticket#2971 on top of
#572.

This PR adds support for versioning, by introducing support for the
following:

1. [PROVIDER PALLET] Generate an identity commitment for a specific
version
2. [PROVIDER PALLET] Delete a previously-created identity commitment of
a specific version
3. [PROVIDER RUNTIME API] Generate a DIP proof for a specific version
4. [CONSUMER] Add `Versioned*` types for DIP verifiers and DIP proofs
running on both a sibling parachain or the parent relaychain
ntn-x2 added a commit that referenced this pull request Dec 14, 2023
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
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
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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant