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

psbt: Use macro to hash instead of relying on Hash trait #2715

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tcharding
Copy link
Member

We have a private function that makes use of the Hash trait to generically hash map entries. This usage makes patching the hashes module difficult. We can achieve the same thing by using a macro and passing in the concrete type.

This is an internal change, no effect on logic or public API.

We have a private function that makes use of the `Hash` trait to
generically hash map entries. This usage makes patching the `hashes`
module difficult. We can achieve the same thing by using a macro and
passing in the concrete type.

This is an internal change, no effect on logic or public API.
@github-actions github-actions bot added the C-bitcoin PRs modifying the bitcoin crate label Apr 25, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8825286212

Details

  • 12 of 12 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 83.111%

Totals Coverage Status
Change from base Build 8801402712: -0.02%
Covered Lines: 19158
Relevant Lines: 23051

💛 - Coveralls

@apoelstra
Copy link
Member

Very interesting. I guess this is an actual use of "being generic over a hash". I suppose we could have used this pattern in rust-miniscript but I don't think it ever occurred to us that these different hashtypes share a trait, bacues it's a bit of a weird thing.

concept ACK convering this to a macro (and, when we remove the Hash trait, forcing downstream users to do the same...though maybe they will be able to be generic over hash engines instead).

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.

ACK 13eb3fb

@tcharding tcharding changed the title psbt: Use macro instead of function psbt: Use macro to hash instead of relying on Hash trait Apr 25, 2024
@tcharding
Copy link
Member Author

Changed PR name to make searching easier because this is related to the hashes crate.

@tcharding tcharding added the one ack PRs that have one ACK, so one more can progress them label May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bitcoin PRs modifying the bitcoin crate one ack PRs that have one ACK, so one more can progress them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants