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

Define an engine type when creating tagged hash engines #2719

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tcharding
Copy link
Member

Done on top of #2717, this is just a demo.

It may be useful for new tagged hash types created with the sha256t_hash_newtype macro to also define a hash engine.

In `bitcoin` we never use the `sha256t::Hash` type directly but only as
an underlying generic type that holds the tagging logic. To get tagged
hash types we currently use the `sha256t_hash_newtype` macro.

Assuming other users implement tagged hashing as we do in `rust-bitcoin`
then the `sha256t::Hash` type currently serves only as a type to
implement `Tag::engine` on - but as long as there exists a way to get
the pre-tagged engine then we do not actually need the type.

If we assume all users will use a provided macro to define tagged hashes
then we have two options. This PR implements the first option.

1. In the `NewTypeHash::engine` function return a pre-tagged
  `sha256::HashEngine`.

2. Have callers of the macro define a new engine type as well as a new
hash type, then we can put the tagging logic in the `Default`
implementation of the new engine.

Remove the `sha256t::Hash` type, and the `Tag` trait. Modify the
`sha256_hash_newtype` macro to match and just implement the tagging
logic in the `engine` function.

Add unit tests to make sure we correctly handle the forward/backward
attribute. `rust-bitcoin` includes test already that verify this patch.
It may be useful for new tagged hash types created with the
`sha256t_hash_newtype` macro to also define a hash engine.

This PR demonstrates doing so.
@github-actions github-actions bot added C-bitcoin PRs modifying the bitcoin crate C-hashes PRs modifying the hashes crate labels Apr 25, 2024
@tcharding tcharding changed the title Define an engine tyep when creating tagged hash engines Define an engine type when creating tagged hash engines Apr 25, 2024
@coveralls
Copy link

coveralls commented Apr 25, 2024

Pull Request Test Coverage Report for Build 8827156010

Details

  • 53 of 62 (85.48%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.07%) to 83.201%

Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/src/psbt/mod.rs 0 1 0.0%
bitcoin/src/taproot/mod.rs 10 12 83.33%
hashes/src/sha256t.rs 41 47 87.23%
Totals Coverage Status
Change from base Build 8801402712: 0.07%
Covered Lines: 19216
Relevant Lines: 23096

💛 - Coveralls

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 C-hashes PRs modifying the hashes crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants