Skip to content

C_x: Represent using own ConnectionIdentifier newtype#260

Merged
geonnave merged 1 commit intolake-rs:mainfrom
chrysn-pull-requests:typed-c_x
May 3, 2024
Merged

C_x: Represent using own ConnectionIdentifier newtype#260
geonnave merged 1 commit intolake-rs:mainfrom
chrysn-pull-requests:typed-c_x

Conversation

@chrysn
Copy link
Copy Markdown
Member

@chrysn chrysn commented May 3, 2024

This makes the APIs clearer and paves the way for adding support for longer C_x.

Contributes-To: #258

Marked as a draft while waiting for CI checks (I only did cargo test which usually misses some)

@geonnave
Copy link
Copy Markdown
Collaborator

geonnave commented May 3, 2024

Thanks, so far looks good.
Evolving it into N-sized identifiers would imply some changes in the encoder/decoder (for which you seem to have a plan as well).
Would not oppose calling the new type ConnId.

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented May 3, 2024

Thanks. Big rename coming up with the squash. Yes, I have a plan, and I'm adding a .from_slice() constructor that'll allow at least the Python API to be nice from the start (so that you pass in bytes, get out bytes, and it just complains when the bytes contain more than 1 byte, of a value unsupported so far -- then, this can progress transparently). For the C API, nothing.

chrysn added a commit to chrysn-pull-requests/edhoc-rs that referenced this pull request May 3, 2024
@chrysn chrysn mentioned this pull request May 3, 2024
@geonnave
Copy link
Copy Markdown
Collaborator

geonnave commented May 3, 2024

Great!

For the C API, nothing.

That concerns me a bit, tracking at #262.

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented May 3, 2024

For the C API, nothing.

More precisely, things do work, but for the same reasons I advocated against having the C API the last time it was removed I don't want so sink too much time there, so I am fixing the examples, but not the API; consequently, unlike the Python API, once we do support longer identifiers, the C API can't use them.

This makes the APIs clearer and paves the way for adding support for
longer C_x.

This alters the Python API to consistently use bytes for connection
identifiers. Once lake-rs#258 is done, it will automatically allow the Python
API to support more values.

Contributes-To: lake-rs#258
@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented May 3, 2024

All green, squashed and ready for final review.

@chrysn chrysn marked this pull request as ready for review May 3, 2024 10:05
@geonnave
Copy link
Copy Markdown
Collaborator

geonnave commented May 3, 2024

Looks good, thanks.

I can take on updating the C API later -- there's an important C-based project in the lab which need lakers working on it.
Still this way it helps a lot, in that we both share parts of the work to advance the library.

@geonnave geonnave merged commit e4a1e94 into lake-rs:main May 3, 2024
@chrysn chrysn deleted the typed-c_x branch May 3, 2024 11:09
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