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

feat: Implementing Pluto Repositories #160

Merged
merged 29 commits into from
Feb 27, 2024
Merged

Conversation

curtis-h
Copy link
Contributor

Description

Large reconsideration of Pluto, reintroducing an implementation that abstracts the storage layer, so we can write the logic and error handling without being coupled to a specific db etc.

The Pluto class now acts as an orchestration layer, only handling Domain objects, their logic and relationships.
The repository layer handles mapping Domain <-> Storable Models.
The Store abtracts the storage implementation with a simplified interface, handling Storable Models.

BREAKING CHANGES:

  • Domain instances now expect to track their storage ID with uuid property.
  • updated Pluto signature to use null instead of undefined
  • Pluto.storePrismDID signature changed - unused arguments (keyPathIndex, metaId) removed
  • Pluto CredentialMetadata
    • updated to use Domain.CredentialMetadata instance
    • fetchCredentialMetadata renamed to getCredentialMetadata
  • Pluto LinkSecret fns updated to use Domain.LinkSecret instance
    • CredentialRequestOptions updated to use Domain.LinkSecret and linkSecretName removed
    • CreatePresentationProof options updated to use Domain.LinkSecret
  • Pluto storeMediator now takes Domain.Mediator, instead of three separate DIDs
  • Pluto storePrivateKeys renamed to storePrivateKey as it only handles a single key
  • PrismDIDs
    • PrismDID added
    • PrismDIDInfo removed
    • PrismDIDMethodId removed - functionality added to PrismDID`
    • Pluto fns
      • getAllPrismDIDs now returns Domain.PrismDIDs
      • getDIDInfoByDID getDIDInfoByAlias getPrismLastKeyPathIndex - removed

Jira link

https://input-output.atlassian.net/browse/ATL-6148
https://input-output.atlassian.net/browse/ATL-5861
https://input-output.atlassian.net/browse/ATL-5863

Checklist

  • Self-reviewed the diff
  • New code has inline documentation
  • New code has proper comments/tests
  • Any changes not covered by tests have been tested manually

@curtis-h curtis-h self-assigned this Jan 30, 2024
@curtis-h curtis-h changed the title Feature/atl 6148 repositories feat/Pluto-Repositories Jan 30, 2024
Copy link

github-actions bot commented Feb 1, 2024

Lines Statements Branches Functions
Coverage: 67%
67.07% (1691/2521) 53.37% (648/1214) 69.75% (512/734)

JUnit

Tests Skipped Failures Errors Time
383 2 💤 0 ❌ 0 🔥 56.782s ⏱️

Copy link
Contributor

@milosh86 milosh86 left a comment

Choose a reason for hiding this comment

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

Hey @curtis-h, great work really!

TBH, I like the implementation a lot, but I miss a bit of higher level context to be able to reason about it properly, like:

  • what are the decision drivers here?
  • what specific problem are we addressing exactly? why is it worth the effort?
  • what's the ideal solution and under what assumptions?
  • what are the tradeoffs of the chosen approach?

I think SDK is mature enough now to start addressing bigger changes in a more structured way (with a bit of good friction). Otherwise, we are going to solve the same problems again and again.

Please consider this only as a proposal to discuss inside the team, I'm ok with proceeding with this one as is, it (de facto) looks better than the existing implementation.

src/domain/buildingBlocks/Pluto.ts Outdated Show resolved Hide resolved
@curtis-h
Copy link
Contributor Author

Hey @curtis-h, great work really!

TBH, I like the implementation a lot, but I miss a bit of higher level context to be able to reason about it properly, like:

  • what are the decision drivers here?
  • what specific problem are we addressing exactly? why is it worth the effort?
  • what's the ideal solution and under what assumptions?
  • what are the tradeoffs of the chosen approach?

I think SDK is mature enough now to start addressing bigger changes in a more structured way (with a bit of good friction). Otherwise, we are going to solve the same problems again and again.

Please consider this only as a proposal to discuss inside the team, I'm ok with proceeding with this one as is, it (de facto) looks better than the existing implementation.

Thanks @milosh86
here's an attempt to answer the questions:

what are the decision drivers here?

  • Pluto interface large, unwieldy to implement
  • any change to interface affects users downstream
  • lack of testing / provability on our side

what specific problem are we addressing exactly? why is it worth the effort?

  • Bloat, not all functions in the interface are required or even used in some cases.
  • Pluto is currently an empty box for our users, with no guidance or tests to prove what they build.

what's the ideal solution and under what assumptions?

  • Ideally, a fully implemented secure storage so there is no work for our users.
  • Assumes no liability for security. Also that the users will be happy with our choice of store.

what are the tradeoffs of the chosen approach?

  • Implmentation and Security is still the users responsibility.
  • Logic and orchestration is managed and proven by us.

@curtis-h
Copy link
Contributor Author

curtis-h commented Feb 13, 2024

Schemas

  • path to migrations
  • encrypt individual fields within a table?
  • needs definition

Query Selectors

  • gt , lt , eq etc

Migrations

Rollbacks?

  • later

Encrypted data

  • in or out of control?
  • if we offer Model, should we opine on what fields should be encrypted?
  • what?
    • PrivateKeys
  • password must be entered by user
    • keep in memory or remove after use?

@curtis-h curtis-h marked this pull request as ready for review February 20, 2024 18:46
@curtis-h curtis-h mentioned this pull request Feb 21, 2024
4 tasks
@elribonazo elribonazo self-requested a review February 21, 2024 09:52
@elribonazo elribonazo changed the title feat/Pluto-Repositories feat: Implementing Pluto Repositories Feb 21, 2024
Copy link
Contributor

@elribonazo elribonazo left a comment

Choose a reason for hiding this comment

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

Have added the missing demos and changes the alias as we agreed.

Awesome work man, well done!

Copy link

# npm audit report

ip  2.0.0
Severity: moderate
NPM IP package incorrectly identifies some private IP addresses as public - https://github.com/advisories/GHSA-78xj-cgh5-2h22
fix available via `npm audit fix`
node_modules/npm/node_modules/ip

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

Copy link
Contributor

@elribonazo elribonazo left a comment

Choose a reason for hiding this comment

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

The whole point of bringing this is to facilitate maintenance and future migrations on the user, developed side.

Migration paths are a key feature or our solution and not including them makes it harder for the user to maintain, etc.

Copy link
Contributor

@elribonazo elribonazo left a comment

Choose a reason for hiding this comment

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

I think this now bundles a complete migration solution that will support the existing community storage packages and allow us to maintain migrations on our side.

We can upgrade model attributes from A to B, but we can also completely remove A and migrate all its rows to a new model which is completely different.

Copy link

# npm audit report

ip  2.0.0
Severity: moderate
NPM IP package incorrectly identifies some private IP addresses as public - https://github.com/advisories/GHSA-78xj-cgh5-2h22
fix available via `npm audit fix`
node_modules/npm/node_modules/ip

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

Copy link
Contributor

@milosh86 milosh86 left a comment

Choose a reason for hiding this comment

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

@curtis-h @elribonazo I reviewed this version in detail and I like it, looks quite clean and haven't noticed anything potentially problematic. We introduced RxDB, but in return we got some nice features out of the box.

@elribonazo one question for you. Are you happy now how this work relates to the pluto-encrypted package. Do you think that migration path for users is obvious?

UPDATE: Great job both of you 🙌💪

@elribonazo
Copy link
Contributor

Are you happy now how this work relates to the pluto-encrypted package. Do you think that migration path for users is obvious?

@milosh86, yes really because in the current scenario this will not require much changes from the user in terms of code, re-implementing the storages, etc. So users still have encrypted (inmemory, or leveldb, or indexdb)

The migration is specified in the models and because models are right now provided by our package, this means that migration of data is something transparent for the users. The data will grow over time and this will facilitate transition to new models. This was in place before but because it was in pluto-encrypted and not in the SDK, for every change in the SDK, i have to release another version of pluto-encrypted.

This will never happen anymore, pluto-encrypted/database, pluto-encrypted/encryption will be deprecated and only the storages will be maintained.

Last, despite we have facilitated lots of things for our users, we are still not taking a decision on where it should be stored and how it should be encrypted, that's something that very advanced user's could implement themselves, the encryption happens at storage level and because storage doesn't speak models, doesn't know what they are maintenance is extremely simple for users.

Copy link

# npm audit report

ip  2.0.0
Severity: moderate
NPM IP package incorrectly identifies some private IP addresses as public - https://github.com/advisories/GHSA-78xj-cgh5-2h22
fix available via `npm audit fix`
node_modules/npm/node_modules/ip

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

# Conflicts:
#	demos/next/package-lock.json
#	demos/next/package.json
#	demos/next/src/app/state.ts
#	demos/next/src/pages/index.tsx
#	package-lock.json
#	package.json
@elribonazo elribonazo merged commit 419930e into master Feb 27, 2024
4 checks passed
@elribonazo elribonazo deleted the feature/ATL-6148-Repositories branch February 27, 2024 15:29
elribonazo added a commit that referenced this pull request Feb 27, 2024
BREAKING CHANGE:
- Domain.Credential now has uuid (string) as an optional attribute in the class
- Change StorePrismDID Function parameters, removing keyPathIndex and privateKeyMetadataId which were not in use
- Small change in the getCredentialMetadata and getLinkSecret to return null when not found
- Change Pluto interface to use CredentialMetadata class VS what it was using before.
- Change Pluto interface, storeMediator now accepts a Mediator class and not 3 attributes, mediator, host and routing
- Rename storePrivateKeys to storePrivateKey
- Change Pluto interface, using Domain.PrismDID instead of Domain.PrismDID which is being removed
- Added new properties to handle Anoncreds credentials properties.
- Re-implemented Pluto which is now available again to all the users.

import SDK from "@atala/prism-wallet-sdk";
import IndexDB from "@pluto-encrypted/indexdb";

const store = new SDK.Store({
name: "test",
storage: IndexDB,
password: Buffer.from("demoapp").toString("hex")
});
const pluto = new SDK.Pluto(store, apollo);

Migrations, schemas, queries and error handling have been moved to the SDK again but user's which were using the existing storages can just pass the indexDB pluto-encrypted storage and it will integrate with the new db schemas, migrations, etc
atala-dev added a commit that referenced this pull request Feb 28, 2024
# [5.0.0](v4.0.0...v5.0.0) (2024-02-28)

### Bug Fixes

* add reference app + mediator live mode ([#177](#177)) ([6ebf48a](6ebf48a))
* e2e tests improvement ([#178](#178)) ([eb6a5ab](eb6a5ab))
* key's id name according to the DID Peer new specs [#126](#126) ([#148](#148)) ([a851b69](a851b69))
* Manually generating the missing changelog and breaking changes b… ([#167](#167)) ([24ecc04](24ecc04))
* Recover JTI field correctly. Allowing to regenerate the original JWT string ([#171](#171)) ([913f3fa](913f3fa))

* feat!: Implementing Pluto Repositories (#160) ([ce7669f](ce7669f)), closes [#160](#160)

### BREAKING CHANGES

* - Domain.Credential now has uuid (string) as an optional attribute in the class
- Change StorePrismDID Function parameters, removing keyPathIndex and privateKeyMetadataId which were not in use
- Small change in the getCredentialMetadata and getLinkSecret to return null when not found
- Change Pluto interface to use CredentialMetadata class VS what it was using before.
- Change Pluto interface, storeMediator now accepts a Mediator class and not 3 attributes, mediator, host and routing
- Rename storePrivateKeys to storePrivateKey
- Change Pluto interface, using Domain.PrismDID instead of Domain.PrismDID which is being removed
- Added new properties to handle Anoncreds credentials properties.
- Re-implemented Pluto which is now available again to all the users.

import SDK from "@atala/prism-wallet-sdk";
import IndexDB from "@pluto-encrypted/indexdb";

const store = new SDK.Store({
name: "test",
storage: IndexDB,
password: Buffer.from("demoapp").toString("hex")
});
const pluto = new SDK.Pluto(store, apollo);

Migrations, schemas, queries and error handling have been moved to the SDK again but user's which were using the existing storages can just pass the indexDB pluto-encrypted storage and it will integrate with the new db schemas, migrations, etc
elribonazo added a commit that referenced this pull request May 2, 2024
BREAKING CHANGE:
- Domain.Credential now has uuid (string) as an optional attribute in the class
- Change StorePrismDID Function parameters, removing keyPathIndex and privateKeyMetadataId which were not in use
- Small change in the getCredentialMetadata and getLinkSecret to return null when not found
- Change Pluto interface to use CredentialMetadata class VS what it was using before.
- Change Pluto interface, storeMediator now accepts a Mediator class and not 3 attributes, mediator, host and routing
- Rename storePrivateKeys to storePrivateKey
- Change Pluto interface, using Domain.PrismDID instead of Domain.PrismDID which is being removed
- Added new properties to handle Anoncreds credentials properties.
- Re-implemented Pluto which is now available again to all the users.

import SDK from "@atala/prism-wallet-sdk";
import IndexDB from "@pluto-encrypted/indexdb";

const store = new SDK.Store({
name: "test",
storage: IndexDB,
password: Buffer.from("demoapp").toString("hex")
});
const pluto = new SDK.Pluto(store, apollo);

Migrations, schemas, queries and error handling have been moved to the SDK again but user's which were using the existing storages can just pass the indexDB pluto-encrypted storage and it will integrate with the new db schemas, migrations, etc
elribonazo pushed a commit that referenced this pull request May 2, 2024
# [5.0.0](v4.0.0...v5.0.0) (2024-02-28)

### Bug Fixes

* add reference app + mediator live mode ([#177](#177)) ([6ebf48a](6ebf48a))
* e2e tests improvement ([#178](#178)) ([eb6a5ab](eb6a5ab))
* key's id name according to the DID Peer new specs [#126](#126) ([#148](#148)) ([a851b69](a851b69))
* Manually generating the missing changelog and breaking changes b… ([#167](#167)) ([24ecc04](24ecc04))
* Recover JTI field correctly. Allowing to regenerate the original JWT string ([#171](#171)) ([913f3fa](913f3fa))

* feat!: Implementing Pluto Repositories (#160) ([ce7669f](ce7669f)), closes [#160](#160)

### BREAKING CHANGES

* - Domain.Credential now has uuid (string) as an optional attribute in the class
- Change StorePrismDID Function parameters, removing keyPathIndex and privateKeyMetadataId which were not in use
- Small change in the getCredentialMetadata and getLinkSecret to return null when not found
- Change Pluto interface to use CredentialMetadata class VS what it was using before.
- Change Pluto interface, storeMediator now accepts a Mediator class and not 3 attributes, mediator, host and routing
- Rename storePrivateKeys to storePrivateKey
- Change Pluto interface, using Domain.PrismDID instead of Domain.PrismDID which is being removed
- Added new properties to handle Anoncreds credentials properties.
- Re-implemented Pluto which is now available again to all the users.

import SDK from "@atala/prism-wallet-sdk";
import IndexDB from "@pluto-encrypted/indexdb";

const store = new SDK.Store({
name: "test",
storage: IndexDB,
password: Buffer.from("demoapp").toString("hex")
});
const pluto = new SDK.Pluto(store, apollo);

Migrations, schemas, queries and error handling have been moved to the SDK again but user's which were using the existing storages can just pass the indexDB pluto-encrypted storage and it will integrate with the new db schemas, migrations, etc
elribonazo added a commit that referenced this pull request May 2, 2024
Co-authored-by: Francisco Javier Ribó Labrador <elribonazo@gmail.com>
Signed-off-by: Francisco Javier Ribó Labrador <elribonazo@gmail.com>
Signed-off-by: Francisco Javier Ribo Labrador <elribonazo@gmail.com>
elribonazo added a commit that referenced this pull request May 2, 2024
BREAKING CHANGE:
- Domain.Credential now has uuid (string) as an optional attribute in the class
- Change StorePrismDID Function parameters, removing keyPathIndex and privateKeyMetadataId which were not in use
- Small change in the getCredentialMetadata and getLinkSecret to return null when not found
- Change Pluto interface to use CredentialMetadata class VS what it was using before.
- Change Pluto interface, storeMediator now accepts a Mediator class and not 3 attributes, mediator, host and routing
- Rename storePrivateKeys to storePrivateKey
- Change Pluto interface, using Domain.PrismDID instead of Domain.PrismDID which is being removed
- Added new properties to handle Anoncreds credentials properties.
- Re-implemented Pluto which is now available again to all the users.

import SDK from "@atala/prism-wallet-sdk";
import IndexDB from "@pluto-encrypted/indexdb";

const store = new SDK.Store({
name: "test",
storage: IndexDB,
password: Buffer.from("demoapp").toString("hex")
});
const pluto = new SDK.Pluto(store, apollo);

Migrations, schemas, queries and error handling have been moved to the SDK again but user's which were using the existing storages can just pass the indexDB pluto-encrypted storage and it will integrate with the new db schemas, migrations, etc
elribonazo pushed a commit that referenced this pull request May 2, 2024
# [5.0.0](v4.0.0...v5.0.0) (2024-02-28)

### Bug Fixes

* add reference app + mediator live mode ([#177](#177)) ([6ebf48a](6ebf48a))
* e2e tests improvement ([#178](#178)) ([eb6a5ab](eb6a5ab))
* key's id name according to the DID Peer new specs [#126](#126) ([#148](#148)) ([a851b69](a851b69))
* Manually generating the missing changelog and breaking changes b… ([#167](#167)) ([24ecc04](24ecc04))
* Recover JTI field correctly. Allowing to regenerate the original JWT string ([#171](#171)) ([913f3fa](913f3fa))

* feat!: Implementing Pluto Repositories (#160) ([ce7669f](ce7669f)), closes [#160](#160)

### BREAKING CHANGES

* - Domain.Credential now has uuid (string) as an optional attribute in the class
- Change StorePrismDID Function parameters, removing keyPathIndex and privateKeyMetadataId which were not in use
- Small change in the getCredentialMetadata and getLinkSecret to return null when not found
- Change Pluto interface to use CredentialMetadata class VS what it was using before.
- Change Pluto interface, storeMediator now accepts a Mediator class and not 3 attributes, mediator, host and routing
- Rename storePrivateKeys to storePrivateKey
- Change Pluto interface, using Domain.PrismDID instead of Domain.PrismDID which is being removed
- Added new properties to handle Anoncreds credentials properties.
- Re-implemented Pluto which is now available again to all the users.

import SDK from "@atala/prism-wallet-sdk";
import IndexDB from "@pluto-encrypted/indexdb";

const store = new SDK.Store({
name: "test",
storage: IndexDB,
password: Buffer.from("demoapp").toString("hex")
});
const pluto = new SDK.Pluto(store, apollo);

Migrations, schemas, queries and error handling have been moved to the SDK again but user's which were using the existing storages can just pass the indexDB pluto-encrypted storage and it will integrate with the new db schemas, migrations, etc
elribonazo added a commit that referenced this pull request May 3, 2024
BREAKING CHANGE:
- Domain.Credential now has uuid (string) as an optional attribute in the class
- Change StorePrismDID Function parameters, removing keyPathIndex and privateKeyMetadataId which were not in use
- Small change in the getCredentialMetadata and getLinkSecret to return null when not found
- Change Pluto interface to use CredentialMetadata class VS what it was using before.
- Change Pluto interface, storeMediator now accepts a Mediator class and not 3 attributes, mediator, host and routing
- Rename storePrivateKeys to storePrivateKey
- Change Pluto interface, using Domain.PrismDID instead of Domain.PrismDID which is being removed
- Added new properties to handle Anoncreds credentials properties.
- Re-implemented Pluto which is now available again to all the users.

import SDK from "@atala/prism-wallet-sdk";
import IndexDB from "@pluto-encrypted/indexdb";

const store = new SDK.Store({
name: "test",
storage: IndexDB,
password: Buffer.from("demoapp").toString("hex")
});
const pluto = new SDK.Pluto(store, apollo);

Migrations, schemas, queries and error handling have been moved to the SDK again but user's which were using the existing storages can just pass the indexDB pluto-encrypted storage and it will integrate with the new db schemas, migrations, etc
elribonazo pushed a commit that referenced this pull request May 3, 2024
# [5.0.0](v4.0.0...v5.0.0) (2024-02-28)

### Bug Fixes

* add reference app + mediator live mode ([#177](#177)) ([6ebf48a](6ebf48a))
* e2e tests improvement ([#178](#178)) ([eb6a5ab](eb6a5ab))
* key's id name according to the DID Peer new specs [#126](#126) ([#148](#148)) ([a851b69](a851b69))
* Manually generating the missing changelog and breaking changes b… ([#167](#167)) ([24ecc04](24ecc04))
* Recover JTI field correctly. Allowing to regenerate the original JWT string ([#171](#171)) ([913f3fa](913f3fa))

* feat!: Implementing Pluto Repositories (#160) ([ce7669f](ce7669f)), closes [#160](#160)

### BREAKING CHANGES

* - Domain.Credential now has uuid (string) as an optional attribute in the class
- Change StorePrismDID Function parameters, removing keyPathIndex and privateKeyMetadataId which were not in use
- Small change in the getCredentialMetadata and getLinkSecret to return null when not found
- Change Pluto interface to use CredentialMetadata class VS what it was using before.
- Change Pluto interface, storeMediator now accepts a Mediator class and not 3 attributes, mediator, host and routing
- Rename storePrivateKeys to storePrivateKey
- Change Pluto interface, using Domain.PrismDID instead of Domain.PrismDID which is being removed
- Added new properties to handle Anoncreds credentials properties.
- Re-implemented Pluto which is now available again to all the users.

import SDK from "@atala/prism-wallet-sdk";
import IndexDB from "@pluto-encrypted/indexdb";

const store = new SDK.Store({
name: "test",
storage: IndexDB,
password: Buffer.from("demoapp").toString("hex")
});
const pluto = new SDK.Pluto(store, apollo);

Migrations, schemas, queries and error handling have been moved to the SDK again but user's which were using the existing storages can just pass the indexDB pluto-encrypted storage and it will integrate with the new db schemas, migrations, etc
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.

3 participants