clarify batch management in the Wallet when the new batch of the credentials was received #452
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
Sakurann
requested review from
danielfett,
c2bo,
tplooker,
paulbastian,
awoie and
bc-pi
c2bo
reviewed
Feb 4, 2025
Co-authored-by: Christian Bormann <[email protected]>
c2bo
approved these changes
Feb 5, 2025
jogu
reviewed
Mar 18, 2025
| @@ -1101,6 +1101,8 @@ If the Client requested an encrypted response by including the `credential_respo | |||
|
|
|||
| If the Credential Response is not encrypted, the media type of the response MUST be set to `application/json`. | |||
|
|
|||
| When the Wallet receives a new batch of Credentials containing a specific Credential Dataset, the Wallet SHOULD delete any Credentials containing the same Credential Dataset it might have received previously. | |||
There was a problem hiding this comment.
I wonder if this is slightly over specific - it doesn't give any advice for the case where the issuer doesn't support batch issuance, nor it is necessarily helpful in the case where the wallet wants a batch of 10 and the issuer only supports batches of 5 (the wallet would hence ask for two batches of 5 and definitely wouldn't delete the first batch of 5 when it got the second batch of 5...).
Maybe this is more generic advice?
Suggested change
| When the Wallet receives a new batch of Credentials containing a specific Credential Dataset, the Wallet SHOULD delete any Credentials containing the same Credential Dataset it might have received previously. | |
| When the Wallet receives new Credentials containing a specific Credential Dataset, the Wallet SHOULD delete any Credentials containing the same Credential Dataset it received in previous sessions. |
(I'm not sure "session" is well defined but can't think of another phrase that means "delete credentials you received a while ago but not ones you received 10 seconds ago".)
There was a problem hiding this comment.
Suggested change
| When the Wallet receives a new batch of Credentials containing a specific Credential Dataset, the Wallet SHOULD delete any Credentials containing the same Credential Dataset it might have received previously. | |
| When the Wallet receives new Credentials containing a specific Credential Dataset, the Wallet SHOULD delete any Credentials containing the same Credential Dataset it received in previous issuance responses if those Credentials are no longer valid. |
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.
resolves #124