Skip to content

feat: added bincode serialization for the EncryptedMessage type#8

Closed
muse254 wants to merge 11 commits into
mainfrom
fix/use-bincode-instead-of-json
Closed

feat: added bincode serialization for the EncryptedMessage type#8
muse254 wants to merge 11 commits into
mainfrom
fix/use-bincode-instead-of-json

Conversation

@muse254
Copy link
Copy Markdown
Contributor

@muse254 muse254 commented Nov 15, 2025

This PR is based on v0.1.1 tag

The changes include:

  • the v0.1.1 changes that are not yet in main
  • added bincode serialization for the EncryptedMessage type
  • we know nonce as [u8; 12] ahead of time, use that Type (rm unnecessary convertions)
  • use &[u8] unless we need to create copies, it's also accepts Vec and &str as a bonus
  • updated Cargo.toml to v0.1.2
  • rm the main.rs file and refactored it as dry_run_test

@muse254 muse254 requested review from dtpthao and stravid87 November 15, 2025 12:12
Copy link
Copy Markdown
Contributor

@dtpthao dtpthao left a comment

Choose a reason for hiding this comment

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

nice refactor for my amateur rust code, thank you

Comment thread src/common.rs
)
}

fn wasm_encrypt(&self, data: &[u8]) -> Result<([u8; 12], Vec<u8>), &'static str> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I remember struggling to make [u8; 32] work in WASM, which is why I originally used Vec. I tested your updates in the interceptor and they compiled successfully, I’m not sure why it didn’t work before.

Since [u8; 32] is now accepted, have you tried removing the wasm_encryption function, exporting EncryptedMessage to WASM, and calling the encrypt function instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, still prepping the layer8-backbone PR. Should be out sometime today

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ahh, have you try to remove wasm_encrypt/decrypt function?

Comment thread src/server.rs Outdated
Copy link
Copy Markdown
Contributor

@dtpthao dtpthao left a comment

Choose a reason for hiding this comment

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

I withdraw my approval, please keep this PR open, I'll review it again

Comment thread src/common.rs
}
}

pub fn public_key(&self) -> &[u8; 32] {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just remembered why I used Vec here. We’re using a single cryptography algorithm right now, but that may change later, and different algorithms may use public keys of different sizes. In that case, we need to update every usage of [u8;32]. How is [u8; 32] better in this context?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, we can revert

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can we version bump when that happens, I'm assuming we will support multiple encryption schemes and not just replace the ntor?

@muse254
Copy link
Copy Markdown
Contributor Author

muse254 commented Dec 1, 2025

Will setup a diff clean PR

@muse254 muse254 closed this Dec 1, 2025
@muse254 muse254 deleted the fix/use-bincode-instead-of-json branch December 2, 2025 13:36
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