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

const constructors for hashes #2377

Open
stevenroose opened this issue Jan 22, 2024 · 10 comments · May be fixed by #2641
Open

const constructors for hashes #2377

stevenroose opened this issue Jan 22, 2024 · 10 comments · May be fixed by #2641
Labels
API hole Important API missing - significantly degrades usability or idiomatic usage

Comments

@stevenroose
Copy link
Collaborator

The hashes currently have no const constructors (not even from_byte_array or all_zeros).

I found out because I was trying to make OutPoint::new and OutPoint::null const fns, but the latter is impossible, it seems.
@Kixunil
Copy link
Collaborator

Kixunil commented Jan 22, 2024

FYI I had a PR to do this but it got stale and I'm actually currently working on a replacement since the hash stuff got complicated.

@Kixunil Kixunil added the API hole Important API missing - significantly degrades usability or idiomatic usage label Jan 22, 2024
@apoelstra
Copy link
Member

from_byte_array and all_zeros could be made const and we could do a point release (assuming there are no other breaking changes in the hashes pipeline yet, which I believe is true).

Actually hashing in constant time is far from trivial, though I haven't tried in a little while.

@Kixunil
Copy link
Collaborator

Kixunil commented Jan 22, 2024

Actually hashing in constant time is far from trivial, though I haven't tried in a little while.

We already have that :)

@apoelstra
Copy link
Member

Oh, lol :). So @stevenroose there is a const constructor.

But agreed that all_zeros and from_byte_array should also be const.

@Kixunil
Copy link
Collaborator

Kixunil commented Jan 22, 2024

Definitely!

@tcharding
Copy link
Member

Seems that Rust does not allow functions within traits to be marked as const.

ref: https://users.rust-lang.org/t/whats-the-reason-that-fn-in-a-trait-cannot-marked-as-a-const/68636

@apoelstra
Copy link
Member

Ah, yeah, that is probably why these aren't const already. We should move those methods out of the trait into inherents.

@Kixunil
Copy link
Collaborator

Kixunil commented Mar 13, 2024

We should move those methods out of the trait into inherents.

Yes!

@15IITian
Copy link
Contributor

15IITian commented Mar 26, 2024 •
edited

image

rust-bitcoin/hashes/src/sha256t.rs

Line 23 in 9df5963

pub struct Hash<T: Tag>([u8; 32], PhantomData<T>);

In all_zeros and from_byte_array methods in Hash<T> impl -> what should be the value of PhantomData part in it?

@apoelstra
Copy link
Member

PhantomData has only one value, called PhantomData. Use that.

15IITian added a commit to 15IITian/rust-bitcoin that referenced this issue Mar 30, 2024
@15IITian
refactor: move all_zeroes & from_byte_array to inherents & make t…
1d6373d 
…hem const

Made these functions const fn & move them from the `Hash` trait to `hash_type` & `hash_newtype`
macros

fix rust-bitcoin#2377
@15IITian 15IITian linked a pull request Mar 30, 2024 that will close this issue
Draft
15IITian added a commit to 15IITian/rust-bitcoin that referenced this issue Apr 6, 2024
@15IITian
refactor: move all_zeroes & from_byte_array to inherents & make t…
8a8be4e 
…hem const

Made these functions const fn & move them from the `Hash` trait to `hash_type` & `hash_newtype`
macros

fix rust-bitcoin#2377
15IITian added a commit to 15IITian/rust-bitcoin that referenced this issue Apr 6, 2024
@15IITian
Move all_zeroes & from_byte_array to inherents & make them const
6c7f81d 
Made these functions const fn & move them from the `Hash` trait to `hash_type` & `hash_newtype`
macros

fix rust-bitcoin#2377
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
API hole Important API missing - significantly degrades usability or idiomatic usage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

move all_zeroes & from_byte_array to inherents & make them const. 15IITian/rust-bitcoin
5 participants
@stevenroose @Kixunil @apoelstra @tcharding @15IITian