search

rust-bitcoin / hex-conservative

Hex crate with a conservative MSRV and dependency policy
Creative Commons Zero v1.0 Universal
8 stars 9 forks source link

Stabalization - roadmap to v1.0 #43

Open tcharding opened 1 year ago

tcharding commented 1 year ago

Tracking issue for what is needed for us to be able to stabalize the hex-conservative crate.

TODO

tcharding commented 3 months ago

@Kixunil can we come up with a list of things that need doing and get it done?

Kixunil commented 3 months ago

At minimum:

apoelstra commented 3 months ago
  • Multi error support (if we still want it, I personally do but I'm now questioning if it should be global)

I don't think we want this. It would be a huge job, incur a ton of complexity to make it nostd/noalloc-compatible in a nonbreaking way, has limited value (if the user is providing invalid hex there are a small number of likely culprits, none of which would benefit from seeing multiple error locations), and also I've literally never seen multi-error support on hex decoding in any user-facing software I've used.

tcharding commented 3 months ago

BufEncoder is used in rust-bitcoin but only in test code, we could remove that?

Kixunil commented 3 months ago

I don't think we want this. It would be a huge job, incur a ton of complexity

I've already done it before, it's not too bad.

make it nostd/noalloc-compatible

It only requires alloc, not std and would be obviously feature gated.

none of which would benefit from seeing multiple error locations

Making multiple typos is certainly realistic scenario. If we assume this library is only used for long hashes then it's quite reasonable to expect that people won't type them but I'd at least expect people accidentally copying whitespace before/after the value.

I've literally never seen multi-error support on hex decoding in any user-facing software I've used.

That doesn't mean such software should never be built. I find seeing multiple errors from rustc/gcc very helpful (unless one causes another which sadly is true) yet I've never seen those in Turbo Pascal... I have long-term plans to improve things in this area in various applications.

BufEncoder is used in rust-bitcoin but only in test code, we could remove that?

Yes, IDK what those tests use BufEncoder for but them existing sounds sus and it's possible that they should just use arrayvec::ArrayString instead.

apoelstra commented 3 months ago

If you're willing to do the work, including to close the error types sufficiently that they present the same API with or without multi-error support, I suppose I'll review and ACK it.

But I don't see how "it's very useful seeing multiple errors in compilation of tens of thousands of lines" implies "it's useful at all to see multiple errors in fewer than ten characters" (with more than ten the data is almost certainly copy/pasted and errors are either "wrong paste" or "crap at the beginning/end" neither of which need any error indication beyond "invalid hex"). I suspect in practice this will just be extra noise. But it's easy for users of the library to just ignore the extra information so no harm done.

Kixunil commented 3 months ago

OK. It'll be almost invisible to those who don't use it. The only difference is they'll have to use a method to access the enum. (We could also have a field but it makes no real difference.)

tcharding commented 2 weeks ago

We are starting to get close to 1.0'ing this crate. @Kixunil it would be great to get your acknowledgement before we do but since primitives 1.0 requires hex to be stable we cannot wait indefinitely. Posting this here and also I'll email you, how about we can give you 3 months from now.