Skip to content

wip: didcomm - #70

Draft
cosminm-affnd wants to merge 33 commits into
mainfrom
didcomm
Draft

wip: didcomm#70
cosminm-affnd wants to merge 33 commits into
mainfrom
didcomm

Conversation

@cosminm-affnd

Copy link
Copy Markdown
Collaborator

No description provided.

@cosminm-affnd
cosminm-affnd requested a review from a team as a code owner April 24, 2025 11:51
@cosminm-affnd
cosminm-affnd marked this pull request as draft April 24, 2025 11:52
Comment thread example/didcomm.dart Outdated

DidcommSignedMessage aliceSignedMessage =
await DidcommSignedMessage.fromPlaintext(
wallet: aliceWallet, keyId: alicePublicKey.id, message: message);

@cosminm-affnd cosminm-affnd Apr 24, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have DidSigner to wrap the wallet and keyId for most simple sign operations.

Is passing the wallet strictly needed for DidcommSignedMessage.fromPlaintext?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you @cosminm-affnd. I somehow tried to avoid using DidSigner as it looked much more complex as a consumer to use. But trying it out again it makes the signing implementation much more easier and flexible within the DidcommSignedMessage class.

Comment thread example/didcomm.dart Outdated
DidcommPlaintextMessage message = DidcommPlaintextMessage(
id: '2fb19055-581d-488e-b357-9d026bee98fc',
to: [bobDidDoc.id],
from: aliceDidDoc.id,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

minor: how do we keep the value here in sync with the key used to sign the message?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I changed DidcommSignedMessage to set from value using DidSigner.did if not set on message before.

@maindotdev
maindotdev force-pushed the didcomm branch 5 times, most recently from 438d076 to ec50724 Compare April 29, 2025 10:38
Comment thread example/didcomm.dart
final bobsEncryptedMessage = await bobsSignedMessage.encrypt(
wallet: bobWallet,
keyId: bobKeyPair.id,
recipientPublicKeyJwks: [aliceJwk.toJson()]);

@ivan-branets-affinidi ivan-branets-affinidi Apr 29, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is conventional in Dart to add , after the last parameter. Here is an example of another affinidi repo

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The example uses ,after last parameter for function definition. But when using a function it doesn't make use of a ,. As we already discussed, it would be nice if this will be done automatically by the linter.

Comment thread example/didcomm.dart
recipientPublicKeyJwks: [aliceJwk.toJson()]);

final getFromJson =
DidcommEncryptedMessage.fromJson(bobsEncryptedMessage.toJson());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

toJson and then fromJson - is it an attempt to clone an object? Could we have clone method for better clarity?
getFromJson - usually method names starts with get, but not object names

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This example just shows how to convert messages to and from json as you would do when you send it to the mediator. After integrating the mediator client we can combine these.

Comment thread lib/src/didcomm/attachment/attachment.dart Outdated
this.lastmodTime,
this.byteCount});

Attachment.fromJson(dynamic jsonData) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It could be beneficial to use JsonSeriflized to reduce toJson and fromJson boilerplate

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes I agree. This is something that should be discussed with the SSI package team as it introduces another dependency.

Comment thread lib/src/didcomm/didcomm_jwt.dart Outdated
Comment thread lib/src/didcomm/didcomm_jwt.dart Outdated
var splitted = jwtCompact.split('.');
if (splitted.length != 3) {
throw FormatException(
'compact serialization must consist of three parts separated by point(.).');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Error should be sentence case, simior to another affinidi package

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I changed it. Is this what you would expect?

Comment thread lib/src/didcomm/didcomm_jwt.dart Outdated
Comment thread lib/src/didcomm/didcomm_jwt.dart Outdated
Comment thread lib/src/didcomm/didcomm_jwt.dart Outdated
Comment thread lib/src/didcomm/didcomm_jwt.dart Outdated
Comment thread lib/src/didcomm/didcomm_jwt.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_encrypted_message.dart
Comment thread lib/src/didcomm/message/didcomm_encrypted_message.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_encrypted_message.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_encrypted_message.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_encrypted_message.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_encrypted_message.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_message.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_plaintext_message.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_plaintext_message.dart Outdated
Comment thread lib/src/didcomm/message/didcomm_plaintext_message.dart Outdated
Comment thread lib/src/didcomm/message/jwe_header.dart Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants