Break public API, simplify key mgmt and tests #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sodiumoxide migration and future plans for this crate
…more buffer passing as arguments. Also taking the opportunity to transition from CLI-integration tests to functional tests instead (we are primarily testing the functions not the CLI) since they introduce OS-dependent constructs (i.e Paths) and difficult WASM code reuse, for instance. [ci skip]
|
Yes, agree here, I think this can be done at once, although we might need to go over it a few times to get it all refactored. I think rewriting the main I'd definitely remove the buffers, and just return Results/byte vectors. We could take some inspiration from the RustCrypto encrypt/decrypt: https://docs.rs/aead/latest/aead/trait.Aead.html. |
… functions, start importing equivalent IO and Async functions written in htsget-rs crate async_crypt4gh
…y type (from ssh_keys, rustls, our custom one...). Also the testsuite is too based on filesystem. [ci skip]
…r count, tests re-factoring is a separate task to tackle later) [ci skip]
…n the root module (as of 0.22?). Several small fixes, renaming of decrypt to decrypt_header, remove advance-related functionality that will go away anyway in DecrypterStream
… since rearrange and reencrypt no longer take buffers as arguments :P)
… towards Header { HeaderInfo, HeaderData } and methods that read/write/encrypt/decrypt those fields accordingly /cc @mmalenic, might need to discuss this for you to have a clean design that we both feel comfortable with?
…h should not... also which of Header/HeaderPackets/EcnrytperHeaderPackets/EncryptedHeaderPacketBytes should hold which functions... [ci skip]
… much easier to follow [ci skip]
…rust-after-the-honeymoon/ for context... why is u8 and not Vec<u8> for those data-bearing enums in point 6, @mmalenic?
…ons from the spec mirroring the actual implementation and FIXMEs that should be considered
… reflect the builder pattern instead of hardcoding header_* and the like
….com:umccr/crypt4gh-rust into break_public_api_simplify_key_mgmt_and_tests
… the direct spec-to-code struct approach (following the structs and their namings defined in the official Crypt4GH spec)
…files and structures
…nt types of headers in toplevel headers.rs too
…ill break most of them anyway. Will re-add them later
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@mmalenic Let's discuss/iterate on this a bit further, but I hope you see where I'm going... I started refactoring the tests to fix https://github.com/EGA-archive/crypt4gh-rust/actions/runs/7733799516/job/21086574881, but I quickly realized that we could just fix the APIs in one go and simplify the tests massively.
We need to agree/decide on the most ergonomic API design for
encrypt/decryptand helper functions (rearrange et al). Ideally this API should be a bit more idiomatic, let go of buffers, OS-level constructs such asPaths which complicate WASM bindgen and embrace the typical builder pattern, IMHO.TODO:
async-crypt4ghbranch here.deserialize_header_infoarguments and return types.keys::PrivateKey/PublicKey/KeyPairtypes are generic enough to accomodate both SSH and GA4GH key formats, perhaps others?get_test_filewith bytes and buffers.encryptfunction.