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

Allow arbitrary verification methods #1334

Merged
merged 8 commits into from
Mar 18, 2024
Merged

Allow arbitrary verification methods #1334

merged 8 commits into from
Mar 18, 2024

Conversation

UMR1352
Copy link
Contributor

@UMR1352 UMR1352 commented Mar 12, 2024

Description of change

Add support for arbitrary (custom) verification methods.

Links to any relevant issues

Fixes issue #1322

Type of change

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

New unit tests for the custom verification method data.

Change checklist

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@UMR1352 UMR1352 added Wasm Related to Wasm bindings. Becomes part of the Wasm changelog Added A new feature that requires a minor release. Part of "Added" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. labels Mar 12, 2024
@UMR1352 UMR1352 added this to the v1.2 milestone Mar 12, 2024
@UMR1352 UMR1352 self-assigned this Mar 12, 2024
@UMR1352 UMR1352 requested a review from a team as a code owner March 12, 2024 16:20
@UMR1352 UMR1352 linked an issue Mar 12, 2024 that may be closed by this pull request
[Task] Allow arbitrary verification methods #1322
Closed
6 tasks
lucagiorgino
lucagiorgino reviewed Mar 14, 2024
View reviewed changes
Copy link
Collaborator

@lucagiorgino lucagiorgino left a comment

Choose a reason for hiding this comment

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

Hi 👋🏻,
I checked the code and didn't find anything worth mentioning. I was able to test this new custom type for my needs, and it works properly.

The only issue I noticed is that the document is not displayed in the Explorer (undefined is displayed) . However, this is unrelated to the code. It's likely that the Explorer needs to be updated to use the next version of the library to reconstruct the DID document correctly.

Example: https://explorer.shimmer.network/testnet/addr/rms1pqqcmsxfsupcmyjreysaewnynlvgu25tdm4csp628ejmak9xscqavlzhfvn?tab=DID

UMR1352 and others added 3 commits March 14, 2024 15:54
@UMR1352 @abdulmth
Update bindings/wasm/src/verification/wasm_method_data.rs
84a5586 
Co-authored-by: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>
@UMR1352
review comments
c42c50f
@UMR1352
fmt
b6772e7
@eike-hass
Copy link
Collaborator

Hi 👋🏻, I checked the code and didn't find anything worth mentioning. I was able to test this new custom type for my needs, and it works properly.

The only issue I noticed is that the document is not displayed in the Explorer (undefined is displayed) . However, this is unrelated to the code. It's likely that the Explorer needs to be updated to use the next version of the library to reconstruct the DID document correctly.

Example: https://explorer.shimmer.network/testnet/addr/rms1pqqcmsxfsupcmyjreysaewnynlvgu25tdm4csp628ejmak9xscqavlzhfvn?tab=DID

Your assumption is correct, the explorer needs to be updated to the upcoming version of the library to support arbitrary verification methods. Not the prettiest error state though 🙈

wulfraem
wulfraem requested changes Mar 18, 2024
View reviewed changes
identity_verification/src/verification_method/method.rs Outdated Show resolved Hide resolved
identity_verification/src/verification_method/method.rs Outdated Show resolved Hide resolved
bindings/wasm/docs/api-reference.md Outdated Show resolved Hide resolved
identity_verification/src/verification_method/method.rs Outdated
Comment on lines 274 to 276
let data_json = serde_json::to_value(&data).unwrap();
let data_type = data_json.as_object().unwrap().into_iter().next().unwrap().0;
properties.remove(data_type);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I guess we can just ignore this rant here, but couldn't bring myself to not complain about the .unwrap() here. :D

Could this be refactored into a match/if-let version that just removes the type (also adressed by its name instead of its relative position in the deserialized JSON) and does nothing if for whatever reasons the field couldn't be found?

Sorry for stirring up a (most probably useless) discussion but I've to be reaaally careful with .unwrap()s in library code. ^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wulfraem Thanks for bringing this up. That was code I forgot to refactor after a first quick and dirty phase just to check if I could work around serde's issue. Lemme know what do you think of it now

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me. ^^

@UMR1352 UMR1352 merged commit 0af29fc into main Mar 18, 2024
11 checks passed
@UMR1352 UMR1352 deleted the feat/custom-verification-method branch March 18, 2024 16:17
@eike-hass eike-hass changed the title Feat/custom verification method Allow arbitrary verification method Mar 27, 2024
@eike-hass eike-hass changed the title Allow arbitrary verification method Allow arbitrary verification method Mar 27, 2024
@eike-hass eike-hass changed the title Allow arbitrary verification method Allow arbitrary verification methods Mar 27, 2024
UMR1352 added a commit that referenced this pull request Apr 25, 2024
@UMR1352
Squashed commit of the following:
be022b0 
commit 30c9bf2
Author: Foorack / Max Faxälv <max@foorack.com>
Date:   Tue Apr 2 10:32:48 2024 +0200

    inherit `repository` in identity_verification (#1348)

commit 1e9c9a3
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Mar 27 15:35:29 2024 +0100

    Release wasm-v1.2.0 (#1345)

commit 84a630d
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Mar 27 15:32:19 2024 +0100

    Release v1.2.0 (#1347)

commit 1aba4b5
Author: Eike Haß <eike-hass@web.de>
Date:   Wed Mar 27 13:13:27 2024 +0100

    removed dev_dep version

commit 0352b84
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Wed Mar 27 10:44:43 2024 +0100

    Support %-encoded characters in DID method id (#1303)

commit e68538f
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Tue Mar 26 11:58:35 2024 +0100

    gRPC bindings (#1264)

commit e53561e
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Tue Mar 26 11:18:14 2024 +0100

    allow large result err variants (#1342)

commit 4a144a3
Author: Eike Haß <eike-hass@web.de>
Date:   Tue Mar 19 09:51:52 2024 +0100

    fix readme links (#1336)

commit 0af29fc
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Mon Mar 18 17:16:57 2024 +0100

    Feat/custom verification method (#1334)

    * Add support for arbitrary (custom) verification method data

    * wasm bindings

    * custom method type + wasm

    * workaround serde's issue

    * Update bindings/wasm/src/verification/wasm_method_data.rs

    Co-authored-by: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>

    * review comments

    * fmt

    * review comment

    ---------

    Co-authored-by: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>

commit edb9150
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Tue Mar 12 14:45:04 2024 +0100

    use latest release of sd-jwt-payload (#1333)

    * use latest release of sd-jwt-payload

    * make clippy happy

commit 0794379
Author: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>
Date:   Wed Mar 6 14:16:00 2024 +0100

    Wasm bindings for `BlockChainAccountId` verification method. (#1326)

commit 59d38f7
Author: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>
Date:   Wed Mar 6 10:56:23 2024 +0100

    Add constructor for VerificationMethod in TS (#1321)
@lucagiorgino lucagiorgino mentioned this pull request May 24, 2024
Open
UMR1352 added a commit that referenced this pull request May 24, 2024
@UMR1352 @AlbertoSvg @wulfraem
Add ZK BBS+-based selectively disclosable credentials (JPT) (#1355)
f25c593 
* Support BBS+ and JWP (#1285)

* merge main

* Wasm bindings for Jpt credentials

* JPT presentation bindings

* docs

* jsonprooftoken payloads

* Refactor `RevocationTimeframeStatus` to align with other setups (#1340)

* refactor `RevocationTimeframeStatus` to other setups

* fix smaller typos

* binding coverage for jsonprooftoken

* Use latest releases of zkryptium/json-proof-token and add new BLS key representation (#1339)

* update zkryptium/json-proof-token deps and new BLS key representation

* minor fix

* Use zkryptium for cryptographic operations inside Memstore (#1351)

* update zkryptium/json-proof-token deps and new BLS key representation

* minor fix

* use zkryptium for crypto operations and JPT for serialization

* fix format

* Feat/jpt bbs+ sd stronghold impl (#1354)

* Implement JwkStorageExt for StrongholdStorage

* reorganize code

* persist changes to stronghold when creating bbs+ keypair, clippy, fmt

* feature gate

* zkp wasm example

* zkp_revocation wasm example

* wasm bindings

* fix docs

* rename JwkStorageExt to JwkStorageBbsPlusExt

* JwkStorageBbsPlusExt impl refactor for Stronghold, MemStore, WasmStore

* Squashed commit of the following:

commit 30c9bf2
Author: Foorack / Max Faxälv <max@foorack.com>
Date:   Tue Apr 2 10:32:48 2024 +0200

    inherit `repository` in identity_verification (#1348)

commit 1e9c9a3
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Mar 27 15:35:29 2024 +0100

    Release wasm-v1.2.0 (#1345)

commit 84a630d
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Mar 27 15:32:19 2024 +0100

    Release v1.2.0 (#1347)

commit 1aba4b5
Author: Eike Haß <eike-hass@web.de>
Date:   Wed Mar 27 13:13:27 2024 +0100

    removed dev_dep version

commit 0352b84
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Wed Mar 27 10:44:43 2024 +0100

    Support %-encoded characters in DID method id (#1303)

commit e68538f
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Tue Mar 26 11:58:35 2024 +0100

    gRPC bindings (#1264)

commit e53561e
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Tue Mar 26 11:18:14 2024 +0100

    allow large result err variants (#1342)

commit 4a144a3
Author: Eike Haß <eike-hass@web.de>
Date:   Tue Mar 19 09:51:52 2024 +0100

    fix readme links (#1336)

commit 0af29fc
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Mon Mar 18 17:16:57 2024 +0100

    Feat/custom verification method (#1334)

    * Add support for arbitrary (custom) verification method data

    * wasm bindings

    * custom method type + wasm

    * workaround serde's issue

    * Update bindings/wasm/src/verification/wasm_method_data.rs

    Co-authored-by: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>

    * review comments

    * fmt

    * review comment

    ---------

    Co-authored-by: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>

commit edb9150
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Tue Mar 12 14:45:04 2024 +0100

    use latest release of sd-jwt-payload (#1333)

    * use latest release of sd-jwt-payload

    * make clippy happy

commit 0794379
Author: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>
Date:   Wed Mar 6 14:16:00 2024 +0100

    Wasm bindings for `BlockChainAccountId` verification method. (#1326)

commit 59d38f7
Author: Abdulrahim Al Methiab <31316147+abdulmth@users.noreply.github.com>
Date:   Wed Mar 6 10:56:23 2024 +0100

    Add constructor for VerificationMethod in TS (#1321)

* clippy

* fmt

* add stronghold bbs+ tests

* review comments

* add license header

* fix wasm bindings

* Persist Stronghold's changes only when its handle is dropped

* Fix StrongholdStorage::get_public_key

* rename stronghold_jwk_storage_ext

* Add inx-faucet profile in CI

* change stronghold crate's structure, revert persist changes on drop

* review comments

* Update identity_credential/src/presentation/jwp_presentation_builder.rs

Co-authored-by: wulfraem <wulfraem@users.noreply.github.com>

* fix wasm bindings

* expose stronghold's key types

* revert last commit

* Add "Fondazione Links" to license header

* Squashed commit of the following:

commit 9abdb38
Author: Sven <sven.feuchtmueller@gmx.de>
Date:   Tue May 14 09:16:09 2024 +0200

    Add EcDSA verifier (#1353)

    * add ecdsa verifier

    * add identity_ecdsa_verifier to workspace, add license headers

    * Update identity_ecdsa_verifier/Cargo.toml

    Co-authored-by: wulfraem <wulfraem@users.noreply.github.com>

    * Update identity_ecdsa_verifier/src/secp256k1.rs

    Co-authored-by: wulfraem <wulfraem@users.noreply.github.com>

    * Update identity_ecdsa_verifier/Cargo.toml

    Co-authored-by: wulfraem <wulfraem@users.noreply.github.com>

    * Update identity_ecdsa_verifier/src/secp256k1.rs

    Co-authored-by: wulfraem <wulfraem@users.noreply.github.com>

    * Update identity_ecdsa_verifier/src/secp256r1.rs

    Co-authored-by: wulfraem <wulfraem@users.noreply.github.com>

    * add feedback

    * add OpenSSL installation to windows runner in CI

    * update license headers and authors for ecdsa verifier

    * update license template to allow multiple contributors

    ---------

    Co-authored-by: Sebastian Wolfram <wulfraem@users.noreply.github.com>

commit 149bfac
Author: wulfraem <wulfraem@users.noreply.github.com>
Date:   Mon May 13 10:44:09 2024 +0200

    Fix findings after clippy update (#1365)

    * fix clippy findings

    * fix formatting

    * refactor .clone_into calls into .to_string

    * fix previous edit

    * disable empty_docs for wasm binding for now

    * fix missing newline

    * disable self update from rust setup in ci for now

    * update self update skip to skip only for windows build

commit 51aedd5
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Tue Apr 30 16:16:36 2024 +0200

    Use STRONGHOLD_PWD_FILE env variable to pass stronghold's password (#1363)

commit edec26c
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Tue Apr 30 15:40:55 2024 +0200

    Arbitrary data signing service (#1350)

commit f59e75a
Author: Eike Haß <eike-hass@web.de>
Date:   Tue Apr 30 15:34:40 2024 +0200

    Fix dockerhub workflow (#1343)

commit 993cfec
Author: Enrico Marconi <31142849+UMR1352@users.noreply.github.com>
Date:   Fri Apr 26 13:39:29 2024 +0200

    add inx-faucet profile (#1356)

* update stronghold and sdk

---------

Co-authored-by: Alberto Solavagione <albertosolavagione30@gmail.com>
Co-authored-by: wulfraem <wulfraem@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wulfraem wulfraem wulfraem approved these changes

@abdulmth abdulmth abdulmth left review comments

@lucagiorgino lucagiorgino lucagiorgino approved these changes

Assignees

@UMR1352 UMR1352

Labels
Added A new feature that requires a minor release. Part of "Added" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

[Task] Allow arbitrary verification methods
5 participants
@UMR1352 @eike-hass @wulfraem @lucagiorgino @abdulmth