Implement identity (recovery) request #6
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t functions UDL signatures: - `string identity_issuance_request_json(IdentityIssuanceRequestParameters params)` - `string identity_recovery_request_json(IdentityRecoveryRequestParameters params)` - `AccountCredentialResult account_credential(AccountCredentialParameters params)` - `string account_credential_deployment_hash_hex(AccountCredential credential, u64 expiry_unix)` - `string account_credential_deployment_signed_payload_hex(SignedAccountCredential credential)` The parameter types mirror the corresponding "input" types from `wallet_library` but use only types supported by UniFFI. The values are translated into these library types via JSON encoding/decoding. This is in contrast to the Java SDK where the value is passed as a JSON encoded string which is then decoded directly into the library input type. Doing it this way ensures that type unsafe conversions happen internally in this library where it's easily tested rather than at the FFI boundary. So it makes the FFI boundary type safe and of course also generates the Swift types that we do need on the SDK side anyway. The identity request functions return their result as JSON encoded strings because the protocol actually is to just send the object as JSON in a URL parameter. So there's no point in decoding them into structured types - they would just be converted right back to JSON on the SDK side. We do decode the payload in a unit test to verify the format.
bisgardo
force-pushed
the
identity-recovery-request
branch
from
3f51714
to
18ff9a7
Compare
bisgardo
force-pushed
the
identity-recovery-request
branch
from
19bc9c7
to
f28f1cd
Compare
|
Note that the PR is stacked on #8. |
bisgardo
commented
Mar 8, 2024
bisgardo
force-pushed
the
identity-recovery-request
branch
from
f28f1cd
to
f8ec3b5
Compare
|
Ping @eb-concordium @abizjak, I'm currently blocked by this to proceed on the SDK side. If it helps I can confirm that it all works from there. |
It's no longer stacked. |
abizjak
suggested changes
Mar 15, 2024
- Inlined type aliases: They were a somewhat arbitrary set of the used types anyway and weren't used for FFI. There are similar types in concordium-base except those are not aliases which means that UniFFI doesn't support them automatically. - Reconcile `CredentialPublicKeysHex` with `CredentialPublicKeysWithScheme` into `CredentialPublicKeys` as it turns out that the string (hex) variant is unnecessary because the scheme variant can be used instead of it. - Unexport internal struct`CredentialDeploymentPayloadHashInput` and rename its field `expiry_unix` to `expiry_unix_secs` to clarify the unit. - Remove test structs that are just boiled down versions of the actual types. This changes the tests to verify the complete format but no longer any concrete values. - Changelog: Minor clarification of rationale.
abizjak
reviewed
Mar 19, 2024
bisgardo
force-pushed
the
identity-recovery-request
branch
from
2248a46
to
10d3c9f
Compare
abizjak
approved these changes
Mar 19, 2024
In the Rust code, the docstrings explain the link between the types generated from UDL and the "original" ones from the core Concordium libraries that they bridge to. To properly refer to types located in concordium-base in the docstrings, an explicit dependency to that library has been added. The docstrings on the UDL types are mostly what I could find in the docstrings on the original types. As the dependency to concordium-base was added, the struct `Versioned` in `lib_test` was replaced by its original.
bisgardo
force-pushed
the
identity-recovery-request
branch
from
10d3c9f
to
cb30d87
Compare
Before this we only verified that the result has the correct JSON format.
bisgardo
force-pushed
the
identity-recovery-request
branch
from
9049f01
to
8e02aeb
Compare
This was referenced Apr 2, 2024
bisgardo
added a commit
to Concordium/concordium-swift-sdk
that referenced
this pull request
Apr 9, 2024
The `NodeClient` protocol is extended with methods for querying IPs and ARs and a method for sending a credential deployment. The IP info exposed via gRPC doesn't include the URLs for starting identity issuance and recovery, so Wallet Proxy integration is added for use when those are needed. The crypto functions for construction identity/credential requests added in Concordium/concordium-wallet-crypto-swift#6 are integrated via `WalletSeed`. The credential deployment serialization functions are integrated in `Transaction.swift`. URLs to invoke the IPs are conveniently built using helpers in `WalletIdentity.swift`. Due to the fact that IPs respond with identity objects etc. encoded as JSON, the relevant types from `ConcordiumWalletCrypto` are extended to conform with `Decodable`. A convenient helper for building the request payloads is included as well. The example CLI is extended to support all the new functionality via the commands: - `wallet identity issue` (example: `concordium-example-client wallet --seed-phrase="..." --identity-provider-id=1 --identity-index=2 identity issue`) - `wallet identity recover` (example: `concordium-example-client wallet --seed-phrase="..." --identity-provider-id=1 --identity-index=2 identity recover`) - `wallet identity create-account` (example: `concordium-example-client wallet --seed-phrase="..." --identity-provider-id=1 --identity-index=2 identity create-account --credential-counter=0`) The readme includes all relevant details.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds the functions with the following UDL signatures:
string identity_issuance_request_json(IdentityIssuanceRequestParameters params)string identity_recovery_request_json(IdentityRecoveryRequestParameters params)AccountCredentialResult account_credential(AccountCredentialParameters params)string account_credential_deployment_hash_hex(AccountCredential credential, u64 expiry_unix)string account_credential_deployment_signed_payload_hex(SignedAccountCredential credential)The parameter types mirror the corresponding "input" types from
wallet_librarybut use only types supported by UniFFI. The values are translated into these library types via JSON encoding/decoding. This is in contrast to the Java SDK where the value is passed as a JSON encoded string which is then decoded directly into the library input type. Doing it this way ensures that type unsafe conversions happen internally in this library where it's easily tested rather than at the FFI boundary. So it makes the FFI boundary type safe and of course also generates the Swift types that we do need on the SDK side anyway.The identity request functions return their result as JSON encoded strings because the protocol actually is to just send the object as JSON in a URL parameter. So there's no point in decoding them into structured types - they would just be converted right back to JSON on the SDK side. We do decode the payload in a unit test to verify the format.
The existing functions have been renamed and reordered to match the conventions and usage in the SDK, so this is a breaking change. Their implementations have not changed.