Redesign hex::BufEncoder to accept owned arrays

[Kixunil]

Not being able to create an owned BufEncoder prevented returning it from functions which need to allocate the buffer on stack. Such is the case in WIP serde via consensus serialzation.

This change refactors OutBytes to be unsized, adds an AsOutBytes trait and uses that one instead of Into to perform the conversion.

Closes #1270

This is meant as potentially mergeable demonstration. Interestingly, it was easier than I expected.

[Kixunil]

Note I found that miri setup requires xargo. Is it worth adding to CI for something this simple? FTR I tried on my machine and it passes.

[apoelstra]

This is really elegant! Out of curiosity where in std did you copy this from?

concept ACK, even with the unsafety.

Regarding xargo/miri, yes it's worth it!! We should MIRI-check a ton of stuff (annoyingly last I looked, anything that called into libsecp we couldn't do with miri which eliminates a lot of crypto..). But we should do this in a separate PR (even though initially they'll be "dumb" tests that don't even involve unsafety so theoretically miri should never get mad about).

[Kixunil]

Same as before: std::path::Path::new

miri is only useful for unsafe (as you noted libsecp256k1 where it's unusable) and potentially for dependencies with unsafe that don't run it themselves or have poor coverage (can we avoid such dependencies?)

I'll try to add it.

[Kixunil]

Note one would normally have two traits - one for mut the other for non-mut. The limitation is that immutable version of AsOutBytes can not be implemented for &T where T: AsOutBytes.

I think this is fine, we really don't want to deal with immutable slices beyond providing non-weird API of BufEncoder

[Kixunil]

I propose to add miri tests after #1272 is merged which adds the test script. There shouldn't be much harm merging this sooner and it'd unblock my other work. But I can wait as well.

[apoelstra]

Sounds good.

re-concept ACK -- I had an idea that I could evade the unsafety by clever use of AsRef<[u8]> and AsMut<[u8]> but it didn't pan out. FYI the comment on the OutBytes type still says "we are trying to avoid unsafety for as long as possible". I'm not sure if this is stale or if it's meant to specifically refer to the use of MaybeUninit.

[apoelstra]

ACK 9a29b5b

[Kixunil]

We can't use AsRef or any other core trait because none of them guarantees pure functions. (Goes into the collection of traits that should be in core.) Without this change to MaybeUninit would be unsound and we'd have to break the API to fix it - what my design avoids.

Indeed the unsafe refers to MaybeUninit which is much more tricky than those two casts. (Requires Out type for instance.)

[tcharding]

Except for the question about the out_bytes module, ACK 1bf8855

[tcharding]

I don't understand this comment (and module use), the OutBytes inner array is private so cannot be accessed by other modules anyways? Or am I missing something? I would have expected to see OutBytes as part of buf_encoder module and private::Sealed.

[Kixunil]

If it was in buf_encoder it'd make the field accessible to the rest of the module which we don't want because it raises auditing cost. This is not critical now but if we ever switch to MaybeUninit we won't forget about it and will have a much smaller diff. :)

[tcharding]

Cool, can we change it to "This prevents the rest of the module from accessing the field of OutBytes" please.

[tcharding]

nit only, of course :)

[Kixunil]

and the crate :)

I'd prefer not to mess with a PR that has two ACKs for something this trivial.

[apoelstra]

ACK 1bf8855