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

Add support for additional data (AD) in Sel.SecretKey.Stream #183

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

intricate
Copy link

Add support for specifying AAD in Sel.SecretKey.Stream.

Comment on lines +515 to +520
additionalDataFromHexByteString
:: Base16 StrictByteString
-> Either AdditionalDataHexDecodingError AdditionalData
additionalDataFromHexByteString hexBs =
case Base16.decodeBase16Untyped (Base16.extractBase16 hexBs) of
Copy link
Author

Choose a reason for hiding this comment

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

This pattern of calling decodeBase16Untyped . extractBase16 instead of decodeBase16 on a Base16 a value confuses me a bit (#144 (comment)), but I've done this here to be consistent with similar functions in this module.

@@ -51,6 +51,13 @@ module Sel.SecretKey.Stream
-- ** Message Tags
, MessageTag (..)

-- ** Additional data (AD)
, AdditionalData (..)
Copy link
Author

Choose a reason for hiding this comment

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

Thoughts on me exposing this constructor? Or would you prefer a function like additionalDataFromBinary or something like that?

Copy link
Member

Choose a reason for hiding this comment

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

If the constructor is not needed to use the API and would just be used for access, then you can just expose additionalDataFromBinary indeed. :)

@intricate intricate force-pushed the support-aad-secretstream-sel branch from 7238194 to dbc8aa6 Compare February 20, 2025 01:16
@Kleidukos Kleidukos self-requested a review February 20, 2025 08:43
@Kleidukos
Copy link
Member

@intricate Thanks! I'll take time to review and think about your comment in #144 :)

@intricate intricate force-pushed the support-aad-secretstream-sel branch from dbc8aa6 to 3d619d4 Compare February 20, 2025 14:20
@intricate intricate force-pushed the support-aad-secretstream-sel branch from 3d619d4 to 3c7c6ca Compare February 20, 2025 14:22
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