Skip to content

Conversation

@NB-MikeRichardson
Copy link
Contributor

Merge of jsonld-credentials -> 0.3.0-pre

NB-MikeRichardson and others added 30 commits April 4, 2022 12:53
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Mike Richardson <mike.richardson@northernblock.io>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Karim <karim@animo.id>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
@TimoGlastra TimoGlastra changed the base branch from 0.3.0-pre to main October 24, 2022 06:18
NB-MikeRichardson and others added 4 commits October 24, 2022 10:46
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Merge AFJ main -> feat/jsonld-credentials
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
@TimoGlastra TimoGlastra self-requested a review October 27, 2022 14:57
NB-MikeRichardson and others added 4 commits October 31, 2022 09:46
Merge AFJ Main -> feat/jsonld-credentials
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
AFJ Main -> feat/jsonld-credentials
@morrieinmaas
Copy link
Contributor

This is urgent (to be merged) as it costs @NB-MikeRichardson a lot of overhead.

NB-MikeRichardson and others added 2 commits November 8, 2022 10:51
fix(connections): do not log AgentContext object (openwallet-foundation#1085)
Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
Comment on lines +20 to +25
createProposal: SignCredentialOptionsRFC0593
acceptProposal: SignCredentialOptionsRFC0593
createOffer: SignCredentialOptionsRFC0593
acceptOffer: SignCredentialOptionsRFC0593
createRequest: SignCredentialOptionsRFC0593
acceptRequest: JsonLdAcceptRequestOptions
Copy link
Contributor

Choose a reason for hiding this comment

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

The acceptXXX methods can take the values from the previous message. So if I receive a proposal and I agree with the contents of it, I call acceptProposal. What this will do is take the contents of the proposal and put those in the request. If i don't agree I will call negotiateProposal, which will in turn call createOffer.

So the types for acceptXX can be a lot simpler than createXXX. I'm fine with merging this PR now, but that means we'll have breaking changes soon to better differntiate between accept and create. We can mark the JSON ld api as experimental


import { SubjectInboundTransport } from '../../../tests/transport/SubjectInboundTransport'
import { SubjectOutboundTransport } from '../../../tests/transport/SubjectOutboundTransport'
import { BbsModule } from '../../bbs-signatures/src/BbsModule'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need the dependency on bbs here? It's fine for now, but ideally we only depend from bbs on core, not vice versa (also in tests)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am using the BbsModule here to set up agent modules in setting up e2e credential tests.

Is there some way to avoid this? Maybe we need to move the agent set up out of helper.ts...??

@TimoGlastra
Copy link
Contributor

Last remarks, then we can merge

@TimoGlastra
Copy link
Contributor

This is urgent (to be merged) as it costs @NB-MikeRichardson a lot of overhead.

Yes this has been outstanding for way too long. Sorry on this from my side for reviewing it slowly. I think as discussed before: smaller PRs that are easier to review. As a tip to you in the future Mike, please read the RFC carefully and handle all edge cases before making a PR. This will save me a lot of time in reviewing, and save you a lot of time in doing multiple round trips.

@NB-MikeRichardson
Copy link
Contributor Author

NB-MikeRichardson commented Nov 8, 2022

Advice on reading RFCs more carefully noted.

I will get these done asap. Please can you raise an issue with all these small issues that need to be done 'post merge.'

I am happy to work on them.

I have raised the issue: Issue #1089

Signed-off-by: Mike Richardson <mike.richardson@ontario.ca>
@NB-MikeRichardson NB-MikeRichardson deleted the feat/jsonld-credentials branch November 14, 2022 08:42
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.

Add support for RFC 0593: JSON-LD Issue Credential

10 participants