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

Write grant per atomic issuance #68

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

sapience
Copy link
Contributor

@sapience sapience commented Feb 4, 2025

Task

https://www.notion.so/idos-association/Write-Grant-per-atomic-Issuance-15788c585fe080a1833edf8e39505492?pvs=4

Changes in API

TBD

How do you know this works?

TBD

TODO

  • Description, API changes
  • Tests
  • Adopt SDK and demo apps
  • Delete write_grants table

@sapience sapience requested a review from pkoch February 4, 2025 11:19
@@ -91,6 +91,16 @@ table write_grants {
foreign_key (wg_owner_user_id) references users(id) on_delete cascade
}

table consumed_write_grants {
Copy link
Member

Choose a reason for hiding this comment

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

During the meeting, I'm pretty sure (but my memory sucks) that we settled on saving the whole message. So we either keep the message bytes, or we keep all the fields necessary to re-generate the message.

Copy link
Contributor Author

@sapience sapience Feb 4, 2025

Choose a reason for hiding this comment

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

In the meeting we decided that we need to prevent the following cases:

  1. Reuse consumed WG
  2. If there were orig_id and copy_id credentials had created, and then orig_id was deleted, there is no way to recreate orig_id (and pair it with copy_id) by the consumed or any other new WGs.
  3. If there were orig_id and copy_id credentials had created, and then copy_id was deleted, there is no way to recreate copy_id (and pair it with orig_id) by the consumed or any other new WGs.

Also we agreed that User can create another WG (message) to allow to Issuer to create another instance of the same VC (with different orig_id and copy_id).

So to achieve prevent to reuse consumed WG the simplest way is to generate WG id by User and put it in the message. It is unique id and User can't put it in another WG. Storing message signature in consumed_write_grants also solve this requirement, but what the reason to do it this way if storing WG id is enough?

or we keep all the fields necessary to re-generate the message

Please remind me why we might need to re-generate the message?

Copy link
Member

Choose a reason for hiding this comment

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

You are correct in your exposition, but (again, if I recall correctly) keeping the whole message was decided to sidestep the problem of "what fields should we keep around". I proposed that we don't make a decision for now, keep everything, and trim things in the future when we're more familiar with how people will want to use the system.

I'll also disambiguate this with Martin.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added by this 3ec5a97

SELECT CASE WHEN (SELECT count(1) FROM access_grants WHERE id = uuid_generate_v5('31276fd4-105f-4ff7-9f64-644942c14b79'::uuid, format('%s-%s-%s', $grantee_wallet_identifier::text, $id::text, $locked_until::text))) > 0 THEN
error('a grant with the same grantee, original_credential_id, and locked_until already exists')
END;
$dwg_result = idos.dwg_veryfy_owner($dwg_owner, @caller, $dwg_id, $dwg_not_before, $dwg_not_after, $dwg_signature);
Copy link
Member

Choose a reason for hiding this comment

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

I would not assume that the caller is the grantee. I'd say that anybody can transmit this transaction (to enable flows where the IdV is the one integrating idOS, like in Fractal's case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, you are right. I will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed by this 6d717dd

get_write_grant_id($user_id)::text
@caller,
$copy_credential_id,
0,
Copy link
Member

Choose a reason for hiding this comment

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

Timelock should be part of the message, if I recall correctly.

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 was guided by this
ONE Access Grant for the Issuer that has NO Timelock.
So is this needed to ask timelock in the message and then not to use?

Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure that was a decision born out of trying to make things simple. Since adding the timelock to our current implementation is trivial, I argue that we should add it.

Let's disambiguate with Martin.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants