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

Deprecate Message::from_digest_slice #712

Merged

Conversation

Kixunil
Copy link
Collaborator

@Kixunil Kixunil commented Jul 25, 2024

Closes #710

On top of #709

@Kixunil Kixunil added the 1.0 Issues and PRs required or helping to stabilize the API label Jul 25, 2024
@Kixunil Kixunil force-pushed the deprecate-message-from-digest-slice branch from 72b753f to c536871 Compare July 25, 2024 07:53
@tcharding
Copy link
Member

Can be rebased now, #709 is in.

@Kixunil Kixunil force-pushed the deprecate-message-from-digest-slice branch from c536871 to ff5188d Compare July 27, 2024 14:35
@Kixunil Kixunil marked this pull request as ready for review July 27, 2024 14:35
@apoelstra
Copy link
Member

The hex-lit thing requires you update the lockfiles.

I'm not sure why CI isn't failing.

The tests defined custom `hex!` macros (yes, two actually) that
evaluated to `Vec<u8>`. While the performance didn't matter it made it
harder to use with interfaces that require arrays and all current uses
were passing it as slices anyway.

So, in preparation for upcoming changes, this commit introduces
`hex_lit` dev-dependency which evaluates to array allowing better
interaction with type checker.
All sensible hash engines return arrays, not slices or other things,
therefore `Message::from_digest_slice` is most likely entirely unneeded
since the array version does a better job and in those rare cases where
it is, the users can just call `.try_into()` themselves.

This commit deprecates `from_digest_slice` and changes all tests to use
`from_digest` except the test that tests `from_digest_slice`. It also
simplifies its code to use `try_into` rather than convert manually and
inefficiently.
@Kixunil Kixunil force-pushed the deprecate-message-from-digest-slice branch from ff5188d to 939bf9e Compare July 28, 2024 07:13
@Kixunil
Copy link
Collaborator Author

Kixunil commented Jul 28, 2024

Done.

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 939bf9e

@apoelstra apoelstra merged commit 3f067d5 into rust-bitcoin:master Jul 28, 2024
21 checks passed
@Kixunil Kixunil deleted the deprecate-message-from-digest-slice branch July 28, 2024 18:35
apoelstra added a commit that referenced this pull request Jul 29, 2024
72e09c1 Improve the comment on `Message::from_digest` (Martin Habovstiak)

Pull request description:

  Minor improvement on top of #712

ACKs for top commit:
  apoelstra:
    ACK 72e09c1

Tree-SHA512: 06e8e706bb9732ea46ef3488ed33f7c7c84ea5afa5b1b2bca03cd2641524ff61156133436c1dd62df62769c8544644e1a4453fbacf4413fece73282ae154a387
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 Issues and PRs required or helping to stabilize the API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate Message::from_digest_slice?
3 participants