`BorshSerialize::serialize` method is unergonomic for mutable slices

[jstarry]

Hello! First of all thanks for this library ❤️

Problem

BorshSerialize::serialize method signature is not very ergonomic when using mutable slices. This is because a &mut [T] already implements Write but the method expects a &mut W where W itself implements Write. This means that in order to pass a mutable slice, you need to pass a mutable reference to the mutable slice: &mut &mut [T].

Proposed Solution

Deprecate serialize and create a bincode inspired serialize_into method which looks like this: https://docs.rs/bincode/1.3.3/bincode/fn.serialize_into.html

However this is a 0.x library, so up to you 😉

[frol]

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.

[matklad]

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 {}

[jstarry]

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

[frol]

@jstarry Would you like to contribute a PR?

@matklad Thanks for the detailed overview :+1:

[jstarry]

@frol no problem, just opened a PR