Skip to content

Conversation

@odd
Copy link
Member

@odd odd commented Aug 19, 2025

  • Added KeyChain to manage cryptographic keys for better modular storage and operations.
  • Refactored encryption and decryption traits for increased flexibility (Encrypt, Decrypt, etc.).
  • Updated dependent APIs (Cryptic, Encrypted) for enhanced modularity and type safety.

@odd odd requested a review from cyberzac August 19, 2025 09:13
@odd odd self-assigned this Aug 19, 2025
@odd odd marked this pull request as ready for review August 19, 2025 09:13
Copilot AI review requested due to automatic review settings August 19, 2025 09:13
Copy link
Contributor

Copilot AI left a 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 KeyChain for 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 Cryptic and Encrypted classes 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
Copy link

Copilot AI Aug 19, 2025

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.

Suggested change
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]

Copilot uses AI. Check for mistakes.
override def toString: String = s"KeyChain(${keys.keys.mkString(", ")})"

object KeyChain:
type Id = Int
Copy link

Copilot AI Aug 19, 2025

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.

Suggested change
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)

Copilot uses AI. Check for mistakes.
package crypto

/** The KeyChain crypto provides a way of managing cryptographic keys for a
* multitude of cyptos (using different or identical underlying cryptos for
Copy link

Copilot AI Aug 19, 2025

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'.

Suggested change
* multitude of cyptos (using different or identical underlying cryptos for
* multitude of cryptos (using different or identical underlying cryptos for

Copilot uses AI. Check for mistakes.
)
case Failure(e) => Failure(e)
final class OrElsed[V: Codec, W >: V: Codec](
final class OrElsed[V, W >: V: Encode](
Copy link

Copilot AI Aug 19, 2025

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.

Suggested change
final class OrElsed[V, W >: V: Encode](
final class OrElsed[V: Decode, W >: V: Encode](

Copilot uses AI. Check for mistakes.
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)
Copy link

Copilot AI Aug 19, 2025

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.

Suggested change
final case class Filtered[V](src: Cryptic[V], pred: V => Boolean)
final case class Filtered[V: Encode: Decode](src: Cryptic[V], pred: V => Boolean)

Copilot uses AI. Check for mistakes.
)
)
final case class Collected[V: Codec, W: Codec](
final case class Collected[V, W](
Copy link

Copilot AI Aug 19, 2025

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.

Suggested change
final case class Collected[V, W](
final case class Collected[V: Decode, W: Encode](

Copilot uses AI. Check for mistakes.
…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.
@odd odd force-pushed the feature/type-classes branch from 08369c8 to fc1ee23 Compare August 22, 2025 09:37
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.

2 participants