-
Notifications
You must be signed in to change notification settings - Fork 2
Introduce KeyChain and refactor cryptographic traits
#40
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new KeyChain class for managing cryptographic keys and refactors the cryptographic trait system to improve modularity and type safety.
- Introduces
KeyChainfor centralized key management with type-safe key storage and retrieval - Refactors cryptographic operations from function types to parameterized traits (
Encrypt[K],Decrypt[K], etc.) - Updates the
CrypticandEncryptedclasses to work with the new trait-based system using context parameters
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| KeyChain.scala | Adds new KeyChain class for managing cryptographic keys with type-safe operations |
| cryptic.scala | Refactors crypto traits from function types to parameterized traits and updates dependent classes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| override def toString: String = s"KeyChain(${keys.keys.mkString(", ")})" | ||
|
|
||
| object KeyChain: | ||
| type Id = Int |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using existential type ? for values in the Map makes the KeyChain unsafe. The get method requires unsafe casting with asInstanceOf[T]. Consider using a type-safe approach like phantom types or sealed trait hierarchies to maintain type safety.
| type Id = Int | |
| keys: Map[KeyChain.Key[_], Any] | |
| ): | |
| require(keys.nonEmpty, "Key chain must contain at least one key") | |
| def add[T](key: KeyChain.Key[T], value: T): KeyChain = | |
| copy(keys = keys + (key -> value)) | |
| def get[T](key: KeyChain.Key[T]): T = | |
| keys | |
| .getOrElse( | |
| key, | |
| throw new NoSuchElementException(s"Key $key not found") | |
| ) | |
| .asInstanceOf[T] | |
| override def toString: String = s"KeyChain(${keys.keys.mkString(", ")})" | |
| object KeyChain: | |
| // Type-safe key definition | |
| sealed trait Key[T] | |
| // Example keys (add as needed) | |
| case object IntKey extends Key[Int] | |
| case object StringKey extends Key[String] |
| override def toString: String = s"KeyChain(${keys.keys.mkString(", ")})" | ||
|
|
||
| object KeyChain: | ||
| type Id = Int |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsafe type casting can cause ClassCastException at runtime if the wrong type is requested for a key. This breaks type safety and can lead to runtime failures.
| type Id = Int | |
| keys: Map[KeyChain.Id, KeyChain.Entry[_]] | |
| ): | |
| require(keys.nonEmpty, "Key chain must contain at least one key") | |
| def add[T](id: KeyChain.Id, key: T): KeyChain = | |
| copy(keys = keys + (id -> KeyChain.Entry(key))) | |
| def get[T](id: KeyChain.Id): T = | |
| keys | |
| .getOrElse( | |
| id, | |
| throw new NoSuchElementException(s"Key ID $id not found") | |
| ) match | |
| case KeyChain.Entry(value: T @unchecked) => value | |
| case _ => throw new ClassCastException(s"Key ID $id does not contain a value of the expected type") | |
| override def toString: String = s"KeyChain(${keys.keys.mkString(", ")})" | |
| object KeyChain: | |
| type Id = Int | |
| case class Entry[T](value: T) |
| package crypto | ||
|
|
||
| /** The KeyChain crypto provides a way of managing cryptographic keys for a | ||
| * multitude of cyptos (using different or identical underlying cryptos for |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: 'cyptos' should be 'cryptos'.
| * multitude of cyptos (using different or identical underlying cryptos for | |
| * multitude of cryptos (using different or identical underlying cryptos for |
| ) | ||
| case Failure(e) => Failure(e) | ||
| final class OrElsed[V: Codec, W >: V: Codec]( | ||
| final class OrElsed[V, W >: V: Encode]( |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OrElsed class requires Encode[W] context bound but also needs Decode[V] for the decrypted method to work properly. The missing Decode[V] constraint will cause compilation errors.
| final class OrElsed[V, W >: V: Encode]( | |
| final class OrElsed[V: Decode, W >: V: Encode]( |
| src.decrypted.flatMap(v => ev(v).decrypted) | ||
| */ | ||
| final case class Filtered[V: Codec](src: Cryptic[V], pred: V => Boolean) | ||
| final case class Filtered[V](src: Cryptic[V], pred: V => Boolean) |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Filtered class is missing required context bounds. It needs Encode[V] for the run method and Decode[V] for the decrypted method to compile properly.
| final case class Filtered[V](src: Cryptic[V], pred: V => Boolean) | |
| final case class Filtered[V: Encode: Decode](src: Cryptic[V], pred: V => Boolean) |
| ) | ||
| ) | ||
| final case class Collected[V: Codec, W: Codec]( | ||
| final case class Collected[V, W]( |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Collected class is missing required context bounds. It needs Decode[V] and Encode[W] for its methods to compile properly.
| final case class Collected[V, W]( | |
| final case class Collected[V: Decode, W: Encode]( |
…raits for flexibility (#37) Introduce `KeyChain` to handle multipurpose cryptographic key storage and operations. Refactor core encryption and decryption traits (`Encrypt`, `Decrypt`, etc.) and adjust dependent APIs (`Cryptic`, `Encrypted`) for enhanced modularity and type safety.
08369c8 to
fc1ee23
Compare
KeyChainto manage cryptographic keys for better modular storage and operations.Encrypt,Decrypt, etc.).Cryptic,Encrypted) for enhanced modularity and type safety.