update rust-bitcoin to 0.31 #67
Conversation
RCasatta
force-pushed
the
2024-01--bitcoin31
branch
4 times, most recently
from
4c44f34 to
01c52ee
Compare
RCasatta
force-pushed
the
2024-01--bitcoin31
branch
from
01c52ee to
e7a853e
Compare
|
Sorry, some cargo fmt slipped in by error on autofmt on save, let me know if I have to fix those |
|
utACK e7a853e. |
| forward | ||
| ); | ||
| sha256t_hash_newtype! { | ||
| pub struct Elip151Tag = hash_str("ELIP-151 Deterministic descriptor blinding keys"); |
There was a problem hiding this comment.
Lol, derp. But we really should have had tests to catch this!!
There was a problem hiding this comment.
would it be feasible and makes sense to have a parameter of the macro requiring to specify the result of the hash for the empty string and the macro generates a test for it?
There was a problem hiding this comment.
oo I like that idea.
cc @tcharding what do you think?
There was a problem hiding this comment.
I don't quite get it, what is the test hoping to catch? Is it hoped to be a regression test? (I don't see how one would get the hash of the empty string without first using the new tagged type.) Am I missing some context?
There was a problem hiding this comment.
Yes, you'd run the test once to get the correct string.
And then yes, it would be a regression test which would (hopefully) detect whenever anybody poked at the macro, or replaced the macro, or something, causing the hash to change.
Having said that, having such a test as part of the macro limits its value, since it's likely to go away when the macro is changed, which is exactly when we want it..
There was a problem hiding this comment.
We could definitely, and probably should already, have a single unit test in each repo that calls the macro and tests the empty hash - "don't trust verify" and all that.
There was a problem hiding this comment.
I hear those lads that maintain hashes chop and change every week like a bunch of cowboys.
4 participants
Solves only hard errors for now