`BorshSerialize::serialize` method is unergonomic for mutable slices about borsh-rs HOT 5 CLOSED

Tough question! API guidelines recommend taking Read/Write by value in API:

https://rust-lang.github.io/api-guidelines/interoperability.html#generic-readerwriter-functions-take-r-read-and-w-write-by-value-c-rw-value

However, as borsh impls are typically recursive, I am not entirely sure that it would be the right call in this case. Haven't thought too deeply about this, but it seems that taking an W in serialize and passing &mut W to recursive calls will cause excessive monomorphisations:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=c5b3ba6bd001be17dd2fb75bfd30a90e

So, I'd say the current signature is the right one for implementing serialization, no need to change or deprecate it. However, it's a bad API for using serialization, as it requires a trait import. Better to follow serde_json / bincode and introduce convenience top-level functions:

fn bosh::to_vec(valueh: &impl BorshSerialize) -> Vec<u8> {}
fn bosh::to_writer(value: &impl BorshSerialize, w: impl Writer) -> Vec<u8> {}
// Maybe?
fn bosh::to_base58(value: &impl BorshSerialize) -> String {}

from borsh-rs.

Ah, this was more nuanced than I expected. Additional high-level convenience functions would do the trick for us, thanks for looking into this!

from borsh-rs.

Thanks for bringing this topic! I have never evaluated this API. I will think through it once I have the capacity, meanwhile, I want to invite @evgenykuzyakov and @matklad to hear their thoughts.

from borsh-rs.

@jstarry Would you like to contribute a PR?

@matklad Thanks for the detailed overview 👍

from borsh-rs.

@frol no problem, just opened a PR

from borsh-rs.