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.
upstream libsecp now has a CMakeLists.txt file. Many years ago we added
some things to .gitignore which appear to be local developers committing
the names of their own stray files, and now this is causing the
revendoring script to lose track of vendored files.
936421476e Use range dependency for hashes v0.12.0 - v0.13.0 (Tobin C. Harding)
Pull request description:
There are zero code changes required to support v.0.13.0 so we elect to use a range dependency to make the upgrade path for downstream users more pleasant.
Upgrade the dependency of `hashes` to be either v0.12.0 or v0.13.0
Use v0.13.0 in the recent/minimal lockfiles.
ACKs for top commit:
apoelstra:
ACK 936421476e
Tree-SHA512: c411acbd6025a751920fe2c432bf8c9ab78e891a3b32170f85873732c595777e075bd8b1277bf7b156e941a3caf532af71af36e99f88aa36dadbd66c61384166
There are zero code changes required to support v.0.13.0 so we elect to
use a range dependency to make the upgrade path for downstream users
more pleasant.
Upgrade the dependency of `hashes` to be either v0.12.0 or v0.13.0
Use v0.13.0 in the recent/minimal lockfiles.
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
33747bb16f Rename KeyPair to Keypair (Tobin C. Harding)
Pull request description:
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"
Fix#647
ACKs for top commit:
apoelstra:
ACK 33747bb16f
Tree-SHA512: 2cb67cde3a39c4681e0628b17b12724fe17d00bc71e985b4eb0b3ad58d574f4a372aa782fc9cb7221cd2032d8312c579195ad2230a3eeb4c550865fa36e42c0b
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"
6fdd3b1da5 Clean up hashes import statements (Tobin C. Harding)
6d7c653b64 Use hashes instead of bitcoin_hashes (Tobin C. Harding)
Pull request description:
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.
ACKs for top commit:
apoelstra:
ACK 6fdd3b1da5
Tree-SHA512: 2c6c81b6ef900bfe930df48ddd32e795997c107a2a94b9e2ea014da0be7aefaa952e1259ddb570b30f5ad7e94c0a9443b691645cb386a8d0349dc7a81f659ed6
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?
3da39c6fb6 Run test with recent/minimal lock files (Tobin C. Harding)
4b9168ca25 Run WASM tests from test wrapper script (Tobin C. Harding)
637d08f1fe Add a layer of indirection to the test script (Tobin C. Harding)
d9b70d27b0 Remove trailing whitespace (Tobin C. Harding)
461bae9244 Move recent/minimal lock files (Tobin C. Harding)
Pull request description:
A while back we added two lock files, one for testing with recent dependency versions and one for testing with minimal dependency versions but at the time we never used them in CI.
Move the two current lock files and use them in CI (mirroring what is done in `rust-bitcoin`).
ACKs for top commit:
apoelstra:
ACK 3da39c6fb6
Tree-SHA512: 5d293689e8a67373cbf0d6b04894c38e636bb7da19db62ac2cc1b83f1dc8184e92169a834d9adf4de3c61c34d5f6f443a1be1d0c2503bb03f08fc486d68beb71
The `wasm-pack` command does not honour `cargo` flags passed to it so we
cannot use `--locked` and test against pre-made lock files. Instead just
run the WASM test from the test script wrapper.
We would like to be able to run the test script with different lock
files, in preparation for doing so move the `test.sh` script to
`_test.sh` and add a new `test.sh` that runs `_test.sh`.
Keep the outer script as `test.sh` so that we do not change the workflow
for those running the script including the github actions.
A while back we added two lock files that were to track dependencies of
two successful test runs, one with a minimal set of dependencies and one
with a recent set of dependencies (ie, recent dependency versions). We
never used these lock files in CI however.
In preparation for using the lock files in CI, and in order to be
uniform with `rust-bitcoin`, move the lock files to the crate root and
rename them to:
- Cargo-minimal.lock
- Cargo-recent.lock
b9cb37d69f Add a verify function to PublicKey (Tobin C. Harding)
Pull request description:
To be uniform with `XOnlyPublicKey` add a `verify` function to the `PublicKey`.
Should have been done when we did #618
ACKs for top commit:
apoelstra:
ACK b9cb37d69f
Tree-SHA512: e1d8127daafd18d3c9b5df6edc46a961ed49e87a44c650b92c695606002f1d4c1aee3e89822e188a65ba888abd50c5b6f247570d73fa8508d739efa8bc4df7f0
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
92778efe92 CI: Pin cc in ASAN no_std_test crate (Tobin C. Harding)
5e6dd8a467 CI: Use bash instead of sh (Tobin C. Harding)
cf5f1034ca Target panic message at lib users (Tobin C. Harding)
ec9c9643d7 Allow stuff after unconditional panic (Tobin C. Harding)
3bbf08348e no_std_test: Remove internal_features (Tobin C. Harding)
cff7a3dae7 CI: Grep for exact error message (Tobin C. Harding)
79e184f08a CI: Update MSRV pins (Tobin C. Harding)
Pull request description:
CI has a bunch of things broken.
This is #640 followed by #639 followed by a few addition fixes to get CI green again. Includes clippy warnings in feature gated code which is not strictly necessary and a fix to a panic message, also not strictly necessary.
ACKs for top commit:
apoelstra:
ACK 92778efe92
Tree-SHA512: f99b01e17fade7df394299bdb6bf385bec3f88d6568d43962238049b33a94c364d48c266acb358e72a48dd55a4aac6300ace6478b0821275b89cb86eba639d8b
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.
`cargo +nightly` output of panic recently changed breaking our grep
statement by adding the code line and a newline.
Grep for the exact second line of the error message.
Pinning is broken again, update the pins it CI so that the following
sequence of commands would work
```bash
rm Cargo.lock
cargo +1.48 update -p wasm-bindgen-test --precise 0.3.34
cargo +1.48 update -p serde_test --precise 1.0.175
cargo +1.48 test --all-features
```
Note, solely out of interest, `cargo +1.48 build` does not need
pinning (at the moment :)
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.
567c39c7f1 Revert "WIP: Add toolchain matrix to job" (Tobin C. Harding)
e6643c083d CI: Pin dependencies for MSRV build ... properly (Tobin C. Harding)
Pull request description:
Goodness me, I made a mess.
Commit `0e0dcb7f CI: Pin dependencies required for MSRV build` is totally wrong, why did it get through CI?
Fix broken stuff in the CI script by doing:
- `serde_json` is not a dependency of `secp256k1`, remove the pinning
- Put the pinning _before_ any call to `cargo`
- Pin the transient dependency `wasm-bindgen-test`
And then `Revert "WIP: Add toolchain matrix to job"`
Fix: #626
ACKs for top commit:
apoelstra:
ACK 567c39c7f1
Tree-SHA512: f60a95e1c840e265dba1d10d2e87b970f1ebc5f01514ef9edaaf0475d833dbce15b8e715a6c53052786036b1dbbba73a2be0e470afd0d37320f540081c51c8e8
This reverts commit 77808b7d83.
dtolnay/rust-toolchain does not support using a matrix as far as I can
tell. Since the PR brief description contains "WIP" it looks like the
original author (me) was testing this using CI, no idea how this patch
got merged.
Commit `0e0dcb7f CI: Pin dependencies required for MSRV build` is
totally wrong, why did it get through CI?
In the CI script do:
- `serde_json` is not a dependency of `secp256k1`, remove the pinning
- Put the pinning _before_ any call to `cargo`
- Pin the transient dependency `wasm-bindgen-test`
81b154fed5 Remove docsrs cfg_attributes (Tobin C. Harding)
Pull request description:
We no longer need to manually configure the docsrs build to highlight feature guards since we use the `doc_auto_cfg` feature. Somehow when we added usage of that feature we forgot to remove the other attributes.
Found in CI fail of #624
ACKs for top commit:
sanket1729:
utACK 81b154fed5. I don't follow this completely, but yay for removing unnecessary code.
apoelstra:
ACK 81b154fed5
Tree-SHA512: 9decdc3f71d8f592047eee89f7f4aaf3a08b2643148c6bc5ad7de9edf61acab0ee56bf3c6dbc14493a9d089d492e31f1d379539e256b5eb96c8873c3be702256
We no longer need to manually configure the docsrs build to highlight
feature guards since we use the `doc_auto_cfg` feature. Somehow when we
added usage of that feature we forgot to remove the other attributes.
0e0dcb7f54 CI: Pin dependencies required for MSRV build (Tobin C. Harding)
Pull request description:
Whinge, whinge, whinge, and pin the dependencies.
ACKs for top commit:
apoelstra:
ACK 0e0dcb7f54
Tree-SHA512: 3eb65a844f632ea324705fe371c3876b96d62ddabe17085ed1bf32183eb491ff5006836c3945f4b0e11886049e9bd62245d2eb2c646cbf1a2f10cbff5c54bd53
3b8a82eb2b Add changelog entry for removal of ONE_KEY (Tobin C. Harding)
Pull request description:
In the 0.25.0 release we removed the `ONE_KEY` const but did not mention it in the release notes. This makes upgrade harder than it should be.
In hindsight we should have deprecated the const then removed it.
Add a changelog entry to the 0.25.0 release mentioning the removal and the work around.
ACKs for top commit:
apoelstra:
ACK 3b8a82eb2b
Tree-SHA512: 0bbf47a2f0e5221719b3d04a895c8f784623e31fd149911cf1b92afa2487037ee9619cedc94ea1d22bffd540ef5aa94a56e98daa56751c94274c973825ec9d5d
d60b891126 Add a verify function to PublicKey (Tobin C. Harding)
Pull request description:
Expose signature verification functionality for schnorr signatures on the `XOnlyPublicKey` type.
Idea from Kixunil: https://github.com/rust-bitcoin/rust-bitcoin/pull/1744#issuecomment-1534200841
ACKs for top commit:
apoelstra:
ACK d60b891126
Tree-SHA512: 2ffa3de528b857c5b0a402815b71b35da913c668bea53b63801705fa6a86eb6d44766aa2395c02f67a4712b451c77caf627af9450183ae70957abf246a63c279
In the 0.25.0 release we removed the `ONE_KEY` const but did not mention
it in the release notes. This makes upgrade harder than it should be.
In hindsight we should have deprecated the const then removed it.
Add a changelog entry to the 0.25.0 release mentioning the removal and
the work around.
47aa740c74 Improve the README files (Tobin C. Harding)
Pull request description:
Improve the README files
Improve the secp256k1 readme by:
- ~Use a top level markdown header (level 1)~ Use HTML for header and badges
- Add a link to the SECG's website (www.secg.org)
- Add a link for `secp256k1` to bitcoin.it explaining the curve
Improve the secp256k1-sys readme by:
- Use HTML for header and badges (a subset of the badges used in `rust-secp256k1` readme)
- Basic cleanup
- Use 100 column width
- Use backticks
- Use capitals
ACKs for top commit:
apoelstra:
ACK 47aa740c74
Tree-SHA512: 8f818ffcda93424430abd72da68d86215c2313479449775e9851aff854d3691180aadfe5052338c2695d85c6cac32e764c4f789301867407eef64d8c3990ef10
Improve the secp256k1 readme by:
- Use a top level markdown header (level 1)
- Add a link to the SECG's website (www.secg.org)
- Add a link for `secp256k1` to bitcoin.it explaining the curve
Improve the secp256k1-sys readme by:
- Mirror secp256k1 readme badges, heading, docs link
- Basic cleanup
- Use 100 column width
- Use backticks
- Use capitals
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
7467b23a8d rustfmt: Use now config option fn_params_layout (Tobin C. Harding)
Pull request description:
`rustfmt` warns:
Warning: the `fn_args_layout` option is deprecated. Use
`fn_params_layout`. instead
As suggested, use now config option `fn_params_layout`.
ACKs for top commit:
apoelstra:
ACK 7467b23a8d
Tree-SHA512: c786607200f550839300234d72e387e76e648b51ccebb96d8fc7a3b607b1702e8410dca486930cf71321f7e0a0546ad55f589da58395c8b3e86d783b40f776a7
`rustfmt` warns:
Warning: the `fn_args_layout` option is deprecated. Use
`fn_params_layout`. instead
As suggested, use now config option `fn_params_layout`.
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
b6d0c3bfcd Use doc_auto_cfg (Tobin C. Harding)
Pull request description:
We can build docs using feature markers by using `doc_auto_cfg` now, no need to manually call the `doc` attribute.
ACKs for top commit:
Kixunil:
ACK b6d0c3bfcd
apoelstra:
ACK b6d0c3bfcd
Tree-SHA512: ab95968dcb664543d6e1ab5f00866fda1ac2862b86793bda0e19cdc354fbf22471c46a044ceabe8cba2d2fc32671604219fdcb5e96107e14096d20d2aceab0f3
Andrew Poelstra
Tobin C. Harding
isaac-asdf