Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace unmaintained cbor implementation #587

Open
apoelstra opened this issue Mar 14, 2023 · 6 comments
Open

Replace unmaintained cbor implementation #587

apoelstra opened this issue Mar 14, 2023 · 6 comments

Comments

@apoelstra
Copy link
Member

I noticed when trying to check minimal dependency versions that cbor depends on rustc_serialize 0.3.0 (lol) which doesn't compile. rustc_serialize 0.3.24 does compile, so there is no problem, except when you try to use -Z minimal-versions.

However apparently, cbor has been unmaintained for the last 4 years.. Furthermore, its replacement serde_cbor has been unmaintained for 18 months. So maybe we need to move away from it anyway.

It looks like a good replacement would be minicbor but I haven't investigated yet. Noting for later.

@apoelstra
Copy link
Member Author

rustc-serialize 0.3.20 is the oldest version that compiles, for future reference.

@apoelstra
Copy link
Member Author

In addition to replacing cbor we also need cargo update -p serde_test --precise 1.0.19 (or rather, a minimal serde_test version of 1.0.19) because we depend on the Configure struct which is not available in earlier versions.

@Kixunil
Copy link
Collaborator

Kixunil commented Mar 15, 2023

Since it's only used in test I think just using serde_test deferring to this statement is OK?

If the crate works for you there is no need to switch to another implementation.

Security vulnerabilities shouldn't be a problem unless they start needlessly annoying people using cargo audit or something.

we also need cargo update -p serde_test --precise 1.0.19

No, that's actually our Cargo.toml being wrong. It should say serde_test = "1.0.19", not serde_test = "1.0".

@apoelstra
Copy link
Member Author

I think we need to move away from cbor, because right now we cannot compile this library with -Z minimal-versions, and one reason is that cbor has mis-specified their rustc-serialize dependency as being too low, and we can't fix it because the library is unmaintained.

@apoelstra
Copy link
Member Author

I'm pretty skeptical of using serde_test and nothing else. AFAICT serde_test provides zero value for something where all the objects are bytestrings, because it interprets ownership in ways that aren't consistent with actual deserializers.

@Kixunil
Copy link
Collaborator

Kixunil commented Mar 15, 2023

we can't fix it because the library is unmaintained.

Actually we can if we rename cbor crate and have cbor feature which depends on whichever rustc-serialize version is the right one. I'm not suggesting this is great, moving is better.

I'm pretty skeptical of using serde_test and nothing else.

Yeah, while it's annoying to have to test multiple formats it seems to be the least bad solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants