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

Adjust how Watermarks are collected to eliminate redundant OCA Bundles #2342

Open
swcurran opened this issue Jan 10, 2025 · 5 comments
Open

Comments

@swcurran
Copy link

The current design for letting the BC Wallet know about Credential Watermarks is doubling the work in managing OCA Bundles, and we need to make it easier. The following is a proposal for changing the approach.

PROBLEM The use (or not) of a watermark is not an attribute of the type of credential, but rather where an instance of the credential is rooted -- on a Dev / Test / Prod network. On each of those the OCA Bundle for the credential should be the same, but because we put the watermark in the OCA Bundle itself, we need an OCA Bundle for each credential type in each environment. As a result, we have duplicate OCA Bundles for every credential type, with the only difference being the presence (or not) of the watermark value. Further, it would probably be even better if there were watermark difference between Dev and Test, but in the current situation, that would make things worse -- three OCA Bundles per credential type. We need to make it eliminate the redundency.

SUGGESTION We associate the "watermark" value with the Identifiers to which a Bundle applies (e.g. the CredDevId on CANdy Dev, Test and Prod). We do this by putting a "watermark" item into the ocabundles.json file, along with the existing values of <identifier>: { "path": <path to Bundle>, "sha256": <checksum> } data. The new data element would be optional (assume NULL if not present) and would be array in the form:

  • "watermark": [ { <lang>: <watermark> } ]
  • Example: "watermark": [ { "en": "Development", }, { "fr", "Développement" } ]

This would require a change in Bifold (or BC Wallet?) to collect the value if present. Proposed handling is that the identifier level watermark value has priority over the one in the OCA Bundle, such that:

  • If the watermark is found at the identifier level, use it, regardless of whether it is found in the OCA Bundle.
  • If the watermark is NOT found at the identifier level, and is found at the OCA Bundle, use the OCA Bundle value.

This means that the OCA Style Guide does not have to change (more on that below) and has the added benefit that we don't need to coordinate the release of the BC Wallet (or other apps using these OCA Bundles) with an update to the new approach.

TRANSITION The process of changing this would take the following form:

  1. Test out if the BC Wallet continues to "work" if there is the "watermark" value in the ocabundles.json. If so, update the Aries OCA Bundles repo to at least gracefully ignore the watermark value in that file.
    1. If not, we need to coordinate a fix for the not erroring if the "watermark" is in the ocabundles.json file before making the change in the Aries OCA Bundle file -- waiting for that to get to production.
  2. Update the aries-oca-bundles repo to set the watermark as appropriate on all identifiers, based on the existing values in the respective OCA Bundles and generate those values into the ocabundles.json file. All of the existing OCA Bundles will continue remain as is, with the "watermark" value will be kept in each OCA Bundle. I can take care of this step.
  3. Update Bifold/BC Wallet to use the "identifier-level watermark" value, as described above. Wait for that to get to production.
  4. Reorganize the aries-oca-bundles repository to eliminate the redundant OCA Bundles. We can make that change, notifying the owners of each Bundle. For those where the only difference is the "watermark", we can just go ahead. Where there are other changes, we will work the owners to see if we need to keep the extra OCA Bundles or if they can be consolidated into one. At the same time, we will likely reorganize the folder structure to better organize the OCA Bundles -- they are a bit of a mess right now. Again, I can take care of this step.

OCA Style Guide Updates Given that we are not precluding the use of "watermark" as part of the OCA Bundle, I propose that we do a "clarification" to the RFC saying, more or less "you can do that, but here is our experience and recommendation...". Thus, the spec is not changed, but best practices are encouraged. We can deprecate the use of "watermark" in the Bundle. A later version can remove the value, as appropriate.

@swcurran
Copy link
Author

@cvarjao, @jeznorth -- It would be nice to do this sooner than later. Its a pain for clients to develop and submit their OCA Bundles into the repo, and eliminating the duplication would be a big help. I'm hoping the impact on the Wallet is small. I have some other ideas that don't impact the wallet, but that will make creating OCA Bundles easier.

Thanks

@swcurran swcurran changed the title Adjust how Watermarks are collected Adjust how Watermarks are collected to eliminate redundant OCA Bundles Jan 10, 2025
@jleach
Copy link
Member

jleach commented Jan 11, 2025

I don't think the Bifold or BC Wallet will care, but if the path to it changes in the JSON it will break. We can easily look for it in it original locaiton or the new one. I wonder if it will be better to do this or just make a new CredentialCard v1.2. I think as we continue to adjust the OCA over time versioning the credential card may be easier. Just pick a CredentialCard that aligns with a particular OCA Bundle Versions?

watermark={overlay.metaOverlay?.watermark}

Right now it lives with other overlay data:

  constructor(overlay: IMetaOverlayData) {
    super(overlay)
    this.language = overlay.language
    this.name = overlay.name
    this.description = overlay.description
    this.#credential_help_text = overlay.credential_help_text
    this.#credential_support_url = overlay.credential_support_url
    this.issuer = overlay.issuer
    this.#issuer_description = overlay.issuer_description
    this.#issuer_url = overlay.issuer_url
    this.#watermark = overlay.watermark
  }

EDIT: Well, it won't really break since the metaOverlay is optional. But I still think our logic well get ugly as we make more adjustments to the OCA structure without versioning the cards. Easier to just say Credentialcard11 (1.1) goes with Bundle Version 1.1 and put them at different URLs similar to how we version API /api/v1.1/hocuspocus. As we get more people using the BC Wallet we'll get users all over the version spectrum.

@swcurran
Copy link
Author

Not entirely clear on what you are saying re CredentialCard (presumably, that is a code module? So that’s a developer to developer discussion). The idea is to add the collection of the watermark data (if available) when finding the OCA Bundle, and only using the the watermark data in the Overlay if it is not already known. So it will take logic changes at the point you indicate.

What I do want to know about is what will happen if we add the watermark data into the ocabundles.json file in the current BC Wallet. If there is suddenly an extra item in the JSON structure, will BC Wallet crash, or will it just ignore the item until we add code to process it?

@jleach
Copy link
Member

jleach commented Jan 11, 2025

Not entirely clear on what you are saying re CredentialCard (presumably, that is a code module? So that’s a developer to developer discussion).

Affirmative. It's an implementation detail.. But I still wonder if we should version the bundles (if not already) and or their location.

What I do want to know about is what will happen if we add the watermark data into the ocabundles.json file in the current BC Wallet. If there is suddenly an extra item in the JSON structure, will BC Wallet crash, or will it just ignore the item until we add code to process it?

Adding fields to JSON won't break anything. Only removing or restructuring. I've manually edited and published one (don't judge me) so I can do a quick conformation test next week.

@jleach
Copy link
Member

jleach commented Jan 20, 2025

Tested. I cleared the cache and data on Android. Updated the URL and re-launched the app. Fetch a new credential. Worked as expected. This change appears to have no impact on the current release.

 LOG  ********* RemoteOCABundleResolver URL =  https://raw.githubusercontent.com/swcurran/aries-oca-bundles/refs/heads/identifier-watermark/

Image

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

No branches or pull requests

2 participants