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

Rewrite verification algorithm in a way that does not cause layer violations #1377

Closed
msporny opened this issue Dec 7, 2023 · 9 comments
Closed

Comments

@msporny
Copy link
Member

msporny commented Dec 7, 2023

Both @selfissued and @OR13 raised architectural layering concerns around the way that the verification algorithm is written. This issue is being raised to track their concerns.

@OR13
Copy link
Contributor

OR13 commented Dec 7, 2023

I think this is solved so long as there is no language in the core data model about validating proof, or extracting proof from a credential or presentation.

@iherman
Copy link
Member

iherman commented Dec 13, 2023

The issue was discussed in a meeting on 2023-12-13

  • no resolutions were taken
View the transcript

2.6. Rewrite verification algorithm in a way that does not cause layer violations (issue vc-data-model#1377)

See github issue vc-data-model#1377.

Brent Zundel: rewrite verification algorithm in a way that does not cause layering violations.
… can we close this issue.

Manu Sporny: I think the text in the spec is not optimal, and it needs to align with the new interface langauge to align with the securing mechanism, and its done.
… I could not figure out how to write that text, you orie, please propose some text.

Orie Steele: Yes, seems like we're in the process of addressing follow-up PRs on verification algorithm. I expect those follow up PRs to resolve this. I'm not sure I have special text to add. Current language isn't acceptable, heading in a good direction.

Brent Zundel: Can you make a concrete suggestion that would make the text acceptable?

Orie Steele: Yes, I think Mike Jones is planning to do that already? I'll leave it to him.

Brent Zundel: can you make a suggestion for text?

Manu Sporny: I prefer to explicitly assign mike jones to this.

Brent Zundel: Mike is assigned, orie, hopefully you can communicate to mike that he is assigned.

@msporny
Copy link
Member Author

msporny commented Dec 18, 2023

PR #1393 has started to define an interface that, if merged, will enable this PR to be closed (by utilizing an interface that won't cause layer violations).

@msporny
Copy link
Member Author

msporny commented Dec 18, 2023

PR #1394 has been raised to address this issue. This issue will be closed when PR #1394 has been merged.

@msporny msporny added pr exists and removed ready for PR This issue is ready for a Pull Request to be created to resolve it labels Dec 18, 2023
@msporny msporny self-assigned this Dec 18, 2023
@iherman
Copy link
Member

iherman commented Dec 20, 2023

The issue was discussed in a meeting on 2023-12-19

  • no resolutions were taken
View the transcript

2.3. Use new Securing Mechanism Verify Proof interface (pr vc-data-model#1394)

See github pull request vc-data-model#1394.

Brent Zundel: I'm wondering if it makes sense, Manu, to talk about 1394 before something else.

Manu Sporny: I'm fine with any order. There are 3-4 that all build on each other, but they change different things.

Brent Zundel: I was seeing the building -- but if the directionality isn't vital we'll just go.

See github issue vc-data-model#1377.

Brent Zundel: "Use new securing mechanism verify proof interface". This PR has been out for a day.

Manu Sporny: This PR is an attempt -- at addressing the same concerns that Mike is trying to address in 1397. The way Jeffrey Yaskin and I are trying to address this is by saying the securing mechanisms must provide an interface that can give you back the secured data.
… This basically says that that interface is in place. It says that a securing mechanism must define that interface and you need to take these inputs and give back these outputs.
… Once that's in place, then we can address the layering violations -- it means we can drop a lot of language around processing proof and so on. I think it achieves what Mike's PR is also achieving, or part of it, by just using the interface.
… By using the interface, we have gotten rid of mention of proof in the algorithms and the need to have a different verification mechanism algorithm. The interface makes it so we don't have to define that extra algorithm.
… It defines what the interface should be for the securing mechanisms and uses that in the verification algorithm and we have a simple, three step algorithm.

Brent Zundel: Please jump on the queue if you've read this PR.

Michael Jones: I will review it, I saw that it existed and skimmed it but decided to finish my PR first.

Brent Zundel: Thank you, Mike.
… Any other comments?

Manu Sporny: Mike, when you're reviewing, keep in mind that this isn't an either-or -- it just simplifies a lot of language in your PR from there and we can see what deltas remain.

Michael Jones: Makes sense.

Brent Zundel: Thank you both.

@iherman
Copy link
Member

iherman commented Dec 21, 2023

The issue was discussed in a meeting on 2023-12-20

  • no resolutions were taken
View the transcript

4.7. Rewrite verification algorithm in a way that does not cause layer violations (issue vc-data-model#1377)

See github issue vc-data-model#1377.

Michael Jones: PR 1397 is also about this.

Brent Zundel: Issue 1377 a number of PRs 1393, 1394. 1394 has a number of approvals. Though some comments. 1393 not as clear. Already discussed.
… a number of PRs are in a good place likely to be merged soon. Some are not. PRs would be closed and issues closed or deferred.
… Expressing gratitude to all.

Manu Sporny: Yes, thank you to everyone for the work and passion they've put into the work! Happy Holidays! Happy New Year! :).

Phillip Long: +1 to Brent's expression of thankfulness ;-).

David Chadwick: +1.


@msporny
Copy link
Member Author

msporny commented Dec 26, 2023

PR #1394 has been merged. Delaying closing this until PR #1397 is revised or closed.

@iherman
Copy link
Member

iherman commented Jan 4, 2024

The issue was discussed in a meeting on 2024-01-03

  • no resolutions were taken
View the transcript

2.1. Rewrite verification algorithm in a way that does not cause layer violations (issue vc-data-model#1377)

See github issue vc-data-model#1377.

Brent Zundel: there are 4 issues, let's talk about them. first one 1377: Rewrite verification algorithm in a way that does not cause layer violations. looking for a quick status update. all have PR exists. can we successfully address this issue?

Manu Sporny: for 1377 believe the vast majority of the changes are already in. Need Mike (selfissued) to look and see what delta is remaining. Believe changes applied through other PRs.

Michael Jones: are there more that you believe impact my PR? should I try to resolve merge conflicts and see what happens?

Manu Sporny: try to resolve merge conflicts and see what's left.

Michael Jones: ack.

Ted Thibodeau Jr.: Not just merge conflicts. A number of suggestions have not yet been acted on.

Brent Zundel: speaking to a pull request?

Ted Thibodeau Jr.: 1397.
… suggestions should be applied before the delta is analyzed/conflicts resolved.

Manu Sporny: quick heads up to selfissued, there are still some open PRs which impact your PR. just be aware of that. all listed in 1377.

Michael Jones: do those look like they're going to land?

Manu Sporny: yes.

Michael Jones: thanks will look at those first.

@msporny msporny removed their assignment Jan 6, 2024
@msporny
Copy link
Member Author

msporny commented Jan 13, 2024

Both PRs #1394 and #1397 have been merged, closing.

@msporny msporny closed this as completed Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants