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.

Sign up for GitHub

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

Jump to bottom

Fixing hashes core dependency and fuzz feature #602

Merged
merged 1 commit into from May 7, 2021
Merged

Fixing hashes core dependency and fuzz feature #602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.toml
View file
Open in desktop
Expand Up @@ -14,7 +14,7 @@ readme = "README.md"
[features]
default = [ "secp-recovery" ]
base64 = [ "base64-compat" ]
fuzztarget = ["bitcoin_hashes/fuzztarget"]
fuzztarget = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should drop this feature entirely now, its been replaced with an explicit --cfg=fuzzing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance that people integrated this recusively into their fuzzing pipeline just as we did with bitcoin_hashes? If so this is a breaking change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be very surprised if that were the case, though, indeed, its possible. Ultimately its still just in fuzzing pipeline(s), so hopefully not anything that would break any release builds anywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I assume we need to leave this as is for 0.26.1 and drop in 0.27?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're not going to yank bitcoin_hashes (which I'm fine with either way) then we can call it a consistent policy. Probably the only thing that will break is rust-Lightning's fuzz crates, and I'm fine dealing with that :).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a little bit confusing that we now have a fuzztarget feature which does nothing at all - you have to set cfg=fuzzing to get any behavior out of it. I think we should drop it.

unstable = []
rand = ["secp256k1/rand-std"]
use-serde = ["serde", "bitcoin_hashes/serde", "secp256k1/serde"]
Expand All @@ -23,7 +23,7 @@ secp-recovery = ["secp256k1/recovery"]

[dependencies]
bech32 = "0.7.2"
bitcoin_hashes = "0.9.1"
bitcoin_hashes = "0.9.6"
sgeisler marked this conversation as resolved.
Show resolved Hide resolved
secp256k1 = "0.20.0"

base64-compat = { version = "1.0.0", optional = true }
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Open in desktop
Expand Up @@ -99,8 +99,8 @@ pub use util::ecdsa::PublicKey;

#[cfg(all(test, feature = "unstable"))]
mod tests {
use hashes::core::fmt::Arguments;
use std::io::{IoSlice, Result, Write};
use std::fmt::Arguments;

#[derive(Default, Clone, Debug, PartialEq, Eq)]
pub struct EmptyWrite;
Expand Down