fix: correctly encode multihashes with serde #7
Conversation
Stebalien
force-pushed
the
feat/enc-dec
branch
from
6d2cc08 to
0c065d7
Compare
The number of digest bytes written needs to match the size. See the upstream issue here: multiformats/rust-multihash#140 All credit goes to @koushiro. My one change is to not allocate.
Stebalien
force-pushed
the
feat/enc-dec
branch
from
0c065d7 to
2077805
Compare
|
Note: I'm not entirely sure if this is the correct fix. At a minimum, we shouldn't be writing the extra bytes. But I'm not sure if just spitting out the buffer without calling |
|
And it's definitely a breaking change. |
|
Looking at the upstream issue, this makes sense to me, and resolves some of the confusion I had around truncation. Not 100% sure it's completely right either, but it seems more right than what exists currently, so in the interest of moving fast I've merged it. Do the other multihash implementations do truncation correctly? Would be useful to compare if so |
Yes. My concern here is whether or not this is supposed to be encoded as a multihash or as something else. Specifically, should the digest be length-prefixed, or written as raw bytes. |
|
I'm pretty sure it should be encoded as a multihash, as per multiformats/multihash#133 |
The number of digest bytes written needs to match the size.
See the upstream issue here: multiformats/rust-multihash#140
The one change is to not allocate.