I wouldn't call this a "hack", but it does stuff in the wrong order.

What we should do instead is add a dep to bitcoin-io on consensus_encoding. Then in bitcoin-io we can provide a ReadError and decode_from_read method analogous to the existing ones in consensus_encoding for std Read.

Then most of your "ugly hack" would be replaced with a call to bitcoin_io::decode_from_read. The remaining ugliness is about converting the bitcoin_io::ReadError into the old bitcoin::consensus::Error. Well, that error is #[non_exhaustive] so we can add variants (and actually we're totally breaking the API so in principle we can do whatever we want, but everything I've said thus-far is backportable to 0.32.x so let's try to keep it that way as long as possible).

So let's add a variant to bitcoin::consensus::Error which holds a primitives::WitnessDecoderError. This is pretty ugly, adding a type-specific error variant to an error type which is supposed to be a general-purpose error type. But another way to look at it is that it's a less-ugly expression of the inherent ugliness of from_parts__unstable.

So let's add a variant to bitcoin::consensus::Error which holds a primitives::WitnessDecoderError.

I did try that this morning but ran into trouble, I stopped at the IterReader IIRC.

EDIT: Oh I remember now, deserialize_partial doesn't work, there is no way to get a ParseError.

You should add the variant to ParseError, not consensus::Error. (Or you can keep using ParseError::ParseFailed as you do now, but that loses a lot of information).

In this current PR you add a variant to Error but AFAICT you never actually use it.

Oh I didn't think of that, it will mean the io::Error is in two places but anyways, I'll have a go.

Lint failure is real (clippy)

Looks like this still inlines "decode from bitcoin::io::BufRead" logic which should be in io.

Thanks, I'll come back.

Looks like this still inlines "decode from bitcoin::io::BufRead" logic which should be in io.

I'm not sure what you mean? This is a copy of encoding::decode_from_read but with a WitnessDecoder instead of generic decoder (T) and using the ParseError instead of ReadError::Decode.

It should be a single call to io::decode_from_read, not a copy of the inside of encoding::decode_from_read.

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

The API break is expected and comes from #5249.