refactor(node): Remove prism identity #840

shotexa · 2024-04-27T23:58:33Z

Overview

Screenshots

Checklists

Pre-submit checklist:

Self-reviewed the diff
New code has proper comments/documentation/tests
Any changes not covered by tests have been tested manually
The README files are updated
If new libraries are included, they have licenses compatible with our project
If there is a db migration altering existing tables, there is a proper migration test

Pre-merge checklist:

Commits have useful messages
Review clarifications made it into the code

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

github-actions · 2024-04-28T00:00:28Z

Unit Test Results

286 tests 285 ✔️ 23s ⏱️
  39 suites     1 💤
  39 files     0 ❌

Results for commit 7dac03d.

♻️ This comment has been updated with latest results.

atala-dev · 2024-04-28T00:03:01Z

🦙 MegaLinter status: ⚠️ WARNING

Descriptor	Linter	Files	Errors
✅ ACTION	actionlint	1	0
⚠️ MARKDOWN	markdownlint	5	90
⚠️ MARKDOWN	markdown-link-check	5	10
✅ MARKDOWN	markdown-table-formatter	5	0
⚠️ PROTOBUF	protolint	5	5
✅ REPOSITORY	dustilock	yes	no
✅ REPOSITORY	git_diff	yes	no
✅ REPOSITORY	grype	yes	no
⚠️ REPOSITORY	kics	yes	49
✅ REPOSITORY	syft	yes	no
✅ REPOSITORY	trivy-sbom	yes	no
⚠️ REPOSITORY	trufflehog	yes	1
✅ SQL	sql-lint	2	0
⚠️ YAML	prettier	2	1
✅ YAML	v8r	2	0
✅ YAML	yamllint	2	0

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

build.sbt

EzequielPostan

Thank you for the PR @shotexa

there are no tests for the new code. It would be good to check that it is interoperable with the SDK identity package. At least the encoding/decoding of long form DIDs

I would push towards merging this in order to unblock one refactor I need to do. Please add the task to add the tests after the build cleanup story

EzequielPostan · 2024-04-29T20:28:18Z

node/src/main/scala/io/iohk/atala/prism/node/utils/Base64.scala

+  def encodeWithoutPadding(bytes: Array[Byte]): String = {
+    Base64.getUrlEncoder.withoutPadding().encodeToString(bytes)
+  }


any reason to have 2 encoders? There is one with padding defined above

I just checked and it looks like the other one is only used in tests (so not used anywhere), so I'll remove it. Same with the decoder.

node/src/test/scala/io/iohk/atala/prism/node/crypto/CryptoTestUtils.scala

EzequielPostan · 2024-04-29T21:23:13Z

node/src/main/scala/io/iohk/atala/prism/node/identity/DidPublicKey.scala

+  lazy val values: List[PublicKeyUsage] =
+    List(
+      MasterKeyUsage,
+      IssuingKeyUsage,
+      KeyAgreementKeyUsage,
+      AuthenticationKeyUsage,
+      RevocationKeyUsage,
+      CapabilityInvocationKeyUsage,
+      CapabilityDelegationKeyUsage
+    )
+
+  def fromProto(keyUsage: KeyUsage): PublicKeyUsage = {
+    values.find(_.toProto == keyUsage).getOrElse(throw new IllegalStateException("Key usage not supported"))
+  }


this is not used

I've removed keyUsage all together from this file

EzequielPostan · 2024-04-29T21:23:58Z

node/src/main/scala/io/iohk/atala/prism/node/identity/DidPublicKey.scala

+  def derivationIndex: Int
+
+  def name: String =
+    if (toProto.name.nonEmpty) toProto.name else throw new IllegalStateException("Key usage must have name")


I think these are not used

EzequielPostan · 2024-04-29T21:27:30Z

node/src/main/scala/io/iohk/atala/prism/node/identity/DidPublicKey.scala

+    )
+}
+
+sealed trait PublicKeyUsage {


there already exists a KeyUsage class in models package object

Thanks for noticing, I've removed the key usage I've added, refactored keyUsage from models a little bit, and reused it in my code.

EzequielPostan · 2024-04-29T21:37:03Z

node/src/main/scala/io/iohk/atala/prism/node/identity/PrismDid.scala

+    buildCanonical(stateHash)
+  }
+
+  def buildCanonicalFromKeys(


This method is not used
Also:

buildExperimentalCanonicalFromKeys

buildCanonicalFromMasterPublicKey

buildLongFormFromPublicKeys

buildExperimentalLongFormFromKeys
are never used

Regarding some unused functions, there are many other functions in the codebase right now that are unused right now, there is a tool in intelij that shows all of them, but some of them appeared to me useful so I've not removed them. In general, do you think we should remove all unused functions right now? I think if we touch this codebase in the future, there is a chance some of them might still be used because some of them are kind of useful in the context of the class or object they are part of.

Anyway, I'm merging this PR now, let's continue in slack and we can address all leftovers in the final round when we go over what we have left in the end.

EzequielPostan · 2024-04-29T21:39:02Z

node/src/main/scala/io/iohk/atala/prism/node/identity/PrismDid.scala

+    }
+  }
+
+  private def safeLongFormDidFromAtalaOperation(


the name is misleading. How is this safe? In Scala "unsafe" means that exceptions can be thrown. This method is unsafe

yeah I guess here it means that it checks for possibly invalid data and throws an exception, but I agree in scala it is misleading.

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

shotexa · 2024-04-30T10:28:47Z

Thank you for the PR @shotexa

there are no tests for the new code. It would be good to check that it is interoperable with the SDK identity package. At least the encoding/decoding of long form DIDs

I would push towards merging this in order to unblock one refactor I need to do. Please add the task to add the tests after the build cleanup story

I'll add a ticket for tests, merging this now to unblock you.

* refactor: clean up migrations (#830) * Add everything Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Remove migration tests Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * add ingnore invalid create option for postres mega linter Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Try mega linter 7 Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * edit megalinter.yaml Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Disable some linters Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> --------- Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * refactor(node): remove legacy BE files (#832) * Start cleaning up docs Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Remove more files Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Fix compilation issue Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Trim docs Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Fix compilation errors Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Remove some bitcoin test resources and old docs Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Lint Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Fix tests Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> --------- Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * refactor: Delete legacy operation related to credentials (#831) * ATL-6668: Delete legacy operation related to credentials This commit deletes the IssueCredentialBatchOperation and RevokeCredentialsOperation. It does not delete node API, daos nor repositories related to credentials * ATL-6668: Delete more files related to VCs This commit deletes even more files related to VC operations. It also removes some dependencies on the old SDK * ATL-6668: Delete unused tables This commit deletes tables and indexes from VC legacy operations * ATL-6668: Fix formatting * ATL-6668: Address review comments This commit deletes references to legacy VC code in protobuf and sql definitions * refactor(ATL-6924): add support for other keys (#834) * ATL-6924: Replace users of ECPublicKey This commit replaces the used of ECPublicKey * ATL-6924: Simplify models This commit simplifies models by deleting classes not needed at parsing time * ATL-6924: Simplify types This commit replaces some uses of Array and uses Vector instead to allow more transparent equality checks * ATL-6924: Add support for encoding functions This commit adds support to decode hex strings into Sha256Hash bytes * ATL-6924: Refactor CryptoUtils and remove SHA256Digest This commit improves the organization of CryptoUtils methods It also delete the used of SDK methods related to Sha256 hashing * refactor(ATL-6926): remove crypto sdk (#838) * ATL-6926: Remove uses of EC classes This commit removes most uses of the legacy crypto SDK * ATL-6926: Fix missing implementations This commit adds missing implementations for key encoding algorithms * ATL-6926: Fix tests * ATL-6926: Clean up code and tests * ATL-6926: Correct typo and add tests This commit adds the final tests to validate the cryptography implementation that replaces the SDK * refactor(node): Remove prism identity (#840) * clean up source files, replace DID usages Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Fix some tests Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Linting Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Fix test Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Fix fromString on DID Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * formatting Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Address some PR comments Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> --------- Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * refactor: crypto utils and tests (#841) * Remove uses of the old SDK This commit removes the uses of the old SDK and update tests * Refactor CryptoUtils class This commit re-organizes and renames methods for better use * Remove last dependency on old SDK This commit removes the last dependencies on the old SDK * docs(ATL-6669): update readme (#833) * ATL-6669: Update README This commits updates the README to focus oly on the PRISM node. * ATL-6669: Add IDE notes in README This commit adds instructions to load the project in different IDEs. The commit also removes incorrect sentences * ATL-6669: Delete old README and complete new one * ATL-6669: Add pre-commit configuration This commit adds a pre-commit hook that runs scalafmt and instructions to configure it * ATL-6669: Refer to identus configuration This commit updates the README and refers users to read identus documentation in order to configure the node. * ATL-7040: Delete DID based authentication (#843) This commit deletes the legacy DID based authentication * refactor(node): build files and add default port for gRPC server (#844) * Change some build files Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Rmove unused files Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Remove unused files Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Remove outer node folder Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Revert version name Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Remove unneeded resolver from the build Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Update fs2 Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Start adding tests Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Add more tests Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Fix 1 failing test Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Minor adjustment in docs Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * Remove test report aggregation step Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> --------- Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> * fix: Add http scheme support and removed the constraint from public keys table (#848) * Add http scheme support and removed the contraint from publickeys table Signed-off-by: mineme0110 <shailesh.patil@iohk.io> * ran scalafmt Signed-off-by: mineme0110 <shailesh.patil@iohk.io> --------- Signed-off-by: mineme0110 <shailesh.patil@iohk.io> --------- Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io> Signed-off-by: mineme0110 <shailesh.patil@iohk.io> Co-authored-by: shotexa <shota.jolbordi@iohk.io> Co-authored-by: Ezequiel Postan <ezequiel.postan@iohk.io> Co-authored-by: Shailesh Patil <53746241+mineme0110@users.noreply.github.com>

Shota Jolbordi added 2 commits April 28, 2024 03:46

clean up source files, replace DID usages

47e6f4f

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

Merge branch 'open-source-node' into ATL-6950-remove-prism-identity

f7ce5be

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

shotexa changed the base branch from main to open-source-node April 27, 2024 23:58

Shota Jolbordi added 5 commits April 28, 2024 04:32

Fix some tests

5fa563a

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

Linting

2e50e95

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

Fix test

89ff999

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

Fix fromString on DID

7726f5a

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

formatting

166b623

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

shotexa commented Apr 28, 2024

View reviewed changes

build.sbt Show resolved Hide resolved

shotexa commented Apr 28, 2024

View reviewed changes

build.sbt Show resolved Hide resolved

EzequielPostan approved these changes Apr 29, 2024

View reviewed changes

Address some PR comments

7dac03d

Signed-off-by: Shota Jolbordi <shota.jolbordi@iohk.io>

shotexa merged commit df626ee into open-source-node Apr 30, 2024
4 checks passed

shotexa deleted the ATL-6950-remove-prism-identity branch April 30, 2024 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(node): Remove prism identity #840

refactor(node): Remove prism identity #840

shotexa commented Apr 27, 2024

github-actions bot commented Apr 28, 2024 •

edited

Loading

atala-dev commented Apr 28, 2024 •

edited

Loading

EzequielPostan left a comment

EzequielPostan Apr 29, 2024

shotexa Apr 29, 2024

shotexa Apr 30, 2024

EzequielPostan Apr 29, 2024

shotexa Apr 30, 2024

EzequielPostan Apr 29, 2024

EzequielPostan Apr 29, 2024

shotexa Apr 30, 2024

EzequielPostan Apr 29, 2024

shotexa Apr 30, 2024

EzequielPostan Apr 29, 2024

shotexa Apr 29, 2024

shotexa Apr 30, 2024

shotexa commented Apr 30, 2024

refactor(node): Remove prism identity #840

refactor(node): Remove prism identity #840

Conversation

shotexa commented Apr 27, 2024

Overview

Screenshots

Checklists

github-actions bot commented Apr 28, 2024 • edited Loading

Unit Test Results

atala-dev commented Apr 28, 2024 • edited Loading

🦙 MegaLinter status: ⚠️ WARNING

EzequielPostan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shotexa commented Apr 30, 2024

github-actions bot commented Apr 28, 2024 •

edited

Loading

atala-dev commented Apr 28, 2024 •

edited

Loading