8000 Remove the hex dependency by elichai · Pull Request #381 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove the hex dependency #381

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

Merged
merged 1 commit into from
Jan 24, 2020
Merged

Conversation

elichai
Copy link
Member
@elichai elichai commented Jan 8, 2020

This removes the hex dependecy from the tests.
I had to leave the hex_hash! macro because here https://github.com/rust-bitcoin/rust-bitcoin/blob/master/src/util/bip143.rs#L143 I needed to use that instead of SigHash::from_hex because the latter requires the hex to be in reverse ordering.

FYI currently even non tests will build hex because of #[cfg(any(test, feature = "serde"))] extern crate hex; This is no longer true as of rust-bitcoin/bitcoin_hashes#71 (I confued myself. even though the hex crate isn't used in non-test code it is still compiled and linked for that reason)

Closes #332

@elichai elichai force-pushed the 2020-01-hex branch 2 times, most recently from 37b1ab8 to 865d4ae Compare January 8, 2020 17:50
@elichai
Copy link
Member Author
elichai commented Jan 8, 2020

This won't work until rust-bitcoin/bitcoin_hashes#71 will get merged because the hash_newtype! currently require the hex crate

@elichai
Copy link
Member Author
elichai commented Jan 20, 2020

Now that rust-bitcoin/bitcoin_hashes#71 is merged this should is mergeable.

@codecov-io
Copy link
codecov-io commented Jan 20, 2020

Codecov Report

Merging #381 into master will increase coverage by 0.51%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
+ Coverage   85.15%   85.66%   +0.51%     
==========================================
  Files          40       40              
  Lines        7478     7745     +267     
==========================================
+ Hits         6368     6635     +267     
  Misses       1110     1110
Impacted Files Coverage Δ
src/lib.rs 100% <ø> (ø) ⬆️
src/util/contracthash.rs 78.5% <ø> (ø) ⬆️
src/util/psbt/macros.rs 100% <ø> (ø) ⬆️
src/internal_macros.rs 59.47% <ø> (ø) ⬆️
src/util/base58.rs 82.16% <100%> (+1.47%) ⬆️
src/blockdata/constants.rs 82.81% <100%> (ø) ⬆️
src/util/address.rs 87.87% <100%> (+1.49%) ⬆️
src/network/message.rs 95.13% <100%> (ø) ⬆️
src/network/message_network.rs 69.64% <100%> (ø) ⬆️
src/network/stream_reader.rs 95.62% <100%> (ø) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5452260...c19b736. Read the comment docs.

@@ -84,7 +84,7 @@ fn bitcoin_genesis_tx() -> Transaction {

// Outputs
let out_script = script::Builder::new()
.push_slice(&Vec::<u8>::from_hex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f").unwrap())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah that's really cool :) I guess Vec<u8> is the only vec FromHex is defined on?

Copy link
Member Author

Choose a reason for hiding this comment

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

It wouldn't make much sense on any other Vec :) (without getting into endianness lol )

Copy link
Collaborator
@stevenroose stevenroose left a comment

Choose a reason for hiding this comment

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

Great!

@elichai elichai requested a review from apoelstra January 21, 2020 11:11
Copy link
Member
@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

nicee

@apoelstra apoelstra merged commit 659f2ed into rust-bitcoin:master Jan 24, 2020
@elichai elichai deleted the 2020-01-hex branch January 26, 2020 09:48
yancyribbens pushed a commit to yancyribbens/rust-bitcoin that referenced this pull request Mar 23, 2024
We manually implement a bunch of traits on `Miniscript` that pass
through to the `node` field (e.g. `PartialEq`). We should do the same
for `hash::Hash` instead of deriving it.

Found by clippy.

Fixes: rust-bitcoin#381
yancyribbens pushed a commit to yancyribbens/rust-bitcoin that referenced this pull request Mar 23, 2024
…script

4ac68c9 Implement hash::Hash for Miniscript (Tobin C. Harding)

Pull request description:

  We manually implement a bunch of traits on `Miniscript` that pass through to the `node` field (e.g. `PartialEq`). We should do the same for `hash::Hash` instead of deriving it.

  Found by clippy.

  Fixes: rust-bitcoin#381

ACKs for top commit:
  apoelstra:
    ACK 4ac68c9

Tree-SHA512: 7920481034bee12fbfdbdfac2f38e9598e25a13a8a89b6b50cc7f63ce528c2de47e959ad073ac1c8f1ab64d4b0e432d264a5caf74ddb656c1a0ffbd8923faf6f
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

Successfully merging this pull request may close these issues.

Use bitcoin_hashes::hex instead of crate hex
4 participants
0