Currently we are panicing if neither `global-context` or `alloc`
features are enabled. We do not need to do so, we can just disable the
whole impl of `FromStr`.
Currently we are attempting to truncate the hash created using
`bitcoin_hashes` by using the "width" formatting parameter instead of
the "precision" parameter. `hex-conservative` truncates with the
"precision" parameter as is expected since a hash is not an integral
type.
Use the formatting string `"{:.16}"` which is the "precision"
formatting parameter.
The example claimed it'd be unsafe, which is a specific Rust term and
thus confusing. It'd just be cryptographically broken. Also the example
passes in a constant which looks ridiculously unrealistic.
Fix these by
* changing the comment to say cryptographically broken
* making the example pass the input through invisible fake hash function
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.
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.
The version 1.63 satisfies our requirements for MSRV and provides
significant benefits so this commit bumps it. This commit also starts
using weak dependencies.
This is a legacy constant and it's better to just use `i32::MAX`. Note
that one cannot `use` an associated constant so this just removed the
import. This is better anyway since it's only used once and it didn't
provide meaningful line length reduction.
The implementations of `ThirtyTwoByteHash` for types from the `hashes`
crate are problematic during upgrades because both `bitcoin` and
`secp256k1` depend on `hashes` and when the versions of `hashes` get
out of sync usage of the trait breaks.
Deprecate the `ThirtyTwoByteHash` trait and remove the impls for types
from `bitcoin_hashes`.
Add an explanation in the changelog because its too long to go in the
deprecation message.
Wildcards make it hard to grep for where stuff comes from, explicit
imports and re-exports are ... more explicit.
Re-export the `key` types explicitly.
Wildcards make it hard to grep for where stuff comes from, explicit
imports and re-exports are ... more explicit.
Import and re-export explicitly instead of by using wildcards.
The inner bytes of `ElligatorSwiftSharedSecret` were almost inaccessible
making the type almost useless, so this commit adds methods to access
inner bytes.
Closes#675
dd6bf7c10d Fix unit test import statements (Tobin C. Harding)
Pull request description:
In `lib.rs` unit tests we are getting build warnings because of how we are importing things, just import with `super::*` unconditionally and be done with it.
This patch is the only good one out of #661.
ACKs for top commit:
apoelstra:
ACK dd6bf7c10d
Kixunil:
ACK dd6bf7c10d
Tree-SHA512: 3970f4c1374ec6de4798bfb52b561e9ac4611ec3a3885edc79639566f777e1fbb502cb36fa7abd015f3fd4a9ca4b6a4931b4ecb2e629e967b4e49391db97a97f
In `lib.rs` unit tests we are getting build warnings because of how we
are importing things, just import with `super::*` unconditionally and be
done with it.
Create bindings for all methods and static types in ellswift.h in
secp256k1-sys and their respective safe-rust types.
All methods are extensively commented and tested using BIP324's
test vectors
This is just a bad test. It constructs a preallocated context object by
starting from a non-preallocated context object, in a way that can't be
done by users (since it directly constructs a `Secp256k1` struct) and a
way that is very difficult to unwind, because you wind up with two
pointers to the same underlying context object, one a "preallocated" one
and one a normal one.
If you then drop the preallocated one, it will call
`secp256k1_context_destroy`, forcing you to manually deallocate the
other one. If you drop the normally-allocated one, you need to
mem::forget the preallocated one to avoid calling
`secp256k1_context_destroy` twice. The whole thing is pretty fragile.
There is another unit test, `test_raw_ctx`, which gets into the same
situation but using the public API, and demonstrates a few ways to get
out of it.
aa4489c71b key: Improve docs (Tobin C. Harding)
Pull request description:
`PublicKey` types are for verifying ECDSA signatures, when these docs where written there were no other types of signatures. With the addition of taproot these docs have become stale.
ACKs for top commit:
apoelstra:
ACK aa4489c71b
Tree-SHA512: bb24d82f2bf316f8907b1bf02132d454d21f0b13d57f06f09f9985bc7fbf7b36e6972a0fdaf3a68967577dbe1995f2a14fd06fddd38eb46718f04bca1c50a445
We use "keypair" in identifiers (local vars and function names) but
`KeyPair` - one of them is wrong.
Elect to follow upstream and define keypair as a single word i.e., use
`Keypair` for type name and `keypair` in identifiers.
This patch can be reproduced mechanically by doing two
search-and-replace operations on all files excluding the CHANGELOG
- Replace "KeyPair" with "Keypair"
- Replace "key_pair" with "keypair"
Now that we have `hashes` as the crate name of `bitcoin_hashes` we can
slightly clean up the import statements.
This is based on the convention we have to import things directly from
the crate if we depend on it and not from the crate level re-export.
Use the more terse `hashes` by way of the `package` field in the
manifest.
Allows us to remove the ugly feature alias "bitcoin-hashes" ->
"bitcoin_hashes" and removes all the bother with the underscore.
Why did we not think of this 2 years ago?
cd40ae7f19 Improve Message constructors (Tobin C. Harding)
Pull request description:
Observe:
- The word "hash" can be a verb or a noun, its usage in function names is therefore at times ambiguous.
- The function name `from_slice` gives no indication as to what the slice input is.
Improve Message constructors by doing:
- Add a constructor `Message::from_digest` that takes a 32 byte array as input.
- Rename `Message::from_slice` to `Message::from_digest_slice` (deprecate `from_slice` and add `from_digest_slice`)
- Improve the docs while we are at it.
### Note
The original PR conflate 2 separate issues, the `Message` constructor naming clarity issue and the upgrade difficulty issue, PR is now only a solution to the first. The second will be done as a separate PR.
ACKs for top commit:
apoelstra:
ACK cd40ae7f19
Tree-SHA512: 4e5aeccf15cca95073f4c3a518b9e1f54f0e33c92c45dfecd1daa31d052022cd28c71bb6df6cff8a6548993e3e22788f11cd2633214ab5a580c753e66d2ea749
Currently the panic message refers to stuff related to development of
the library, this is meaningless for users of the lib. Target panic
message at secp users instead.
We have an unconditional panic for some combination of features, this
causes clippy to give a bunch of useless warnings.
Add allow attributes to quieten down clippy.
Observe:
- The word "hash" can be a verb or a noun, its usage in function names
is therefore at times ambiguous.
- The function name `from_slice` gives no indication as to what the
slice input is.
Improve Message constructors by doing:
- Add a constructor `Message::from_digest` that takes a 32 byte array as
input.
- Rename `Message::from_slice` to `Message::from_digest_slice`
(deprecate `from_slice` and add `from_digest_slice`)
- Improve the docs while we are at it.
8af2cf12da add .serialize() function to schnorr signature (isaac-asdf)
Pull request description:
convert from Signature to a byte_array
ACKs for top commit:
Kixunil:
ACK 8af2cf12da
tcharding:
ACK 8af2cf12da
apoelstra:
ACK 8af2cf12da
Tree-SHA512: b69d58646cdba4d83a79189f18628590970f471771feef0e11e089d73bd934777e3554a448b88a3643203522fde98084fd7570a5cec400516166583a3433c000
896e6c7f2d Introduce SPDX license identifiers (Tobin C. Harding)
Pull request description:
Licenses are boring as hell, so is are all the comments at the top of each file. This patch makes no comment on the merit of license comments in each file, rather this patch reduces the license comment to the minimum possible with no loss of meaning - an SPDX license identifier.
Note also please that we remove the "written by" comments as well for the following reasons (discussed recently on rust-bitcoin repo):
- they are not descriptive because many devs contributed
- they have a tendency to include the wrong date because of cut'n'pasta
- all this info is in the git history
ref: https://spdx.dev/ids/#how
cc elichai because this PR removes your name but you were not explicitly part of the conversation on `rust-bitcoin` about this topic. Here is the issue: https://github.com/rust-bitcoin/rust-bitcoin/issues/1816 also for more on SPDX see https://github.com/rust-bitcoin/rust-bitcoin/pull/1076
ACKs for top commit:
Kixunil:
ACK 896e6c7f2d
apoelstra:
ACK 896e6c7f2d
Tree-SHA512: 6f0ff7ec2632aed510df362e2fb9cf25fe02cae347bdd4a481804a3ea2b9e060c4ec2c85de3e9d1d40920e4b9c4eecfab127e61f3d076886fe8f2fb4bff9f5a7
Licenses are boring as hell, so is are all the comments at the top of
each file. This patch makes no comment on the merit of license comments
in each file, rather this patch reduces the license comment to the
minimum possible with no loss of meaning - an SPDX license identifier.
Note also please that we remove the "written by" comments as well for
the following reasons (discussed recently on rust-bitcoin repo):
- they are not descriptive because many devs contributed
- they have a tendency to include the wrong date because of cut'n'pasta
- all this info is in the git history
ref: https://spdx.dev/ids/#how
I was reading the docs for `normalize_s` and got confused what the
point was - it says that libsecp "will only accept" signatures that
are normalized, which led me to believe it would refuse to
deserialize such signatures. This is untrue, it only refuses to
*validate* such signatures.
Now that we have Rust 1.48 as the MSRV we no longer need the custom
implementations of `PartialEq`, `Eq`, `PartialOrd`, `Ord`, and `Hash`.
We can just let users of the `impl_array_newtype` macro derive these
traits if they want them.
Remove the custom implementations and add derives to our two users of
the macro.
Yuki Kishimoto
Tobin C. Harding
Andrew Poelstra
Martin Habovstiak
Marko Bencun
Elichai Turkel
Steven Roose
benthecarman
Jiri Jakes
Davidson Souza
isaac-asdf
Matt Corallo