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

Introduce Message type to simplify the API #105

Open
tomusdrw opened this issue May 31, 2023 · 3 comments
Open

Introduce Message type to simplify the API #105

tomusdrw opened this issue May 31, 2023 · 3 comments

Comments

@tomusdrw
Copy link
Owner

tomusdrw commented May 31, 2023

Split off from #93

It would be nice to have SecretKey::sing directly use an array [u8; 32] instead of slice &[u8]. This would improve the api since the message should always be 32 bytes.

I guess using a slice is a carry-over form secp256k1 library. I agree that using [u8; 32] would be more convenient and could reduce the number of Results returned. However in some cases this might trigger unnecessary copying of data, so perhaps instead we could introduce a Message type:

pub struct Message<'a> {
  msg: InternalMessage<'a>,
}

impl<'a> Message {
  pub fn from_slice(slice: &'a [u8]) -> Result<Self, _> {
    // verify length
  }
}
impl<'a> From<[u8; 32]> for Message<'a> {
  ...
}
impl<'a> From<&'a [u8; 32]> for Message<'a> {
...
}

enum InternalMessage<'a> {
   ValidatedSlice(&'a [u8]),
   Borrowed(&'a [u8; 32]),
   Owned([u8; 32]),
}

And take impl Into<Message<'a>> in the API. Wondering how ergonomic the new API would be, since the errors might sometimes be confusing.

@elpiel wdyt?

@elpiel
Copy link
Contributor

elpiel commented Jun 26, 2023

It might be a good approach although it would make the API a bit less user-friendly.

Since arrays are what's used I suspect that it fits the API better but then the user of the crate should manually convert a slice to a fixed-length array.

There is an impl of TryFrom for slice to array so it's not that bad.

https://doc.rust-lang.org/std/primitive.array.html#impl-TryFrom%3C%26'a+%5BT%5D%3E-for-%26'a+%5BT;+N%5D

@tomusdrw
Copy link
Owner Author

Ah, the conversion looks ideal, I didn't know about it! Yeah, I think the Message type is not needed in that case and just having &[u8; 32] would be good enough, thanks!

@elpiel
Copy link
Contributor

elpiel commented Jun 27, 2023

Thank you as well for taking the time to review this idea 😋

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

No branches or pull requests

2 participants